Tiled >= 0.2.10 breaks the copied access-policy plugin (PrincipalType.external removed) in p46/p47/p48/p49-services #219

Activity

1. gilesknap commented on Sep 24, 2026

   

   gilesknap

   on Sep 24, 2026

   MemberAuthor

   More actions

   Chased this upstream. The diagnosis holds, the port is already solved inside DLS, and one detail in the issue needs correcting. Opened four guard PRs; did not do the port itself, for the reason in "Sequencing" below.

   Where external went, and why

   PrincipalType.external was removed by bluesky/tiled#1377 ("Reconcile device code flow and API keys with ProxiedOIDCAuthenticator", commit a44a8f8, merged 2026-05-11, first released in v0.2.10 on 2026-05-22).

   The rationale, from the PR body: ProxiedOIDCAuthenticator "does not create a Principal with Roles or Identities in Tiled, which breaks API key functionality" — whoami() returned {'type': 'external', 'identities': [], 'roles': [], 'api_keys': []}. The same commit rewrote tiled/server/authentication.py so a proxied-OIDC request resolves to a real DB-backed principal:

   • type=PrincipalType.external → type=PrincipalType.user (both sites)
   • identities=[] → identities=[Identity(id=identity_id, provider=provider)], where identity_id = decoded_access_token.get("user") or decoded_access_token["sub"]
   • the whoami short-circuit for external principals was deleted

   The enum member was then dropped because nothing produced it any more. A follow-up, bluesky/tiled#1385 (commit ebd6376, also in v0.2.10), is the half that matters most to us — its own description: "Carry the access token in the Principal model (needed for DLS' authorization code)." So Principal.access_token is still populated for proxied-OIDC principals in current tiled; schemas.py:436 still has access_token: Optional[SecretStr] = Field(exclude=True, default=None).

   There is no CHANGELOG entry for any of this. git grep PrincipalType v0.2.18 -- CHANGELOG.md docs/ is empty. It was an undocumented removal of a public symbol, which is why this sat unnoticed.

   The port is three lines, and DLS has already made it — twice

   1. The canonical copy, DiamondLightSource/blueapi, tests/system_tests/services/tiled_config/dls.py, was ported in DiamondLightSource/blueapi#1488 ("chore(tests): Update tiled to v0.2.12", commit b2d8ee9, merged 2026-06-24). The entire patch is a substitution:

   -        if principal.type != PrincipalType.external:
   +        if principal.type != PrincipalType.user:
                raise HTTPException(
                    status_code=HTTP_401_UNAUTHORIZED,
   -                detail=f"Principal of type {PrincipalType.external}"
   +                detail=f"Principal of type {PrincipalType.user}"
                    f" required but given {principal.type}",
                    headers={"WWW-Authenticate": "Bearer"},
                )

                if (
                    isinstance(principal, Principal)
   -                and principal.type is PrincipalType.external
   +                and principal.type is PrincipalType.user
                    and principal.access_token is not None
                ):

   2. epics-containers/t11-services already runs it. services/t11-tiled/templates/configmap-access-policy.yaml carries the ported plugin and values.yaml has image.tag: 0.2.18. I extracted the embedded dls.py from t11 and diffed it against p46's: the only difference is those three lines. So there is a working, deployed reference pair inside this org.

   A GitHub code search for PrincipalType.external now returns these four configmaps (plus one unrelated fork) as the only remaining hits in the tiled ecosystem. p46/p47/p48/p49 are the last holdouts.

   Nothing else in the plugin's API moved

   git diff v0.2.9..v0.2.18 -- tiled/access_control/ touches only scopes.py. tiled/access_control/access_policies.py is byte-identical across those tags, so ExternalPolicyDecisionPoint.__init__, init_node, modify_node, filters, build_input, _get_external_decision, ResultHolder, ALL_ACCESS and NO_ACCESS all keep their signatures. tiled/type_aliases.py still exports AccessBlob, AccessTags, Filters, Scopes; tiled.queries.AccessBlobFilter is unchanged (user_id, tags); tiled.adapters.protocols.BaseAdapter is still there. (The one scopes.py change, USER_SCOPES → SINGLE_USER_SCOPES, does not affect us — the plugin never imports it.)

   So the port really is three lines and nothing more.

   Correction: Renovate is not the trigger

   The issue says a Renovate bump "will hit this in all four repos at once". Not as things stand:

   • The four open Renovate PRs — p46#193, p47#169, p48#44, p49#70, all titled "Update tiled Docker tag to v0.2.18" — actually change Chart.yaml, the Helm chart dependency version, not the image.
   • The image comes from image.tag in values.yaml, which overrides the chart's default. That default is tag: "" (falling back to appVersion) in both chart 0.2.2 and chart 0.2.18, so the explicit 0.2.7 still wins after a chart bump.
   • Renovate cannot see image.tag either: its helm-values manager needs an image.repository sibling to identify the dependency, and there isn't one. Consistent with history — p46's services/p46-tiled/values.yaml has six commits, all human, none from Renovate.
   • Live proof: p47 already merged a chart bump to 0.2.14 (p47-services#157, 2026-07-26) and is still serving image 0.2.7 today.

   So the four open Renovate PRs are not the landmine — merging them is a chart-only change. The landmine is a human editing tag:, which is why the guard belongs next to that line rather than in renovate.json. That is a lower urgency than the issue implies, but the same eventual outcome.

   Sequencing — why I did not just do the port

   Porting the plugin without bumping the image is an outage, not a no-op. Under 0.2.7 a proxied-OIDC principal genuinely is external, so _check_principal testing != PrincipalType.user would 401 every user on all four beamlines. The port and the tag bump have to be the same commit.

   And the bump is not free for three of the four:

   repo	auth DB
   p46	sqlite:///file:authdb?mode=memory&cache=shared — throwaway, no migration concern
   p47, p48, p49	sqlite:////storage/auth.db — persistent

   tiled's authn_database/migrations/versions goes from 12 revisions at v0.2.7 to 13 at v0.2.18, so the persistent auth DBs need an alembic upgrade (or to be recreated) as part of the move. That is a deployment decision with a beamline window attached, not something to land unattended, so I have left it to you.

   What I did open

   Four PRs, each a YAML comment only — no value changes, rendered output byte-identical — putting the warning immediately above tag: where the next bump will happen:

   • Warn against bumping tiled past 0.2.9 without porting the access policy #220
   • Warn against bumping tiled past 0.2.9 without porting the access policy p47-services#177
   • Warn against bumping tiled past 0.2.9 without porting the access policy p48-services#50
   • Warn against bumping tiled past 0.2.9 without porting the access policy p49-services#80

   They deliberately do not say Fixes, since the actual port is still outstanding.

   One judgement call for whoever does the port

   != PrincipalType.user is not an exact equivalent of the old != PrincipalType.external: it now also admits a principal from tiled's own internally-minted user tokens, which would reach build_input with no access token and send OPA an input carrying only audience. Upstream's own equivalent example, example_configs/keycloak_oidc/tiled_policy/example_pdp.py, took the stricter route in the same release — it dropped the type test entirely and guards on if principal.access_token is not None:, which is exact regardless of type.

   Matching t11 and blueapi (is PrincipalType.user) keeps all three DLS copies identical, which is probably worth more than the marginal strictness — but it is worth a deliberate decision rather than a copy-paste.

   Investigated in the epics-containers sweep follow-up; drafted by Claude Code.
2. gilesknap commented on Sep 24, 2026

   

   gilesknap

   on Sep 24, 2026

   MemberAuthor

   More actions

   Correction to this issue's description, from the independent review of the guard PRs.

   This issue (and the guard comments as first written) said the server "fails to
   import the plugin and will not start" on Tiled >= 0.2.10. That is wrong, and the
   real failure mode is worse to diagnose:

   All three PrincipalType.external uses are inside function bodies, so the module
   imports fine and the server starts normally. ExternalPolicyDecisionPoint
   (v0.2.10) calls build_input from filters, allowed_scopes, init_node and
   modify_node, so every authenticated request raises AttributeError and
   returns HTTP 500. There is no startup error to notice.

   The comments added by p46#220, p47#177, p48#50 and p49#80 have been reworded
   accordingly. The commit messages of those PRs still carry the original wording.

   Correction posted by Claude Code after independent review.