From ee670d5e1994407e32ad5862c5f6a52383e0a491 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Sun, 18 Oct 2020 14:34:22 +0200 Subject: [PATCH] core: add key field to token for easier rotation --- passbook/admin/views/users.py | 2 +- passbook/api/auth.py | 57 ++++++----- passbook/core/api/tokens.py | 20 +++- passbook/core/channels.py | 24 ++--- .../migrations/0014_auto_20201018_1158.py | 50 ++++++++++ passbook/core/models.py | 25 +++-- passbook/outposts/controllers/docker.py | 4 +- .../0009_fix_missing_token_identifier.py | 1 + .../templates/outposts/deployment_modal.html | 2 +- .../templates/outposts/setup_custom.html | 68 ------------- .../outposts/templates/outposts/setup_dc.html | 59 ------------ .../outposts/setup_k8s_integrated.html | 59 ------------ .../templates/outposts/setup_k8s_manual.html | 96 ------------------- .../commands/create_recovery_key.py | 10 +- passbook/recovery/tests.py | 13 +-- passbook/recovery/urls.py | 2 +- passbook/recovery/views.py | 12 +-- swagger.yaml | 25 +++-- 18 files changed, 168 insertions(+), 361 deletions(-) create mode 100644 passbook/core/migrations/0014_auto_20201018_1158.py delete mode 100644 passbook/outposts/templates/outposts/setup_custom.html delete mode 100644 passbook/outposts/templates/outposts/setup_dc.html delete mode 100644 passbook/outposts/templates/outposts/setup_k8s_integrated.html delete mode 100644 passbook/outposts/templates/outposts/setup_k8s_manual.html diff --git a/passbook/admin/views/users.py b/passbook/admin/views/users.py index 6f490ebc6..b1cd5a674 100644 --- a/passbook/admin/views/users.py +++ b/passbook/admin/views/users.py @@ -158,7 +158,7 @@ class UserPasswordResetView(LoginRequiredMixin, PermissionRequiredMixin, DetailV token, _ = Token.objects.get_or_create( identifier="password-reset-temp", user=self.object ) - querystring = urlencode({"token": token.token_uuid}) + querystring = urlencode({"token": token.key}) link = request.build_absolute_uri( reverse("passbook_flows:default-recovery") + f"?{querystring}" ) diff --git a/passbook/api/auth.py b/passbook/api/auth.py index 06eebd3ce..8d0f3f18b 100644 --- a/passbook/api/auth.py +++ b/passbook/api/auth.py @@ -1,43 +1,54 @@ """API Authentication""" from base64 import b64decode -from typing import Any, Tuple, Union +from typing import Any, Optional, Tuple, Union -from django.utils.translation import gettext as _ -from rest_framework import HTTP_HEADER_ENCODING, exceptions from rest_framework.authentication import BaseAuthentication, get_authorization_header from rest_framework.request import Request +from structlog import get_logger from passbook.core.models import Token, TokenIntents, User +LOGGER = get_logger() + + +def token_from_header(raw_header: bytes) -> Optional[Token]: + """raw_header in the Format of `Basic dGVzdDp0ZXN0`""" + auth_credentials = raw_header.decode() + # Accept headers with Type format and without + if " " in auth_credentials: + auth_type, auth_credentials = auth_credentials.split() + if auth_type.lower() != "basic": + LOGGER.debug( + "Unsupported authentication type, denying", type=auth_type.lower() + ) + return None + auth_credentials = b64decode(auth_credentials.encode()).decode() + # Accept credentials with username and without + if ":" in auth_credentials: + _, password = auth_credentials.split(":") + else: + password = auth_credentials + if password == "": + return None + tokens = Token.filter_not_expired(key=password, intent=TokenIntents.INTENT_API) + if not tokens.exists(): + LOGGER.debug("Token not found") + return None + return tokens.first() + class PassbookTokenAuthentication(BaseAuthentication): """Token-based authentication using HTTP Basic authentication""" def authenticate(self, request: Request) -> Union[Tuple[User, Any], None]: """Token-based authentication using HTTP Basic authentication""" - auth = get_authorization_header(request).split() + auth = get_authorization_header(request) - if not auth or auth[0].lower() != b"basic": + token = token_from_header(auth) + if not token: return None - if len(auth) == 1: - msg = _("Invalid basic header. No credentials provided.") - raise exceptions.AuthenticationFailed(msg) - if len(auth) > 2: - msg = _( - "Invalid basic header. Credentials string should not contain spaces." - ) - raise exceptions.AuthenticationFailed(msg) - - header_data = b64decode(auth[1]).decode(HTTP_HEADER_ENCODING).partition(":") - - tokens = Token.filter_not_expired( - token_uuid=header_data[2], intent=TokenIntents.INTENT_API - ) - if not tokens.exists(): - raise exceptions.AuthenticationFailed(_("Invalid token.")) - - return (tokens.first().user, None) + return (token.user, None) def authenticate_header(self, request: Request) -> str: return 'Basic realm="passbook"' diff --git a/passbook/core/api/tokens.py b/passbook/core/api/tokens.py index 5fde0f55d..bffbaf42e 100644 --- a/passbook/core/api/tokens.py +++ b/passbook/core/api/tokens.py @@ -1,7 +1,14 @@ """Tokens API Viewset""" +from uuid import UUID + +from django.http.response import Http404 +from rest_framework.decorators import action +from rest_framework.request import Request +from rest_framework.response import Response from rest_framework.serializers import ModelSerializer from rest_framework.viewsets import ModelViewSet +from passbook.audit.models import Event, EventAction from passbook.core.models import Token @@ -17,6 +24,17 @@ class TokenSerializer(ModelSerializer): class TokenViewSet(ModelViewSet): """Token Viewset""" - queryset = Token.objects.all() lookup_field = "identifier" + queryset = Token.filter_not_expired() serializer_class = TokenSerializer + + @action(detail=True) + # pylint: disable=invalid-name + def view_key(self, request: Request, pk: UUID) -> Response: + """Return token key and log access""" + tokens = Token.filter_not_expired(pk=pk) + if not tokens.exists(): + raise Http404 + token = tokens.first() + Event.new(EventAction.TOKEN_VIEW, token=token).from_http(request) + return Response({"key": token.key}) diff --git a/passbook/core/channels.py b/passbook/core/channels.py index ba5eaef12..5598ef8fa 100644 --- a/passbook/core/channels.py +++ b/passbook/core/channels.py @@ -1,9 +1,9 @@ """Channels base classes""" from channels.generic.websocket import JsonWebsocketConsumer -from django.core.exceptions import ValidationError from structlog import get_logger -from passbook.core.models import Token, TokenIntents, User +from passbook.api.auth import token_from_header +from passbook.core.models import User LOGGER = get_logger() @@ -20,19 +20,13 @@ class AuthJsonConsumer(JsonWebsocketConsumer): self.close() return False - token = headers[b"authorization"] - try: - token_uuid = token.decode("utf-8") - tokens = Token.filter_not_expired( - token_uuid=token_uuid, intent=TokenIntents.INTENT_API - ) - if not tokens.exists(): - LOGGER.warning("WS Request with invalid token") - self.close() - return False - except ValidationError: - LOGGER.warning("WS Invalid UUID") + raw_header = headers[b"authorization"] + + token = token_from_header(raw_header) + if not token: + LOGGER.warning("Failed to authenticate") self.close() return False - self.user = tokens.first().user + + self.user = token.user return True diff --git a/passbook/core/migrations/0014_auto_20201018_1158.py b/passbook/core/migrations/0014_auto_20201018_1158.py new file mode 100644 index 000000000..a40101019 --- /dev/null +++ b/passbook/core/migrations/0014_auto_20201018_1158.py @@ -0,0 +1,50 @@ +# Generated by Django 3.1.2 on 2020-10-18 11:58 +from django.apps.registry import Apps +from django.db import migrations, models +from django.db.backends.base.schema import BaseDatabaseSchemaEditor + +import passbook.core.models + + +def set_default_token_key(apps: Apps, schema_editor: BaseDatabaseSchemaEditor): + db_alias = schema_editor.connection.alias + Token = apps.get_model("passbook_core", "Token") + + for token in Token.objects.using(db_alias).all(): + token.key = token.pk.hex + token.save() + + +class Migration(migrations.Migration): + + dependencies = [ + ("passbook_core", "0013_auto_20201003_2132"), + ] + + operations = [ + migrations.AddField( + model_name="token", + name="key", + field=models.TextField(default=passbook.core.models.default_token_key), + ), + migrations.AlterUniqueTogether( + name="token", + unique_together=set(), + ), + migrations.AlterField( + model_name="token", + name="identifier", + field=models.CharField(max_length=255), + ), + migrations.AddIndex( + model_name="token", + index=models.Index(fields=["key"], name="passbook_co_key_e45007_idx"), + ), + migrations.AddIndex( + model_name="token", + index=models.Index( + fields=["identifier"], name="passbook_co_identif_1a34a8_idx" + ), + ), + migrations.RunPython(set_default_token_key), + ] diff --git a/passbook/core/models.py b/passbook/core/models.py index 1e8bc0752..3e6d3df83 100644 --- a/passbook/core/models.py +++ b/passbook/core/models.py @@ -32,6 +32,11 @@ def default_token_duration(): return now() + timedelta(minutes=30) +def default_token_key(): + """Default token key""" + return uuid4().hex + + class Group(models.Model): """Custom Group model which supports a basic hierarchy""" @@ -274,10 +279,8 @@ class ExpiringModel(models.Model): def filter_not_expired(cls, **kwargs) -> QuerySet: """Filer for tokens which are not expired yet or are not expiring, and match filters in `kwargs`""" - query = Q(**kwargs) - query_not_expired_yet = Q(expires__lt=now(), expiring=True) - query_not_expiring = Q(expiring=False) - return cls.objects.filter(query & (query_not_expired_yet | query_not_expiring)) + expired = Q(expires__lt=now(), expiring=True) + return cls.objects.exclude(expired).filter(**kwargs) @property def is_expired(self) -> bool: @@ -298,6 +301,7 @@ class TokenIntents(models.TextChoices): # Allow access to API INTENT_API = "api" + # Recovery use for the recovery app INTENT_RECOVERY = "recovery" @@ -305,7 +309,8 @@ class Token(ExpiringModel): """Token used to authenticate the User for API Access or confirm another Stage like Email.""" token_uuid = models.UUIDField(primary_key=True, editable=False, default=uuid4) - identifier = models.TextField() + identifier = models.CharField(max_length=255) + key = models.TextField(default=default_token_key) intent = models.TextField( choices=TokenIntents.choices, default=TokenIntents.INTENT_VERIFICATION ) @@ -313,13 +318,19 @@ class Token(ExpiringModel): description = models.TextField(default="", blank=True) def __str__(self): - return f"Token {self.identifier} (expires={self.expires})" + description = f"{self.identifier}" + if self.expiring: + description += f" (expires={self.expires})" + return description class Meta: verbose_name = _("Token") verbose_name_plural = _("Tokens") - unique_together = (("identifier", "user"),) + indexes = [ + models.Index(fields=["identifier"]), + models.Index(fields=["key"]), + ] class PropertyMapping(models.Model): diff --git a/passbook/outposts/controllers/docker.py b/passbook/outposts/controllers/docker.py index e78624951..1749cf651 100644 --- a/passbook/outposts/controllers/docker.py +++ b/passbook/outposts/controllers/docker.py @@ -30,7 +30,7 @@ class DockerController(BaseController): return { "PASSBOOK_HOST": self.outpost.config.passbook_host, "PASSBOOK_INSECURE": str(self.outpost.config.passbook_host_insecure), - "PASSBOOK_TOKEN": self.outpost.token.token_uuid.hex, + "PASSBOOK_TOKEN": self.outpost.token.key, } def _comp_env(self, container: Container) -> bool: @@ -136,7 +136,7 @@ class DockerController(BaseController): "PASSBOOK_INSECURE": str( self.outpost.config.passbook_host_insecure ), - "PASSBOOK_TOKEN": self.outpost.token.token_uuid.hex, + "PASSBOOK_TOKEN": self.outpost.token.key, }, } }, diff --git a/passbook/outposts/migrations/0009_fix_missing_token_identifier.py b/passbook/outposts/migrations/0009_fix_missing_token_identifier.py index 0021a979a..52461cbf2 100644 --- a/passbook/outposts/migrations/0009_fix_missing_token_identifier.py +++ b/passbook/outposts/migrations/0009_fix_missing_token_identifier.py @@ -18,6 +18,7 @@ def fix_missing_token_identifier(apps: Apps, schema_editor: BaseDatabaseSchemaEd class Migration(migrations.Migration): dependencies = [ + ("passbook_core", "0014_auto_20201018_1158"), ("passbook_outposts", "0008_auto_20201014_1547"), ] diff --git a/passbook/outposts/templates/outposts/deployment_modal.html b/passbook/outposts/templates/outposts/deployment_modal.html index 7ebbf6171..d84b7bb16 100644 --- a/passbook/outposts/templates/outposts/deployment_modal.html +++ b/passbook/outposts/templates/outposts/deployment_modal.html @@ -24,7 +24,7 @@ - +

{% trans 'If your passbook Instance is using a self-signed certificate, set this value.' %}

diff --git a/passbook/outposts/templates/outposts/setup_custom.html b/passbook/outposts/templates/outposts/setup_custom.html deleted file mode 100644 index e391a98fa..000000000 --- a/passbook/outposts/templates/outposts/setup_custom.html +++ /dev/null @@ -1,68 +0,0 @@ -{% load i18n %} -{% load static %} -
- - -
- - - - diff --git a/passbook/outposts/templates/outposts/setup_dc.html b/passbook/outposts/templates/outposts/setup_dc.html deleted file mode 100644 index 7c7121482..000000000 --- a/passbook/outposts/templates/outposts/setup_dc.html +++ /dev/null @@ -1,59 +0,0 @@ -{% extends "administration/base.html" %} - -{% load i18n %} -{% load humanize %} -{% load passbook_utils %} - -{% block head %} -{{ block.super }} - -{% endblock %} - -{% block content %} -
-
-

- - {% trans 'Outpost Setup' %} -

-

{% trans "Outposts are deployments of passbook components to support different environments and protocols, like reverse proxies." %}

-
-
-
- -
    -
  • - -
    • -
  • - -
    • -
  • - -
    • -
- -
-
-
- -
-
-{% endblock %} diff --git a/passbook/outposts/templates/outposts/setup_k8s_integrated.html b/passbook/outposts/templates/outposts/setup_k8s_integrated.html deleted file mode 100644 index 7c7121482..000000000 --- a/passbook/outposts/templates/outposts/setup_k8s_integrated.html +++ /dev/null @@ -1,59 +0,0 @@ -{% extends "administration/base.html" %} - -{% load i18n %} -{% load humanize %} -{% load passbook_utils %} - -{% block head %} -{{ block.super }} - -{% endblock %} - -{% block content %} -
-
-

- - {% trans 'Outpost Setup' %} -

-

{% trans "Outposts are deployments of passbook components to support different environments and protocols, like reverse proxies." %}

-
-
-
- -
    -
  • - -
    • -
  • - -
    • -
  • - -
    • -
- -
-
-
- -
-
-{% endblock %} diff --git a/passbook/outposts/templates/outposts/setup_k8s_manual.html b/passbook/outposts/templates/outposts/setup_k8s_manual.html deleted file mode 100644 index e22bed335..000000000 --- a/passbook/outposts/templates/outposts/setup_k8s_manual.html +++ /dev/null @@ -1,96 +0,0 @@ -{% extends "administration/base.html" %} - -{% load i18n %} -{% load humanize %} -{% load passbook_utils %} - -{% block content %} -
-
-

- - {% trans 'Outpost Setup' %} -

-

{% trans "Outposts are deployments of passbook components to support different environments and protocols, like reverse proxies." %}

-
-
-
-
- 
apiVersion: apps/v1
-kind: Deployment
-metadata:
-  labels:
-    app.kubernetes.io/name: "passbook-{{ outpost.type }}"
-    app.kubernetes.io/instance: "{{ outpost.name }}"
-    passbook.beryju.org/outpost: "{{ outpost.pk.hex }}"
-  name: "passbook-{{ outpost.type }}-{{ outpost.name }}"
-spec:
-  replicas: 1
-  selector:
-    matchLabels:
-      app.kubernetes.io/name: "passbook-{{ outpost.type }}"
-      app.kubernetes.io/instance: "{{ outpost.name }}"
-      passbook.beryju.org/outpost: "{{ outpost.pk.hex }}"
-  template:
-    metadata:
-      labels:
-        app.kubernetes.io/name: "passbook-{{ outpost.type }}"
-        app.kubernetes.io/instance: "{{ outpost.name }}"
-        passbook.beryju.org/outpost: "{{ outpost.pk.hex }}"
-    spec:
-      containers:
-      - env:
-        - name: PASSBOOK_HOST
-          value: "{{ host }}"
-        - name: PASSBOOK_TOKEN
-          value: "{{ outpost.token.pk.hex }}"
-        image: beryju/passbook-{{ outpost.type }}:{{ version }}
-        name: "passbook-{{ outpost.type }}"
-        ports:
-        - containerPort: 4180
-          protocol: TCP
-          name: http
-        - containerPort: 4443
-          protocol: TCP
-          name: https
----
-apiVersion: v1
-kind: Service
-metadata:
-  labels:
-    app.kubernetes.io/name: "passbook-{{ outpost.type }}"
-    app.kubernetes.io/instance: "{{ outpost.name }}"
-    passbook.beryju.org/outpost: "{{ outpost.pk.hex }}"
-  name: "passbook-{{ outpost.type }}-{{ outpost.name }}"
-spec:
-  ports:
-  - name: http
-    port: 4180
-    protocol: TCP
-    targetPort: 4180
-  - name: https
-    port: 4443
-    protocol: TCP
-    targetPort: 4443
-  selector:
-    app.kubernetes.io/name: "passbook-{{ outpost.type }}"
-    app.kubernetes.io/instance: "{{ outpost.name }}"
-    passbook.beryju.org/outpost: "{{ outpost.pk.hex }}"
----
-apiVersion: extensions/v1beta1
-kind: Ingress
-metadata:
-  name: "passbook-{{ outpost.type }}-{{ outpost.name }}"
-spec:
-  rules:
-  - host: "{{ provider.external_host }}"
-    http:
-      paths:
-      - backend:
-          serviceName: "passbook-{{ outpost.type }}-{{ outpost.name }}"
-          servicePort: 4180
-        path: "/pbprox"
-
-
-
-{% endblock %} diff --git a/passbook/recovery/management/commands/create_recovery_key.py b/passbook/recovery/management/commands/create_recovery_key.py index ff2a6486a..48fd083d5 100644 --- a/passbook/recovery/management/commands/create_recovery_key.py +++ b/passbook/recovery/management/commands/create_recovery_key.py @@ -9,7 +9,6 @@ from django.utils.translation import gettext as _ from structlog import get_logger from passbook.core.models import Token, TokenIntents, User -from passbook.lib.config import CONFIG LOGGER = get_logger() @@ -32,22 +31,17 @@ class Command(BaseCommand): def get_url(self, token: Token) -> str: """Get full recovery link""" - path = reverse( - "passbook_recovery:use-token", kwargs={"uuid": str(token.token_uuid)} - ) - return f"https://{CONFIG.y('domain')}{path}" + return reverse("passbook_recovery:use-token", kwargs={"key": str(token.key)}) def handle(self, *args, **options): """Create Token used to recover access""" duration = int(options.get("duration", 1)) - delta = timedelta(days=duration * 365.2425) _now = now() - expiry = _now + delta + expiry = _now + timedelta(days=duration * 365.2425) user = User.objects.get(username=options.get("user")) token = Token.objects.create( expires=expiry, user=user, - identifier="recovery", intent=TokenIntents.INTENT_RECOVERY, description=f"Recovery Token generated by {getuser()} on {_now}", ) diff --git a/passbook/recovery/tests.py b/passbook/recovery/tests.py index 52cb83ab7..e12d5a3df 100644 --- a/passbook/recovery/tests.py +++ b/passbook/recovery/tests.py @@ -5,8 +5,7 @@ from django.core.management import call_command from django.shortcuts import reverse from django.test import TestCase -from passbook.core.models import Token, User -from passbook.lib.config import CONFIG +from passbook.core.models import Token, TokenIntents, User class TestRecovery(TestCase): @@ -17,21 +16,19 @@ class TestRecovery(TestCase): def test_create_key(self): """Test creation of a new key""" - CONFIG.update_from_dict({"domain": "testserver"}) out = StringIO() self.assertEqual(len(Token.objects.all()), 0) call_command("create_recovery_key", "1", self.user.username, stdout=out) - self.assertIn("https://testserver/recovery/use-token/", out.getvalue()) + token = Token.objects.get(intent=TokenIntents.INTENT_RECOVERY, user=self.user) + self.assertIn(token.key, out.getvalue()) self.assertEqual(len(Token.objects.all()), 1) def test_recovery_view(self): """Test recovery view""" out = StringIO() call_command("create_recovery_key", "1", self.user.username, stdout=out) - token = Token.objects.first() + token = Token.objects.get(intent=TokenIntents.INTENT_RECOVERY, user=self.user) self.client.get( - reverse( - "passbook_recovery:use-token", kwargs={"uuid": str(token.token_uuid)} - ) + reverse("passbook_recovery:use-token", kwargs={"key": token.key}) ) self.assertEqual(int(self.client.session["_auth_user_id"]), token.user.pk) diff --git a/passbook/recovery/urls.py b/passbook/recovery/urls.py index 60e2736dc..136b78f4a 100644 --- a/passbook/recovery/urls.py +++ b/passbook/recovery/urls.py @@ -5,5 +5,5 @@ from django.urls import path from passbook.recovery.views import UseTokenView urlpatterns = [ - path("use-token//", UseTokenView.as_view(), name="use-token"), + path("use-token//", UseTokenView.as_view(), name="use-token"), ] diff --git a/passbook/recovery/views.py b/passbook/recovery/views.py index b12fee663..cc0ccacb8 100644 --- a/passbook/recovery/views.py +++ b/passbook/recovery/views.py @@ -2,22 +2,22 @@ from django.contrib import messages from django.contrib.auth import login from django.http import Http404, HttpRequest, HttpResponse -from django.shortcuts import get_object_or_404, redirect +from django.shortcuts import redirect from django.utils.translation import gettext as _ from django.views import View -from passbook.core.models import Token +from passbook.core.models import Token, TokenIntents class UseTokenView(View): """Use token to login""" - def get(self, request: HttpRequest, uuid: str) -> HttpResponse: + def get(self, request: HttpRequest, key: str) -> HttpResponse: """Check if token exists, log user in and delete token.""" - token: Token = get_object_or_404(Token, pk=uuid) - if token.is_expired: - token.delete() + tokens = Token.filter_not_expired(key=key, intent=TokenIntents.INTENT_RECOVERY) + if not tokens.exists(): raise Http404 + token = tokens.first() login(request, token.user, backend="django.contrib.auth.backends.ModelBackend") token.delete() messages.warning(request, _("Used recovery-link to authenticate.")) diff --git a/swagger.yaml b/swagger.yaml index 413eb5c8a..f2757d1bd 100755 --- a/swagger.yaml +++ b/swagger.yaml @@ -529,6 +529,23 @@ paths: in: path required: true type: string + /core/tokens/{identifier}/view_key/: + get: + operationId: core_tokens_view_key + description: Return token key and log access + parameters: [] + responses: + '200': + description: '' + schema: + $ref: '#/definitions/Token' + tags: + - core + parameters: + - name: identifier + in: path + required: true + type: string /core/users/: get: operationId: core_users_list @@ -6098,6 +6115,7 @@ definitions: - user_write - suspicious_request - password_set + - token_view - invitation_created - invitation_used - authorize_application @@ -6108,11 +6126,6 @@ definitions: - model_updated - model_deleted - custom_ - date: - title: Date - type: string - format: date-time - readOnly: true app: title: App type: string @@ -6214,7 +6227,6 @@ definitions: type: object Token: required: - - identifier - user type: object properties: @@ -6226,6 +6238,7 @@ definitions: identifier: title: Identifier type: string + readOnly: true minLength: 1 intent: title: Intent