Page MenuHomeSoftware Heritage

Add DRF bearer token authentication using OpenID Connect
ClosedPublic

Authored by anlambert on Mar 2 2020, 6:47 PM.

Details

Summary

This backend for Django REST Framework enables to authenticate users through the
use of bearer tokens (provided by Keycloak) sent in HTTP request headers.

Related to T1927
Related to T2249

Depends on D2746

Diff Detail

Repository
rDWAPPS Web applications
Branch
oidc-bearer-token-drf-auth
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 10849
Build 16305: Cypress tests for swh-web diffsJenkins
Build 16304: tox-on-jenkinsJenkins
Build 16303: arc lint + arc unit

Event Timeline

vlorentz requested changes to this revision.Mar 3 2020, 6:40 PM
vlorentz added a subscriber: vlorentz.

Nice too! Just a few nitpicks:

swh/web/auth/backends.py
121

You don't need to call .get() again.

And mypy might complain some day that the type of auth_header changes

122

token = auth_header[-1].

But why do we need to support missing auth type? RFC 7235 always requires it (https://tools.ietf.org/html/rfc7235#section-4.2 and https://tools.ietf.org/html/rfc7235#appendix-C )

And it should also check the auth type in case we add more types in the future, so:

(auth_scheme, token) = auth_header.split(' ', 1)
if auth_scheme != 'Bearer':
    raise ...
136

Does the client have control over decoded['exp']? If yes, we should clamp the value of the ttl.

This revision now requires changes to proceed.Mar 3 2020, 6:40 PM
swh/web/auth/backends.py
121

ack

122

Ok, I will improve that part.

136

Ack, will clamp to token max active time.

anlambert marked 3 inline comments as done.

Update: Address @vlorentz comments

Thanks for the review again, diff updated accordingly.

Update: Fix cypress tests launch

Since the addition of DRF authentication backends, the following error was reported when
launching swh-web in end to end tests mode:

Traceback (most recent call last):
  File "swh/web/manage.py", line 46, in <module>
    execute_from_command_line(sys.argv)
  File "/home/jenkins/.local/lib/python3.7/site-packages/django/core/management/__init__.py", line 381, in execute_from_command_line
    utility.execute()
  File "/home/jenkins/.local/lib/python3.7/site-packages/django/core/management/__init__.py", line 325, in execute
    settings.INSTALLED_APPS
  File "/home/jenkins/.local/lib/python3.7/site-packages/django/conf/__init__.py", line 79, in __getattr__
    self._setup(name)
  File "/home/jenkins/.local/lib/python3.7/site-packages/django/conf/__init__.py", line 66, in _setup
    self._wrapped = Settings(settings_module)
  File "/home/jenkins/.local/lib/python3.7/site-packages/django/conf/__init__.py", line 157, in __init__
    mod = importlib.import_module(self.SETTINGS_MODULE)
  File "/usr/lib/python3.7/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1006, in _gcd_import
  File "<frozen importlib._bootstrap>", line 983, in _find_and_load
  File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 677, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 728, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/home/jenkins/workspace/DWAPPS/cypress-diff/swh/web/settings/tests.py", line 107, in <module>
    from swh.web.tests.data import get_tests_data, override_storages # noqa
  File "/home/jenkins/workspace/DWAPPS/cypress-diff/swh/web/tests/data.py", line 12, in <module>
    from rest_framework.decorators import api_view
  File "/home/jenkins/.local/lib/python3.7/site-packages/rest_framework/decorators.py", line 13, in <module>
    from rest_framework.views import APIView
  File "/home/jenkins/.local/lib/python3.7/site-packages/rest_framework/views.py", line 17, in <module>
    from rest_framework.schemas import DefaultSchema
  File "/home/jenkins/.local/lib/python3.7/site-packages/rest_framework/schemas/__init__.py", line 33, in <module>
    authentication_classes=api_settings.DEFAULT_AUTHENTICATION_CLASSES,
  File "/home/jenkins/.local/lib/python3.7/site-packages/rest_framework/settings.py", line 220, in __getattr__
    val = perform_import(val, attr)
  File "/home/jenkins/.local/lib/python3.7/site-packages/rest_framework/settings.py", line 168, in perform_import
    return [import_from_string(item, setting_name) for item in val]
  File "/home/jenkins/.local/lib/python3.7/site-packages/rest_framework/settings.py", line 168, in <listcomp>
    return [import_from_string(item, setting_name) for item in val]
  File "/home/jenkins/.local/lib/python3.7/site-packages/rest_framework/settings.py", line 177, in import_from_string
    return import_string(val)
  File "/home/jenkins/.local/lib/python3.7/site-packages/django/utils/module_loading.py", line 17, in import_string
    module = import_module(module_path)
  File "/usr/lib/python3.7/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "/home/jenkins/workspace/DWAPPS/cypress-diff/swh/web/auth/backends.py", line 20, in <module>
    from swh.web.auth.models import OIDCUser
  File "/home/jenkins/workspace/DWAPPS/cypress-diff/swh/web/auth/models.py", line 9, in <module>
    from django.contrib.auth.models import User
  File "/home/jenkins/.local/lib/python3.7/site-packages/django/contrib/auth/models.py", line 2, in <module>
    from django.contrib.auth.base_user import AbstractBaseUser, BaseUserManager
  File "/home/jenkins/.local/lib/python3.7/site-packages/django/contrib/auth/base_user.py", line 47, in <module>
    class AbstractBaseUser(models.Model):
  File "/home/jenkins/.local/lib/python3.7/site-packages/django/db/models/base.py", line 103, in __new__
    app_config = apps.get_containing_app_config(module)
  File "/home/jenkins/.local/lib/python3.7/site-packages/django/apps/registry.py", line 252, in get_containing_app_config
    self.check_apps_ready()
  File "/home/jenkins/.local/lib/python3.7/site-packages/django/apps/registry.py", line 135, in check_apps_ready
    raise AppRegistryNotReady("Apps aren't loaded yet.")
django.core.exceptions.AppRegistryNotReady: Apps aren't loaded yet.

So move end to end test DRF views implementation in a dedicated module and import it only when required.

This revision is now accepted and ready to land.Mar 5 2020, 12:37 PM