diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 352d42d98..aa0e4251f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -58,6 +58,7 @@ jobs: pip install bson pip install pymongo pip install jwcrypto + pip install passlib pip install git+https://github.com/TeskaLabs/asab.git#egg=asab[encryption] - name: Test with unittest diff --git a/CHANGELOG.md b/CHANGELOG.md index bdfeb2bd8..2aa3f40a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## v24.06 ### Pre-releases +- `v24.06-alpha11` - `v24.06-alpha10` - `v24.06-beta3` - `v24.06-alpha9` @@ -29,6 +30,7 @@ - Fix the initialization of NoTenantsError (#346, `v24.06-alpha2`) ### Features +- Client secret management (#359, `v24.06-alpha11`) - External login provider label contains just the display name (#352, `v24.06-alpha10`) - ElasticSearch index and Kibana space authorization (#353, `v24.06-alpha7.2`) - Disable special characters in tenant ID (#349, `v24.06-alpha6`) diff --git a/Dockerfile b/Dockerfile index 33533001e..6e70ae038 100644 --- a/Dockerfile +++ b/Dockerfile @@ -37,6 +37,7 @@ RUN apk add --no-cache \ fastjsonschema \ passlib \ bcrypt \ + argon2_cffi \ python-ldap \ aiomysql \ jinja2 \ diff --git a/seacatauth/client/handler.py b/seacatauth/client/handler.py index af7447325..6706a2387 100644 --- a/seacatauth/client/handler.py +++ b/seacatauth/client/handler.py @@ -5,7 +5,7 @@ import asab.exceptions from seacatauth.decorators import access_control -from .service import REGISTER_CLIENT_SCHEMA, UPDATE_CLIENT_SCHEMA, CLIENT_TEMPLATES +from .service import REGISTER_CLIENT_SCHEMA, UPDATE_CLIENT_SCHEMA, CLIENT_TEMPLATES, is_client_confidential # @@ -84,12 +84,8 @@ async def get(self, request): Get client by client_id """ client_id = request.match_info["client_id"] - result = self._rest_normalize( - await self.ClientService.get(client_id), - include_client_secret=True) - return asab.web.rest.json_response( - request, result - ) + result = self._rest_normalize(await self.ClientService.get(client_id)) + return asab.web.rest.json_response(request, result) @access_control("seacat:client:access") @@ -120,11 +116,18 @@ async def register(self, request, *, json_data): if not self.ClientService._AllowCustomClientID: raise asab.exceptions.ValidationError("Specifying custom client_id is not allowed.") json_data["_custom_client_id"] = json_data.pop("preferred_client_id") - result = await self.ClientService.register(**json_data) - data = self._rest_normalize( - await self.ClientService.get(result["client_id"]), - include_client_secret=True) - return asab.web.rest.json_response(request, data=data) + client_id = await self.ClientService.register(**json_data) + client = await self.ClientService.get(client_id) + response_data = self._rest_normalize(client) + + if is_client_confidential(client): + # Set a secret for confidential client + client_secret, client_secret_expires_at = await self.ClientService.reset_secret(client_id) + response_data["client_secret"] = client_secret + if client_secret_expires_at: + response_data["client_secret_expires_at"] = client_secret_expires_at + + return asab.web.rest.json_response(request, data=response_data) @asab.web.rest.json_schema_handler(UPDATE_CLIENT_SCHEMA) @@ -151,10 +154,13 @@ async def reset_secret(self, request): Reset client secret """ client_id = request.match_info["client_id"] - response = await self.ClientService.reset_secret(client_id) + client_secret, client_secret_expires_at = await self.ClientService.reset_secret(client_id) + response_data = {"client_secret": client_secret} + if client_secret_expires_at: + response_data["client_secret_expires_at"] = client_secret_expires_at return asab.web.rest.json_response( request, - data=response, + data=response_data, ) @@ -171,7 +177,7 @@ async def delete(self, request): ) - def _rest_normalize(self, client: dict, include_client_secret: bool = False): + def _rest_normalize(self, client: dict): cookie_service = self.ClientService.App.get_service("seacatauth.CookieService") rest_data = { @@ -181,8 +187,8 @@ def _rest_normalize(self, client: dict, include_client_secret: bool = False): } rest_data["client_id"] = rest_data["_id"] rest_data["client_id_issued_at"] = int(rest_data["_c"].timestamp()) - if include_client_secret and "__client_secret" in client: - rest_data["client_secret"] = client["__client_secret"] + if "__client_secret" in client: + rest_data["client_secret"] = True if "client_secret_expires_at" in rest_data: rest_data["client_secret_expires_at"] = int(rest_data["client_secret_expires_at"].timestamp()) rest_data["cookie_name"] = cookie_service.get_cookie_name(rest_data["_id"]) diff --git a/seacatauth/client/service.py b/seacatauth/client/service.py index a61bb04c1..df7f1268c 100644 --- a/seacatauth/client/service.py +++ b/seacatauth/client/service.py @@ -3,6 +3,7 @@ import re import secrets import urllib.parse +import passlib.hash import asab.storage.exceptions import asab.exceptions @@ -280,9 +281,7 @@ async def count(self, query_filter: dict = None): async def get(self, client_id: str): cookie_svc = self.App.get_service("seacatauth.CookieService") - client = await self.StorageService.get(self.ClientCollection, client_id, decrypt=["__client_secret"]) - if "__client_secret" in client: - client["__client_secret"] = client["__client_secret"].decode("ascii") + client = await self.StorageService.get(self.ClientCollection, client_id) client["cookie_name"] = cookie_svc.get_cookie_name(client_id) return client @@ -303,61 +302,27 @@ async def register( :type _custom_client_id: str :return: Dict containing the issued client_id and client_secret. """ - response_types = kwargs.get("response_types", frozenset(["code"])) + response_types = kwargs.get("response_types", {"code"}) for v in response_types: assert v in RESPONSE_TYPES - grant_types = kwargs.get("grant_types", frozenset(["authorization_code"])) + grant_types = kwargs.get("grant_types", {"authorization_code"}) for v in grant_types: assert v in GRANT_TYPES application_type = kwargs.get("application_type", "web") assert application_type in APPLICATION_TYPES - token_endpoint_auth_method = kwargs.get("token_endpoint_auth_method", "none") - # TODO: The default should be "client_secret_basic". Change this once implemented. - assert token_endpoint_auth_method in TOKEN_ENDPOINT_AUTH_METHODS - if _custom_client_id is not None: client_id = _custom_client_id - L.warning("Creating a client with custom ID", struct_data={"client_id": client_id}) + L.warning("Creating a client with custom ID.", struct_data={"client_id": client_id}) else: client_id = secrets.token_urlsafe(self.ClientIdLength) upsertor = self.StorageService.upsertor(self.ClientCollection, obj_id=client_id) - if token_endpoint_auth_method == "none": - # The client is PUBLIC - # Clients incapable of maintaining the confidentiality of their - # credentials (e.g., clients executing on the device used by the - # resource owner, such as an installed native application or a web - # browser-based application), and incapable of secure client - # authentication via any other means. - # The authorization server MAY establish a client authentication method - # with public clients. However, the authorization server MUST NOT rely - # on public client authentication for the purpose of identifying the - # client. - client_secret = None - client_secret_expires_at = None - elif token_endpoint_auth_method == "client_secret_basic": - raise NotImplementedError("Token endpoint auth method 'client_secret_basic' is not supported.") - # TODO: Finish implementing authorization with client secret - # The client is CONFIDENTIAL - # Clients capable of maintaining the confidentiality of their - # credentials (e.g., client implemented on a secure server with - # restricted access to the client credentials), or capable of secure - # client authentication using other means. - # Confidential clients are typically issued (or establish) a set of - # client credentials used for authenticating with the authorization - # server (e.g., password, public/private key pair). - client_secret, client_secret_expires_at = self._generate_client_secret() - upsertor.set("__client_secret", client_secret.encode("ascii"), encrypt=True) - if client_secret_expires_at is not None: - upsertor.set("client_secret_expires_at", client_secret_expires_at) - else: - # The client is CONFIDENTIAL - # Valid method type, not implemented yet - raise NotImplementedError("token_endpoint_auth_method = {!r}".format(token_endpoint_auth_method)) - + # TODO: The default should be "client_secret_basic". + token_endpoint_auth_method = kwargs.get("token_endpoint_auth_method", "none") + assert token_endpoint_auth_method in TOKEN_ENDPOINT_AUTH_METHODS upsertor.set("token_endpoint_auth_method", token_endpoint_auth_method) self._check_redirect_uris(redirect_uris, application_type, grant_types) @@ -385,10 +350,11 @@ async def register( upsertor.set("session_expiration", session_expiration) # Optional client metadata - for k in frozenset([ + for k in { "client_name", "client_uri", "logout_uri", "cookie_domain", "custom_data", "login_uri", "authorize_anonymous_users", "authorize_uri", "cookie_webhook_uri", "cookie_entry_uri", - "anonymous_cid"]): + "anonymous_cid" + }: v = kwargs.get(k) if v is not None and not (isinstance(v, str) and len(v) == 0): upsertor.set(k, v) @@ -398,40 +364,26 @@ async def register( except asab.storage.exceptions.DuplicateError: raise asab.exceptions.Conflict(key="client_id", value=client_id) - L.log(asab.LOG_NOTICE, "Client created", struct_data={"client_id": client_id}) - - response = { - "client_id": client_id, - "client_id_issued_at": int(datetime.datetime.now(datetime.timezone.utc).timestamp())} - - if client_secret is not None: - response["client_secret"] = client_secret - if client_secret_expires_at is not None: - response["client_secret_expires_at"] = client_secret_expires_at - - return response + L.log(asab.LOG_NOTICE, "Client created.", struct_data={"client_id": client_id}) + return client_id async def reset_secret(self, client_id: str): + """ + Set or reset client secret + """ + # TODO: Use M2M credentials provider. client = await self.get(client_id) - if client["token_endpoint_auth_method"] == "none": - # The authorization server MAY establish a client authentication method with public clients. - # However, the authorization server MUST NOT rely on public client authentication for the purpose - # of identifying the client. [rfc6749#section-3.1.2] - raise asab.exceptions.ValidationError("Cannot set secret for public client") upsertor = self.StorageService.upsertor(self.ClientCollection, obj_id=client_id, version=client["_v"]) client_secret, client_secret_expires_at = self._generate_client_secret() - upsertor.set("__client_secret", client_secret.encode("ascii"), encrypt=True) + client_secret_hash = passlib.hash.argon2.hash(client_secret) + upsertor.set("__client_secret", client_secret_hash) if client_secret_expires_at is not None: upsertor.set("client_secret_expires_at", client_secret_expires_at) - await upsertor.execute(event_type=EventTypes.CLIENT_SECRET_RESET) - L.log(asab.LOG_NOTICE, "Client secret updated", struct_data={"client_id": client_id}) - response = {"client_secret": client_secret} - if client_secret_expires_at is not None: - response["client_secret_expires_at"] = client_secret_expires_at - - return response + await upsertor.execute(event_type=EventTypes.CLIENT_SECRET_RESET) + L.log(asab.LOG_NOTICE, "Client secret updated.", struct_data={"client_id": client_id}) + return client_secret, client_secret_expires_at async def update(self, client_id: str, **kwargs): @@ -471,14 +423,15 @@ async def update(self, client_id: str, **kwargs): upsertor.set(k, v) await upsertor.execute(event_type=EventTypes.CLIENT_UPDATED) - L.log(asab.LOG_NOTICE, "Client updated", struct_data={ + L.log(asab.LOG_NOTICE, "Client updated.", struct_data={ "client_id": client_id, - "fields": " ".join(client_update.keys())}) + "fields": " ".join(client_update.keys()) + }) async def delete(self, client_id: str): await self.StorageService.delete(self.ClientCollection, client_id) - L.log(asab.LOG_NOTICE, "Client deleted", struct_data={"client_id": client_id}) + L.log(asab.LOG_NOTICE, "Client deleted.", struct_data={"client_id": client_id}) async def validate_client_authorize_options( @@ -504,22 +457,39 @@ async def validate_client_authorize_options( return True - async def authenticate_client(self, client: dict, client_secret: str = None): + async def authenticate_client(self, client: dict, client_id: str, client_secret: str = None): """ - Verify client credentials (client_id and client_secret). + Verify client ID and secret. """ - # TODO: Use a client credential provider. + if client_id != client["_id"]: + raise exceptions.ClientError( + "Client IDs do not match.", + client_id=client["_id"], + attempted_client_id=client_id + ) if client_secret is None: # The client MAY omit the parameter if the client secret is an empty string. # [rfc6749#section-2.3.1] client_secret = "" + + client_secret_hash = client.get("__client_secret") + if not client_secret_hash: + # No client secret is set, none is expected! + if not client_secret: + return True + else: + raise exceptions.ClientError( + "Got non-empty secret from non-confidential client.", client_id=client["_id"]) + if "client_secret_expires_at" in client \ and client["client_secret_expires_at"] != 0 \ and client["client_secret_expires_at"] < datetime.datetime.now(datetime.timezone.utc): - L.warning("Client secret expired.", struct_data={"client_id": client["_id"]}) - if client_secret != client.get("__client_secret", ""): - L.warning("Incorrect client secret.", struct_data={"client_id": client["_id"]}) - return True + raise exceptions.ClientError("Expired client secret.", client_id=client["_id"]) + + if not passlib.hash.argon2.verify(client_secret, client_secret_hash): + raise exceptions.ClientError("Incorrect client secret.", client_id=client["_id"]) + else: + return True def _check_grant_types(self, grant_types, response_types): @@ -622,3 +592,13 @@ def validate_redirect_uri(redirect_uri: str, registered_uris: list, validation_m raise ValueError("Unsupported redirect_uri_validation_method: {!r}".format(validation_method)) return False + + +def is_client_confidential(client: dict): + token_endpoint_auth_method = client.get("token_endpoint_auth_method", "none") + if token_endpoint_auth_method == "none": + return False + elif token_endpoint_auth_method in {"client_secret_basic", "client_secret_post"}: + return True + else: + raise NotImplementedError("Unsupported token_endpoint_auth_method: {!r}".format(token_endpoint_auth_method))