From a0164d9128fe974d6a080b538df73874b3d815e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robin=20Hru=C5=A1ka?= Date: Wed, 10 Apr 2024 11:10:38 +0200 Subject: [PATCH 1/7] add argon2 to dependencies --- Dockerfile | 1 + 1 file changed, 1 insertion(+) 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 \ From 89c03a153b1ead961e9472cd02e3dc48298730a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robin=20Hru=C5=A1ka?= Date: Wed, 10 Apr 2024 11:25:51 +0200 Subject: [PATCH 2/7] add support for client secret --- seacatauth/client/handler.py | 11 +++-- seacatauth/client/service.py | 88 +++++++++++++++++++++--------------- 2 files changed, 58 insertions(+), 41 deletions(-) diff --git a/seacatauth/client/handler.py b/seacatauth/client/handler.py index af7447325..a54f30f75 100644 --- a/seacatauth/client/handler.py +++ b/seacatauth/client/handler.py @@ -120,9 +120,9 @@ 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) + client_id = await self.ClientService.register(**json_data) data = self._rest_normalize( - await self.ClientService.get(result["client_id"]), + await self.ClientService.get(client_id), include_client_secret=True) return asab.web.rest.json_response(request, data=data) @@ -181,8 +181,11 @@ 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: + if include_client_secret: + rest_data["client_secret"] = client["__client_secret"] + else: + 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..66e17b5d9 100644 --- a/seacatauth/client/service.py +++ b/seacatauth/client/service.py @@ -1,8 +1,10 @@ +import base64 import datetime import logging import re import secrets import urllib.parse +import passlib.hash import asab.storage.exceptions import asab.exceptions @@ -36,8 +38,8 @@ ] TOKEN_ENDPOINT_AUTH_METHODS = [ "none", - # "client_secret_basic", - # "client_secret_post", + "client_secret_basic", + "client_secret_post", # "client_secret_jwt", # "private_key_jwt" ] @@ -280,7 +282,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"]) + client = await self.StorageService.get(self.ClientCollection, client_id) if "__client_secret" in client: client["__client_secret"] = client["__client_secret"].decode("ascii") client["cookie_name"] = cookie_svc.get_cookie_name(client_id) @@ -303,11 +305,11 @@ 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 @@ -320,7 +322,7 @@ async def register( 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) @@ -338,9 +340,7 @@ async def register( # 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 + elif token_endpoint_auth_method in {"client_secret_basic", "client_secret_post"}: # The client is CONFIDENTIAL # Clients capable of maintaining the confidentiality of their # credentials (e.g., client implemented on a secure server with @@ -350,7 +350,8 @@ async def register( # 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) + 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) else: @@ -385,10 +386,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,18 +400,8 @@ 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): @@ -419,18 +411,20 @@ async def reset_secret(self, client_id: str): # 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}) + 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 @@ -471,14 +465,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 +499,41 @@ 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 + ) + + # TODO: Use M2M credentials provider. 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 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): From d099cdc5774effc297355252288325976af679f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robin=20Hru=C5=A1ka?= Date: Wed, 10 Apr 2024 12:02:58 +0200 Subject: [PATCH 3/7] add support for client secret --- seacatauth/client/handler.py | 39 ++++++++++---------- seacatauth/client/service.py | 70 ++++++++++-------------------------- 2 files changed, 39 insertions(+), 70 deletions(-) diff --git a/seacatauth/client/handler.py b/seacatauth/client/handler.py index a54f30f75..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") @@ -121,10 +117,17 @@ async def register(self, request, *, json_data): raise asab.exceptions.ValidationError("Specifying custom client_id is not allowed.") json_data["_custom_client_id"] = json_data.pop("preferred_client_id") client_id = await self.ClientService.register(**json_data) - data = self._rest_normalize( - await self.ClientService.get(client_id), - include_client_secret=True) - return asab.web.rest.json_response(request, data=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 = { @@ -182,10 +188,7 @@ 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 "__client_secret" in client: - if include_client_secret: - rest_data["client_secret"] = client["__client_secret"] - else: - rest_data["client_secret"] = True + 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 66e17b5d9..ea6294c19 100644 --- a/seacatauth/client/service.py +++ b/seacatauth/client/service.py @@ -283,8 +283,6 @@ 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) - if "__client_secret" in client: - client["__client_secret"] = client["__client_secret"].decode("ascii") client["cookie_name"] = cookie_svc.get_cookie_name(client_id) return client @@ -316,10 +314,6 @@ async def register( 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}) @@ -327,38 +321,9 @@ async def register( 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 in {"client_secret_basic", "client_secret_post"}: - # 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() - 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) - 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) @@ -405,13 +370,11 @@ async def register( 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() client_secret_hash = passlib.hash.argon2.hash(client_secret) @@ -421,11 +384,7 @@ async def reset_secret(self, client_id: str): 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 + return client_secret, client_secret_expires_at async def update(self, client_id: str, **kwargs): @@ -509,8 +468,6 @@ async def authenticate_client(self, client: dict, client_id: str, client_secret: client_id=client["_id"], attempted_client_id=client_id ) - - # TODO: Use M2M credentials provider. if client_secret is None: # The client MAY omit the parameter if the client secret is an empty string. # [rfc6749#section-2.3.1] @@ -530,7 +487,7 @@ async def authenticate_client(self, client: dict, client_id: str, client_secret: and client["client_secret_expires_at"] < datetime.datetime.now(datetime.timezone.utc): raise exceptions.ClientError("Expired client secret.", client_id=client["_id"]) - if passlib.hash.argon2.verify(client_secret, client_secret_hash): + if not passlib.hash.argon2.verify(client_secret, client_secret_hash): raise exceptions.ClientError("Incorrect client secret.", client_id=client["_id"]) else: return True @@ -636,3 +593,12 @@ 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)) From a74208732b71497fe3f185676c77e7a1247d9bab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robin=20Hru=C5=A1ka?= Date: Wed, 10 Apr 2024 12:09:02 +0200 Subject: [PATCH 4/7] disable TOKEN_ENDPOINT_AUTH options until implemented --- seacatauth/client/service.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/seacatauth/client/service.py b/seacatauth/client/service.py index ea6294c19..3be1e8347 100644 --- a/seacatauth/client/service.py +++ b/seacatauth/client/service.py @@ -38,8 +38,8 @@ ] TOKEN_ENDPOINT_AUTH_METHODS = [ "none", - "client_secret_basic", - "client_secret_post", + # "client_secret_basic", + # "client_secret_post", # "client_secret_jwt", # "private_key_jwt" ] From de53ea80c49da082e777016b773d4231d2d6787f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robin=20Hru=C5=A1ka?= Date: Wed, 10 Apr 2024 12:11:12 +0200 Subject: [PATCH 5/7] update CHANGELOG.md --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) 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`) From 2f6563f4b6d45e0deb853949eee331cf77a5ef3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robin=20Hru=C5=A1ka?= Date: Wed, 10 Apr 2024 12:11:43 +0200 Subject: [PATCH 6/7] flake8 --- seacatauth/client/service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/seacatauth/client/service.py b/seacatauth/client/service.py index 3be1e8347..df7f1268c 100644 --- a/seacatauth/client/service.py +++ b/seacatauth/client/service.py @@ -1,4 +1,3 @@ -import base64 import datetime import logging import re @@ -594,6 +593,7 @@ def validate_redirect_uri(redirect_uri: str, registered_uris: list, validation_m 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": From 93713fade0ed309f45e9be54868046fd95b1a8b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robin=20Hru=C5=A1ka?= Date: Wed, 10 Apr 2024 12:41:13 +0200 Subject: [PATCH 7/7] add passlib to test dependencies --- .github/workflows/ci.yml | 1 + 1 file changed, 1 insertion(+) 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