Skip to content

Commit

Permalink
fix!: remove oidc group api fetching
Browse files Browse the repository at this point in the history
  • Loading branch information
Yelinz committed Jan 11, 2023
1 parent b984054 commit e64e9d5
Show file tree
Hide file tree
Showing 19 changed files with 179 additions and 321 deletions.
3 changes: 3 additions & 0 deletions .mypy.ini
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,6 @@ ignore_missing_imports = True

[mypy-openpyxl.*]
ignore_missing_imports = True

[mypy-generic_permissions.*]
ignore_missing_imports = True
6 changes: 0 additions & 6 deletions CONFIGURATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,9 @@ supporting Open ID Connect. If not available, you might consider using
[Keycloak](https://www.keycloak.org/).

* `REQUIRE_AUTHENTICATION`: Force authentication to be required (default: False)
* `GROUP_ACCESS_ONLY`: Force visibility to templates of group only. See also `OIDC_GROUPS_CLAIM` or `OIDC_GROUPS_API` (default: False)
* `OIDC_USERINFO_ENDPOINT`: Url of userinfo endpoint as [described](https://openid.net/specs/openid-connect-core-1_0.html#UserInfo)
* `OIDC_VERIFY_SSL`: Verify ssl certificate of oidc userinfo endpoint (default: True)
* `OIDC_GROUPS_CLAIM`: Name of claim to be used to define group membership (default: document_merge_service_groups)
* `OIDC_GROUPS_API`: In case authorization is done in a different service than your OpenID Connect provider you may specify an API endpoint returning JSON which is called after authentication. Use `{sub}` as placeholder for username. Replace `sub` with any claim name. Example: https://example.net/users/{sub}/
* `OIDC_GROUPS_API_REVALIDATION_TIME`: Time in seconds before groups api is called again.
* `OIDC_GROUPS_API_VERIFY_SSL`: Verify ssl certificate of groups api endpoint (default: True)
* `OIDC_GROUPS_API_JSONPATH`: [Json path expression](https://goessner.net/articles/JsonPath/index.html) to match groups in json returned by `OIDC_GROUPS_API`. See also [JSONPath online evaluator](https://jsonpath.com/)
* `OIDC_GROUPS_API_HEADER`: List of headers which are passed on to groups api. (default: ['AUTHENTICATION'])
* `OIDC_BEARER_TOKEN_REVALIDATION_TIME`: Time in seconds before bearer token validity is verified again. For best security token is validated on each request per default. It might be helpful though in case of slow Open ID Connect provider to cache it. It uses [cache](#cache) mechanism for memorizing userinfo result. Number has to be lower than access token expiration time. (default: 0)

## Cache
Expand Down
87 changes: 87 additions & 0 deletions MIGRATE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
# Migration from v1 to v2
**Warning**

The `group` attribute will be removed from the Template model.
A suggested migration would be to move the value to `meta` before migrating.

----

The previous pre-defined permission and visibility system was removed in favour of [dgap](https://github.com/adfinis/django-generic-api-permissions).

The integration of `OIDC_GROUPS_API` and `OIDC_GROUPS_API_JSONPATH` was removed with it.
Because every consuming app can now define its own way to handle the permissions.

Example Permissions:
```py
import requests
from rest_framework import exceptions
from generic_permissions.permissions import object_permission_for

from document_merge_service.models import Template


class CustomPermission:
"""
Current settings and how to refactor them
OIDC_GROUPS_API = "https://example.com/users/{sub}/group"
OIDC_GROUPS_API_JSONPATH = "$$.included[?(@.type=='services')].id"
"""
@object_permission_for(Template)
def has_object_permission_template(self, request, instance):
uri = "https://example.com/users/{sub}/group"
# extract header
token = request.headers["AUTHORIZATION"]

# previously OIDC_GROUPS_API
groups_api = f"https://example.com/users/{request.user.username}/group"

response = requests.get(
groups_api, verify=True, headers={"authorization": token}
)
try:
response.raise_for_status()
except requests.HTTPError as e:
raise exceptions.AuthenticationFailed(
f"Retrieving groups from {uri} "
f"failed with error '{str(e)}'."
)

result = response.json()

# previously OIDC_GROUPS_API_JSONPATH was used here to extract the group from the response
for data in result["included"]:
if data.type == "services"
groups = data.id

return instance.meta["group"] in groups
```

After creating the permission define it in `settings.py` for dgap.
```py
GENERIC_PERMISSIONS_PERMISSION_CLASSES = ['app.permissions.CustomPermission']
```

Example Visibility:
```py
from django.db.models import Q
from generic_permissions.visibilities import filter_queryset_for

from document_merge_service.models import Template


class CustomVisibility:
"""Example Visibility class to replicate previous behaviour."""

@filter_queryset_for(Template)
def filter_templates(self, queryset, request):
queryset = queryset.filter(
Q(meta__group__in=self.request.user.groups or []) | Q(meta__group__isnull=True)
)

return queryset
```

After creating the visibility define it in `settings.py` for dgap.
```py
GENERIC_PERMISSIONS_VISIBILITY_CLASSES = ['app.visibilites.CustomVisibility']
```
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ shell: ## Shell into document merge service

format: ## Format python code with black
@docker-compose exec document-merge-service poetry run black .
@docker-compose exec document-merge-service poetry run isort .

dmypy: ## Run mypy locally (starts a deamon for performance)
dmypy run document_merge_service
11 changes: 4 additions & 7 deletions USAGE.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,16 +49,13 @@ When you enable authentication, all access must be with a valid OIDC token.
Authenticated users are able to upload and merge templates, but anonymous users
won't be anymore.

You can configure the DMS to allow users to only see templates created by other
people in the same group (by enabling the setting `GROUP_ACCESS_ONLY`).

Any further restriction is currently not possible. If you have the need for
further rules, please open an issue (or a pull request) on Github, so we can
have a discussion and extend the authorization process accordingly.

For the full details on how to configure it, see the
[configuration guide](CONFIGURATION.md).

### Permissions / Visibility

[dgap](https://github.com/adfinis/django-generic-api-permissions) is being used for custom permissions and visibilites. Refer to the README over at [dgap](https://github.com/adfinis/django-generic-api-permissions) on how to configure.


## Uploading templates

Expand Down
44 changes: 4 additions & 40 deletions document_merge_service/api/authentication.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import functools
import hashlib

import jsonpath
import requests
from django.conf import settings
from django.core.cache import cache
Expand Down Expand Up @@ -40,7 +39,6 @@ def __str__(self):
return self.username


# rewrite as permission
class BearerTokenAuthentication(authentication.BaseAuthentication):
header_prefix = "Bearer"

Expand Down Expand Up @@ -82,33 +80,6 @@ def get_userinfo(self, token):

return response.json()

def get_groups_from_api(self, request, userinfo):
headers = {
key: value
for key, value in request.headers.items()
if key.upper() in settings.OIDC_GROUPS_API_HEADERS
}

# replace placeholders
groups_api = settings.OIDC_GROUPS_API
placeholders = {k: v for k, v in userinfo.items() if isinstance(v, str)}
for key, value in placeholders.items():
groups_api = groups_api.replace("{" + key + "}", value)

response = requests.get(
groups_api, verify=settings.OIDC_GROUPS_API_VERIFY_SSL, headers=headers
)
try:
response.raise_for_status()
except requests.HTTPError as e:
raise exceptions.AuthenticationFailed(
f"Retrieving groups from {settings.OIDC_GROUPS_API} "
f"failed with error '{str(e)}'."
)

result = response.json()
return jsonpath.jsonpath(result, settings.OIDC_GROUPS_API_JSONPATH)

def authenticate(self, request):
token = self.get_bearer_token(request)
if token is None:
Expand All @@ -123,21 +94,14 @@ def authenticate(self, request):
timeout=settings.OIDC_BEARER_TOKEN_REVALIDATION_TIME,
)

# retrieve groups
groups = []
if settings.OIDC_GROUPS_CLAIM:
groups = userinfo[settings.OIDC_GROUPS_CLAIM]
elif settings.OIDC_GROUPS_API:
groups_api_method = functools.partial(
self.get_groups_from_api, userinfo=userinfo, request=request
)
groups = cache.get_or_set(
f"authentication.groups.{hashsum_token}",
groups_api_method,
timeout=settings.OIDC_GROUPS_API_REVALIDATION_TIME,
)

return AuthenticatedUser(userinfo["sub"], groups), token
return (
AuthenticatedUser(userinfo["sub"], groups),
token,
)

def authenticate_header(self, request):
return f"{self.header_prefix} realm={settings.OIDC_USERINFO_ENDPOINT}"
1 change: 0 additions & 1 deletion document_merge_service/api/factories.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ class TemplateFactory(DjangoModelFactory):
description = Faker("text")
engine = Faker("word", ext_word_list=models.Template.ENGINE_CHOICES_LIST)
template = None
group = None
meta = {} # type: ignore

class Meta:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Generated by Django 3.2.16 on 2022-12-23 12:54

from django.db import migrations


class Migration(migrations.Migration):

dependencies = [
("api", "0005_xlsx_template_engine"),
]

operations = [
migrations.RemoveField(
model_name="template",
name="group",
),
]
3 changes: 0 additions & 3 deletions document_merge_service/api/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,6 @@ class Template(models.Model):
engine: models.CharField = models.CharField(
max_length=20, choices=ENGINE_CHOICES_TUPLE
)
group: models.CharField = models.CharField(
max_length=255, db_index=True, blank=True, null=True
)
meta = models.JSONField(default=dict)


Expand Down
41 changes: 4 additions & 37 deletions document_merge_service/api/permissions.py
Original file line number Diff line number Diff line change
@@ -1,42 +1,9 @@
import inspect

from django.conf import settings
from django.core.exceptions import ImproperlyConfigured

from .collections import list_duplicates
from .models import Template

class AllowAny(BasePermission):
pass


class IsAuthenticated(BasePermission):
"""
Allow access only to authenticated users.
You can either use this in combination with your own permission
classes, or inherit from it if you want *some* models to be accessible
publicly.
"""
from rest_framework import permissions

@permission_for(Template)
def base_permission(self, request):
return request.user.is_authenticated

@object_permission_for(Template)
def base_object_permission(self, request, instance):
return self.base_permission(request)


class AsConfigured(IsAuthenticated):
@permission_for(Template)
def base_permission(self, request):
if settings.REQUIRE_AUTHENTICATION:
return super().base_permission(request)
return True

@object_permission_for(Template)
def base_object_permission(self, request, instance):
class AsConfigured(permissions.IsAuthenticated):
def has_permission(self, request, view):
if settings.REQUIRE_AUTHENTICATION:
return super().base_object_permission(request, instance)
return super().has_permission(request, view)
return True
24 changes: 5 additions & 19 deletions document_merge_service/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@

from django.conf import settings
from django.urls import reverse
from django.utils.translation import gettext as _
from rest_framework import exceptions, serializers
from generic_permissions.validation import ValidatorMixin
from rest_framework import exceptions, serializers

from . import engines, models

Expand Down Expand Up @@ -34,32 +33,17 @@ def to_representation(self, value):
return reverse("template-download", args=[value.pk])


class CurrentGroupDefault:
requires_context = True

def __call__(self, serializer_field):
return serializer_field.context["request"].user.group


class TemplateSerializer(ValidatorMixin ,serializers.ModelSerializer):
class TemplateSerializer(ValidatorMixin, serializers.ModelSerializer):
disable_template_validation = serializers.BooleanField(
allow_null=True, default=False
)
group = serializers.CharField(allow_null=True, default=CurrentGroupDefault())
available_placeholders = serializers.ListField(allow_null=True, required=False)
sample_data = serializers.JSONField(allow_null=True, required=False)
files = serializers.ListField(
child=CustomFileField(write_only=True, allow_empty_file=False), required=False
)
template = TemplateFileField()

def validate_group(self, group):
request = self.context["request"]
if group and group not in request.user.groups:
raise exceptions.ValidationError(_(f"User is not member of group {group}"))

return group

def _sample_to_placeholders(self, sample_doc):
@singledispatch
def _doc(doc):
Expand Down Expand Up @@ -129,7 +113,6 @@ class Meta:
"description",
"template",
"engine",
"group",
"available_placeholders",
"sample_data",
"files",
Expand All @@ -151,3 +134,6 @@ class TemplateMergeSerializer(serializers.Serializer):
files = serializers.ListField(
child=CustomFileField(write_only=True, allow_empty_file=False), required=False
)

class Meta:
model = models.Template
Loading

0 comments on commit e64e9d5

Please sign in to comment.