Store and validate requested oauth scopes

/authorize now expects client to send valid scopes and mints the
selected scopes into tokens.

The scope vocabulary and its wire format live in cmk.gui.scopes, free of
Checkmk dependencies. Resolving scopes to permissions is a separate
concern and comes later.

The MCP requests read and write by default.

Existing tokens in from before this commit carry the old "mcp" scope and
will need to re-authenticate.

Change-Id: I03a31b20a14e8296df30071938bcf6be84caed1e

Author: hrantzsch

cmk/gui/BUILD

@@ -497,6 +497,17 @@ py_library(
     ],
 )
 
+py_library(
+    name = "scopes",
+    srcs = ["scopes.py"],
+    imports = ["../.."],
+    visibility = [
+        "//cmk:__subpackages__",
+        "//non-free/packages:__subpackages__",
+        "//tests:__subpackages__",
+    ],
+)
+
 py_library(
     name = "auth_core",
     srcs = [

cmk/gui/OWNERS

@@ -3,6 +3,9 @@ set noparent  # don't inherit owners information from parent folders
 per-file auth.py=file:/component_owners/user_management_and_authentication/OWNERS_DEFINITION
 per-file auth.py=set noparent
 
+per-file authorization.py=file:/component_owners/user_management_and_authentication/OWNERS_DEFINITION
+per-file authorization.py=set noparent
+
 per-file login.py=file:/component_owners/user_management_and_authentication/OWNERS_DEFINITION
 per-file login.py=set noparent
 

cmk/gui/oauth/BUILD

@@ -16,6 +16,7 @@ py_library(
         requirement("redis"),
         "//cmk/gui:core_toolkit",
         "//cmk/gui:page_toolkit",
+        "//cmk/gui:scopes",
         "//cmk/gui/utils:csrf_token",
         "//cmk/gui/wato:base",
         "//cmk/gui/watolib",

cmk/gui/oauth/pages/_authorize.py

@@ -19,6 +19,13 @@
 from cmk.gui.oauth import client_store
 from cmk.gui.oauth.store._auth_code_store import AuthCodeRecord, AuthCodeStore
 from cmk.gui.pages import Page, PageContext, PageResult
+from cmk.gui.scopes import (
+    DEFAULT_SCOPE,
+    format_scopes,
+    InvalidScopeError,
+    parse_scopes,
+    ScopeId,
+)
 from cmk.gui.utils.csrf_token import check_csrf_token
 from cmk.gui.utils.security_log_events import OAuthAuthorizationFailureEvent
 from cmk.gui.utils.transaction_manager import transactions
@@ -35,13 +42,17 @@ class OAuthAuthorizePage(Page):
     code. Returns 404 while no OAuth-consuming feature is enabled for the
     site (the enabled predicate is injected at registration).
 
+    This is where the granted scope is decided: the requested scope is
+    validated and normalized (see cmk.gui.scopes), shown to the user,
+    and bound to the code in that form, so the client's raw scope string never
+    reaches the token. There is no per-scope selection UI; approving grants
+    what was asked for.
+
     Codes minted on approval are persisted PKCE-bound via AuthCodeStore; the
     token endpoint later redeems them single-use. Validates client_id against
     the registered-client store (see cmk.gui.oauth.client_store()) and requires
     redirect_uri to exactly match one of that client's registered
-    redirect_uris. _token.py does not yet validate that a code was issued to
-    the client redeeming it -- that's separate follow-up work. Rejected
-    requests are logged as security events (see
+    redirect_uris. Rejected requests are logged as security events (see
     OAuthAuthorizationFailureEvent).
     """
 
@@ -106,18 +117,35 @@ def page(self, ctx: PageContext) -> PageResult:
             self._error_redirect(ctx, redirect_uri, "invalid_request")
             return None
 
+        if len(request.values.getlist("scope")) > 1:
+            # RFC 6749 section 3.1 forbids repeating a request parameter, and
+            # with duplicates there is no answer to what the user is approving.
+            self._log_authorization_failure("repeated scope parameter")
+            self._error_redirect(ctx, redirect_uri, "invalid_request")
+            return None
+
+        raw_scope = request.var("scope", "").strip()
+        try:
+            # RFC 6749 section 3.3 leaves what an omitted scope means to us.
+            granted_scopes = parse_scopes(raw_scope) if raw_scope else DEFAULT_SCOPE
+        except InvalidScopeError as exc:
+            # RFC 6749 section 4.1.2.1. Rejected rather than downscoped.
+            self._log_authorization_failure(f"unknown scope: {exc}")
+            self._error_redirect(ctx, redirect_uri, "invalid_scope")
+            return None
+
         # received authorization form OK
         if request.request_method == "POST":
             check_csrf_token()
             if transactions.check_transaction():
                 if request.var("_deny") is not None:
                     self._error_redirect(ctx, redirect_uri, "access_denied")
                     return None
-                self._issue_code(ctx, redirect_uri, client_id, code_challenge)
+                self._issue_code(ctx, redirect_uri, client_id, code_challenge, granted_scopes)
                 return None
 
         # show concent page
-        self._show_consent_page(ctx, redirect_uri)
+        self._show_consent_page(ctx, redirect_uri, granted_scopes)
         return None
 
     def _open_login_frame(self, ctx: PageContext, title: str) -> None:
@@ -150,7 +178,12 @@ def _close_login_frame(self) -> None:
         html.footer()
 
     def _issue_code(
-        self, ctx: PageContext, redirect_uri: str, client_id: str, code_challenge: str
+        self,
+        ctx: PageContext,
+        redirect_uri: str,
+        client_id: str,
+        code_challenge: str,
+        granted_scopes: frozenset[ScopeId],
     ) -> None:
         # The bound user is the server-side session user; the page registry
         # guarantees an authenticated session before this code runs.
@@ -160,7 +193,9 @@ def _issue_code(
             user_id=user.id,
             client_id=client_id,
             redirect_uri=redirect_uri,
-            scope=request.var("scope"),
+            # The normalized grant the consent page showed, not the client's
+            # raw scope string.
+            scope=format_scopes(granted_scopes),
             resource=request.var("resource"),
             code_challenge=code_challenge,
         )
@@ -220,7 +255,9 @@ def _show_redirect_page(
         html.a(_("Click here if you are not redirected automatically."), href=target_url)
         self._close_login_frame()
 
-    def _show_consent_page(self, ctx: PageContext, redirect_uri: str) -> None:
+    def _show_consent_page(
+        self, ctx: PageContext, redirect_uri: str, granted_scopes: frozenset[ScopeId]
+    ) -> None:
         client_id = request.var("client_id")
 
         self._open_login_frame(ctx, _("Authorize access"))
@@ -232,6 +269,16 @@ def _show_consent_page(self, ctx: PageContext, redirect_uri: str) -> None:
                 _('The application "%(client_id)s" is requesting access to this Checkmk site.')
                 % {"client_id": client_id}
             )
+        descriptions = {
+            ScopeId.READ: _("read data"),
+            ScopeId.WRITE: _("change data and configuration"),
+        }
+        html.p(
+            _("It is requesting permission to: %(grants)s.")
+            # ScopeId order, so a given grant always reads the same way.
+            % {"grants": ", ".join(descriptions[s] for s in ScopeId if s in granted_scopes)}
+        )
+        html.p(_("Your own user permissions still apply."))
         html.p(_("Redirect target: %(redirect_uri)s") % {"redirect_uri": redirect_uri})
         # Explicit action: this page is also reachable via the external OAuth
         # issuer alias (/oauth-<site>/authorize, see system_apache.py), where

cmk/gui/oauth/pages/_models.py

@@ -8,6 +8,8 @@
 
 from pydantic import BaseModel, Field, field_validator
 
+from cmk.gui.scopes import SUPPORTED_SCOPES
+
 
 class OAuthAuthorizationServerMetadata(BaseModel):
     """RFC 8414 authorization server metadata document.
@@ -29,6 +31,7 @@ class OAuthAuthorizationServerMetadata(BaseModel):
     grant_types_supported: list[str] = ["authorization_code"]
     token_endpoint_auth_methods_supported: list[str] = ["none"]
     code_challenge_methods_supported: list[str] = ["S256"]
+    scopes_supported: list[str] = list(SUPPORTED_SCOPES)
 
 
 class OAuthClientRegistrationRequest(BaseModel):
@@ -70,10 +73,16 @@ class OAuthClientRegistrationErrorResponse(BaseModel):
 
 
 class OAuthTokenResponse(BaseModel):
-    """RFC 6749 section 5.1 access token response."""
+    """RFC 6749 section 5.1 access token response.
+
+    scope is always sent, not only when it differs from the request as section
+    5.1 requires at a minimum: normalization means it usually does differ, and
+    no client should have to infer that.
+    """
 
     access_token: str
     token_type: str = "Bearer"
+    scope: str
 
 
 class OAuthTokenErrorResponse(BaseModel):

cmk/gui/oauth/pages/_token.py

@@ -20,6 +20,7 @@
 from cmk.gui.oauth.store._auth_code_store import AuthCodeStore
 from cmk.gui.oauth.store.token_store import UnknownClientError
 from cmk.gui.pages import Page, PageContext, PageResult
+from cmk.gui.scopes import format_scopes, parse_scopes
 from cmk.gui.utils.security_log_events import OAuthTokenFailureEvent
 from cmk.utils.security_event import log_security_event
 
@@ -78,8 +79,11 @@ class OAuthTokenPage(Page):
     single-use against the store the authorize endpoint fills, and every
     binding of the redeemed record is enforced: the PKCE S256 challenge, the
     client_id (which must still be registered), and redirect_uri/resource if
-    sent. A scope parameter is
-    ignored; the eventual token's user and scope come only from the record.
+    sent. A scope parameter on the token request is ignored; the token's user
+    and scope come only from the record, so the scope is the one the user
+    approved rather than one the client picks at redemption. It is echoed back
+    in the response body, so a client that asked for more than it got finds out
+    here instead of on its first rejected write.
     Rejections follow the RFC 6749 section 5.2 error format. The returned
     access token is a real, store-backed token issued for the record's user
     (see cmk.gui.oauth.token_store).
@@ -189,12 +193,16 @@ def page(self, ctx: PageContext) -> PageResult:
             _error("invalid_grant")
             return None
 
+        # We assume that parse_scopes will be fine here, as we serialized the previously parsed
+        # scopes into the AuthCodeStore ourselves. If it still goes wrong it's not a user error.
+        granted_scopes = parse_scopes(record.scope)
+
         try:
             access_token = token_store().issue_token(
                 UserId(record.user_id),
                 expires_at=datetime.now(UTC) + _ACCESS_TOKEN_TTL,
                 resource=record.resource,
-                scope=record.scope,
+                scope=granted_scopes,
                 client_id=record.client_id,
             )
         except UnknownClientError:
@@ -205,5 +213,9 @@ def page(self, ctx: PageContext) -> PageResult:
             return None
 
         response.set_content_type("application/json")
-        response.set_data(OAuthTokenResponse(access_token=access_token).model_dump_json())
+        response.set_data(
+            OAuthTokenResponse(
+                access_token=access_token, scope=format_scopes(granted_scopes)
+            ).model_dump_json()
+        )
         return None

cmk/gui/oauth/store/_auth_code_store.py

@@ -26,7 +26,7 @@ class AuthCodeRecord(BaseModel):
     user_id: str
     client_id: str
     redirect_uri: str
-    scope: str | None
+    scope: str
     resource: str | None
     code_challenge: str
 

cmk/gui/oauth/store/token_store.py

@@ -11,7 +11,9 @@
 from datetime import datetime, UTC
 
 from cmk.ccc.user import UserId
+from cmk.gui.log import logger
 from cmk.gui.oauth.store.backend import Backend
+from cmk.gui.scopes import format_scopes, InvalidScopeError, parse_scopes, ScopeId
 
 
 class UnknownClientError(LookupError):
@@ -29,7 +31,7 @@ class TokenRecord:
     issued_at: datetime
     expires_at: datetime
     resource: str | None
-    scope: str | None
+    scope: frozenset[ScopeId]
     client_id: str
 
     def is_valid(self, *, at: datetime | None = None) -> bool:
@@ -65,13 +67,15 @@ def issue_token(
         *,
         expires_at: datetime,
         resource: str | None,
-        scope: str | None,
+        scope: frozenset[ScopeId],
         client_id: str,
     ) -> str:
         if not user_id:
             raise ValueError("user_id must not be empty")
         if not client_id:
             raise ValueError("client_id must not be empty")
+        if not scope:
+            raise ValueError("scope must not be empty")
 
         issued_at_timestamp = int(_utc_now().timestamp())
         expires_at_timestamp = _to_timestamp(expires_at)
@@ -93,7 +97,7 @@ def issue_token(
                     issued_at_timestamp,
                     expires_at_timestamp,
                     resource,
-                    scope,
+                    format_scopes(scope),
                     client_id,
                 ),
             )
@@ -133,16 +137,26 @@ def list_by_user(self, user_id: UserId) -> list[TokenRecord]:
             """,
             (user_id,),
         ).fetchall()
-        return [_row_to_record(row) for row in rows]
+        return [record for row in rows if (record := _row_to_record(row)) is not None]
 
 
-def _row_to_record(row: sqlite3.Row) -> TokenRecord:
+def _row_to_record(row: sqlite3.Row) -> TokenRecord | None:
+    """The row as a record, or None if its scope is not one we could have written."""
+    try:
+        scope = parse_scopes(row["scope"] or "")
+    except InvalidScopeError:
+        logger.warning(
+            "Refusing OAuth access token of user %(user_id)s: stored scope %(scope)r is not usable",
+            {"user_id": row["user_id"], "scope": row["scope"]},
+        )
+        return None
+
     return TokenRecord(
         user_id=UserId(row["user_id"]),
         issued_at=datetime.fromtimestamp(row["issued_at"], tz=UTC),
         expires_at=datetime.fromtimestamp(row["expires_at"], tz=UTC),
         resource=row["resource"],
-        scope=row["scope"],
+        scope=scope,
         client_id=row["client_id"],
     )