From db27c41b810bb965e2e112bfa8bb0c0adc3bb13c Mon Sep 17 00:00:00 2001 From: "C. Weaver" Date: Fri, 9 Feb 2024 22:22:28 -0500 Subject: [PATCH 1/4] Remove duplicated code --- scimma_admin/hopskotch_auth/api_views.py | 133 +---------------------- 1 file changed, 1 insertion(+), 132 deletions(-) diff --git a/scimma_admin/hopskotch_auth/api_views.py b/scimma_admin/hopskotch_auth/api_views.py index c0a80c6..3f46a2b 100644 --- a/scimma_admin/hopskotch_auth/api_views.py +++ b/scimma_admin/hopskotch_auth/api_views.py @@ -109,137 +109,6 @@ def do_scram_first(client_first: str): ex.save() return (ex,s) -def do_scram_final(client_final: str, sid: Optional[str]=None): - """ - Return: If successful, the (completed) SCRAMExchange and the SCRAM server object - """ - if sid: - print("Client supplied sid:",sid) - ex = SCRAMExchange.objects.get(sid=sid) - else: - # a bit ugly: To find the previously started exchange session, if any, we need to extract - # the nonce from the request. We can either reimplement the parsing logic, or underhandedly - # reach inside of scramp to use its parse function. We do the latter. - try: - parsed = scramp.core._parse_message(client_final, "client final", "crp") - except: - return Response(status=status.HTTP_400_BAD_REQUEST) - ex = SCRAMExchange.objects.get(j_nonce=parsed['r']) - # recreate the SCRAM server state from our stored exchange record - s = scramp.ScramMechanism("SCRAM-SHA-512").make_server(scram_user_lookup, - s_nonce=ex.s_nonce()) - s.set_client_first(ex.client_first) - s.get_server_first() # waste of time, but scramp requires this to be called - # if we reach this point, we are ready to process the second half of the exchange - s.set_client_final(client_final) - # if scramp hasn't objected, the authentication has now succeeded - return (ex,s) - -def parse_list_header(header: str): - return [v[1:-1] if v[0] == v[-1] == '"' else v for v in parse_http_list(header)] - -def parse_dict_header(header: str): - def unquote(v: str): - return v[1:-1] if v[0] == v[-1] == '"' else v - d = dict() - for item in parse_list_header(header): - if '=' in item: - k, v = item.split('=', 1) - d[k] = unquote(v) - else: - d[k] = None - return d - -class ScramState(object): - def __init__(self, mech, sid, s): - self.mech = mech - self.sid = sid - self.s = s - -class ScramAuthentication(BaseAuthentication): - def authenticate(self, request): - auth_header = get_authorization_header(request) - if not auth_header or len(auth_header)==0: - return None - try: - auth_header=auth_header.decode("utf-8") - except: - raise AuthenticationFailed("Malformed authentication header") - - if not auth_header.upper().startswith("SCRAM-"): - return None - m = re.fullmatch("(SCRAM-[A-Z0-9-]+) *([^ ].*)", auth_header, flags=re.IGNORECASE) - if not m: - raise AuthenticationFailed("Malformed SCRAM authentication header") - scram_mech=m.group(1).upper() - auth_data = parse_dict_header(m.group(2)) - if "data" in auth_data and "sid" in auth_data: - # If we have both of these we are in the final phase of the SCRAM handshake - sid = auth_data.get("sid") - data = auth_data.get("data") - if not sid or not data: - raise AuthenticationFailed("Malformed SCRAM authentication header") - client_final=base64.b64decode(data).decode("utf-8") - ex,s = do_scram_final(client_final, sid) - request.META["scram_state"]=ScramState(scram_mech, sid, s) - return (ex.cred.owner, ex.cred) - # Otherwise, SCRAM has not yet succeeded - return None - - def authenticate_header(self, request): - auth_header = get_authorization_header(request) - if not auth_header or len(auth_header)==0: - return "SCRAM-SHA-512" - try: - auth_header=auth_header.decode("utf-8") - except: - raise AuthenticationFailed("Malformed SCRAM authentication header") - if auth_header.upper().startswith("SCRAM-"): - m = re.fullmatch("(SCRAM-[A-Z0-9-]+) *([^ ].*)", auth_header, flags=re.IGNORECASE) - if not m: - return "SCRAM-SHA-512" - scram_mech=m.group(1).upper() - auth_data = parse_dict_header(m.group(2)) - if not auth_data.get("data", None): - return "SCRAM-SHA-512" - client_first=base64.b64decode(auth_data.get("data")).decode("utf-8") - try: - # This function will only be called during the SCRAM first phase, so we do that - ex, s = do_scram_first(client_first) - sfirst=base64.b64encode(s.get_server_first().encode("utf-8")).decode('utf-8') - return f"{scram_mech} sid={ex.sid}, data={sfirst}" - except (scramp.ScramException): - raise AuthenticationFailed("SCRAM authentication failed") - -def set_scram_auth_info_header(get_response): - def middleware(request): - response = get_response(request) - scram_state = request.META.get("scram_state", None) - if scram_state: - sfinal=base64.b64encode(scram_state.s.get_server_final().encode("utf-8")).decode('utf-8') - response["Authentication-Info"]=f"{scram_state.mech} sid={scram_state.sid}, data={sfinal}" - return response - return middleware - -def do_scram_first(client_first: str): - """ - Return: a tuple (sid, server first data) - """ - # all credentials we issue are SHA-512 - s = scramp.ScramMechanism("SCRAM-SHA-512").make_server(scram_user_lookup) - s.set_client_first(client_first) - - # If scramp did not complain, the exchange can proceed. - # First, we record the state so that it can be picked up later. - ex = SCRAMExchange() - ex.cred = SCRAMCredentials.objects.get(username=s.user) - ex.j_nonce = s.nonce - ex.s_nonce_len = len(s.s_nonce) - ex.client_first = client_first - ex.began = datetime.datetime.now(datetime.timezone.utc) - ex.save() - return (ex,s) - def do_scram_final(client_final: str, sid: Optional[str]=None): """ Return: If successful, the (completed) SCRAMExchange and the SCRAM server @@ -476,7 +345,7 @@ def header_transform(name): if "body" in rdata: # Icky Hack: DRF wants to decode JSON for us, so we must re-encode the # sub-request's body to be decoded. . . again. - # If there's a way to tell DRF to do no parsing on this request (beacuse it + # If there's a way to tell DRF to do no parsing on this request (because it # already did it), that could be much more efficient try: raw_body = json.dumps(rdata["body"]).encode("utf-8") From f6ef426407d9a6c21bc24e4d068ab3906ffa7b57 Mon Sep 17 00:00:00 2001 From: "C. Weaver" Date: Fri, 9 Feb 2024 23:13:24 -0500 Subject: [PATCH 2/4] Add an authentication pseudo-mechanism for requests within mutli-requests --- scimma_admin/hopskotch_auth/api_views.py | 16 ++++++++++++++++ .../static/hopskotch_auth/api/schema.yml | 6 +++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/scimma_admin/hopskotch_auth/api_views.py b/scimma_admin/hopskotch_auth/api_views.py index 3f46a2b..8e046b5 100644 --- a/scimma_admin/hopskotch_auth/api_views.py +++ b/scimma_admin/hopskotch_auth/api_views.py @@ -158,6 +158,14 @@ def __init__(self, mech, sid, s): class ScramAuthentication(BaseAuthentication): def authenticate(self, request): + # This is a bit tricky, as it doesn't directly have anything to do with SCRAM Auth: + # If the request wraps one which is already authenticated, we hoist out that authentication + # information and just return it immediately. + # This is used by the multi request mechanism to cascade authentication down to sub-requests + if hasattr(request._request,"user") and request._request.user.is_authenticated \ + and hasattr(request._request,"auth"): + return (request._request.user, request._request.auth) + auth_header = get_authorization_header(request) if not auth_header or len(auth_header)==0: return None @@ -335,6 +343,14 @@ def header_transform(name): continue sr_headers = { header_transform(k):v for k,v in rdata["headers"].items()} sub_request.META.update(sr_headers) + # Implement our own auth pseudo-mecahnism, allowing the sub-request to re-use the + # parent request's auth. Note that what we replicate is not the authentication data + # which was sent, but the end result of the authentication, so that authentication + # is not repeated. + if "HTTP_AUTHORIZATION" in sub_request.META \ + and sub_request.META["HTTP_AUTHORIZATION"] == "Inherit": + sub_request.user = request.user + sub_request.auth = request.auth # overwrite headers which should not be inherited sub_request.META["REQUEST_METHOD"] = rdata["method"] sub_request.META["REQUEST_URI"] = rdata["path"] diff --git a/scimma_admin/hopskotch_auth/static/hopskotch_auth/api/schema.yml b/scimma_admin/hopskotch_auth/static/hopskotch_auth/api/schema.yml index 6970bf9..5d0314c 100644 --- a/scimma_admin/hopskotch_auth/static/hopskotch_auth/api/schema.yml +++ b/scimma_admin/hopskotch_auth/static/hopskotch_auth/api/schema.yml @@ -21,7 +21,11 @@ paths: /hopauth/api/v{version}/multi: post: operationId: multiRequest - description: 'Submit a request to perform a bundle of sub-requests. Each sub-request is processed independently, including its authentication. The request body is a mapping of user-chosen keys to sub-requests, and the response will be in the form of a mapping with the same keys, so that sub-responses can be matched to the sub-requests the client wanted to make. Each sub-request must include a method (HTTP verb) and path requested. Each may optionally include headers (useful for including authorization tokens), and a request body if applicable. Each sub-response will include a status and response body, and may also include response headers.' + description: "Submit a request to perform a bundle of sub-requests. Each sub-request is processed independently, including its authentication. The request body is a mapping of user-chosen keys to sub-requests, and the response will be in the form of a mapping with the same keys, so that sub-responses can be matched to the sub-requests the client wanted to make. Each sub-request must include a method (HTTP verb) and path requested. Each may optionally include headers (useful for including authorization tokens), and a request body if applicable. Each sub-response will include a status and response body, and may also include response headers. Authentication (and authorization) +is generally checked separately for each sub-request independent of both other sub-requests and the +original multi-request, so in most cases each sub-request should include its own `Authorization` +header. Besides using the `Token` scheme, a speical 'pseudo-scheme', `Inherit`, is supported, which +causes the sub-request to share the parent multi-request's authentication." parameters: - name: version in: path From d72e0aa8e0e7d82e0f3a50fdc4761b20eb58eeee Mon Sep 17 00:00:00 2001 From: "C. Weaver" Date: Fri, 9 Feb 2024 23:14:26 -0500 Subject: [PATCH 3/4] Remove more duplicated code --- scimma_admin/hopskotch_auth/api_views.py | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/scimma_admin/hopskotch_auth/api_views.py b/scimma_admin/hopskotch_auth/api_views.py index 8e046b5..c0efd47 100644 --- a/scimma_admin/hopskotch_auth/api_views.py +++ b/scimma_admin/hopskotch_auth/api_views.py @@ -69,19 +69,6 @@ def find_current_credential(request) -> Optional[SCRAMCredentials]: return None return None -def describe_auth(request) -> str: - auth = getattr(request, "auth", None) - if auth: - if isinstance(auth, SCRAMCredentials): - return f"Authentication was HTTP SCRAM with credential {auth.username}" - if isinstance(auth, bytes): - token = RESTAuthToken.get_token(auth) - if token is not None: - return f"Authentication was with token {token}" - else: - return "Authentication was with unknown token" - return "Request was not authenticated" - class Version(APIView): # This is non-sensitive information, which a client may need to read in order to authenticate # correctly, so it is not itself subject to authentication From 258273db8e7f13f8357e2fe7fffd7356e549782a Mon Sep 17 00:00:00 2001 From: "C. Weaver" Date: Mon, 12 Feb 2024 18:06:42 -0500 Subject: [PATCH 4/4] Require authentication for multi-requests. This fixes HTTP SCRAM auth for these requests, as we need to return the 401 status on the first request to complete the handshake. --- scimma_admin/hopskotch_auth/api_views.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/scimma_admin/hopskotch_auth/api_views.py b/scimma_admin/hopskotch_auth/api_views.py index c0efd47..90cf8c6 100644 --- a/scimma_admin/hopskotch_auth/api_views.py +++ b/scimma_admin/hopskotch_auth/api_views.py @@ -101,7 +101,6 @@ def do_scram_final(client_final: str, sid: Optional[str]=None): Return: If successful, the (completed) SCRAMExchange and the SCRAM server """ if sid: - print("Client supplied sid:",sid) ex = SCRAMExchange.objects.get(sid=sid) else: # a bit ugly: To find the previously started exchange session, if any, we need to extract @@ -288,6 +287,7 @@ def post(self, request, version): class MultiRequest(APIView): authentication_classes = [ScramAuthentication, rest_authtoken.auth.AuthTokenAuthentication] + permission_classes = [IsAuthenticated] def post(self, request, version): auth_header_names = ["HTTP_AUTHORIZATION", "HTTP_PROXY_AUTHORIZATION"] @@ -836,7 +836,6 @@ def create(self, request, *args, **kwargs): serializer = serializers[version].GroupMembershipCreationSerializer(data=request.data) serializer.is_valid(raise_exception=True) - print(f"validated_data: {serializer.validated_data}") group = serializer.validated_data['group'] target_user = serializer.validated_data['user']