From cc73705b2277494d159f832cd3bcb915edde749b Mon Sep 17 00:00:00 2001 From: Mikel Alejo Date: Thu, 22 Feb 2024 20:23:22 +0100 Subject: [PATCH] RHCLOUD-31103 | fix: token not validated (#1036) * fix: stop validating the audience claim We don't have a unified audience claim that we can validate for the incoming tokens. Depending on which method is used to request a token, the audience claim changes, and that causes the tokens to not be validated correctly. There are ongoing discussions about this specific claim and in which way we need to validate it, but until we decide what to do, we can safely rely on the service accounts scope that we're already validating to make sure that the token is valid. RHCLOUD-31103 * refactor: remove comments and unused variables RHCLOUD-31103 * refactor: improve logging RHCLOUD-31103 * fix: linting issues RHCLOUD-31103 --- .../authorization/token_validator.py | 23 ++++++++----------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/rbac/management/authorization/token_validator.py b/rbac/management/authorization/token_validator.py index 1b389ede6..32cfa9ee4 100644 --- a/rbac/management/authorization/token_validator.py +++ b/rbac/management/authorization/token_validator.py @@ -34,8 +34,6 @@ logger = logging.getLogger(__name__) # pylint: disable=invalid-name -# The audience claim we are expecting to find in the token. -AUDIENCE_CLAIM = "cloud-services" # The service accounts claim we are expecting to find in the token. SERVICE_ACCOUNTS_CLAIM = "api.iam.service_accounts" @@ -45,14 +43,11 @@ class ITSSOTokenValidator: def __init__(self): """Get the OIDC configuration URL.""" - # TODO replace it with: it_host = settings.IT_SERVICE_HOST it_port = settings.IT_SERVICE_PORT self.it_request_timeout_seconds = settings.IT_SERVICE_TIMEOUT_SECONDS it_scheme = settings.IT_SERVICE_PROTOCOL_SCHEME - # it_host = "https://sso.stage.redhat.com" - # The host contains the URL including the port... self.host = f"{it_scheme}://{it_host}:{it_port}/auth/realms/redhat-external" # ... but the issuer does not. We need to make this distinction so that when validating the token, the issuer @@ -146,15 +141,13 @@ def validate_token(self, request: Request) -> str: # Decode the token. try: token: Token = jwt.decode(value=bearer_token, key=key_set) - except Exception: + except Exception as e: + logging.warning("[request_id: %s] Unable to decode token: %s", getattr(request, "req_id", None), str(e)) raise InvalidTokenError("Unable to decode token") - # Make sure that the token issuer matches the IT issuer, that the audience is set to the "cloud-services" - # client, and that the scope contains the "service accounts" claim. - claim_requests = JWTClaimsRegistry( - iss={"essential": True, "value": self.issuer}, - aud={"essential": True, "value": AUDIENCE_CLAIM}, - ) + # Make sure that the token issuer matches the IT issuer and that the scope contains the "service accounts" + # claim. + claim_requests = JWTClaimsRegistry(iss={"essential": True, "value": self.issuer}) # Make sure that the token is valid. try: @@ -171,7 +164,11 @@ def validate_token(self, request: Request) -> str: # below. claim_requests.validate(token.claims) except Exception as e: - logger.debug('Token "%s" rejected for having invalid claims: %s', token, str(e)) + logging.warning( + "[request_id: %s] Token rejected for having invalid claims: %s", + getattr(request, "req_id", "no-request-id-present"), + str(e), + ) raise InvalidTokenError("The token's claims are invalid") return bearer_token