Apply SCIMAuthCheckMiddleware directly to SCIMView.dispatch #85
Conversation
| @@ -229,6 +228,7 @@ | |||
| 'SCHEME': 'https', | |||
| # use default value, this will be overridden by value returned by BASE_LOCATION_GETTER | |||
| 'NETLOC': 'localhost', | |||
| 'AUTH_CHECK_MIDDLEWARE': 'app.middleware.CustomSCIMAuthCheckMiddleware', | |||
There was a problem hiding this comment.
Can we change this back to app.middleware.SCIMAuthCheckMiddleware. As a developer, I would want to name my class CustomSCIMAuthCheckMiddleware and have the library's version be either SCIMAuthCheckMiddleware, BaseSCIMAuthCheckMiddleware, etc.
There was a problem hiding this comment.
This isn’t the library’s version; it’s part of the demo app (demo/app/middleware.py), and has been since 0759c04. The only thing I changed here is to move it from MIDDLEWARE_CLASSES to AUTH_CHECK_MIDDLEWARE.
The library’s version is still django_scim.middleware.SCIMAuthCheckMiddleware, and developers who want that don’t need to customize AUTH_CHECK_MIDDLEWARE at all since it’s the default (src/django_scim/settings.py).
andersk
changed the base branch from
logston/update-custom-authentication
to
master
|
(Rebased onto |
This way, the user no longer needs to install our middleware in their project settings; this simplifies configuration and avoids a performance penalty for all non-SCIM requests. Additionally, since SCIMView now has a guarantee that SCIMAuthCheckMiddleware is being used (unless the new AUTH_CHECK_MIDDLEWARE setting is explicitly overridden), it no longer needs to perform its own redundant version of the check. Signed-off-by: Anders Kaseorg <andersk@mit.edu>
|
@andersk Thanks so much for this change, one of the things I wanted to address in order to use this library was not adding a site-wide middleware, and it looks like you've already beaten me to it! @logston Is there a release schedule for when these changes might be made available on PyPI? I'd love to take advantage of them! I can install from the git repo at a specific hash for now, but would be a bit harder to manage updates that way. |
This way, the user no longer needs to install our middleware in their project settings; this simplifies configuration and avoids a performance penalty for all non-SCIM requests.
Additionally, since
SCIMViewnow has a guarantee thatSCIMAuthCheckMiddlewareis being used (unless the newAUTH_CHECK_MIDDLEWAREsetting is explicitly overridden), it no longer needs to perform its own redundant version of the check.With the redundant check removed, we could go further and remove the
GET_IS_AUTHENTICATED_PREDICATEsetting, since the same effect can be achieved by setting theAUTH_CHECK_MIDDLEWAREsetting to a custom subclass ofSCIMAuthCheckMiddleware.@logston What do you think about this strategy?