-
Notifications
You must be signed in to change notification settings - Fork 43
Fix AUD default value #33
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
Conversation
# GIVEN userinfo from fake token | ||
KeycloakConnect.userinfo = Mock(return_value=self.fake_userinfo) | ||
|
||
# GIVEN a View endpoint has existis 'diretor' role on GET method |
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.
Small typo 'diretor'
@@ -155,7 +155,7 @@ | |||
'KEYCLOAK_SERVER_URL': 'http://localhost:8080/auth', | |||
'KEYCLOAK_REALM': 'TEST', | |||
'KEYCLOAK_CLIENT_ID': 'client-backend', | |||
'KEYCLOAK_CLIENT_SECRET_KEY': 'E2n41fJgl9BPIS3nDk1DQQ7BIPf6PauH', | |||
'KEYCLOAK_CLIENT_SECRET_KEY': '', | |||
'KEYCLOAK_CACHE_TTL': 60, | |||
'LOCAL_DECODE': False, |
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 believe the default case should be True
, because you dont need to bug the auth server with every request and it will add a delay in the request lifecycle
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.
Thanks! I agree, but wanted to make sure the author gets to make the decision.
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 agree too!
Dont you think there should be a test for the |
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.
def userinfo(self, token, audience=None, options=None, raise_exception=True):
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 tried aud=None
but it didnt work for me.
I am not sure if that is the case for everybody
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 didn't get it.
What does it mean?
Do you suggest replacing these params?
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.
Yes, the github formatting did not work well here. I was giving an example of updating the method signatures to include the arguments to the decode function. That way any audience
and options
may be passed to jwt.decode
.
These three methods use the decode method:
- is_token_active
- roles_from_token
- userinfo
@hassaanalansary I'm gonna create it |
Fix the #32 issue