A plugin installation brings its role with it - #377
Merged
Conversation
Declaring permission keys made them grantable; it did not put them anywhere. The last step was left to the operator: install a plugin, open role administration, click its keys together by hand. Miss it and the plugin is installed, its screens answer 403, and the only account that can use it is the super admin — with nothing anywhere saying why. The same failure class this codebase has now hit three times: enforcement works, granting does not. Every plugin that has permissions at all now gets one role, <pluginId>.admin, holding all of them. Both supply paths, one role. A plugin declares its keys in the manifest or contributes them through IHostAdminApiExtensionContributor, and of the four plugins installed here exactly one uses the first. Reading only the manifest would have left the other three without a role, silently — an empty key list is indistinguishable from "has no permissions". One role, not three. "View", "manage", "announcements only" are cuts the plugin has to know; here only the set of keys is known. A guessed split would be worse than none: it looks considered, so nobody checks it. Created once, then never touched. Take a key out, add another, rename it — that survives every restart. A tuning decision silently reverted at the next start is worse than a missing permission: the missing one is visible, the reverted one is not, and it is invisible exactly when it leaves somebody able to do something they should no longer be able to do. What a plugin update declares on top becomes a log line, deliberately the weaker delivery — it can be missed, but it cannot overwrite a decision. The role is found by (ProvisionedByPluginId, ProvisionedAs), not by name, so renaming it in the UI does not put a second one beside it at the next start. A role a human already created under that name is left alone and reported: it belongs to them, and adopting it would mean deleting it with the plugin. Two entry points, one provisioner: a hosted service registered after plugin rehydration for what is already installed, and a PluginLifecycleChangedEvent subscriber for what arrives later — the same shape the custom-field, snippet and sensitive-field syncs already use. Uninstall removes nothing: a user has exactly one role, and deleting it would take away every access they have, including the parts that have nothing to do with the plugin. Corrected while writing it: the index comment claimed its filter is what lets several hand-made roles coexist. Measured — it is not. Postgres treats NULLs in a unique index as distinct, and the test passes without the filter. The filter is there so the index covers only provisioned rows, and the comment now says that. Not closed by this, and worth knowing: a workspace-scoped session carries the permissions in WorkspaceRolePermissions.ForRole and nothing else — a hardcoded core-key list. A provisioned role therefore works for platform-scoped operators; a workspace administrator still cannot hold a plugin key. That gap predates this mechanism. 13 new tests. Proven red: move the hosted service before rehydration and the ordering gate fails; remove the create-once guard and the drift test fails; remove the name check and the collision test fails. Full suite 1757 + 34 green.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Declaring permission keys made them grantable. It did not put them anywhere.
The last step was the operator's: install a plugin, open role administration, click its keys together
into a role by hand. Miss it, and the plugin is installed, its screens answer
403, and the onlyaccount that can use it is the super admin — with nothing anywhere saying why.
That is the same failure class already on record twice in this codebase (
BackendPermissionKeyValidator,BackendPermissionInventory): Absicherung wirksam, Vergabe unmöglich. This closes the third instance.What
Every plugin that has permissions at all now gets one role,
<pluginId>.admin, holding all of them.Both supply paths, one role. A plugin declares its keys in the manifest or contributes them
through
IHostAdminApiExtensionContributor— and of the four plugins installed here exactly one usesthe first. Reading only the manifest would have left the other three without a role, silently: an empty
key list is indistinguishable from "has no permissions".
One role, not three. "View", "manage", "announcements only" are cuts the plugin has to know; here
only the set of keys is known. A guessed split would be worse than none — it looks considered, so
nobody checks it. Finer roles come from the manifest once it can carry them; until then they are one
click away from this one.
Created once, then never touched. Take a key out, add another, rename it — that survives every
restart. A tuning decision silently reverted at the next start is worse than a missing permission: the
missing one is visible, the reverted one is not, and it is invisible exactly when it leaves somebody
able to do something they should no longer be able to do. What a plugin update declares on top becomes
a log line — deliberately the weaker delivery, because it can be missed but cannot overwrite a
decision.
Found by identity, not by name.
(ProvisionedByPluginId, ProvisionedAs)— renaming the role in theUI does not put a second one beside it at the next start. A role a human already created under that
name is left alone and reported: it belongs to them, and adopting it would mean deleting it with the
plugin.
Two entry points, one provisioner. A hosted service registered after plugin rehydration for what
is already installed, and a
PluginLifecycleChangedEventsubscriber for what arrives later — the sameshape the custom-field, snippet and sensitive-field syncs already use. The ordering is load-bearing and
has its own gate: registered earlier, the contributor-path plugins get no role and the start still
looks clean.
Uninstall removes nothing. A user has exactly one role; deleting it would take away every access
they have, including the parts that have nothing to do with the plugin. A role whose plugin is gone
grants keys that match nothing — harmless and visible.
A comment that was wrong
The index comment claimed its filter is what lets several hand-made roles coexist. Measured: it is not.
Postgres treats NULLs in a unique index as distinct, and the test passes with the filter removed. The
filter exists so the index covers only provisioned rows. Both the comment and the test now say that,
and the test stays — it would notice the day somebody switches the index to
NULLS NOT DISTINCT.What this does not fix
A workspace-scoped session carries the permissions in
WorkspaceRolePermissions.ForRoleand nothingelse — a hardcoded core-key list — and
BackendClaimsTransformationreturns early for workspace scope.So a provisioned role takes effect for platform-scoped operators; a workspace administrator still
cannot hold a plugin key at all.
That gap predates this mechanism and is a separate decision: which plugin keys a workspace admin may
carry (plausibly: those of the plugins activated for their workspace). It is documented in the manifest
guide rather than quietly widened here.
Verification
13 new tests, including 5 against a real Postgres. Proven red first: