Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for conafigurable userinfo_endpoint and a better error handling when this is missing #20

Closed
wants to merge 3 commits into from

Conversation

wiliamsouza
Copy link

Add support for configurable userinfo_endpoint and a better error handling when OpenID connect provider discovery metadata does not contain this field.

Copy link
Contributor

@allardhoeve allardhoeve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, what @osantana said.

@@ -68,7 +68,11 @@ def get_bearer_token(self, request):

@cache(ttl=api_settings.OIDC_BEARER_TOKEN_EXPIRATION_TIME)
def get_userinfo(self, token):
response = requests.get(self.oidc_config['userinfo_endpoint'],
userinfo_endpoint = self.oidc_config.get('userinfo_endpoint', api_settings.USERINFO_ENDPOINT)
if userinfo_endpoint in ['', None, [], (), {}]:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather this be if not userinfo_endpoint:, which is much more readable and probably what you want anyway.

@wiliamsouza
Copy link
Author

@allardhoeve Can you please take a look here.

userinfo_endpoint = self.oidc_config.get('userinfo_endpoint', api_settings.USERINFO_ENDPOINT)
if not userinfo_endpoint:
msg = _('Invalid userinfo_endpoint URL.'
'Not found an URL from OpenID connect discovery metadata nor settings.OIDC_AUTH.USERINFO_ENDPOINT.')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you change this to "Could not find an URL from OpenID connect discovery metadata nor settings.OIDC_AUTH.USERINFO_ENDPOINT"? Just a little nicer :)

@kerrermanisNL
Copy link
Contributor

Looks pretty good @wiliamsouza do you have time to rebase?

@rburgst
Copy link

rburgst commented Apr 9, 2021

I would also need this feature. Any chance we can get this merged?

kerrermanisNL pushed a commit that referenced this pull request Apr 9, 2021
Based on PR for #20
But updated with master.

Adds a possibility for users to configure a user endpoint if it cannot
be discovered via OpenID connect provider discovery metadata. Also
provide and error message when none could be found (including user
specified one).

Special thanks to @wiliamsouza
@kerrermanisNL
Copy link
Contributor

Closing in favour of #54 thanks again @wiliamsouza :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants