Convert internal scopes representation to set

Rather than keeping token scopes as lists internally and converting
them to sets for set math, always keep token scopes as sets
everywhere. This is a more natural and correct representation, but
it required a lot of tedious refactoring since the lists leaked into
a lot of internal APIs.

src/gafaelfawr/cache.py

@@ -335,18 +335,18 @@ class InternalTokenCache(TokenCache):
     """Cache for internal tokens."""
 
     def get(
-        self, token_data: TokenData, service: str, scopes: list[str]
+        self, token_data: TokenData, service: str, scopes: set[str]
     ) -> Token | None:
         """Retrieve an internal token from the cache.
 
         Parameters
         ----------
         token_data
-            The authentication data for the parent token.
+            Authentication data for the parent token.
         service
-            The service of the internal token.
+            Service of the internal token.
         scopes
-            The scopes the internal token should have.
+            Scopes the internal token should have.
 
         Returns
         -------
@@ -367,7 +367,7 @@ def store(
         self,
         token_data: TokenData,
         service: str,
-        scopes: list[str],
+        scopes: set[str],
         token: Token,
     ) -> None:
         """Store an internal token in the cache.
@@ -377,19 +377,19 @@ def store(
         Parameters
         ----------
         token_data
-            The authentication data for the parent token.
+            Authentication data for the parent token.
         service
-            The service of the internal token.
+            Service of the internal token.
         scopes
-            The scopes the internal token should have.
+            Scopes the internal token should have.
         token
-            The token to cache.
+            Token to cache.
         """
         key = self._build_key(token_data, service, scopes)
         self._cache[key] = token
 
     def _build_key(
-        self, token_data: TokenData, service: str, scopes: list[str]
+        self, token_data: TokenData, service: str, scopes: set[str]
     ) -> tuple[str, ...]:
         """Build the cache key for an internal token.
 
@@ -407,9 +407,12 @@ def _build_key(
         tuple
             An object suitable for use as a hash key for this internal token.
         """
-        expires = str(token_data.expires) if token_data.expires else "None"
-        scope = ",".join(sorted(scopes))
-        return (token_data.token.key, expires, service, scope)
+        return (
+            token_data.token.key,
+            str(token_data.expires) if token_data.expires else "None",
+            service,
+            " ".join(sorted(scopes)),
+        )
 
 
 class NotebookTokenCache(TokenCache):

src/gafaelfawr/handlers/ingress.py

@@ -597,18 +597,18 @@ async def build_delegated_token(
         return str(token)
     elif auth_config.delegate_to:
         # Delegated scopes are optional; if the authenticating token doesn't
-        # have the scope, it's omitted from the delegated token.  (To make it
+        # have the scope, it's omitted from the delegated token. (To make it
         # mandatory, require that scope via the scope parameter as well, and
-        # then the authenticating token will always have it.)  Therefore,
+        # then the authenticating token will always have it.) Therefore,
         # reduce the scopes of the internal token to the intersection between
         # the requested delegated scopes and the scopes of the authenticating
         # token.
-        delegate_scopes = auth_config.delegate_scopes & set(token_data.scopes)
+        delegate_scopes = auth_config.delegate_scopes & token_data.scopes
         token_service = context.factory.create_token_service()
         token = await token_service.get_internal_token(
             token_data,
             service=auth_config.delegate_to,
-            scopes=sorted(delegate_scopes),
+            scopes=delegate_scopes,
             ip_address=context.ip_address,
             minimum_lifetime=auth_config.minimum_lifetime,
         )

src/gafaelfawr/models/auth.py

@@ -8,6 +8,8 @@
 
 from pydantic import BaseModel, Field
 
+from ..pydantic import Scopes
+
 __all__ = [
     "APIConfig",
     "APILoginResponse",
@@ -165,11 +167,11 @@ class APILoginResponse(BaseModel):
         examples=["someuser"],
     )
 
-    scopes: list[str] = Field(
+    scopes: Scopes = Field(
         ...,
         title="Access scopes",
         description="Access scopes for this authenticated user",
-        examples=["read:all", "user:token"],
+        examples=[["read:all", "user:token"]],
     )
 
     config: APIConfig = Field(

src/gafaelfawr/models/token.py

@@ -313,7 +313,7 @@ def bootstrap_token(cls) -> Self:
             token=Token(),
             username="<bootstrap>",
             token_type=TokenType.service,
-            scopes=["admin:token"],
+            scopes={"admin:token"},
         )
 
     @classmethod
@@ -332,7 +332,7 @@ def internal_token(cls) -> Self:
             token=Token(),
             username="<internal>",
             token_type=TokenType.service,
-            scopes=["admin:token"],
+            scopes={"admin:token"},
         )
 
 
@@ -380,7 +380,7 @@ class AdminTokenRequest(BaseModel):
     )
 
     scopes: Scopes = Field(
-        [],
+        set(),
         title="Token scopes",
         examples=[["read:all"]],
     )
@@ -486,7 +486,7 @@ class UserTokenRequest(BaseModel):
     )
 
     scopes: Scopes = Field(
-        [],
+        set(),
         title="Token scope",
         examples=[["read:all"]],
     )

src/gafaelfawr/pydantic.py

@@ -47,7 +47,7 @@ def _normalize_ip_address(v: str | IPv4Address | IPv6Address) -> str:
 """
 
 
-def _normalize_scopes(v: str | Iterable[str]) -> list[str]:
+def _normalize_scopes(v: str | Iterable[str]) -> set[str]:
     """Pydantic validator for scope fields.
 
     Scopes are stored in the database as a comma-delimited, sorted list.
@@ -65,15 +65,21 @@ def _normalize_scopes(v: str | Iterable[str]) -> list[str]:
         Scopes as a set.
     """
     if isinstance(v, str):
-        return [] if not v else sorted(v.split(","))
+        return set() if not v else set(v.split(","))
     else:
-        return sorted(v)
+        return set(v)
 
 
-Scopes: TypeAlias = Annotated[list[str], PlainValidator(_normalize_scopes)]
+Scopes: TypeAlias = Annotated[
+    set[str],
+    PlainValidator(_normalize_scopes),
+    PlainSerializer(
+        lambda s: sorted(s), return_type=list[str], when_used="json"
+    ),
+]
 """Type for a list of scopes.
 
-The scopes will be forced to sorted order by validation.
+The scopes will be forced to sorted order on serialization.
 """
 
 

src/gafaelfawr/services/kubernetes.py

@@ -327,7 +327,7 @@ async def _create_token(self, parent: GafaelfawrServiceToken) -> Token:
         request = AdminTokenRequest(
             username=parent.spec.service,
             token_type=TokenType.service,
-            scopes=parent.spec.scopes,
+            scopes=set(parent.spec.scopes),
         )
         return await self._token_service.create_token_from_admin_request(
             request, TokenData.internal_token(), ip_address=None
@@ -342,7 +342,7 @@ async def _is_token_valid(
             return False
         if token_data.username != parent.spec.service:
             return False
-        return sorted(token_data.scopes) == sorted(parent.spec.scopes)
+        return token_data.scopes == set(parent.spec.scopes)
 
     async def _secret_needs_update(
         self, parent: GafaelfawrServiceToken, secret: V1Secret | None

src/gafaelfawr/services/token.py

@@ -186,7 +186,7 @@ async def audit(self, *, fix: bool = False) -> list[str]:
         return alerts
 
     async def create_session_token(
-        self, user_info: TokenUserInfo, *, scopes: list[str], ip_address: str
+        self, user_info: TokenUserInfo, *, scopes: set[str], ip_address: str
     ) -> Token:
         """Create a new session token.
 
@@ -217,9 +217,7 @@ async def create_session_token(
         async with self._session.begin():
             admins = await self._admin_store.list()
         if any(user_info.username == a.username for a in admins):
-            scopes = sorted({*scopes, "admin:token"})
-        else:
-            scopes = sorted(scopes)
+            scopes.add("admin:token")
 
         data = TokenData(
             token=token,
@@ -256,7 +254,7 @@ async def create_session_token(
             token_key=token.key,
             token_username=data.username,
             token_expires=format_datetime_for_logging(expires),
-            token_scopes=scopes,
+            token_scopes=sorted(scopes),
             token_userinfo=data.to_userinfo_dict(),
         )
 
@@ -294,7 +292,7 @@ async def create_oidc_token(
             token=token,
             username=auth_data.username,
             token_type=TokenType.oidc,
-            scopes=[],
+            scopes=set(),
             created=created,
             expires=expires,
             name=auth_data.name,
@@ -342,7 +340,7 @@ async def create_user_token(
         username: str,
         *,
         token_name: str,
-        scopes: list[str],
+        scopes: set[str],
         expires: datetime | None = None,
         ip_address: str,
     ) -> Token:
@@ -393,7 +391,6 @@ async def create_user_token(
         self._validate_scopes(scopes, auth_data)
         if expires:
             expires = expires.replace(microsecond=0)
-        scopes = sorted(scopes)
 
         token = Token()
         created = current_datetime()
@@ -498,7 +495,7 @@ async def create_token_from_admin_request(
             token=token,
             username=request.username,
             token_type=request.token_type,
-            scopes=sorted(request.scopes),
+            scopes=request.scopes,
             created=created,
             expires=expires,
             name=request.name,
@@ -537,7 +534,7 @@ async def create_token_from_admin_request(
                 token_username=request.username,
                 token_expires=format_datetime_for_logging(expires),
                 token_name=request.token_name,
-                token_scopes=data.scopes,
+                token_scopes=sorted(data.scopes),
                 token_userinfo=data.to_userinfo_dict(),
             )
         else:
@@ -546,7 +543,7 @@ async def create_token_from_admin_request(
                 token_key=token.key,
                 token_username=request.username,
                 token_expires=format_datetime_for_logging(expires),
-                token_scopes=data.scopes,
+                token_scopes=sorted(data.scopes),
                 token_userinfo=data.to_userinfo_dict(),
             )
         return token
@@ -751,7 +748,7 @@ async def get_internal_token(
         self,
         token_data: TokenData,
         service: str,
-        scopes: list[str],
+        scopes: set[str],
         *,
         ip_address: str,
         minimum_lifetime: timedelta | None = None,
@@ -783,7 +780,6 @@ async def get_internal_token(
         """
         self._validate_scopes(scopes, token_data)
         self._validate_username(token_data.username)
-        scopes = sorted(scopes)
         return await self._token_cache.get_internal_token(
             token_data,
             service,
@@ -919,7 +915,7 @@ async def modify_token(
         *,
         ip_address: str,
         token_name: str | None = None,
-        scopes: list[str] | None = None,
+        scopes: set[str] | None = None,
         expires: datetime | None = None,
         no_expire: bool = False,
     ) -> TokenInfo | None:
@@ -992,7 +988,7 @@ async def modify_token(
             username=info.username,
             token_type=TokenType.user,
             token_name=token_name if token_name else info.token_name,
-            scopes=sorted(scopes) if scopes is not None else info.scopes,
+            scopes=scopes if scopes is not None else info.scopes,
             expires=info.expires if not (expires or no_expire) else expires,
             actor=auth_data.username,
             action=TokenChange.edit,
@@ -1006,7 +1002,7 @@ async def modify_token(
             info = await self._token_db_store.modify(
                 key,
                 token_name=token_name,
-                scopes=sorted(scopes) if scopes else scopes,
+                scopes=scopes,
                 expires=expires,
                 no_expire=no_expire,
             )
@@ -1111,7 +1107,7 @@ async def _audit_token(
             mismatches.append("username")
         if db.token_type != redis.token_type:
             mismatches.append("type")
-        if db.scopes != sorted(redis.scopes):
+        if db.scopes != redis.scopes:
             # There was a bug where Redis wasn't updated when the scopes were
             # changed but the database was. Redis is canonical, so set the
             # database scopes to match.
@@ -1168,19 +1164,18 @@ def _audit_unknown_scopes(self, tokens: Iterable[TokenData]) -> list[str]:
         alerts = []
         for token_data in tokens:
             known_scopes = set(self._config.known_scopes.keys())
-            for scope in token_data.scopes:
-                if scope not in known_scopes:
-                    self._logger.warning(
-                        "Token has unknown scope",
-                        token=token_data.token.key,
-                        user=token_data.username,
-                        scope=scope,
-                    )
-                    alerts.append(
-                        f"Token `{token_data.token.key}` for"
-                        f" `{token_data.username}` has unknown scope"
-                        f" (`{scope}`)"
-                    )
+            for scope in token_data.scopes - known_scopes:
+                self._logger.warning(
+                    "Token has unknown scope",
+                    token=token_data.token.key,
+                    user=token_data.username,
+                    scope=scope,
+                )
+                alerts.append(
+                    f"Token `{token_data.token.key}` for"
+                    f" `{token_data.username}` has unknown scope"
+                    f" (`{scope}`)"
+                )
         return alerts
 
     def _check_authorization(
@@ -1419,7 +1414,7 @@ def _validate_expires(self, expires: datetime | None) -> None:
 
     def _validate_scopes(
         self,
-        scopes: list[str],
+        scopes: set[str],
         auth_data: TokenData | None = None,
     ) -> None:
         """Check that the requested scopes are valid.
@@ -1439,12 +1434,11 @@ def _validate_scopes(
         """
         if not scopes:
             return
-        scopes_set = set(scopes)
         if auth_data and "admin:token" not in auth_data.scopes:
-            if not (scopes_set <= set(auth_data.scopes)):
+            if not (scopes <= auth_data.scopes):
                 msg = "Requested scopes are broader than your current scopes"
                 raise InvalidScopesError(msg)
-        if not (scopes_set <= self._config.known_scopes.keys()):
+        if not (scopes <= set(self._config.known_scopes.keys())):
             msg = "Unknown scopes requested"
             raise InvalidScopesError(msg)