Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions armasec_lite/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@

from __future__ import annotations

from collections.abc import Callable
from collections.abc import Callable, Collection
from enum import Enum
from typing import Any
from urllib.parse import urlparse
Expand Down Expand Up @@ -269,7 +269,9 @@ class DomainConfig(BaseModel):
match_keys: Key/value pairs that must be present in a decoded token.
A mismatch raises 403.
permission_extractor: Optional function that extracts permissions from the
decoded token when they are not a top level claim.
decoded token when they are not a top level claim. May
return any collection of strings; pydantic coerces the
result into `TokenPayload.permissions`, a set.
"""

domain: str
Expand All @@ -279,7 +281,7 @@ class DomainConfig(BaseModel):
use_https: bool = True
verify_issuer: bool = True
match_keys: dict[str, Any] = {}
permission_extractor: Callable[[dict[str, Any]], list[str]] | None = None
permission_extractor: Callable[[dict[str, Any]], Collection[str]] | None = None

@field_validator("domain")
@classmethod
Expand Down
14 changes: 9 additions & 5 deletions armasec_lite/token_decoder.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@

from __future__ import annotations

from collections.abc import Callable
from collections.abc import Callable, Collection
from functools import partial
from typing import Any

Expand Down Expand Up @@ -86,7 +86,7 @@ def __init__(
algorithm: str = "RS256",
debug_logger: Callable[..., None] | None = None,
decode_options_override: dict[str, Any] | None = None,
permission_extractor: Callable[[dict[str, Any]], list[str]] | None = None,
permission_extractor: Callable[[dict[str, Any]], Collection[str]] | None = None,
jwks_refresher: Callable[[], JWKs] | None = None,
):
"""
Expand All @@ -103,6 +103,9 @@ def __init__(
See the `armasec_lite.jwt` module docstring.
permission_extractor: Optional function that extracts permissions from the
decoded token when they are not a top level claim.
It may return any collection of strings: a list, set
or tuple all validate, since pydantic coerces the
result into `TokenPayload.permissions`, a set.

Consider the example token:

Expand Down Expand Up @@ -356,7 +359,7 @@ def decode(self, token: str, **claims: Any) -> TokenPayload:
return token_payload


def extract_keycloak_permissions(decoded_token: dict[str, Any]) -> list[str]:
def extract_keycloak_permissions(decoded_token: dict[str, Any]) -> set[str]:
"""
Extract permissions from a Keycloak token.

Expand All @@ -374,7 +377,8 @@ def extract_keycloak_permissions(decoded_token: dict[str, Any]) -> list[str]:
}
```

this extractor returns `["read:stuff"]`.
this extractor returns `{"read:stuff"}`. A set rather than a list, matching the
`TokenPayload.permissions` field it feeds; Keycloak roles carry no meaningful order.

Pass it as `DomainConfig(permission_extractor=extract_keycloak_permissions)`. It is
called only after the signature has verified, so the claims it reads are trustworthy.
Expand All @@ -393,4 +397,4 @@ def extract_keycloak_permissions(decoded_token: dict[str, Any]) -> list[str]:
configuration mistake, not a bad token.
"""
resource_key = decoded_token["azp"]
return list(decoded_token["resource_access"][resource_key]["roles"])
return set(decoded_token["resource_access"][resource_key]["roles"])
37 changes: 30 additions & 7 deletions armasec_lite/token_payload.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,21 @@
provider's tokens may be absent on another's, and the model will not tell you in advance.
Read unknown claims with `getattr(payload, name, default)`.

### `permissions` is a set

Upstream armasec typed `permissions` as a list. Here it is a `set[str]`, because every
consumer of the field, the scope check above all, tests membership and intersection and
never order, and a set makes those checks constant time instead of linear. Pydantic
coerces the JSON array in the token's claim on validation, so duplicates collapse and
issuers need not change anything. Only code that indexed or ordered `payload.permissions`
notices; `to_dict` still emits a sorted list, keeping the compatibility shim's shape.

The consumer-visible caveat is serialization order. In code the field is an ordinary
Python set, but when a handler returns the payload as its response body, the JSON encoder
turns the set into a list in iteration order, which for strings varies per process. A
snapshot test or downstream contract asserting that JSON byte-for-byte becomes flaky.
Where a stable shape matters, return `to_dict()` or sort the field before serializing.

### Aliases

`expire` and `client_id` accept two source names each, via `AliasChoices`: the registered
Expand Down Expand Up @@ -55,9 +70,14 @@ class TokenPayload(BaseModel):
The only required field.
permissions: The permissions the token grants, checked against a route's
scopes. Read from a top level `permissions` claim, or produced by
a `permission_extractor` for providers that nest them. Defaults to
empty, so a token with no permissions authenticates but authorizes
nothing.
a `permission_extractor` for providers that nest them. A set,
deliberately: permissions are only ever tested for membership and
intersection, never for order, and validation collapses a claim
that repeats a permission into granting it once. Serializing the
payload renders the set in nondeterministic order; where a stable
shape matters, use `to_dict`, which emits a sorted list. Defaults
to empty, so a token with no permissions authenticates but
authorizes nothing.
expire: The "exp" (or "expire") claim, as a datetime. Informational here:
expiry was already enforced during decoding, so a payload in hand
is not expired.
Expand All @@ -70,7 +90,7 @@ class TokenPayload(BaseModel):
"""

sub: str
permissions: list[str] = Field(default_factory=list)
permissions: set[str] = Field(default_factory=set)
expire: datetime | None = Field(None, validation_alias=AliasChoices("exp", "expire"))
client_id: str | None = Field(None, validation_alias=AliasChoices("azp", "client_id"))
original_token: str | None = None
Expand All @@ -87,12 +107,15 @@ def to_dict(self) -> dict[str, Any]:
`exp` again. Prefer reading attributes off the model directly.

Returns:
A dictionary with `sub`, `permissions`, `exp` and `client_id`. `exp` is None
when the token carried no expiry.
A dictionary with `sub`, `permissions`, `exp` and `client_id`. `permissions`
is a sorted list rather than the set on the model, because the shim's whole
point is upstream's JSON-friendly shape and a set is neither
JSON-serializable nor deterministically ordered. `exp` is None when the token
carried no expiry.
"""
return {
"sub": self.sub,
"permissions": self.permissions,
"permissions": sorted(self.permissions),
"exp": int(self.expire.timestamp()) if self.expire is not None else None,
"client_id": self.client_id,
}
16 changes: 11 additions & 5 deletions armasec_lite/token_security.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,10 @@ class TokenSecurity(APIKeyBase):
Attributes:
domain_configs: The OIDC domains a token may be authenticated against. A token
is accepted if any one of them can decode it.
scopes: Permissions the token must carry, or None to check none.
scopes: Permissions the token must carry, as a frozenset. Empty means
authentication only, with no permission check. Materialized
once at construction because `_check_scopes` runs per request
and must not rebuild it there.
permission_mode: How `scopes` is matched. ALL requires every one, SOME requires
at least one.
debug_logger: A callable such as `logger.debug`. Defaults to `noop`, which
Expand Down Expand Up @@ -140,7 +143,10 @@ def __init__(
token is accepted if any one of them decodes it.
scopes: Optional permission scopes that should be checked. When
empty or None, authentication is required but no permission
check is performed.
check is performed. Any iterable of strings is accepted and
materialized into a frozenset here, so a generator is
consumed exactly once instead of silently emptying after
the first request.
permission_mode: How the scopes are matched. ALL or SOME.
debug_logger: A callable such as `logger.debug`. Defaults to `noop`.
debug_exceptions: If True, raise original exceptions instead of translating
Expand All @@ -149,7 +155,7 @@ def __init__(
skip_plugins: If True, do not evaluate plugin validators.
"""
self.domain_configs = domain_configs
self.scopes = scopes
self.scopes = frozenset(scopes or ())
self.permission_mode = permission_mode

self.debug_logger = debug_logger if debug_logger else noop
Expand Down Expand Up @@ -324,8 +330,8 @@ def _check_scopes(self, token_payload: TokenPayload) -> None:
branch recognizes. Maps to 403: the caller is authenticated, just not
allowed.
"""
token_permissions = set(token_payload.permissions)
my_permissions = set(self.scopes or ())
token_permissions = token_payload.permissions
my_permissions = self.scopes

# Guarded rather than left to the logger to discard: this runs once per request,
# and `unwrap` splits and rejoins the whole composed string.
Expand Down
14 changes: 11 additions & 3 deletions docusaurus/docs/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,11 @@ upstream, instead of breaking all three to save zero bytes.
| `fastapi` | kept |
| (new) | `cryptography` |

## Four differences from upstream
## Five differences from upstream

`armasec-lite` targets a drop-in migration, but four things differ. Each is covered in
full on the [migration guide](./migration.md).
`armasec-lite` targets a drop-in migration, but five things differ. Each is covered in
full on the [migration guide](./migration.md), which carries the complete, authoritative
list.

1. **Import name.** The package imports as `armasec_lite`, not `armasec`. The distribution
name changes too: `armasec-lite`, not `armasec`.
Expand All @@ -60,6 +61,13 @@ full on the [migration guide](./migration.md).
additive and requires no action: the first positional argument is still `JWKs`, so
every existing construction site is unaffected. It is what lets a JWKS key rotation
recover without a process restart.
5. **`TokenPayload.permissions` is a `set[str]`, not a `list[str]`.** Permissions are
only ever tested for membership and intersection, so the scope check runs in constant
time and a claim repeating a permission collapses into granting it once. Tokens need
no changes; pydantic coerces the claim's JSON array. Two things to watch: code that
indexes or orders the field, and a handler that returns the payload as its response
body, whose `permissions` now serialize into the response JSON in nondeterministic
order. Where a stable JSON shape matters, use `to_dict()`, which emits a sorted list.

## Request flow

Expand Down
44 changes: 44 additions & 0 deletions docusaurus/docs/migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,31 @@ project combined. It may ship later as a separate `armasec-lite-cli` distributio
installed alongside `armasec-lite` if you rely on it, or wait for a future
`armasec-lite-cli`.

## 7. `TokenPayload.permissions` is a `set[str]`

Upstream types `permissions` as `list[str]`. `armasec-lite` types it as `set[str]`,
because every consumer of the field, the scope check above all, tests membership and
intersection and never order, and a set makes those checks constant time. Nothing changes
on the way in: the claim in the token is still a JSON array, pydantic coerces it on
validation, and a claim that repeats a permission now collapses into granting it once.
`extract_keycloak_permissions` returns a set as well, matching the field it feeds.

Two things on the way out can break:

- Code that indexes or orders the field (`payload.permissions[0]`, `sorted` assumptions,
strict equality against a list) stops working. Compare against a set, or sort
explicitly where you need an order.
- A handler that returns the `TokenPayload` model as its response body now serializes
`permissions` in nondeterministic order: JSON has no set type, so the encoder turns the
set into a list in iteration order, and set iteration order for strings varies per
process. Nothing changes inside your code, where the field is an ordinary Python set;
only the JSON a client receives is affected. Anything asserting or diffing that JSON
byte-for-byte, such as a snapshot test or a downstream contract test, becomes flaky.

**Fix:** compare permissions with set operations. Where a stable JSON shape matters, use
`payload.to_dict()`, which deliberately emits `permissions` as a sorted list, or sort the
field yourself before serializing.

## Requires no action

A few more differences round out the API diff. None of them breaks anything migrating, so
Expand Down Expand Up @@ -172,6 +197,23 @@ message that names neither the domain nor the configuration.
load a provider, and a `DomainConfig` with an unsupported algorithm could not decode a
token. `Armasec()` with no domain at all still raises the same 422 it always did.

### `TokenSecurity.scopes` is a frozenset, and `permission_extractor` may return any collection

`TokenSecurity` materializes the `scopes` it is given into a `frozenset` at construction,
so the per-request scope check rebuilds nothing. The constructor still accepts any
iterable of strings, exactly as upstream did; only code reading `security.scopes` back
and expecting the original list or tuple notices. Materializing also means a generator
passed as `scopes` is consumed once, at construction, instead of silently emptying after
the first request.

Relatedly, the `permission_extractor` contract widened from returning `list[str]` to
returning any `Collection[str]`. Every existing extractor that returns a list remains
valid; pydantic coerces whatever collection comes back into the `permissions` set.

**Why it is safe:** `scopes` was only ever consumed by the scope check, which compares by
set semantics regardless of input type, and widening an accepted return type breaks no
existing implementation.

### `handle_errors` does not re-wrap an error that is already ours

py-buzz's `handle_errors` wraps every exception raised in its block, including one of its
Expand All @@ -198,6 +240,7 @@ The tables below are the complete list; nothing above adds to or contradicts the
| The pytest fixtures live behind the `[test]` extra | A ported test suite cannot import the fixtures from a plain install, because upstream forced `pytest` into every install and this does not | Depend on `armasec-lite[test]` |
| The OIDC loader cache is process-wide | Tests that expect per-instance provider state now share it | `openid_config_loader.clear_cache()`, or the `mock_openid_server` fixture, which calls it automatically |
| The CLI is not included | `armasec` console script is gone | Out of scope; see [Security](./security/index.md#what-is-out-of-scope) |
| `TokenPayload.permissions` is a `set[str]` | Code that indexes or orders the field, compares it to a list, or asserts byte-stable JSON from a handler that returns the payload as its response body (the encoder serializes the set in nondeterministic order) | Use set operations; for a stable JSON shape use `to_dict()`, which emits a sorted list |

**Requires no action, listed so the API diff is complete:**

Expand All @@ -207,3 +250,4 @@ The tables below are the complete list; nothing above adds to or contradicts the
| `JWK` no longer requires `n` and `e` | Strictly more permissive; an EC or OKP key that upstream rejected now parses |
| `DomainConfig` requires a non-empty `domain` and a supported `algorithm` | Neither shape ever worked; the failure just moved from request time to construction time. `Armasec()` with no domain still raises its own 422 |
| `handle_errors` re-raises an `ArmasecError` subclass unchanged | Everything that is not already ours is still wrapped. Re-wrapping would let the `PayloadMappingError` block turn a genuine 401 into a 500 |
| `TokenSecurity.scopes` is materialized into a frozenset at construction, and `permission_extractor` may return any `Collection[str]` | The constructor accepts the same iterables it always did, the scope check always compared by set semantics, and widening an accepted return type breaks no existing extractor |
11 changes: 8 additions & 3 deletions docusaurus/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3163,10 +3163,10 @@
dependencies:
"@types/yargs-parser" "*"

"@typescript/typescript-linux-x64@7.0.2":
"@typescript/typescript-darwin-arm64@7.0.2":
version "7.0.2"
resolved "https://registry.npmjs.org/@typescript/typescript-linux-x64/-/typescript-linux-x64-7.0.2.tgz"
integrity sha512-EYdf2cNg7rgCWJnxCdJ+F3V39O8ihb37eHAu1LK8oAFizgTQbPOK7zHHXbPt8rX24COqODXeI3sIf0fCXG7H/A==
resolved "https://registry.npmjs.org/@typescript/typescript-darwin-arm64/-/typescript-darwin-arm64-7.0.2.tgz"
integrity sha512-gowzar9MwS/aRWp6f3a4KUqzRjAZjOsmGNCM6LcTgXum+dBfgsBVMN+AgvOCCbguXyick6LJhpBszxMebJ8syA==

"@ungap/structured-clone@^1.0.0":
version "1.4.0"
Expand Down Expand Up @@ -5373,6 +5373,11 @@ fs-extra@^11.1.1, fs-extra@^11.2.0:
jsonfile "^6.0.1"
universalify "^2.0.0"

fsevents@~2.3.2:
version "2.3.3"
resolved "https://registry.npmjs.org/fsevents/-/fsevents-2.3.3.tgz"
integrity sha512-5xoDfX+fL7faATnagmWPpbFtwh/R77WmMMqqHGS65C3vvB0YHrgF+B1YmZ3441tMj5n63k0212XNoJwzlhffQw==

function-bind@^1.1.2:
version "1.1.2"
resolved "https://registry.npmjs.org/function-bind/-/function-bind-1.1.2.tgz"
Expand Down
6 changes: 3 additions & 3 deletions tests/unit/test_token_decoder.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ def test_decode_builds_a_token_payload(jwks, rsa_private, now):
token = _sign(rsa_private, {"sub": "abc", "exp": now + 60, "permissions": ["read:x"]})
payload = decoder.decode(token)
assert payload.sub == "abc"
assert payload.permissions == ["read:x"]
assert payload.permissions == {"read:x"}
assert payload.original_token == token


Expand All @@ -128,7 +128,7 @@ def test_decode_applies_the_permission_extractor(jwks, rsa_private, now):
"resource_access": {"my-client": {"roles": ["read:stuff"]}},
},
)
assert decoder.decode(token).permissions == ["read:stuff"]
assert decoder.decode(token).permissions == {"read:stuff"}


def test_decode_raises_payload_mapping_error_when_the_extractor_misses(jwks, rsa_private, now):
Expand Down Expand Up @@ -168,4 +168,4 @@ def test_decode_restricts_the_algorithm_to_the_configured_one(jwks, rsa_private,

def test_extract_keycloak_permissions_reads_the_nested_roles():
decoded = {"azp": "my-client", "resource_access": {"my-client": {"roles": ["read:stuff"]}}}
assert extract_keycloak_permissions(decoded) == ["read:stuff"]
assert extract_keycloak_permissions(decoded) == {"read:stuff"}
Loading