diff --git a/changelog.d/20240110_125829_rra_DM_42384.md b/changelog.d/20240110_125829_rra_DM_42384.md new file mode 100644 index 000000000..fda683138 --- /dev/null +++ b/changelog.d/20240110_125829_rra_DM_42384.md @@ -0,0 +1,11 @@ +### Backwards-incompatible changes + +- When acting as an OpenID Connect server, Gafaelfawr no longer exposes all claims by default. Instead, it now honors the `scope` parameter in the request, which must include `openid` and may include `profile` and `email`. +- Require the `oidcServer.issuer` configuration setting use the `https` scheme, since this is required by the OpenID Connect 1.0 specification. +- Set the `aud` claim in OpenID Connect ID tokens issued by Gafaelfawr to the client ID of the requesting client instead of a fixed audience used for all tokens. +- OpenID Connect ID tokens issued by Gafaelfawr now inherit their expiration time from the underlying Gafaelfawr token used as the authentication basis for the ID token. Previously, OpenID Connect ID tokens would receive the full default lifetime even when issued on the basis of Gafaelfawr tokens that were about to expire. + +### Bug fixes + +- Include the scope used to issue the ID token in the reply from the OpenID Connect server token endpoint. +- In the response from `/.well-known/openid-configuration`, declare that the only supported response mode of the OpenID Connect server is `query`. diff --git a/docs/user-guide/openid-connect.rst b/docs/user-guide/openid-connect.rst index 33b897f64..f83042ea7 100644 --- a/docs/user-guide/openid-connect.rst +++ b/docs/user-guide/openid-connect.rst @@ -4,28 +4,52 @@ Configuring OpenID Connect ########################## -Basic configuration -=================== +Configure Gafaelfawr +==================== -To protect a service that uses OpenID Connect, first set ``oidc_server.enabled`` to true in the :ref:`helm-settings`. -Then, create (or add to, if already existing) an ``oidc-server-secrets`` Vault secret key. -The value of the key must be a JSON list, with each list member representing one OpenID Connect client. +To protect a service that uses OpenID Connect, first set ``oidcServer.enabled`` to true in the :ref:`helm-settings`. +Then, create (or add to, if already existing) an ``oidc-server-secrets`` secret for the ``gafaelfawr`` Phalanx application. + +The value of the secret must be a JSON list, with each list member representing one OpenID Connect client. Each list member must be an object with two keys: ``id`` and ``secret``. -``id`` can be anything informative that you want to use to uniquely represent this OpenID Connect client. +``id`` is the unique OpenID Connect client ID that the client will present during authentication. ``secret`` should be a randomly-generated secret that the client will use to authenticate. -Then, configure the client. -The authorization endpoint is ``/auth/openid/login``. -The token endpoint is ``/auth/openid/token``. -The userinfo endpoint is ``/auth/openid/userinfo``. -The JWKS endpoint is ``/.well-known/jwks.json``. -As with any other protected service, the client must run on the same URL host as Gafaelfawr, and these endpoints are all at that shared host (and should be specified using ``https``). +Configure the OpenID client +=========================== + +Gafaelfawr exposes the standard OpenID Connect configuration information at ``/.well-known/openid-configuration``. +Clients that can auto-discover their configuration from that may only need to be configured with the client ID and secret matching the Gafaelfawr configuration. + +For clients that require more manual configuration, the OpenID Connect routes are: + +- Authorization endpoint: ``/auth/openid/login``. +- Token endpoint: ``/auth/openid/token``. +- userinfo endpoint: ``/auth/openid/userinfo``. +- JWKS endpoint: ``/.well-known/jwks.json``. + +As with any other protected service, the client must run on the same URL host as Gafaelfawr. +These endpoints are all at that shared host (and should be specified using ``https``). + +The client must use the authentication code OpenID Connect flow (see `OpenID Connect Core 1.0 section 3.1 `__). +The other authentication flows are not supported. + +OpenID scopes +------------- + +The following OpenID Connect scopes are supported and influence what claims are included in the ID token: + +``openid`` + Required, per the OpenID Connect specification. + The standard OAuth 2.0 and OpenID Connect claims will be included, as well as ``scope`` and ``sub``. + For the Gafaelfawr OpenID Connect provider, ``sub`` will always be the user's username. -The OpenID Connect client should be configured to request only the ``openid`` scope. -No other scope is supported. -The client must be able to authenticate by sending a ``client_secret`` parameter in the request to the token endpoint. +``profile`` + Adds ``preferred_username``, with the same value as ``sub``, and, if this information is available, ``name``. + Gafaelfawr by design does not support attempting to break the name into components such as given name or family name. -The JWT returned by the Gafaelfawr OpenID Connect server will include the authenticated username in the ``sub`` and ``preferred_username`` claims, and the numeric UID in the ``uid_number`` claim. +``email`` + Adds the ``email`` claim if the user's email address is known. Examples ======== @@ -50,7 +74,7 @@ Assuming that Gafaelfawr and Chronograf are deployed on the host ``example.com`` ``GENERIC_CLIENT_ID`` and ``GENERIC_CLIENT_SECRET`` should match a client ID and secret configured in the ``oidc-server-secrets`` Vault key. Be aware that this uses the ``sub`` token claim, which corresponds to the user's username, for authentication, rather than the default of the user's email address. -(Gafaelfawr does not always have an email address for a user.) +Gafaelfawr does not always have an email address for a user. Open Distro for Elasticsearch ----------------------------- diff --git a/examples/docker/gafaelfawr.yaml b/examples/docker/gafaelfawr.yaml index c82b3b0cd..707b0f678 100644 --- a/examples/docker/gafaelfawr.yaml +++ b/examples/docker/gafaelfawr.yaml @@ -38,7 +38,7 @@ github: # Configuration for the Gafaelfawr OpenID Connect server. oidc_server: - issuer: "http://localhost:8080" + issuer: "https://localhost:8080" keyId: "localhost-key-id" audience: "http://localhost" keyFile: "/run/secrets/issuer-key" diff --git a/examples/gafaelfawr-dev.yaml b/examples/gafaelfawr-dev.yaml index 1915137f1..22ee283da 100644 --- a/examples/gafaelfawr-dev.yaml +++ b/examples/gafaelfawr-dev.yaml @@ -41,7 +41,7 @@ github: # Configuration for the Gafaelfawr OpenID Connect server. oidcServer: - issuer: "http://localhost:8080" + issuer: "https://localhost:8080" keyId: "localhost-key-id" audience: "http://localhost" keyFile: "examples/secrets/issuer-key" diff --git a/src/gafaelfawr/config.py b/src/gafaelfawr/config.py index 697385925..9b0e1d46d 100644 --- a/src/gafaelfawr/config.py +++ b/src/gafaelfawr/config.py @@ -17,7 +17,7 @@ from datetime import timedelta from ipaddress import IPv4Network, IPv6Network from pathlib import Path -from typing import Self +from typing import Annotated, Self from uuid import UUID import yaml @@ -25,9 +25,11 @@ AnyHttpUrl, BaseModel, Field, + UrlConstraints, field_validator, model_validator, ) +from pydantic_core import Url from safir.logging import LogLevel, Profile, configure_logging from safir.pydantic import CamelCaseModel, validate_exactly_one_of @@ -46,6 +48,7 @@ "GitHubGroup", "GitHubGroupTeam", "GitHubSettings", + "HttpsUrl", "LDAPConfig", "LDAPSettings", "NotebookQuota", @@ -62,6 +65,13 @@ "Settings", ] +HttpsUrl = Annotated[ + Url, + UrlConstraints( + allowed_schemes=["https"], host_required=True, max_length=2083 + ), +] + class GitHubSettings(CamelCaseModel): """pydantic model of GitHub configuration.""" @@ -299,15 +309,12 @@ class FirestoreSettings(CamelCaseModel): class OIDCServerSettings(CamelCaseModel): """pydantic model of issuer configuration.""" - issuer: str + issuer: HttpsUrl """iss (issuer) field in issued tokens.""" key_id: str """kid (key ID) header field in issued tokens.""" - audience: str - """aud (audience) field in issued tokens.""" - key_file: Path """File containing RSA private key for signing issued tokens.""" @@ -745,9 +752,6 @@ class OIDCServerConfig: key_id: str """kid (key ID) header field in issued tokens.""" - audience: str - """aud (audience) field in issued tokens.""" - keypair: RSAKeyPair """RSA key pair for signing and verifying issued tokens.""" @@ -1001,9 +1005,8 @@ def from_file(cls, path: Path) -> Self: # noqa: PLR0912,PLR0915,C901 for c in oidc_secrets ) oidc_server_config = OIDCServerConfig( - issuer=settings.oidc_server.issuer, + issuer=str(settings.oidc_server.issuer), key_id=settings.oidc_server.key_id, - audience=settings.oidc_server.audience, keypair=oidc_keypair, lifetime=timedelta(minutes=settings.token_lifetime_minutes), clients=oidc_clients, diff --git a/src/gafaelfawr/factory.py b/src/gafaelfawr/factory.py index 062de04f4..b840198ea 100644 --- a/src/gafaelfawr/factory.py +++ b/src/gafaelfawr/factory.py @@ -415,11 +415,13 @@ def create_oidc_service(self) -> OIDCService: ) authorization_store = OIDCAuthorizationStore(storage) token_service = self.create_token_service() + user_info_service = self.create_user_info_service() slack_client = self.create_slack_client() return OIDCService( config=self._context.config.oidc_server, authorization_store=authorization_store, token_service=token_service, + user_info_service=user_info_service, slack_client=slack_client, logger=self._logger, ) diff --git a/src/gafaelfawr/handlers/oidc.py b/src/gafaelfawr/handlers/oidc.py index 13b553e48..acbabdf2a 100644 --- a/src/gafaelfawr/handlers/oidc.py +++ b/src/gafaelfawr/handlers/oidc.py @@ -4,7 +4,7 @@ import time from collections.abc import Mapping -from typing import Any +from typing import Annotated, Any from urllib.parse import ParseResult, parse_qsl, urlencode from fastapi import ( @@ -28,6 +28,7 @@ JWKS, OIDCConfig, OIDCErrorReply, + OIDCScope, OIDCTokenReply, OIDCVerifiedToken, ) @@ -67,33 +68,50 @@ tags=["oidc"], ) async def get_login( - client_id: str, - parsed_redirect_uri: ParseResult = Depends(parsed_redirect_uri), - response_type: (str | None) = Query( - None, - title="Requested response type", - description="code is the only supported response type", - examples=["code"], - ), - scope: (str | None) = Query( - None, - title="Requested token scope", - description="openid is the only supported scope", - examples=["openid"], - ), - state: (str | None) = Query( - None, - title="Opaque state cookie", - description=( - "Set by the client to prevent session fixation attacks. Will be" - " returned verbatim in the response. The client should verify" - " that it matches the code sent in the request by, for example" - " comparing it to a code set in a cookie." + client_id: Annotated[ + str, + Query( + title="Client ID", + description="Identifier of the registered OpenID Client", + examples=["https://example.org/chronograf"], ), - examples=["omeKJ7MNv_9dKSKnVNjxMQ"], - ), - token_data: TokenData = Depends(authenticate), - context: RequestContext = Depends(context_dependency), + ], + parsed_redirect_uri: Annotated[ParseResult, Depends(parsed_redirect_uri)], + token_data: Annotated[TokenData, Depends(authenticate)], + context: Annotated[RequestContext, Depends(context_dependency)], + response_type: Annotated[ + str | None, + Query( + title="Requested response type", + description="code is the only supported response type", + examples=["code"], + ), + ] = None, + scope: Annotated[ + str | None, + Query( + title="Requested token scopes", + description=( + "Token scopes separated by spaces. The openid scope is" + " required, and profile and email scopes are supported. All" + " other scopes are ignored." + ), + examples=["openid", "openid profile email"], + ), + ] = None, + state: Annotated[ + str | None, + Query( + title="Opaque state cookie", + description=( + "Set by the client to prevent session fixation attacks. Will" + " be returned verbatim in the response. The client should" + " verify that it matches the code sent in the request by, for" + " example comparing it to a code set in a cookie." + ), + examples=["omeKJ7MNv_9dKSKnVNjxMQ"], + ), + ] = None, ) -> str: oidc_service = context.factory.create_oidc_service() @@ -115,8 +133,10 @@ async def get_login( error = "code is the only supported response_type" elif not scope: error = "Missing scope parameter" - elif scope != "openid": - error = "openid is the only supported scope" + else: + scopes = OIDCScope.parse_scopes(scope) + if OIDCScope.openid not in scopes: + error = "Only OpenID Connect supported (openid not in scope)" if error: e = InvalidRequestError(error) context.logger.warning("%s", e.message, error=str(e)) @@ -129,7 +149,10 @@ async def get_login( # Get an authorization code and return it. code = await oidc_service.issue_code( - client_id, parsed_redirect_uri.geturl(), token_data.token + client_id=client_id, + redirect_uri=parsed_redirect_uri.geturl(), + token=token_data.token, + scopes=scopes, ) return_url = build_return_url( parsed_redirect_uri, state=state, code=str(code) @@ -172,34 +195,46 @@ def build_return_url(redirect_uri: ParseResult, **params: str | None) -> str: ) async def post_token( response: Response, - grant_type: (str | None) = Form( - None, - title="Request type", - description="`authorization_code` is the only supported grant type", - examples=["authorization_code"], - ), - client_id: (str | None) = Form( - None, - title="ID of client", - examples=["oidc-client-name"], - ), - client_secret: (str | None) = Form( - None, - title="Client secret", - examples=["rYTfX6h9-ilGwADfgn7KRQ"], - ), - code: (str | None) = Form( - None, - title="Authorization code", - description="The code returned from the /auth/openid/login endpoint", - examples=["gc-W74I5HltJZRc0fOUAapgVQ.3T1xQQgeD063KgmNinw-tA"], - ), - redirect_uri: (str | None) = Form( - None, - title="URL of client", - description="Must match the redirect_uri in the client registration", - examples=["https://example.com/"], - ), + grant_type: Annotated[ + str | None, + Form( + title="Request type", + description=( + "`authorization_code` is the only supported grant type" + ), + examples=["authorization_code"], + ), + ] = None, + client_id: Annotated[ + str | None, + Form( + title="ID of client", + examples=["https://data.lsst.cloud/oidc-client"], + ), + ] = None, + client_secret: Annotated[ + str | None, + Form( + title="Client secret", + examples=["rYTfX6h9-ilGwADfgn7KRQ"], + ), + ] = None, + code: Annotated[ + str | None, + Form( + title="Authorization code", + description="Code returned from the `/auth/openid/login` endpoint", + examples=["gc-W74I5HltJZRc0fOUAapgVQ.3T1xQQgeD063KgmNinw-tA"], + ), + ] = None, + redirect_uri: Annotated[ + str | None, + Form( + title="URL of client", + description="Must match `redirect_uri` in the client registration", + examples=["https://example.com/"], + ), + ] = None, context: RequestContext = Depends(context_dependency), ) -> OIDCTokenReply | JSONResponse: oidc_service = context.factory.create_oidc_service() @@ -236,6 +271,7 @@ async def post_token( access_token=token.encoded, id_token=token.encoded, expires_in=int(token.claims["exp"] - time.time()), + scope=token.claims["scope"], ) diff --git a/src/gafaelfawr/models/oidc.py b/src/gafaelfawr/models/oidc.py index 7ea615e0b..9348beaf1 100644 --- a/src/gafaelfawr/models/oidc.py +++ b/src/gafaelfawr/models/oidc.py @@ -2,7 +2,9 @@ from __future__ import annotations +from contextlib import suppress from datetime import datetime +from enum import StrEnum from typing import Any, Self from pydantic import BaseModel, Field, field_serializer, field_validator @@ -20,11 +22,49 @@ "OIDCAuthorization", "OIDCAuthorizationCode", "OIDCConfig", + "OIDCScope", "OIDCToken", "OIDCVerifiedToken", ] +class OIDCScope(StrEnum): + """A recognized OpenID Connect scope. + + This should not be directly exposed in the model of any endpoint. Instead, + the `str` scope parameter should be parsed with the `parse_scopes` class + method to yield a list of `OIDCScope` objects. + """ + + openid = "openid" + profile = "profile" + email = "email" + + @classmethod + def parse_scopes(cls, scopes: str) -> list[Self]: + """Parse a space-separated list of scopes. + + Any unknown scopes are silently ignored, following the OpenID Connect + Core specification. + + Parameters + ---------- + scopes + Space-separated list of scopes. + + Returns + ------- + list of OIDCScope + Corresponding enums of recognized scopes. + """ + result = [] + for scope in scopes.split(None): + with suppress(KeyError): + result.append(cls[scope]) + result.sort() + return result + + class OIDCAuthorizationCode(BaseModel): """An OpenID Connect authorization code. @@ -112,6 +152,10 @@ class OIDCAuthorization(BaseModel): title="When the authorization was created", ) + scopes: list[OIDCScope] = Field( + [OIDCScope.openid], title="Requested scopes" + ) + @field_serializer("created_at") def _serialize_datetime(self, time: datetime | None) -> int | None: return int(time.timestamp()) if time is not None else None @@ -220,6 +264,16 @@ class OIDCTokenReply(BaseModel): examples=[86400], ) + scope: str = Field( + ..., + title="Scopes of token", + description=( + "Scopes of the issued token, with any unrecognized or unauthorized" + " scopes from the request filtered out" + ), + examples=["email openid profile"], + ) + token_type: str = Field( "Bearer", title="Type of token", @@ -358,10 +412,10 @@ class OIDCConfig(BaseModel): ) scopes_supported: list[str] = Field( - ["openid"], + [s.value for s in OIDCScope], title="Supported scopes", - description="`openid` is the only supported scope", - examples=[["openid"]], + description="List of supported scopes", + examples=[["openid", "profile", "email"]], ) response_types_supported: list[str] = Field( @@ -371,6 +425,13 @@ class OIDCConfig(BaseModel): examples=[["code"]], ) + response_modes_supported: list[str] = Field( + ["query"], + title="Supported response modes", + description="`query` is the only supported response mode", + examples=[["query"]], + ) + grant_types_supported: list[str] = Field( ["authorization_code"], title="Supported grant types", diff --git a/src/gafaelfawr/services/oidc.py b/src/gafaelfawr/services/oidc.py index 5e2a9ca52..6c7effb7d 100644 --- a/src/gafaelfawr/services/oidc.py +++ b/src/gafaelfawr/services/oidc.py @@ -25,15 +25,26 @@ OIDCAuthorization, OIDCAuthorizationCode, OIDCConfig, + OIDCScope, OIDCToken, OIDCVerifiedToken, ) -from ..models.token import Token, TokenUserInfo +from ..models.token import Token from ..storage.oidc import OIDCAuthorizationStore from .token import TokenService +from .userinfo import UserInfoService __all__ = ["OIDCService"] +_SCOPE_CLAIMS = { + OIDCScope.openid: frozenset( + ["aud", "iat", "iss", "exp", "jti", "scope", "sub"] + ), + OIDCScope.profile: frozenset(["name", "preferred_username"]), + OIDCScope.email: frozenset(["email"]), +} +"""Mapping of scope values to the claims to expose for that scope.""" + class OIDCService: """Minimalist OpenID Connect identity provider. @@ -50,6 +61,8 @@ class OIDCService: The underlying storage for OpenID Connect authorizations. token_service Token manipulation service. + user_info_service + User information service. slack_client If provided, a Slack webhook client to use to report corruption of the underlying Redis store. @@ -77,12 +90,14 @@ def __init__( config: OIDCServerConfig, authorization_store: OIDCAuthorizationStore, token_service: TokenService, + user_info_service: UserInfoService, slack_client: SlackWebhookClient | None = None, logger: BoundLogger, ) -> None: self._config = config self._authorization_store = authorization_store self._token_service = token_service + self._user_info = user_info_service self._slack = slack_client self._logger = logger @@ -117,18 +132,25 @@ def is_valid_client(self, client_id: str) -> bool: return any(c.client_id == client_id for c in self._config.clients) async def issue_code( - self, client_id: str, redirect_uri: str, token: Token + self, + *, + client_id: str, + redirect_uri: str, + token: Token, + scopes: list[OIDCScope], ) -> OIDCAuthorizationCode: """Issue a new authorization code. Parameters ---------- client_id - The client ID with access to this authorization. + Client ID with access to this authorization. redirect_uri - The intended return URI for this authorization. + Intended return URI for this authorization. token - The underlying authentication token. + Underlying authentication token. + scopes + Requested scopes. Returns ------- @@ -144,13 +166,16 @@ async def issue_code( if not self.is_valid_client(client_id): raise UnauthorizedClientError(f"Unknown client ID {client_id}") authorization = OIDCAuthorization( - client_id=client_id, redirect_uri=redirect_uri, token=token + client_id=client_id, + redirect_uri=redirect_uri, + token=token, + scopes=scopes, ) await self._authorization_store.create(authorization) return authorization.code - def issue_token( - self, user_info: TokenUserInfo, **claims: str + async def issue_id_token( + self, authorization: OIDCAuthorization ) -> OIDCVerifiedToken: """Issue an OpenID Connect token. @@ -159,29 +184,47 @@ def issue_token( Parameters ---------- - user_info - The token data on which to base the token. - **claims - Additional claims to add to the token. + authorization + Authorization code used to request a token. Returns ------- OIDCVerifiedToken The new token. + + Raises + ------ + InvalidGrantError + Raised if the underlying authorization or session does not exist. """ + token_data = await self._token_service.get_data(authorization.token) + if not token_data: + code = authorization.code.key + msg = f"Invalid underlying token for authorization {code}" + raise InvalidGrantError(msg) + user_info = await self._user_info.get_user_info_from_token(token_data) + + # Build a payload of every claim we support, and then filter it by the + # list of claims that were requested via either claims or scopes and + # by dropping any claims that were None. now = current_datetime() - expires = now + self._config.lifetime payload: dict[str, Any] = { - "aud": self._config.audience, + "aud": authorization.client_id, + "auth_time": int(token_data.created.timestamp()), "iat": int(now.timestamp()), - "iss": self._config.issuer, - "exp": int(expires.timestamp()), + "iss": str(self._config.issuer), + "email": user_info.email, + "exp": int(token_data.expires.timestamp()), + "jti": authorization.code.key, "name": user_info.name, - "preferred_username": user_info.username, - "sub": user_info.username, + "preferred_username": token_data.username, + "scope": " ".join(s.value for s in authorization.scopes), + "sub": token_data.username, "uid_number": user_info.uid, - **claims, } + payload = self._filter_claims(payload, authorization) + + # Encode the token. encoded_token = jwt.encode( payload, self._config.keypair.private_key_as_pem().decode(), @@ -244,7 +287,11 @@ async def redeem_code( if grant_type != "authorization_code": raise UnsupportedGrantTypeError(f"Invalid grant type {grant_type}") auth_code = OIDCAuthorizationCode.from_str(code) + + # Authorize the client. self._check_client_secret(client_id, client_secret) + + # Retrieve the metadata associated with the authorization code. try: authorization = await self._authorization_store.get(auth_code) except DeserializeError as e: @@ -257,6 +304,7 @@ async def redeem_code( msg = f"Unknown authorization code {auth_code.key}" raise InvalidGrantError(msg) + # Authorize the request. if authorization.client_id != client_id: msg = ( f"Authorization client ID mismatch for {auth_code.key}:" @@ -270,22 +318,18 @@ async def redeem_code( ) raise InvalidGrantError(msg) - user_info = await self._token_service.get_user_info( - authorization.token - ) - if not user_info: - msg = f"Invalid underlying token for authorization {auth_code.key}" - raise InvalidGrantError(msg) - token = self.issue_token(user_info, jti=auth_code.key, scope="openid") - - # The code is valid and we're going to return success, so delete it - # from Redis so that it cannot be reused. + # Construct and return the token. The authorization code has now been + # redeemed, so delete it so that it cannot be reused. + token = await self.issue_id_token(authorization) await self._authorization_store.delete(auth_code) return token def verify_token(self, token: OIDCToken) -> OIDCVerifiedToken: """Verify a token issued by the internal OpenID Connect server. + Any currently-registered client audience is accepted as a valid + audience. + Parameters ---------- token @@ -302,12 +346,13 @@ def verify_token(self, token: OIDCToken) -> OIDCVerifiedToken: The issuer of this token is unknown and therefore the token cannot be verified. """ + audiences = (c.client_id for c in self._config.clients) try: payload = jwt.decode( token.encoded, self._config.keypair.public_key_as_pem().decode(), algorithms=[ALGORITHM], - audience=self._config.audience, + audience=audiences, ) return OIDCVerifiedToken( encoded=token.encoded, claims=payload, jti=payload["jti"] @@ -344,3 +389,31 @@ def _check_client_secret( msg = f"Invalid secret for {client_id}" raise InvalidClientError(msg) raise InvalidClientError(f"Unknown client ID {client_id}") + + def _filter_claims( + self, + payload: dict[str, Any], + authorization: OIDCAuthorization, + ) -> dict[str, Any]: + """Filter claims according to the request. + + Parameters + ---------- + payload + Full set of claims based on the user's metadata and token. + authorization + OpenID Connect authorization, which contains the client-requested + scopes and claims. For now, only the ``id_token`` portion of the + claims request is honored. + + Returns + ------- + dict + Filtered claims based on the requested scopes and claims. + """ + wanted: set[str] = set() + for scope in authorization.scopes: + wanted.update(_SCOPE_CLAIMS.get(scope, set())) + return { + k: v for k, v in payload.items() if v is not None and k in wanted + } diff --git a/src/gafaelfawr/services/token.py b/src/gafaelfawr/services/token.py index 9de38bf9a..62e34f8e2 100644 --- a/src/gafaelfawr/services/token.py +++ b/src/gafaelfawr/services/token.py @@ -753,31 +753,6 @@ async def get_token_info_unchecked( return None return info - async def get_user_info(self, token: Token) -> TokenUserInfo | None: - """Get user information associated with a token. - - Parameters - ---------- - token - Data from the authentication token. - - Returns - ------- - TokenUserInfo or None - User information for the holder of that token, or `None` if the - token is not valid. - """ - data = await self.get_data(token) - if not data: - return None - return TokenUserInfo( - username=data.username, - name=data.name, - uid=data.uid, - email=data.email, - groups=data.groups, - ) - async def list_tokens( self, auth_data: TokenData, username: str | None = None ) -> list[TokenInfo]: diff --git a/tests/cli_test.py b/tests/cli_test.py index 8363b9a43..c1f4fb59e 100644 --- a/tests/cli_test.py +++ b/tests/cli_test.py @@ -29,7 +29,7 @@ from gafaelfawr.factory import Factory from gafaelfawr.models.admin import Admin from gafaelfawr.models.history import TokenChange, TokenChangeHistoryEntry -from gafaelfawr.models.oidc import OIDCAuthorizationCode +from gafaelfawr.models.oidc import OIDCAuthorizationCode, OIDCScope from gafaelfawr.models.token import Token, TokenData, TokenType, TokenUserInfo from gafaelfawr.schema import Base from gafaelfawr.storage.history import TokenChangeHistoryStore @@ -128,7 +128,10 @@ async def setup() -> OIDCAuthorizationCode: ) oidc_service = factory.create_oidc_service() return await oidc_service.issue_code( - "some-id", "https://example.com/", token + client_id="some-id", + redirect_uri="https://example.com/", + token=token, + scopes=[OIDCScope.openid], ) code = event_loop.run_until_complete(setup()) diff --git a/tests/handlers/oidc_test.py b/tests/handlers/oidc_test.py index a166b0762..099790119 100644 --- a/tests/handlers/oidc_test.py +++ b/tests/handlers/oidc_test.py @@ -11,13 +11,19 @@ import pytest from _pytest.logging import LogCaptureFixture from httpx import AsyncClient +from safir.datetime import current_datetime from safir.testing.slack import MockSlackWebhook from gafaelfawr.config import Config, OIDCClient from gafaelfawr.constants import ALGORITHM from gafaelfawr.factory import Factory from gafaelfawr.models.auth import AuthError, AuthErrorChallenge, AuthType -from gafaelfawr.models.oidc import OIDCAuthorizationCode, OIDCToken +from gafaelfawr.models.oidc import ( + OIDCAuthorization, + OIDCAuthorizationCode, + OIDCScope, + OIDCToken, +) from gafaelfawr.util import number_to_base64 from ..support.config import reconfigure @@ -54,7 +60,7 @@ async def test_login( "/auth/openid/login", params={ "response_type": "code", - "scope": "openid", + "scope": " openid unknown profile foo ", "client_id": "some-id", "state": "random-state", "redirect_uri": return_url, @@ -113,29 +119,27 @@ async def test_login( "token_type": "Bearer", "expires_in": ANY, "id_token": ANY, + "scope": "openid profile", } assert isinstance(data["expires_in"], int) - exp_seconds = config.oidc_server.lifetime.total_seconds() - assert exp_seconds - 5 <= data["expires_in"] <= exp_seconds + exp_seconds = (token_data.expires - current_datetime()).total_seconds() + assert exp_seconds - 1 <= data["expires_in"] <= exp_seconds + 5 assert data["access_token"] == data["id_token"] oidc_service = factory.create_oidc_service() token = oidc_service.verify_token(OIDCToken(encoded=data["id_token"])) assert token.claims == { - "aud": config.oidc_server.audience, - "exp": ANY, + "aud": "some-id", + "exp": int(token_data.expires.timestamp()), "iat": ANY, "iss": config.oidc_server.issuer, "jti": OIDCAuthorizationCode.from_str(code).key, "name": token_data.name, "preferred_username": token_data.username, - "scope": "openid", + "scope": "openid profile", "sub": token_data.username, - "uid_number": token_data.uid, } now = time.time() - expected_exp = now + config.oidc_server.lifetime.total_seconds() - assert expected_exp - 5 <= token.claims["exp"] <= expected_exp assert now - 5 <= token.claims["iat"] <= now username = token_data.username @@ -322,7 +326,9 @@ async def test_login_errors( assert r.status_code == 307 assert query_from_url(r.headers["Location"]) == { "error": ["invalid_request"], - "error_description": ["openid is the only supported scope"], + "error_description": [ + "Only OpenID Connect supported (openid not in scope)" + ], } # None of these errors should have resulted in Slack alerts. @@ -348,7 +354,12 @@ async def test_token_errors( token = token_data.token oidc_service = factory.create_oidc_service() redirect_uri = f"https://{TEST_HOSTNAME}/app" - code = await oidc_service.issue_code("some-id", redirect_uri, token) + code = await oidc_service.issue_code( + client_id="some-id", + redirect_uri=redirect_uri, + token=token, + scopes=[OIDCScope.openid], + ) # Missing parameters. request: dict[str, str] = {} @@ -476,7 +487,12 @@ async def test_token_errors( } # Correct code, but invalid client_id for that code. - bogus_code = await oidc_service.issue_code("other-id", redirect_uri, token) + bogus_code = await oidc_service.issue_code( + client_id="other-id", + redirect_uri=redirect_uri, + token=token, + scopes=[OIDCScope.openid], + ) request["code"] = str(bogus_code) r = await client.post("/auth/openid/token", data=request) assert r.status_code == 400 @@ -537,7 +553,13 @@ async def test_invalid( ) token_data = await create_session_token(factory) oidc_service = factory.create_oidc_service() - oidc_token = oidc_service.issue_token(token_data, jti="some-jti") + authorization = OIDCAuthorization( + client_id="some-id", + redirect_uri="https://example.com/", + token=token_data.token, + scopes=[OIDCScope.openid], + ) + oidc_token = await oidc_service.issue_id_token(authorization) caplog.clear() r = await client.get( @@ -663,8 +685,9 @@ async def test_well_known_oidc( "token_endpoint": base_url + "/auth/openid/token", "userinfo_endpoint": base_url + "/auth/openid/userinfo", "jwks_uri": base_url + "/.well-known/jwks.json", - "scopes_supported": ["openid"], + "scopes_supported": [s.value for s in OIDCScope], "response_types_supported": ["code"], + "response_modes_supported": ["query"], "grant_types_supported": ["authorization_code"], "subject_types_supported": ["public"], "id_token_signing_alg_values_supported": [ALGORITHM], diff --git a/tests/services/oidc_test.py b/tests/services/oidc_test.py index 7a40370e4..c2329d390 100644 --- a/tests/services/oidc_test.py +++ b/tests/services/oidc_test.py @@ -21,7 +21,11 @@ UnsupportedGrantTypeError, ) from gafaelfawr.factory import Factory -from gafaelfawr.models.oidc import OIDCAuthorizationCode +from gafaelfawr.models.oidc import ( + OIDCAuthorization, + OIDCAuthorizationCode, + OIDCScope, +) from ..support.config import reconfigure from ..support.tokens import create_session_token @@ -42,9 +46,19 @@ async def test_issue_code(tmp_path: Path, factory: Factory) -> None: assert list(config.oidc_server.clients) == clients with pytest.raises(UnauthorizedClientError): - await oidc_service.issue_code("unknown-client", redirect_uri, token) + await oidc_service.issue_code( + client_id="unknown-client", + redirect_uri=redirect_uri, + token=token, + scopes=[OIDCScope.openid], + ) - code = await oidc_service.issue_code("some-id", redirect_uri, token) + code = await oidc_service.issue_code( + client_id="some-id", + redirect_uri=redirect_uri, + token=token, + scopes=[OIDCScope.openid, OIDCScope.profile], + ) encrypted_code = await factory.redis.get(f"oidc:{code.key}") assert encrypted_code fernet = Fernet(config.session_secret.encode()) @@ -61,6 +75,7 @@ async def test_issue_code(tmp_path: Path, factory: Factory) -> None: "secret": token.secret, }, "created_at": ANY, + "scopes": ["openid", "profile"], } now = time.time() assert now - 2 < serialized_code["created_at"] < now @@ -80,7 +95,12 @@ async def test_redeem_code(tmp_path: Path, factory: Factory) -> None: token_data = await create_session_token(factory) token = token_data.token redirect_uri = "https://example.com/" - code = await oidc_service.issue_code("client-2", redirect_uri, token) + code = await oidc_service.issue_code( + client_id="client-2", + redirect_uri=redirect_uri, + token=token, + scopes=[OIDCScope.openid, OIDCScope.profile], + ) oidc_token = await oidc_service.redeem_code( grant_type="authorization_code", @@ -90,16 +110,15 @@ async def test_redeem_code(tmp_path: Path, factory: Factory) -> None: code=str(code), ) assert oidc_token.claims == { - "aud": config.oidc_server.audience, + "aud": "client-2", "iat": ANY, "exp": ANY, "iss": config.oidc_server.issuer, "jti": code.key, "name": token_data.name, "preferred_username": token_data.username, - "scope": "openid", + "scope": "openid profile", "sub": token_data.username, - "uid_number": token_data.uid, } assert not await factory.redis.get(f"oidc:{code.key}") @@ -121,7 +140,12 @@ async def test_redeem_code_errors( token_data = await create_session_token(factory) token = token_data.token redirect_uri = "https://example.com/" - code = await oidc_service.issue_code("client-2", redirect_uri, token) + code = await oidc_service.issue_code( + client_id="client-2", + redirect_uri=redirect_uri, + token=token, + scopes=[OIDCScope.openid], + ) with pytest.raises(InvalidRequestError): await oidc_service.redeem_code( @@ -245,7 +269,7 @@ async def test_redeem_code_errors( @pytest.mark.asyncio -async def test_issue_token(tmp_path: Path, factory: Factory) -> None: +async def test_issue_id_token(tmp_path: Path, factory: Factory) -> None: clients = [OIDCClient(client_id="some-id", client_secret="some-secret")] config = await reconfigure( tmp_path, "github-oidc-server", factory, oidc_clients=clients @@ -254,24 +278,25 @@ async def test_issue_token(tmp_path: Path, factory: Factory) -> None: oidc_service = factory.create_oidc_service() token_data = await create_session_token(factory) - oidc_token = oidc_service.issue_token( - token_data, jti="new-jti", scope="openid" + authorization = OIDCAuthorization( + client_id="some-id", + redirect_uri="https://example.com/", + token=token_data.token, + scopes=[OIDCScope.openid, OIDCScope.profile], ) + oidc_token = await oidc_service.issue_id_token(authorization) assert oidc_token.claims == { - "aud": config.oidc_server.audience, - "exp": ANY, + "aud": "some-id", + "exp": int(token_data.expires.timestamp()), "iat": ANY, "iss": config.oidc_server.issuer, - "jti": "new-jti", + "jti": authorization.code.key, "name": token_data.name, "preferred_username": token_data.username, - "scope": "openid", + "scope": "openid profile", "sub": token_data.username, - "uid_number": token_data.uid, } now = time.time() assert now - 5 <= oidc_token.claims["iat"] <= now + 5 - expected_exp = now + config.oidc_server.lifetime.total_seconds() - assert expected_exp - 5 <= oidc_token.claims["exp"] <= expected_exp + 5 diff --git a/tests/services/token_test.py b/tests/services/token_test.py index a150d32a8..a3405fceb 100644 --- a/tests/services/token_test.py +++ b/tests/services/token_test.py @@ -89,7 +89,6 @@ async def test_session_token(config: Config, factory: Factory) -> None: expires=data.expires, parent=None, ) - assert await token_service.get_user_info(token) == user_info async with factory.session.begin(): history = await token_service.get_change_history( @@ -151,7 +150,6 @@ async def test_user_token(factory: Factory) -> None: expires=expires, ip_address="192.168.0.1", ) - assert await token_service.get_user_info(user_token) == user_info info = await token_service.get_token_info_unchecked(user_token.key) assert info assert info == TokenInfo( @@ -219,7 +217,6 @@ async def test_notebook_token(config: Config, factory: Factory) -> None: token = await token_service.get_notebook_token( data, ip_address="1.0.0.1" ) - assert await token_service.get_user_info(token) == user_info info = await token_service.get_token_info_unchecked(token.key) assert info assert info == TokenInfo( @@ -359,7 +356,6 @@ async def test_internal_token(config: Config, factory: Factory) -> None: scopes=["read:all"], ip_address="2001:db8::45", ) - assert await token_service.get_user_info(internal_token) == user_info info = await token_service.get_token_info_unchecked(internal_token.key) assert info assert info == TokenInfo( @@ -374,6 +370,17 @@ async def test_internal_token(config: Config, factory: Factory) -> None: parent=session_token.key, ) assert_is_now(info.created) + assert await token_service.get_data(internal_token) == TokenData( + token=internal_token, + username=user_info.username, + token_type=TokenType.internal, + scopes=["read:all"], + created=info.created, + expires=data.expires, + name=user_info.name, + uid=user_info.uid, + groups=user_info.groups, + ) # Cannot request a scope that the parent token doesn't have. with pytest.raises(InvalidScopesError): @@ -949,7 +956,7 @@ async def test_delete(factory: Factory) -> None: assert await token_service.get_data(token) is None async with factory.session.begin(): assert await token_service.get_token_info_unchecked(token.key) is None - assert await token_service.get_user_info(token) is None + assert await token_service.get_data(token) is None async with factory.session.begin(): assert not await token_service.delete_token(