-
Notifications
You must be signed in to change notification settings - Fork 46
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
Conversation
There was a problem hiding this 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.
oidc_auth/authentication.py
Outdated
@@ -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, [], (), {}]: |
There was a problem hiding this comment.
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.
@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.') |
There was a problem hiding this comment.
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 :)
Looks pretty good @wiliamsouza do you have time to rebase? |
I would also need this feature. Any chance we can get this merged? |
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
Closing in favour of #54 thanks again @wiliamsouza :) |
Add support for configurable userinfo_endpoint and a better error handling when OpenID connect provider discovery metadata does not contain this field.