Browse Source

feat(api,webapp): add token expiration and surrounding functionality

Related: #347
Peter Thomassen 4 years ago
parent
commit
a9d5b4e743

+ 11 - 6
api/desecapi/authentication.py

@@ -17,12 +17,20 @@ from desecapi.serializers import AuthenticatedBasicUserActionSerializer, EmailPa
 class TokenAuthentication(RestFrameworkTokenAuthentication):
     model = Token
 
+    # Note: This method's runtime depends on in what way a credential is invalid (expired, wrong client IP).
+    # It thus exposes the failure reason when under timing attack.
     def authenticate(self, request):
         try:
-            user, token = super().authenticate(request)
-        except TypeError:  # TypeError: cannot unpack non-iterable NoneType object
+            user, token = super().authenticate(request)  # may raise exceptions.AuthenticationFailed if token is invalid
+        except TypeError:  # if no token was given
             return None  # unauthenticated
 
+        if not token.is_valid:
+            raise exceptions.AuthenticationFailed('Invalid token.')
+
+        token.last_used = timezone.now()
+        token.save()
+
         # REMOTE_ADDR is populated by the environment of the wsgi-request [1], which in turn is set up by nginx as per
         # uwsgi_params [2]. The value of $remote_addr finally is given by the network connection [3].
         # [1]: https://github.com/django/django/blob/stable/3.1.x/django/core/handlers/wsgi.py#L77
@@ -44,10 +52,7 @@ class TokenAuthentication(RestFrameworkTokenAuthentication):
 
     def authenticate_credentials(self, key):
         key = Token.make_hash(key)
-        user, token = super().authenticate_credentials(key)
-        token.last_used = timezone.now()
-        token.save()
-        return user, token
+        return super().authenticate_credentials(key)
 
 
 class BasicTokenAuthentication(BaseAuthentication):

+ 25 - 0
api/desecapi/migrations/0010_token_expiration.py

@@ -0,0 +1,25 @@
+# Generated by Django 3.1.3 on 2020-11-19 09:55
+
+import datetime
+import django.core.validators
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('desecapi', '0009_token_allowed_subnets'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='token',
+            name='max_age',
+            field=models.DurationField(default=None, null=True, validators=[django.core.validators.MinValueValidator(datetime.timedelta(0))]),
+        ),
+        migrations.AddField(
+            model_name='token',
+            name='max_unused_period',
+            field=models.DurationField(default=None, null=True, validators=[django.core.validators.MinValueValidator(datetime.timedelta(0))]),
+        ),
+    ]

+ 23 - 1
api/desecapi/models.py

@@ -23,7 +23,7 @@ from django.contrib.postgres.constraints import ExclusionConstraint
 from django.contrib.postgres.fields import ArrayField, CIEmailField, RangeOperators
 from django.core.exceptions import ValidationError
 from django.core.mail import EmailMessage, get_connection
-from django.core.validators import RegexValidator
+from django.core.validators import MinValueValidator, RegexValidator
 from django.db import models
 from django.db.models import Manager, Q
 from django.db.models.expressions import RawSQL
@@ -397,10 +397,32 @@ class Token(ExportModelOperationsMixin('Token'), rest_framework.authtoken.models
     last_used = models.DateTimeField(null=True, blank=True)
     perm_manage_tokens = models.BooleanField(default=False)
     allowed_subnets = ArrayField(CidrAddressField(), default=_allowed_subnets_default.__func__)
+    max_age = models.DurationField(null=True, default=None, validators=[MinValueValidator(timedelta(0))])
+    max_unused_period = models.DurationField(null=True, default=None, validators=[MinValueValidator(timedelta(0))])
 
     plain = None
     objects = NetManager()
 
+    @property
+    def is_valid(self):
+        now = timezone.now()
+
+        # Check max age
+        try:
+            if self.created + self.max_age < now:
+                return False
+        except TypeError:
+            pass
+
+        # Check regular usage requirement
+        try:
+            if (self.last_used or self.created) + self.max_unused_period < now:
+                return False
+        except TypeError:
+            pass
+
+        return True
+
     def generate_key(self):
         self.plain = secrets.token_urlsafe(21)
         self.key = Token.make_hash(self.plain)

+ 3 - 1
api/desecapi/serializers.py

@@ -51,10 +51,12 @@ class CaptchaSolutionSerializer(serializers.Serializer):
 class TokenSerializer(serializers.ModelSerializer):
     allowed_subnets = serializers.ListField(child=netfields_rf.CidrAddressField(), required=False)
     token = serializers.ReadOnlyField(source='plain')
+    is_valid = serializers.ReadOnlyField()
 
     class Meta:
         model = models.Token
-        fields = ('id', 'created', 'last_used', 'name', 'perm_manage_tokens', 'allowed_subnets', 'token',)
+        fields = ('id', 'created', 'last_used', 'max_age', 'max_unused_period', 'name', 'perm_manage_tokens',
+                  'allowed_subnets', 'is_valid', 'token',)
         read_only_fields = ('id', 'created', 'last_used', 'token')
 
     def __init__(self, *args, include_plain=False, **kwargs):

+ 118 - 2
api/desecapi/tests/test_authentication.py

@@ -1,7 +1,11 @@
+from datetime import timedelta
 import json
+from unittest import mock
 
+from django.utils import timezone
 from rest_framework.status import HTTP_200_OK, HTTP_401_UNAUTHORIZED
 
+from desecapi.models import Token
 from desecapi.tests.base import DynDomainOwnerTestCase
 
 
@@ -45,8 +49,14 @@ class DynUpdateAuthenticationTestCase(DynDomainOwnerTestCase):
 
 class TokenAuthenticationTestCase(DynDomainOwnerTestCase):
 
-    def assertAuthenticationStatus(self, code, token=None, **kwargs):
-        self.client.set_credentials_token_auth(token or self.token.plain)
+    def setUp(self):
+        super().setUp()
+        # Refresh token from database, but keep plain value
+        self.token, self.token.plain = Token.objects.get(pk=self.token.pk), self.token.plain
+
+    def assertAuthenticationStatus(self, code, plain=None, expired=False ,**kwargs):
+        plain = plain or self.token.plain
+        self.client.set_credentials_token_auth(plain)
 
         # only forward REMOTE_ADDR if not None
         if kwargs.get('REMOTE_ADDR') is None:
@@ -56,6 +66,10 @@ class TokenAuthenticationTestCase(DynDomainOwnerTestCase):
         body = json.dumps({'detail': 'Invalid token.'}) if code == HTTP_401_UNAUTHORIZED else None
         self.assertResponse(response, code, body)
 
+        if expired:
+            key = Token.make_hash(plain)
+            self.assertFalse(Token.objects.get(key=key).is_valid)
+
     def test_token_case_sensitive(self):
         self.assertAuthenticationStatus(HTTP_200_OK)
         self.assertAuthenticationStatus(HTTP_401_UNAUTHORIZED, self.token.plain.upper())
@@ -78,3 +92,105 @@ class TokenAuthenticationTestCase(DynDomainOwnerTestCase):
             self.token.save()
             for client_ip in client_ips:
                 self.assertAuthenticationStatus(status, REMOTE_ADDR=client_ip)
+
+    def test_token_max_age(self):
+        # No maximum age: can use now and in ten years
+        self.token.max_age = None
+        self.token.save()
+
+        self.assertAuthenticationStatus(HTTP_200_OK)
+        with mock.patch('desecapi.models.timezone.now', return_value=timezone.now() + timedelta(days=3650)):
+            self.assertAuthenticationStatus(HTTP_200_OK)
+
+        # Maximum age zero: token cannot be used
+        self.token.max_age = timedelta(0)
+        self.token.save()
+        self.assertAuthenticationStatus(HTTP_401_UNAUTHORIZED, expired=True)
+
+        # Maximum age 10 10:10:10: can use one second before, but not once second after
+        period = timedelta(days=10, hours=10, minutes=10, seconds=10)
+        self.token.max_age = period
+        self.token.save()
+
+        second = timedelta(seconds=1)
+        with mock.patch('desecapi.models.timezone.now', return_value=self.token.created + period - second):
+            self.assertAuthenticationStatus(HTTP_200_OK)
+        with mock.patch('desecapi.models.timezone.now', return_value=self.token.created + period + second):
+            self.assertAuthenticationStatus(HTTP_401_UNAUTHORIZED, expired=True)
+
+    def test_token_max_unused_period(self):
+        plain = self.token.plain
+        second = timedelta(seconds=1)
+
+        # Maximum unused period zero: token cannot be used
+        self.token.max_unused_period = timedelta(0)
+        self.token.save()
+        self.assertAuthenticationStatus(HTTP_401_UNAUTHORIZED, expired=True)
+
+        # Maximum unused period
+        period = timedelta(days=10, hours=10, minutes=10, seconds=10)
+        self.token.max_unused_period = period
+        self.token.save()
+
+        # Can't use after period if token was never used (last_used is None)
+        self.assertIsNone(self.token.last_used)
+        with mock.patch('desecapi.models.timezone.now', return_value=self.token.created + period + second):
+            self.assertAuthenticationStatus(HTTP_401_UNAUTHORIZED, plain=plain, expired=True)
+            self.assertIsNone(Token.objects.get(pk=self.token.pk).last_used)  # unchanged
+
+        # Can use after half the period
+        with mock.patch('desecapi.models.timezone.now', return_value=self.token.created + period/2):
+            self.assertAuthenticationStatus(HTTP_200_OK, plain=plain)
+        self.token = Token.objects.get(pk=self.token.pk)  # update last_used field
+
+        # Can't use once another period is over
+        with mock.patch('desecapi.models.timezone.now', return_value=self.token.last_used + period + second):
+            self.assertAuthenticationStatus(HTTP_401_UNAUTHORIZED, plain=plain, expired=True)
+            self.assertEqual(self.token.last_used, Token.objects.get(pk=self.token.pk).last_used)  # unchanged
+
+        # ... but one second before, and also for one more period
+        with mock.patch('desecapi.models.timezone.now', return_value=self.token.last_used + period - second):
+            self.assertAuthenticationStatus(HTTP_200_OK, plain=plain)
+        with mock.patch('desecapi.models.timezone.now', return_value=self.token.last_used + 2*period - 2*second):
+            self.assertAuthenticationStatus(HTTP_200_OK, plain=plain)
+
+        # No maximum age: can use now and in ten years
+        self.token.max_unused_period = None
+        self.token.save()
+
+        self.assertAuthenticationStatus(HTTP_200_OK, plain=plain)
+        with mock.patch('desecapi.models.timezone.now', return_value=timezone.now() + timedelta(days=3650)):
+            self.assertAuthenticationStatus(HTTP_200_OK, plain=plain)
+
+    def test_token_max_age_max_unused_period(self):
+        hour = timedelta(hours=1)
+        self.token.max_age = 3 * hour
+        self.token.max_unused_period = hour
+        self.token.save()
+
+        # max_unused_period wins if tighter than max_age
+        with mock.patch('desecapi.models.timezone.now', return_value=self.token.created + 1.25*hour):
+            self.assertAuthenticationStatus(HTTP_401_UNAUTHORIZED, expired=True)
+
+        # Can use immediately
+        self.assertAuthenticationStatus(HTTP_200_OK)
+
+        # Can use continuously within max_unused_period
+        with mock.patch('desecapi.models.timezone.now', return_value=self.token.created + 0.75*hour):
+            self.assertAuthenticationStatus(HTTP_200_OK)
+        with mock.patch('desecapi.models.timezone.now', return_value=self.token.created + 1.5*hour):
+            self.assertAuthenticationStatus(HTTP_200_OK)
+
+        # max_unused_period wins again if tighter than max_age
+        with mock.patch('desecapi.models.timezone.now', return_value=self.token.created + 2.75*hour):
+            self.assertAuthenticationStatus(HTTP_401_UNAUTHORIZED, expired=True)
+
+        # Can use continuously within max_unused_period
+        with mock.patch('desecapi.models.timezone.now', return_value=self.token.created + 2.25*hour):
+            self.assertAuthenticationStatus(HTTP_200_OK)
+        with mock.patch('desecapi.models.timezone.now', return_value=self.token.created + 2.75*hour):
+            self.assertAuthenticationStatus(HTTP_200_OK)
+
+        # max_age wins again if tighter than max_unused_period
+        with mock.patch('desecapi.models.timezone.now', return_value=self.token.created + 3.25*hour):
+            self.assertAuthenticationStatus(HTTP_401_UNAUTHORIZED, expired=True)

+ 20 - 8
api/desecapi/tests/test_tokens.py

@@ -26,7 +26,7 @@ class TokenPermittedTestCase(DomainOwnerTestCase):
         self.assertEqual(len(response.data), 2)
         self.assertIn('id', response.data[0])
         self.assertFalse(any(field in response.data[0] for field in ['token', 'key', 'value']))
-        self.assertFalse(any(token.encode() in response.content for token in [self.token.plain, self.token2.plain,]))
+        self.assertFalse(any(token.encode() in response.content for token in [self.token.plain, self.token2.plain]))
         self.assertNotContains(response, self.token.plain)
 
     def test_delete_my_token(self):
@@ -48,8 +48,10 @@ class TokenPermittedTestCase(DomainOwnerTestCase):
         self.assertStatus(response, status.HTTP_200_OK)
         self.assertEqual(
             set(response.data.keys()),
-            {'id', 'created', 'last_used', 'name', 'perm_manage_tokens', 'allowed_subnets'}
+            {'id', 'created', 'last_used', 'max_age', 'max_unused_period', 'name', 'perm_manage_tokens',
+             'allowed_subnets', 'is_valid'}
         )
+        self.assertFalse(any(token.encode() in response.content for token in [self.token.plain, self.token2.plain]))
 
     def test_retrieve_other_token(self):
         token_id = Token.objects.get(user=self.user).id
@@ -62,11 +64,20 @@ class TokenPermittedTestCase(DomainOwnerTestCase):
         url = self.reverse('v1:token-detail', pk=self.token.id)
 
         for method in [self.client.patch, self.client.put]:
-            data = {'name': method.__name__, 'allowed_subnets': ['127.0.0.0/8']}
-            response = method(url, data=data)
-            self.assertStatus(response, status.HTTP_200_OK)
-            self.assertEqual(Token.objects.get(pk=self.token.id).name, method.__name__)
-            self.assertEqual(Token.objects.get(pk=self.token.id).allowed_subnets, [IPv4Network('127.0.0.0/8')])
+            datas = [
+                {'name': method.__name__},
+                {'allowed_subnets': ['127.0.0.0/8']},
+                {'allowed_subnets': ['127.0.0.0/8', '::/0']},
+                {'max_age': '365 00:10:33.123456'},
+                {'max_age': None},
+                {'max_unused_period': '365 00:10:33.123456'},
+                {'max_unused_period': None},
+            ]
+            for data in datas:
+                response = method(url, data=data)
+                self.assertStatus(response, status.HTTP_200_OK)
+                for k, v in data.items():
+                    self.assertEqual(response.data[k], v)
 
         # Revoke token management permission
         response = self.client.patch(url, data={'perm_manage_tokens': False})
@@ -90,7 +101,8 @@ class TokenPermittedTestCase(DomainOwnerTestCase):
             self.assertStatus(response, status.HTTP_201_CREATED)
             self.assertEqual(
                 set(response.data.keys()),
-                {'id', 'created', 'last_used', 'name', 'perm_manage_tokens', 'allowed_subnets', 'token'}
+                {'id', 'created', 'last_used', 'max_age', 'max_unused_period', 'name', 'perm_manage_tokens',
+                 'allowed_subnets', 'is_valid', 'token'}
             )
             self.assertEqual(response.data['name'], data.get('name', ''))
             self.assertEqual(response.data['allowed_subnets'], data.get('allowed_subnets', ['0.0.0.0/0', '::/0']))

+ 43 - 1
docs/auth/tokens.rst

@@ -33,6 +33,8 @@ A JSON object representing a token has the following structure::
             "0.0.0.0/0",
             "::/0"
         ],
+        "max_age": "365 00:00:00",
+        "max_unused_period": null,
         "token": "4pnk7u-NHvrEkFzrhFDRTjGFyX_S"
     }
 
@@ -60,9 +62,16 @@ Field details:
     Token ID, used for identification only (e.g. when deleting a token). This
     is *not* the token value.
 
+``is_valid``
+    :Access mode: read-only
+    :Type: boolean
+
+    Indicates whether this token is valid.  Currently, this reflects validity
+    based on ``max_age`` and ``max_unused_period``.
+
 ``last_used``
     :Access mode: read-only
-    :Type: timestamp (nullable)
+    :Type: timestamp or ``null``
 
     Timestamp of when the token was last successfully authenticated, or
     ``null`` if the token has never been used.
@@ -72,6 +81,30 @@ Field details:
     executed because it was found that the token did not have sufficient
     permission, this field will still be updated.
 
+``max_age``
+    :Access mode: read, write
+    :Type: string (time duration: ``[DD] [HH:[MM:]]ss[.uuuuuu]``) or ``null``
+
+    Maximum token age.  If ``created + max_age`` is less than the current time,
+    the token is invalidated.  Invalidated tokens are not automatically deleted
+    and can be resurrected by adjusting the expiration settings (using another
+    valid token with sufficient privileges).
+
+    If ``null``, the token is valid regardless of age (setting disabled).
+
+``max_unused_period``
+    :Access mode: read, write
+    :Type: string (time duration: ``[DD] [HH:[MM:]]ss[.uuuuuu]``) or ``null``
+
+    Maximum allowed time period of disuse without invalidating the token.  If
+    ``max(created, last_used) + max_unused_period`` is less than the current
+    time, the token is invalidated.  Invalidated tokens are not automatically
+    deleted and can be resurrected by adjusting the expiration settings (using
+    another valid token with sufficient privileges).
+
+    If ``null``, the token is valid regardless of prior usage (setting
+    disabled).
+
 ``name``
     :Access mode: read, write
     :Type: string
@@ -146,6 +179,15 @@ configuration during creation:
 - ``perm_manage_tokens``:  If set to ``true``, the token can be used to
   authorize token management operations (as described in this chapter).
 
+Additionally, you can configure an expiration policy with the following fields:
+
+- ``max_age``:  Force token expiration when a certain time period has passed
+  since its creation.  If ``null``, the token does not expire due to age.
+
+- ``max_unused_period``:  Require that the token is used a least once within
+  the given time period to prevent it from expiring.  If ``null``, the token
+  does not expire due to it not being used.
+
 If a field is provided but has invalid content, ``400 Bad Request`` is
 returned, with error details in the body.
 

+ 49 - 0
webapp/src/components/Field/Checkbox.vue

@@ -0,0 +1,49 @@
+<template>
+  <v-checkbox
+    :label="label"
+    :disabled="disabled || readonly"
+    :error-messages="errorMessages"
+    :input-value="value"
+    :required="required"
+    :rules="[v => !required || !!v || 'Required.']"
+    @change="change"
+  />
+</template>
+
+<script>
+export default {
+  name: 'Checkbox',
+  props: {
+    disabled: {
+      type: Boolean,
+      required: false,
+    },
+    errorMessages: {
+      type: [String, Array],
+      default: () => [],
+    },
+    label: {
+      type: String,
+      required: false,
+    },
+    readonly: {
+      type: Boolean,
+      required: false,
+    },
+    required: {
+      type: Boolean,
+      default: false,
+    },
+    value: {
+      type: Boolean,
+      required: true,
+    },
+  },
+  methods: {
+    change(event) {
+      this.$emit('input', event);
+      this.$emit('dirty', {target: this.$el});
+    },
+  },
+};
+</script>

+ 7 - 1
webapp/src/components/Field/GenericText.vue

@@ -6,6 +6,8 @@
     :value="value"
     :type="type || ''"
     :placeholder="required ? '' : '(optional)'"
+    :hint="hint"
+    persistent-hint
     :required="required"
     :rules="[v => !required || !!v || 'Required.']"
     @input="$emit('input', $event)"
@@ -26,6 +28,10 @@ export default {
       type: [String, Array],
       default: () => [],
     },
+    hint: {
+      type: String,
+      default: '',
+    },
     label: {
       type: String,
       required: false,
@@ -40,7 +46,7 @@ export default {
     },
     value: {
       type: [String, Number],
-      required: true,
+      required: false,
     },
     type: {
       type: String,

+ 1 - 1
webapp/src/components/Field/SwitchBox.vue → webapp/src/components/Field/Switchbox.vue

@@ -12,7 +12,7 @@
 
 <script>
 export default {
-  name: 'SwitchBox',
+  name: 'Switchbox',
   props: {
     disabled: {
       type: Boolean,

+ 4 - 2
webapp/src/views/CrudList.vue

@@ -302,11 +302,12 @@
 import { HTTP, withWorking } from '@/utils';
 import RRSetType from '@/components/Field/RRSetType';
 import TimeAgo from '@/components/Field/TimeAgo';
+import Checkbox from '@/components/Field/Checkbox';
 import Code from '@/components/Field/Code';
 import GenericText from '@/components/Field/GenericText';
 import Record from '@/components/Field/Record';
 import RecordList from '@/components/Field/RecordList';
-import SwitchBox from '@/components/Field/SwitchBox';
+import Switchbox from '@/components/Field/Switchbox';
 import TTL from '@/components/Field/TTL';
 
 // safely access deeply nested objects
@@ -317,7 +318,8 @@ export default {
   components: {
     RRSetType,
     TimeAgo,
-    SwitchBox,
+    Switchbox,
+    Checkbox,
     Code,
     GenericText,
     Record,

+ 35 - 1
webapp/src/views/TokenList.vue

@@ -68,7 +68,41 @@ export default {
             value: 'perm_manage_tokens',
             readonly: false,
             writeOnCreate: true,
-            datatype: 'SwitchBox',
+            datatype: 'Switchbox',
+            searchable: false,
+          },
+          max_age: {
+            name: 'item.max_age',
+            text: 'Maximum age',
+            align: 'left',
+            sortable: true,
+            value: 'max_age',
+            readonly: false,
+            writeOnCreate: true,
+            datatype: 'GenericText',
+            searchable: false,
+            fieldProps: () => ({ hint: 'Format: [DD] [HH:[MM:]]ss' }),
+          },
+          max_unused_period: {
+            name: 'item.max_unused_period',
+            text: 'Maximum unused period',
+            align: 'left',
+            sortable: true,
+            value: 'max_unused_period',
+            readonly: false,
+            writeOnCreate: true,
+            datatype: 'GenericText',
+            searchable: false,
+            fieldProps: () => ({ hint: 'Format: [DD] [HH:[MM:]]ss' }),
+          },
+          is_valid: {
+            name: 'item.is_valid',
+            text: 'Valid',
+            align: 'left',
+            sortable: true,
+            value: 'is_valid',
+            readonly: true,
+            datatype: 'Checkbox',
             searchable: false,
           },
           created: {