scim: Upgrade to django-scim2 0.17.1.

The new release adds the commit:
20ac22b96d

Which allows us to get rid of the entire ugly override that was needed
to do this commit's job in our code. What we do here in this commit:
* Use django-scim2 0.17.1
* Revert the relevant parts of f5a65846a8
* Adjust the expected error message in test_exception_details_not_revealed_to_client
  since the message thrown by django-scim2 in this release is slightly
  different.

We do not have to add anything to set EXPOSE_SCIM_EXCEPTIONS, since
django-scim2 uses False as the default, which is what we want - and we
have the aforementioned test verifying that indeed information doesn't
get revealed to the SCIM client.
This commit is contained in:
Mateusz Mandera 2022-02-04 19:57:11 +01:00 committed by Tim Abbott
parent 1045737be6
commit a1fa2a8cf5
6 changed files with 18 additions and 90 deletions

View File

@ -432,8 +432,8 @@ django-phonenumber-field==6.0.0 \
--hash=sha256:9695d3beda772c503ad4e04a4f7012a8227e9e3e4fd0ea4ffb07c43245bf4a8d \
--hash=sha256:bbb9cb2e6fc53c476de40428e1354c313a040e8b2fb69ea8ead4ba41a60f926a
# via django-two-factor-auth
django-scim2==0.17.0 \
--hash=sha256:dc3cb3c0d5b6ebf4ae8a28dd1dac1e0658fb543c8c178e72e3c19975816da092
django-scim2==0.17.1 \
--hash=sha256:346e9b3e9bff6aab59e533c735b9892bcc52d06ed042772b4d48fcb494c2e22a
# via -r requirements/common.in
django-sendfile2==0.6.1 \
--hash=sha256:312b4501960e6b3a3390c48a6bdcfdae2c0516efacf24bdd0c97c6f2f2d2fc30 \

View File

@ -289,8 +289,8 @@ django-phonenumber-field==6.0.0 \
--hash=sha256:9695d3beda772c503ad4e04a4f7012a8227e9e3e4fd0ea4ffb07c43245bf4a8d \
--hash=sha256:bbb9cb2e6fc53c476de40428e1354c313a040e8b2fb69ea8ead4ba41a60f926a
# via django-two-factor-auth
django-scim2==0.17.0 \
--hash=sha256:dc3cb3c0d5b6ebf4ae8a28dd1dac1e0658fb543c8c178e72e3c19975816da092
django-scim2==0.17.1 \
--hash=sha256:346e9b3e9bff6aab59e533c735b9892bcc52d06ed042772b4d48fcb494c2e22a
# via -r requirements/common.in
django-sendfile2==0.6.1 \
--hash=sha256:312b4501960e6b3a3390c48a6bdcfdae2c0516efacf24bdd0c97c6f2f2d2fc30 \

View File

@ -48,4 +48,4 @@ API_FEATURE_LEVEL = 115
# historical commits sharing the same major version, in which case a
# minor version bump suffices.
PROVISION_VERSION = "173.4"
PROVISION_VERSION = "174.0"

View File

@ -2,17 +2,11 @@ from typing import Any, Callable, Dict, List, Optional, Type, Union
import django_scim.constants as scim_constants
import django_scim.exceptions as scim_exceptions
import orjson
from django.conf import settings
from django.contrib.auth.decorators import login_required
from django.core.exceptions import ValidationError
from django.db import models, transaction
from django.http import HttpRequest, HttpResponse
from django.utils.decorators import method_decorator
from django.views.decorators.csrf import csrf_exempt
from django.http import HttpRequest
from django_scim.adapters import SCIMUser
from django_scim.views import SCIMView, SearchView, UserSearchView, UsersView
from django_scim.views import logger as scim_views_logger
from scim2_filter_parser.attr_paths import AttrPath
from zerver.lib.actions import (
@ -368,58 +362,3 @@ class ConflictError(scim_exceptions.IntegrityError):
"""
scim_type = "uniqueness"
class ZulipSCIMViewMixin(SCIMView):
"""
Default django-scim2 behavior is to convert any exception that occurs while processing
the request within the view code to a string and put it
in the HttpResponse. We don't want that due to the risk of leaking sensitive information
through the error message.
The way we implement this override is by having this mixin override the main dispatch()
method - and then all the specific view classes are re-defined to inherit from this mixin
and the original django-scim2 class. This means that we have to also re-register all
the URL patterns so that our View classes are used.
"""
@method_decorator(csrf_exempt)
@method_decorator(login_required)
def dispatch(self, request: HttpRequest, *args: Any, **kwargs: Any) -> HttpResponse:
"""
This method through which all SCIM views are processed needs to be forked
to change its logic of how exceptions are handled.
"""
if not self.implemented:
return self.status_501(request, *args, **kwargs)
try:
return super(SCIMView, self).dispatch(request, *args, **kwargs)
except Exception as e:
if not isinstance(e, scim_exceptions.SCIMException):
# This is where we adjust the exception-handling behavior. Instead of
# putting str(e) in the response, we use a generic error that won't leak
# information.
scim_views_logger.exception("Unable to complete SCIM call.")
e = scim_exceptions.SCIMException("Exception while processing SCIM request.")
content = orjson.dumps(e.to_dict())
return HttpResponse(
content=content, content_type=scim_constants.SCIM_CONTENT_TYPE, status=e.status
)
class ZulipSCIMView(ZulipSCIMViewMixin, SCIMView):
pass
class ZulipSCIMUsersView(ZulipSCIMViewMixin, UsersView):
pass
class ZulipSCIMSearchView(ZulipSCIMViewMixin, SearchView):
pass
class ZulipSCIMUserSearchView(ZulipSCIMViewMixin, UserSearchView):
pass

View File

@ -101,7 +101,7 @@ class TestExceptionDetailsNotRevealedToClient(SCIMTestCase):
result.json(),
{
"schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"],
"detail": "Exception while processing SCIM request.",
"detail": "Exception occurred while processing the SCIM request",
"status": 500,
},
)

View File

@ -754,45 +754,34 @@ urls += [path("saml/metadata.xml", saml_sp_metadata)]
# SCIM2
from zerver.lib.scim import (
ZulipSCIMSearchView,
ZulipSCIMUserSearchView,
ZulipSCIMUsersView,
ZulipSCIMView,
)
from django_scim import views as scim_views
urls += [
# We have to register all the SCIM URL patterns first, because we override
# all the SCIM View classes and we need Django to use them instead of
# the django-scim2 Views that the app will register.
re_path(r"^scim/v2/$", ZulipSCIMView.as_view(implemented=False)),
re_path(r"^scim/v2/.search$", ZulipSCIMSearchView.as_view(implemented=False)),
re_path(r"^scim/v2/Users/.search$", ZulipSCIMUserSearchView.as_view()),
re_path(r"^scim/v2/Users(?:/(?P<uuid>[^/]+))?$", ZulipSCIMUsersView.as_view()),
# Everything below here are features that we don't yet support and we want
# to explicitly mark them to return "Not Implemented" rather than running
# the django-scim2 code for them.
re_path(
r"^scim/v2/Groups/.search$",
ZulipSCIMView.as_view(implemented=False),
scim_views.SCIMView.as_view(implemented=False),
),
re_path(
r"^scim/v2/Groups(?:/(?P<uuid>[^/]+))?$",
ZulipSCIMView.as_view(implemented=False),
scim_views.SCIMView.as_view(implemented=False),
),
re_path(r"^scim/v2/Me$", ZulipSCIMView.as_view(implemented=False)),
re_path(r"^scim/v2/Me$", scim_views.SCIMView.as_view(implemented=False)),
re_path(
r"^scim/v2/ServiceProviderConfig$",
ZulipSCIMView.as_view(implemented=False),
scim_views.SCIMView.as_view(implemented=False),
),
re_path(
r"^scim/v2/ResourceTypes(?:/(?P<uuid>[^/]+))?$",
ZulipSCIMView.as_view(implemented=False),
scim_views.SCIMView.as_view(implemented=False),
),
re_path(r"^scim/v2/Schemas(?:/(?P<uuid>[^/]+))?$", ZulipSCIMView.as_view(implemented=False)),
re_path(r"^scim/v2/Bulk$", ZulipSCIMView.as_view(implemented=False)),
# At the end we still register the django-scim2 url patterns (even though we override them all above)
# so that reverse("scim:viewname") still works like the internal library code expects.
re_path(
r"^scim/v2/Schemas(?:/(?P<uuid>[^/]+))?$", scim_views.SCIMView.as_view(implemented=False)
),
re_path(r"^scim/v2/Bulk$", scim_views.SCIMView.as_view(implemented=False)),
# This registers the remaining SCIM endpoints.
path("scim/v2/", include("django_scim.urls", namespace="scim")),
]