From 71d6ed4e3c416a00e572f89c9599de9ad79b0038 Mon Sep 17 00:00:00 2001 From: Ethan Paul Date: Sat, 22 Feb 2020 22:07:37 -0500 Subject: [PATCH] Fix linting and typing errors Document token model Fix typo accessing incorrect crypto hash in passlib Fix cyclical import around domain admin settings Deprecate unused token usage enum Document constants module Fix typing errors in fields module Fix creation order of database models Add uuid default factory for uuid field --- keyosk/config/storage.py | 2 +- keyosk/constants.py | 2 ++ keyosk/database/__init__.py | 14 ++++---- keyosk/database/_shared.py | 5 ++- keyosk/database/account.py | 6 ++-- keyosk/database/domain.py | 11 ++++--- keyosk/database/domain_admin.py | 8 +++-- keyosk/database/token.py | 57 ++++++++++++++++++++++----------- keyosk/datatypes.py | 9 +----- keyosk/fields.py | 12 +++---- 10 files changed, 75 insertions(+), 51 deletions(-) diff --git a/keyosk/config/storage.py b/keyosk/config/storage.py index a589638..8fdb118 100644 --- a/keyosk/config/storage.py +++ b/keyosk/config/storage.py @@ -49,7 +49,7 @@ class SQLiteStorageConfigSerializer(msh.Schema): :class:`KeyoskSQLiteStorageConfig` class. """ - path = custom_fields.Path() + path = custom_fields.PathString() pragmas = msh.fields.Dict(keys=msh.fields.String(), values=msh.fields.Raw()) # pylint: disable=unused-argument,no-self-use diff --git a/keyosk/constants.py b/keyosk/constants.py index 95f45d9..e6d8461 100644 --- a/keyosk/constants.py +++ b/keyosk/constants.py @@ -1,3 +1,5 @@ +"""Constant parameter definitions""" + DEFAULT_CONFIG_PATH = "/etc/keyosk/conf.toml" ENV_CONFIG_PATH = "KYSK_CONF_PATH" diff --git a/keyosk/database/__init__.py b/keyosk/database/__init__.py index 9026813..6aaced9 100644 --- a/keyosk/database/__init__.py +++ b/keyosk/database/__init__.py @@ -28,21 +28,21 @@ from keyosk import datatypes from keyosk.database._shared import INTERFACE as interface from keyosk.database._shared import KeyoskBaseModel from keyosk.database.account import Account +from keyosk.database.account import AccountAssignment +from keyosk.database.account_acl import AccountACLEntry from keyosk.database.domain import Domain -from keyosk.database.mappings import AccountACL -from keyosk.database.mappings import AccountAssignment -from keyosk.database.mappings import DomainAccessList -from keyosk.database.mappings import DomainAdmin -from keyosk.database.mappings import DomainPermission +from keyosk.database.domain import DomainAccessList +from keyosk.database.domain import DomainPermission +from keyosk.database.domain_admin import DomainAdmin MODELS: List[Type[KeyoskBaseModel]] = [ Account, - Domain, DomainAccessList, DomainPermission, DomainAdmin, - AccountACL, + Domain, + AccountACLEntry, AccountAssignment, ] diff --git a/keyosk/database/_shared.py b/keyosk/database/_shared.py index a165769..fc9b52c 100644 --- a/keyosk/database/_shared.py +++ b/keyosk/database/_shared.py @@ -11,6 +11,7 @@ Thus the :func:`initialize` function and :class:`KeyoskBaseModel` class need to separate modules, and for somewhat arbitrary reasons the base model was put here and the init function kept in init. """ +import uuid from typing import Any from typing import Generator from typing import List @@ -36,7 +37,9 @@ class KeyoskBaseModel(peewee.Model): class Meta: # pylint: disable=missing-docstring,too-few-public-methods database = INTERFACE - uuid = peewee.UUIDField(null=False, unique=True, primary_key=True) + uuid = peewee.UUIDField( + null=False, unique=True, primary_key=True, default=uuid.uuid4 + ) @staticmethod def dict_keys() -> List[str]: diff --git a/keyosk/database/account.py b/keyosk/database/account.py index a2e0ee6..f5638ad 100644 --- a/keyosk/database/account.py +++ b/keyosk/database/account.py @@ -66,7 +66,7 @@ class Account(KeyoskBaseModel): :returns: Boolean indicating whether the provided value matches the encrypted secret """ - return passlib.hash.pdkdf2_sha512.verify( + return passlib.hash.pbkdf2_sha512.verify( value, self.encrypted_server_set_secret ) @@ -75,7 +75,7 @@ class Account(KeyoskBaseModel): :param value: The string to set the encrypted client-set-secret to """ - self.encrypted_client_set_secret = passlib.hash.pdkdf2_sha512.hash(value) + self.encrypted_client_set_secret = passlib.hash.pbkdf2_sha512.hash(value) def update_server_set_secret(self, length: int = 42) -> str: """Update the server set secret @@ -84,7 +84,7 @@ class Account(KeyoskBaseModel): :returns: The new value of the server set secret """ value = secrets.token_urlsafe(length) - self.encrypted_server_set_secret = passlib.hash.pdkdf2_sha512.hash(value) + self.encrypted_server_set_secret = passlib.hash.pbkdf2_sha512.hash(value) return value @staticmethod diff --git a/keyosk/database/domain.py b/keyosk/database/domain.py index 6f3acee..4aff05f 100644 --- a/keyosk/database/domain.py +++ b/keyosk/database/domain.py @@ -5,7 +5,6 @@ from typing import List import peewee from keyosk.database._shared import KeyoskBaseModel -from keyosk.database.domain_admin import DomainAdmin class Domain(KeyoskBaseModel): @@ -29,8 +28,8 @@ class Domain(KeyoskBaseModel): be valid for :attribute lifespan_refresh: Number of seconds an an issued JWT refresh token should be valid for - :attribute administration: Container of additional settings related to the - administration of the domain itself + :property administration: Container of additional settings related to the + administration of the domain itself :property access_list_names: List of Access Control Lists under the domain that accounts can have permission entries on :property permission_names: List of permissions that can be assigned to an account's ACL @@ -53,7 +52,6 @@ class Domain(KeyoskBaseModel): enable_refresh = peewee.BooleanField(null=False) lifespan_access = peewee.IntegerField(null=False) lifespan_refresh = peewee.IntegerField(null=False) - administration = peewee.ForeignKeyField(DomainAdmin, null=False, unique=True) @property def access_list_names(self) -> List[str]: @@ -65,6 +63,11 @@ class Domain(KeyoskBaseModel): """Return the list of permission names from the backref""" return [item.name for item in self._permissions] + @property + def administration(self): + """Return administration settings container""" + return self._administration[0] + @staticmethod def dict_keys() -> List[str]: return [ diff --git a/keyosk/database/domain_admin.py b/keyosk/database/domain_admin.py index a11bdea..6e1d7d4 100644 --- a/keyosk/database/domain_admin.py +++ b/keyosk/database/domain_admin.py @@ -16,8 +16,9 @@ from typing import Tuple import peewee from keyosk.database._shared import KeyoskBaseModel -from keyosk.database.mappings import DomainAccessList -from keyosk.database.mappings import DomainPermission +from keyosk.database.domain import Domain +from keyosk.database.domain import DomainAccessList +from keyosk.database.domain import DomainPermission class DomainAdmin(KeyoskBaseModel): @@ -55,6 +56,9 @@ class DomainAdmin(KeyoskBaseModel): class Meta: # pylint: disable=missing-docstring,too-few-public-methods table_name = "domain_admin" + domain = peewee.ForeignKeyField( + Domain, unique=True, null=False, backref="_administration" + ) access_list = peewee.ForeignKeyField(DomainAccessList, null=True) domain_read = peewee.ForeignKeyField(DomainPermission, null=True) domain_update = peewee.ForeignKeyField(DomainPermission, null=True) diff --git a/keyosk/database/token.py b/keyosk/database/token.py index 93fcca4..d5c6ba2 100644 --- a/keyosk/database/token.py +++ b/keyosk/database/token.py @@ -1,5 +1,7 @@ +"""Access token model definition""" import datetime import json +import secrets from collections import OrderedDict from typing import Sequence @@ -13,42 +15,54 @@ from keyosk.database.domain import Domain class Token(KeyoskBaseModel): - class Meta: + """Issued access token storage model + + :attribute account: Account the token was issued to + :attribute domain: Domain the token was issued for + :attribute issuer: Value of the issuer parameter at generation time + :attribute issued: Datetime indicating when the token was issued + :attribute expires: Datetime indicating when the token expires + :attribute revoked: Whether the token has been revoked + :attribute refresh: Refresh token attached to the issued access token; can be + ``None`` if refresh tokens are disabled for the domain + :property claims: Claims generated for the token + + .. note:: Settings and parameters may be changed on linked records. However, the + ``claims`` property will always contain the set of claims as assigned at + issuance time. + """ + + class Meta: # pylint: disable=missing-docstring,too-few-public-methods table_name = "token" - account = peewee.ForeignKeyField(Account, backref="tokens") - domain = peewee.ForeignKeyField(Domain, backref="tokens") + account = peewee.ForeignKeyField(Account, backref="tokens", null=True) + domain = peewee.ForeignKeyField(Domain, backref="tokens", null=True) issuer = peewee.CharField(null=False) issued = peewee.DateTimeField(null=False, default=datetime.datetime.utcnow) expires = peewee.DateTimeField(null=False) revoked = peewee.BooleanField(null=False) + refresh = peewee.CharField(null=True) _claims = peewee.CharField(null=False) - _usage = peewee.CharField(null=False) @property - def claims(self): + def claims(self) -> datatypes.TokenClaims: + """Return the claims dictionary""" return json.loads(self._claims) @claims.setter - def claims(self, value): + def claims(self, value: datatypes.TokenClaims): + """Set the claims dictionary""" self._claims = json.dumps(value) - @property - def usage(self) -> datatypes.TokenUsage: - return datatypes.TokenUsage[self._usage] - - @usage.setter - def usage(self, value: datatypes.TokenUsage): - self._usage = value.name - def make_public_claims(self): + """Generate the public JWT claims from current state data""" return { "jti": self.uuid, "sub": self.account.username, "aud": self.domain.audience, "iss": self.issuer, - "exp": int(self.expires.timestamp()), - "iat": int(self.issued.timestamp()), + "exp": int(self.expires.timestamp()), # pylint: disable=no-member + "iat": int(self.issued.timestamp()), # pylint: disable=no-member } @classmethod @@ -58,16 +72,21 @@ class Token(KeyoskBaseModel): domain: Domain, issuer: str, lifespan: datetime.timedelta, - usage: datatypes.TokenUsage, permissions: Sequence[AccountACLEntry], + generate_refresh: bool, ): + """Create a new token using provided data + + This function is intentionally not documented, as I expect it will not survive + first contact with a practical implementation + """ new = cls( account=account, domain=domain, issuer=issuer, expires=(datetime.datetime.utcnow() + lifespan), - usage=usage, revoked=False, + refresh=secrets.token_urlsafe(42) if generate_refresh else None, ) acls = {} @@ -96,7 +115,7 @@ class Token(KeyoskBaseModel): } claims = new.make_public_claims() - claims.update({"ksk-usg": new.usage.value, "ksk-pem": bitmasks}) + claims.update({"ksk-pem": bitmasks}) new.claims = claims diff --git a/keyosk/datatypes.py b/keyosk/datatypes.py index 0ddc1a8..088b4e6 100644 --- a/keyosk/datatypes.py +++ b/keyosk/datatypes.py @@ -7,14 +7,7 @@ from typing import Union Extras = Dict[str, Union[int, float, bool, str, None]] -class TokenUsage(enum.Enum): - """Possible usage values for an issued JWT - - Values will be the value of the ``ksk-usg`` claim in the issued token - """ - - REFRESH = "ref" - ACCESS = "acc" +TokenClaims = Dict[str, Union[str, int, bool, Dict[str, int]]] @enum.unique diff --git a/keyosk/fields.py b/keyosk/fields.py index bdec0fc..b185954 100644 --- a/keyosk/fields.py +++ b/keyosk/fields.py @@ -59,7 +59,7 @@ class EnumItem(msh.fields.Field): return self.enum[self._from_pretty_name(value)] return self.enum[value] except ValueError as err: - raise msh.ValidationError(err) + raise msh.ValidationError(str(err)) except (KeyError, AttributeError) as err: raise msh.ValidationError(f"No {self.enum} named {err}") @@ -72,11 +72,11 @@ class EnumItem(msh.fields.Field): return value.replace("-", "_").upper() -class Path(msh.fields.String): +class PathString(msh.fields.String): """Translate between a string and a path object""" - def _serialize(self, value: Union[str, Path], *args, **kwargs) -> str: - return super()._serialize(str(value), *args, **kwargs) + def _serialize(self, value: Union[str, Path], attr, obj, **kwargs) -> str: + return super()._serialize(str(value), attr, obj, **kwargs) - def _deserialize(self, value: str, *args, **kwargs) -> Path: - return Path(super()._deserialize(value, *args, **kwargs)) + def _deserialize(self, value: str, attr, data, **kwargs) -> Path: + return Path(super()._deserialize(value, attr, data, **kwargs))