Permissions are unmodifiable after issue: shift sign-time auth from materialized SigningCondition snapshots to live policy lookup #11
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
nsecbunkerd's admin RPC surface is insert-only for everything that affects authorization:create_new_policyrevoke_user(binary)In practical terms: once
applyTokenruns for a KeyUser, the set of(method, kind)pairs that user is allowed to sign is frozen forever. The Policy and its rules are template data consulted only at that one moment; after that, the auth check (lib/acl/checkIfPubkeyAllowed) readsSigningConditionrows scoped tokeyUserIdand never re-consults the source.That's the wrong model for any organization-style deployment where permissions evolve. Adding a new event kind to a policy (e.g. NIP-52 calendar kinds 31922/31923 for an LNbits events-extension integration — a real case that surfaced this) does nothing for any KeyUser already bound to that policy. They'd need to be revoked + re-onboarded, which on the lnbits side means re-provisioning the account (new wallet, new identity, etc) — an unacceptable cost.
Repro of the staleness
create_new_policywithrules: [{method: "sign_event", kind: 0}]. Policy id = N.create_new_tokenunder policy N. Take the token.connectwith that token. Bunker creates KeyUser + a singleSigningConditionrow for(method=sign_event, kind=0).sign_eventfor kind 0 → ✅ allowed.sign_eventfor kind 7 → ❌ denied (noSigningConditionrow).add_policy_ruleRPC exists. They'd have to wipe + recreate the policy.SigningConditionsnapshot. Its row set wouldn't change.add_signing_conditionRPC to fix that KeyUser either.revoke_user+ re-onboard. For a deployment with N existing accounts, that's N re-onboardings.Proposed change — auth-by-policy at sign-time
Shift
checkIfPubkeyAllowedfrom snapshot-of-rules to live-lookup-of-rules:Current (
src/daemon/lib/acl/index.ts:35-45):Proposed:
SigningConditionbecomes a per-user override layer (deny-list or extra-grant), not the primary auth source.Policy.rulesbecomes the live source of truth.What this unlocks
Once auth is policy-driven at sign-time:
SigningCondition(e.g. a specific user gets a kind their policy doesn't, or is denied a kind their policy allows).KeyUser(or mark it revoked) without needing per-policy gymnastics.Companion admin RPCs
With (B) landed, the missing-but-not-blocking RPCs become straightforward:
add_policy_rule(policyId, {method, kind})— INSERT intoPolicyRule. Effective immediately for every bound KeyUser.remove_policy_rule(policyId, ruleId)— DELETE. Same.update_policy(policyId, {name?, expiresAt?})— UPDATE. Affects bound users only at next sign attempt.add_signing_condition(keyUserId, {method, kind, allowed})— INSERT intoSigningCondition. Per-user override.remove_signing_condition(signingConditionId)— DELETE. Per-user.revoke_token(tokenId)— UPDATE token'srevokedAtfield; auth check then ignores SigningConditions / policy bindings sourced from this token.These are all single-table mutations with no migration story because the schema already supports them.
Migration concerns
The auth flip is backwards-compatible for existing deployments:
SigningConditionrows continue to work — they're still consulted (now as the override layer).SigningConditionrows onapplyToken(or we can keep that behavior — they're redundant but harmless).(method, kind)immediately. That's the whole point — but worth noting as a behavior change for any deployment that was depending on the snapshot freeze (we don't know of any, but worth flagging in release notes).For the LNbits IdP integration specifically: dropping
applyToken'sSigningCondition-fanout loop becomes the cleanest implementation. TheKeyUserbinding remains (carrieskeyName,userPubkey,description,revokedAt), but thefor (rule of policy.rules) await prisma.signingCondition.create(...)block atsrc/daemon/backend/index.ts:60-75can be deleted entirely once auth is live-from-policy.Cross-references
aiolabs/lnbitsPR #33 — eager-bind chain that surfaced the LNbits-specific need for permission additions across already-onboarded accounts.aiolabs/lnbitsissue forthcoming —DEFAULT_POLICY_RULESis missing NIP-52 kinds 31922/31923. Today that's a hard block; with this change, fixing it would be a one-line bump + RPC call.~/dev/coordination/log.mdentries2026-05-30T10:30Zand later.Out of scope for this issue
maxUsageCount) — the schema has these but they're not enforced anywhere I could find. Worth a separate audit but unrelated to the propagation model.Follow-up: three gaps from the original issue body worth fleshing out before implementation starts
Three things I deferred or under-specified in the issue body. Adding them here so the design has a complete picture.
1. Concrete API shape for the companion admin RPCs
The original body just named the RPCs. Proposed parameter shapes, all following the existing
create_new_policy.ts/create_new_token.tspattern (single JSON-stringified param, response shipped over kind-24134):Symmetrical with the existing
revoke_user(binary update on a single field). Each is one prisma mutation, no migration. The clients inaiolabs/lnbits#35's reconciliation pass would call these in a loop afterget_policiesreturns the current state.2. Multi-lnbits-instance bunker sharing
The current
aiolabs/lnbitsprovisioning convention shares one bunker across multiple lnbits instances by converging onpolicy_name = "lnbits-default". The existing docstring inlnbits/core/signers/remote_bunker.py:90-93acknowledges this:Implication for (B): once auth is policy-driven at sign-time, a rule change on the shared policy propagates instantly to every lnbits instance's user base simultaneously. That's a feature, not a bug — but it has two operational consequences worth surfacing in the implementation:
Concurrent rule additions are safe because they're additive. An lnbits-v1 instance reconciling its rule set against the live policy can call
add_policy_rulefor any kinds it has in itsDEFAULT_POLICY_RULESthat aren't yet on the policy. An lnbits-v2 instance doing the same with a broader set just adds the extra rules. Both converge harmlessly.Concurrent rule removals can race between instances on differing versions. If lnbits-v1's
DEFAULT_POLICY_RULESshrinks (operator drops some kinds), and lnbits-v1 callsremove_policy_rule, every other lnbits instance using the same shared bunker loses those permissions for its user base too. Recommendation: lnbits-side reconciliation should ONLY ADD rules, never remove them, leaving deletions as a manual admin op. Worth noting in lnbits#35's reconciliation pass.This is purely operational, not a schema concern. The bunker doesn't need to know about instance identity.
3. Test plan for the auth-shift change
Regression cases that should cover the migration safely:
add_signing_condition(allowed: false)denies request even when policy allowsadd_signing_condition(allowed: true)allows request even when policy doesn'tadd_policy_rule→ immediately following sign request for that kind from a previously-bound KeyUser succeedsremove_policy_rule→ immediately following sign request for that kind from a previously-bound KeyUser is rejected (assuming no per-user override)revoke_usercontinues to deny all subsequent ops for that KeyUser regardless of policyrevoke_tokendenies ops bound to that specific token without affecting other tokens issued to the same KeyUsermaxUsageCountenforcementThe first two are critical for any deployment with already-onboarded users (the migration safety net). The next three exercise the new live-policy semantic. The last three guard the existing revoke flows.
A property-based test would also be reasonable here — generate a random policy + KeyUser graph, evaluate the auth check against both the snapshot model and the live model, assert results match for all already-bound rules. That would catch any subtle divergence from the existing behavior.
Where these gaps live now
These three additions cover the design surface that the original body deferred or implied. Implementation can proceed off the issue body for the auth shift itself; this comment is the supplement for the companion RPCs + operational + test concerns.
expiresAt(TTL) is not enforced post-bind — sign-time ACL ignores it #24