feat(registry): per-user access lists (verdaccio-style groups)#12048
Conversation
Model each packages[*] access/publish permission as a list of tokens (verdaccio's space-separated groups) instead of a single rule, and match it against the caller's identity. Tokens are the built-in pseudo-groups ($all/$authenticated/$anonymous and their @/bare aliases) or a name; a name matches an authenticated caller whose username equals it — verdaccio's per-user access, where the htpasswd backend contributes the username and richer groups would come from a group-providing auth backend. - AccessRule (single enum) -> AccessList (Vec<AccessToken>) + Identity. - Accept both a space-separated string and a YAML sequence for each permission, like verdaccio. - Bare names are now valid tokens (drops the InvalidAccessRule stopgap from #12043 that rejected them). - enforce_access: anonymous denial -> 401, authenticated denial -> 403. Refs #11973.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
📝 WalkthroughWalkthroughThis PR refactors the pnpm-registry access control system from a single-token enum model to a multi-token permission list model. ChangesAccess Control Model Refactor
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant Identity as identify()
participant Policy as PackagePolicies
participant AccessList
Client->>Server: request (authenticated/anonymous)
Server->>Identity: derive from auth state
Identity-->>Server: User{username} or Anonymous
Server->>Policy: for_package(name)
Policy-->>Server: Effective { access, publish }
Server->>AccessList: select by Action
Server->>AccessList: allows(&identity)
AccessList-->>Server: true/false
alt allowed
Server-->>Client: permit request
else denied
alt anonymous
Server-->>Client: 401 Unauthenticated
else authenticated
Server-->>Client: 403 Forbidden
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Review Summary by QodoSupport multi-token access lists with per-user access control
WalkthroughsDescription• Model access/publish permissions as token lists instead of single rules • Support per-user access by allowing usernames as valid tokens • Accept both space-separated strings and YAML sequences for permissions • Replace InvalidAccessRule error with flexible token parsing • Introduce Identity type to represent caller authentication state Diagramflowchart LR
Config["Config YAML<br/>access/publish fields"] -->|AccessSpec| Parse["Parse string or<br/>sequence form"]
Parse -->|AccessList| Tokens["List of<br/>AccessTokens"]
Tokens -->|evaluate| Identity["Caller Identity<br/>Anonymous/User"]
Identity -->|satisfies| Allow["Permission<br/>allowed/denied"]
File Changes1. registry/crates/pnpm-registry/src/config.rs
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #12048 +/- ##
==========================================
+ Coverage 88.81% 88.93% +0.12%
==========================================
Files 234 235 +1
Lines 30594 30871 +277
==========================================
+ Hits 27172 27456 +284
+ Misses 3422 3415 -7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@registry/crates/pnpm-registry/src/policy.rs`:
- Around line 25-45: The documentation for the AccessToken enum's Authenticated
and Anonymous variants is incomplete: the From<&str> implementation also accepts
the bare strings "authenticated" and "anonymous" (in addition to the $/@ forms),
so update the doc comments on Authenticated and Anonymous to mention the plain
aliases ("authenticated" and "anonymous") as accepted pseudo-group tokens;
locate the enum AccessToken and its variants Authenticated and Anonymous and
expand their doc comments to list all accepted forms to match the From<&str>
mapping.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d544c330-5f02-4b49-a30c-34664c4fc5bf
📒 Files selected for processing (6)
registry/crates/pnpm-registry/src/config.rsregistry/crates/pnpm-registry/src/error.rsregistry/crates/pnpm-registry/src/lib.rsregistry/crates/pnpm-registry/src/policy.rsregistry/crates/pnpm-registry/src/server.rsregistry/crates/pnpm-registry/tests/policy.rs
💤 Files with no reviewable changes (1)
- registry/crates/pnpm-registry/src/error.rs
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Code Coverage
- GitHub Check: Lint and Test (macos-latest)
- GitHub Check: Lint and Test (windows-latest)
🧰 Additional context used
📓 Path-based instructions (1)
registry/crates/**/*.rs
📄 CodeRabbit inference engine (registry/AGENTS.md)
Follow the pacquet code-style guide (../pacquet/CODE_STYLE_GUIDE.md) for Rust-level conventions including imports, naming, ownership, error handling, and test layout
Files:
registry/crates/pnpm-registry/tests/policy.rsregistry/crates/pnpm-registry/src/server.rsregistry/crates/pnpm-registry/src/lib.rsregistry/crates/pnpm-registry/src/config.rsregistry/crates/pnpm-registry/src/policy.rs
🔇 Additional comments (10)
registry/crates/pnpm-registry/src/policy.rs (4)
47-92: LGTM!
94-102:$anonymousis stricter here than in verdaccio — confirm the divergence is intended.
satisfiesmakesAnonymousmatch only logged-out callers. In verdaccio the anonymous group is granted to everyone: all users receive those groups independently of whether they are anonymous, so an authenticated user's group list still includes$anonymous/anonymous. So a migrated config usingaccess: $anonymousto mean "everyone may read" would, under this implementation, deny authenticated readers. The doc on Lines 28-29 states this stricter behavior is deliberate — flagging only so the verdaccio-compatibility gap is a conscious choice.
105-204: LGTM!
206-307: LGTM!registry/crates/pnpm-registry/src/config.rs (3)
10-10: LGTM!Also applies to: 232-268
553-579: LGTM!
635-645: LGTM!Also applies to: 1335-1452
registry/crates/pnpm-registry/src/server.rs (1)
20-20: LGTM!registry/crates/pnpm-registry/src/lib.rs (1)
28-28: LGTM!registry/crates/pnpm-registry/tests/policy.rs (1)
99-116: LGTM!
AccessToken::from also accepts the unprefixed `authenticated` / `anonymous` forms; document them on the variants so they aren't misread as usernames.
* feat(registry): support multi-token access lists with usernames Model each packages[*] access/publish permission as a list of tokens (verdaccio's space-separated groups) instead of a single rule, and match it against the caller's identity. Tokens are the built-in pseudo-groups ($all/$authenticated/$anonymous and their @/bare aliases) or a name; a name matches an authenticated caller whose username equals it — verdaccio's per-user access, where the htpasswd backend contributes the username and richer groups would come from a group-providing auth backend. - AccessRule (single enum) -> AccessList (Vec<AccessToken>) + Identity. - Accept both a space-separated string and a YAML sequence for each permission, like verdaccio. - Bare names are now valid tokens (drops the InvalidAccessRule stopgap from #12043 that rejected them). - enforce_access: anonymous denial -> 401, authenticated denial -> 403. Refs #11973. * docs(registry): note the bare authenticated/anonymous token aliases AccessToken::from also accepts the unprefixed `authenticated` / `anonymous` forms; document them on the variants so they aren't misread as usernames.
Summary
Models each package's
access/publishpermission as a list of tokens (verdaccio's space-separated groups) matched against the caller's identity, instead of a single rule. This adds per-user access (list usernames directly) and lays the matching groundwork for named groups.How verdaccio does it (and what we mirror)
verdaccio's
access/publish/unpublishare space-separated lists; a request is allowed when the caller's group set intersects the list. A logged-in user's groups are the built-ins plus their username, plus whatever the auth plugin returns — and the default htpasswd plugin contributes only the username. Named groups beyond that come from other auth backends (LDAP/GitLab/…).So there is intentionally no new
groups:config key: that's not how verdaccio works with htpasswd. Usernames give per-user access today; group names will resolve here for free once a group-providing auth backend lands (the separate "pluggable auth" item in #11973).What's in
AccessRule(single enum) →AccessList(Vec<AccessToken>) +Identity. A permission is satisfied if the identity matches any token (union).$all/@all/all,$authenticated/@authenticated,$anonymous/@anonymous, or a name. A name matches an authenticated caller whose username equals it.access: alice bob) and a YAML sequence (access: [alice, bob]), like verdaccio.enforce_access: a denied anonymous caller gets401(can authenticate); a denied authenticated caller gets403.Behavior change to note
This reverses the
InvalidAccessRulestopgap from #12043, which rejected bare names because groups weren't modeled yet. Bare names are now valid tokens (usernames/groups) — verdaccio's actual behavior. The error variant and its two "unknown token is a config error" tests are removed, replaced by per-user / list tests.Scope
unpublishis still parsed for config compatibility but folds intopublishat enforcement (unchanged; separate item).Test plan
$all/$authenticated/$anonymous/username/empty-list semantics, mixed-list union, YAML string-vs-sequence equivalence, defaults, first-match-wins, and bundled-config protections preserved.tests/policy.rs):$authenticatedgating,$anonymousadmit/forbid, and a newusername_in_access_list_grants_only_that_user(anon → 401, wrong user → 403, listed user → allowed).cargo test,cargo clippy --deny warnings,cargo dylint(perfectionist),cargo fmt --check,cargo doc -D warnings,typosall clean.Refs #11973 (Access control → "Named groups, per-user group membership").
Written by an agent (Claude Code, claude-opus-4-8).
Summary by CodeRabbit
New Features
Improvements
Tests