-
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 commentThe reason will be displayed to describe this comment to others. Learn more. I believe the default case should be There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. I agree too! |
||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ | |
from .middleware import KeycloakMiddleware, KeycloakConnect | ||
from core import views | ||
from unittest.mock import Mock | ||
from django.conf import settings | ||
|
||
|
||
class KeycloakConfigTestCase(TestCase): | ||
|
@@ -346,8 +347,38 @@ def test_when_token_is_active_and_has_roles_request_not_authorizated(self): | |
|
||
# THEN not allows endpoint to be accessed (401) | ||
self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) | ||
|
||
def test_when_token_is_active_and_has_roles_request_authorizated(self): | ||
|
||
# GIVEN LOCAL_DECODE as False in settings.py | ||
settings.KEYCLOAK_CONFIG['LOCAL_DECODE'] = False | ||
|
||
# GIVEN token as valid | ||
KeycloakConnect.is_token_active = Mock(return_value=True) | ||
|
||
# GIVEN token has roles | ||
KeycloakConnect.roles_from_token = Mock(return_value=['director']) | ||
|
||
# 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 commentThe reason will be displayed to describe this comment to others. Learn more. Small typo 'diretor' |
||
view = views.loans | ||
|
||
# GIVEN a fake request | ||
request = self.factory.get(self.uri) | ||
request.META['HTTP_AUTHORIZATION'] = 'Bearer token_fake' | ||
|
||
# WHEN middleware is processed | ||
KeycloakMiddleware(Mock()).process_view(request, view, [], {}) | ||
|
||
# THEN allows endpoint and pass token roles and userinfo to request View | ||
self.assertEqual(['director'], request.roles) | ||
|
||
def test_when_cached_token_is_active_and_has_roles_request_authorizated(self): | ||
|
||
# GIVEN LOCAL_DECODE as True in settings.py | ||
settings.KEYCLOAK_CONFIG['LOCAL_DECODE'] = True | ||
|
||
# GIVEN token as valid | ||
KeycloakConnect.is_token_active = Mock(return_value=True) | ||
|
@@ -369,4 +400,4 @@ def test_when_token_is_active_and_has_roles_request_authorizated(self): | |
KeycloakMiddleware(Mock()).process_view(request, view, [], {}) | ||
|
||
# THEN allows endpoint and pass token roles and userinfo to request View | ||
self.assertEqual(['director'], request.roles) | ||
self.assertEqual([], request.roles) |
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.
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
andoptions
may be passed tojwt.decode
.These three methods use the decode method: