Skip to content

Commit

Permalink
Bugfix: Fix exempt URI method views (#28)
Browse files Browse the repository at this point in the history
* Fix tests assertEquals calls

When running tests in Python 3.12.0, an error indicating `assertEquals`
is not available.

* Modify tests for exempted URI with explicit override in view

The tests were previously testing against the following configuration:

```python
{'GET': ['director', 'judge', 'employee'], 'POST': ['director', 'judge'], 'UPDATE': ['director', 'judge'], 'DELETE': ['director', 'judge'], 'PATCH': ['director', 'judge', 'employee']}
```

It was not possible to override the test cases. The `'POST'` method was
removed from keycloak_roles in order to be able to appropriately test
the intended behavior, as described in the test case, `test_when_some_EXEMPT_URI_is_permitted_on_authentication_with_keycloak_roles_on_view`.

* Add explicit override logic for exempted URIs in middleware

This will close and fix #27.

Using a simple `if` statement, adding one more check to see if the
processing of a given view should occur makes it simple to allow for
more complex behaviors on a `ModelViewSet` than either fully exempted or
exempted, yet able to be overriden in a View.

* Update version number

* Fix AttributeError for is_api_view check

When attempting to access the `/admin` interface in Django, an
`AttributeError` gets `raise`d:

```
Traceback (most recent call last):
  File "/Users/thomasfex/development/github/ItkBackend/env/lib/python3.8/site-packages/django/core/handlers/exception.py", line 47, in inner
    response = get_response(request)
  File "/Users/thomasfex/development/github/ItkBackend/env/lib/python3.8/site-packages/django/core/handlers/base.py", line 171, in _get_response
    response = middleware_method(request, callback, callback_args, callback_kwargs)
  File "/Users/thomasfex/development/github/ItkBackend/env/lib/python3.8/site-packages/django_keycloak_auth/middleware.py", line 91, in process_view
    is_api_view = True if str(view_func.cls.__qualname__) == "WrappedAPIView" else False
AttributeError: 'function' object has no attribute 'cls'
[03-11-23 13:38:09] django.server 500 "GET /admin/ HTTP/1.1" 500 95736
```

A `try`/`except` block successfully allows the access of `/admin` pages.

Co-authored-by: Thomas Cross <[email protected]>

---------

Co-authored-by: Marcelo Vinicius de Sousa Campos <[email protected]>
Co-authored-by: Thomas Cross <[email protected]>
  • Loading branch information
3 people authored Nov 16, 2023
1 parent 976708d commit f360b5d
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 48 deletions.
1 change: 0 additions & 1 deletion core/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ class BankViewSet(viewsets.ModelViewSet):
queryset = models.Bank.objects.all()
keycloak_roles = {
'GET': ['director', 'judge', 'employee'],
'POST': ['director', 'judge', ],
'UPDATE': ['director', 'judge', ],
'DELETE': ['director', 'judge', ],
'PATCH': ['director', 'judge', 'employee'],
Expand Down
11 changes: 8 additions & 3 deletions django_keycloak_auth/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,17 @@ def process_view(self, request, view_func, view_args, view_kwargs):
if hasattr(settings, 'KEYCLOAK_EXEMPT_URIS'):
path = request.path_info.lstrip('/')
if any(re.match(m, path) for m in settings.KEYCLOAK_EXEMPT_URIS):
return None
# Checks to see if a request.method explicitly overwrites exemptions in SETTINGS
if hasattr(view_func.cls, "keycloak_roles") and request.method not in view_func.cls.keycloak_roles:
return None

# There's condictions for these view_func.cls:
# 1) @api_view -> view_func.cls is WrappedAPIView (validates in 'keycloak_roles' in decorators.py) -> True
# 2) When it is a APIView, ViewSet or ModelViewSet with 'keycloak_roles' attribute -> False
is_api_view = True if str(view_func.cls.__qualname__) == "WrappedAPIView" else False
# 2) When it is a APIView, ViewSet or ModelViewSet with 'keycloak_roles' attribute -> False
try:
is_api_view = True if str(view_func.cls.__qualname__) == "WrappedAPIView" else False
except AttributeError:
is_api_view = False

# Read if View has attribute 'keycloak_roles' (for APIView, ViewSet or ModelViewSet)
# Whether View hasn't this attribute, it means all request method routes will be permitted.
Expand Down
32 changes: 17 additions & 15 deletions django_keycloak_auth/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,12 @@ def test_when_some_EXEMPT_URI_is_permitted_on_authentication_with_keycloak_roles
settings.KEYCLOAK_EXEMPT_URIS = ['core/banks']

# WHEN makes a request that has 'keycloak_roles' attribute on View
response = self.client.get(self.uri)
get_response = self.client.get(self.uri)
post_response = self.client.post(self.uri, {"name": "fake", "code": "test"})

# THEN allows endpoint to be accessed
self.assertEquals(response.status_code, status.HTTP_200_OK)
self.assertEqual(get_response.status_code, status.HTTP_401_UNAUTHORIZED)
self.assertEqual(post_response.status_code, status.HTTP_201_CREATED)

def test_when_some_URI_is_permitted_on_authentication_without_keycloak_roles_attribute_on_view(self):
# GIVEN i've got a URI without 'keycloak_roles' on the View
Expand All @@ -111,7 +113,7 @@ def test_when_some_URI_is_permitted_on_authentication_without_keycloak_roles_att
response = self.client.get(uri_no_roles)

# THEN allows endpoint to be accessed
self.assertEquals(response.status_code, status.HTTP_200_OK)
self.assertEqual(response.status_code, status.HTTP_200_OK)

def test_when_some_URI_without_authorization_on_http_header(self):
# GIVEN a View endpoint
Expand All @@ -122,7 +124,7 @@ def test_when_some_URI_without_authorization_on_http_header(self):
response = KeycloakMiddleware(Mock()).process_view(request, view, [], {})

# THEN not allows endpoint to be accessed (401)
self.assertEquals(response.status_code, status.HTTP_401_UNAUTHORIZED)
self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED)

def test_when_token_not_active(self):
# GIVEN token as not valid
Expand All @@ -139,7 +141,7 @@ def test_when_token_not_active(self):
response = KeycloakMiddleware(Mock()).process_view(request, view, [], {})

# THEN not allows endpoint to be accessed (401)
self.assertEquals(response.status_code, status.HTTP_401_UNAUTHORIZED)
self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED)

def test_when_token_as_active_and_no_roles_request_not_authorizated(self):
# GIVEN token as valid
Expand All @@ -159,7 +161,7 @@ def test_when_token_as_active_and_no_roles_request_not_authorizated(self):
response = KeycloakMiddleware(Mock()).process_view(request, view, [], {})

# THEN not allows endpoint to be accessed (401)
self.assertEquals(response.status_code, status.HTTP_401_UNAUTHORIZED)
self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED)

def test_when_token_as_active_and_has_roles_request_not_authorizated(self):
# GIVEN token as valid
Expand All @@ -180,7 +182,7 @@ def test_when_token_as_active_and_has_roles_request_not_authorizated(self):
response = KeycloakMiddleware(Mock()).process_view(request, view, [], {})

# THEN does't allow endpoint authorization
self.assertEquals(response.status_code, status.HTTP_403_FORBIDDEN)
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)

def test_when_token_as_active_and_has_roles_request_authorizated(self):
# GIVEN token as valid
Expand All @@ -201,7 +203,7 @@ def test_when_token_as_active_and_has_roles_request_authorizated(self):
response = KeycloakMiddleware(Mock()).process_view(request, view, [], {})

# THEN allows endpoint and pass token roles to request View
self.assertEquals(['director'], request.roles)
self.assertEqual(['director'], request.roles)

def test_when_realm_roles_and_client_roles_are_present_both_are_returned(self):
fake_token = {
Expand All @@ -221,7 +223,7 @@ def test_when_realm_roles_and_client_roles_are_present_both_are_returned(self):
self.keycloak.userinfo = Mock(return_value=fake_token)
roles = self.keycloak.roles_from_token(Mock())

self.assertEquals(['judge', 'director'], roles)
self.assertEqual(['judge', 'director'], roles)

def test_when_only_realm_roles_are_present_realm_roles_are_returned(self):
fake_token = {
Expand All @@ -235,7 +237,7 @@ def test_when_only_realm_roles_are_present_realm_roles_are_returned(self):
self.keycloak.userinfo = Mock(return_value=fake_token)
roles = self.keycloak.roles_from_token(Mock())

self.assertEquals(['director'], roles)
self.assertEqual(['director'], roles)

def test_when_only_client_roles_are_present_client_roles_are_returned(self):
fake_token = {
Expand All @@ -251,15 +253,15 @@ def test_when_only_client_roles_are_present_client_roles_are_returned(self):
self.keycloak.userinfo = Mock(return_value=fake_token)
roles = self.keycloak.roles_from_token(Mock())

self.assertEquals(['judge'], roles)
self.assertEqual(['judge'], roles)

def test_when_no_role_is_present_none_is_returned(self):
fake_token = {}
self.keycloak.introspect = Mock(return_value=fake_token)
self.keycloak.userinfo = Mock(return_value=fake_token)
roles = self.keycloak.roles_from_token(Mock())

self.assertEquals(None, roles)
self.assertEqual(None, roles)

def test_when_only_user_info_is_present_user_info_are_returned(self):
fake_token = {
Expand All @@ -272,7 +274,7 @@ def test_when_only_user_info_is_present_user_info_are_returned(self):

userinfo = self.keycloak.userinfo(Mock())

self.assertEquals(fake_token['sub'], userinfo['sub'])
self.assertEqual(fake_token['sub'], userinfo['sub'])

def test_keycloak_connect_well_known(self):
"""Test keycloak endpoint well_known when status >= 400"""
Expand Down Expand Up @@ -334,7 +336,7 @@ def test_when_token_is_active_and_has_roles_request_not_authorizated(self):
response = KeycloakMiddleware(Mock()).process_view(request, view, [], {})

# THEN not allows endpoint to be accessed (401)
self.assertEquals(response.status_code, status.HTTP_401_UNAUTHORIZED)
self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED)

def test_when_token_is_active_and_has_roles_request_authorizated(self):

Expand All @@ -358,4 +360,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.assertEquals(['director'], request.roles)
self.assertEqual(['director'], request.roles)
36 changes: 8 additions & 28 deletions docker-compose.yml
Original file line number Diff line number Diff line change
@@ -1,38 +1,18 @@
version: "3"

volumes:
my_data:

networks:
my_network:
driver: bridge

services:
db:
image: postgres:12.0-alpine
volumes:
- my_data:/var/lib/postgresql/data
keycloak:
image: jboss/keycloak
restart: "no"
environment:
POSTGRES_DB: deposito
POSTGRES_USER: admin
POSTGRES_PASSWORD: 123
- DB_VENDOR=H2
- KEYCLOAK_USER=admin
- KEYCLOAK_PASSWORD=123
ports:
- "8080:8080"
networks:
- my_network

keycloak:
image: jboss/keycloak
environment:
DB_VENDOR: POSTGRES
DB_ADDR: db
DB_DATABASE: deposito
DB_USER: admin
DB_SCHEMA: public
DB_PASSWORD: 123
KEYCLOAK_USER: admin
KEYCLOAK_PASSWORD: 123
ports:
- 8080:8080
depends_on:
- db
networks:
- my_network
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

setup(
name="django-keycloak-auth",
version="0.9.7",
version="0.9.8",
packages=find_packages(),

# Project uses reStructuredText, so ensure that the docutils get
Expand Down

0 comments on commit f360b5d

Please sign in to comment.