refactor(usecase): gate repository administration in adminAccess
adminAccess refuses a branch-limited token that holds repo:admin, so Update and Delete share one rule and canAdminRepo goes away. Principal gains IsBranchLimited and CoversBranchPrefix, which hold the empty-prefix guard in one place for AllowsRef, canCreateRepo, adminAccess and checkNoEscalation.
- commit
- e0dab236084077a78904a245a992ce5bb160e4d0
- author
- brice · · 2026-10-12 16:14 UTC
- parent
- 5a81f18
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
5 files · +65 -39
M internal/domain/principal.go +21 -10
| @@ -82,7 +82,7 @@ func (p Principal) HasOwnerRights() bool { | |||
| 82 | 82 | if p.IsRoot() { | |
| 83 | 83 | return true | |
| 84 | 84 | } | |
| 85 | - | if p.kind != kindToken || len(p.repos) > 0 || len(p.branchPrefixes) > 0 { | |
| 85 | + | if p.kind != kindToken || len(p.repos) > 0 || p.IsBranchLimited() { | |
| 86 | 86 | return false | |
| 87 | 87 | } | |
| 88 | 88 | for _, s := range AllScopes() { | |
| @@ -113,6 +113,24 @@ func (p Principal) Repos() []RepoName { return slices.Clone(p.repos) } | |||
| 113 | 113 | // every branch. | |
| 114 | 114 | func (p Principal) BranchPrefixes() []string { return slices.Clone(p.branchPrefixes) } | |
| 115 | 115 | ||
| 116 | + | // IsBranchLimited reports whether p is limited to branch prefixes. A limited | |
| 117 | + | // principal has no owner rights, creates no repository and administers none. | |
| 118 | + | func (p Principal) IsBranchLimited() bool { return len(p.branchPrefixes) > 0 } | |
| 119 | + | ||
| 120 | + | // CoversBranchPrefix reports whether s lies under one of the branch prefixes of | |
| 121 | + | // p: s starts with one of them. An empty prefix, which ParseBranchPrefix never | |
| 122 | + | // lets in, covers nothing instead of everything, so that a corrupt record | |
| 123 | + | // limits a token and never widens it. It is false without prefixes: callers | |
| 124 | + | // test IsBranchLimited first. | |
| 125 | + | func (p Principal) CoversBranchPrefix(s string) bool { | |
| 126 | + | for _, prefix := range p.branchPrefixes { | |
| 127 | + | if prefix != "" && strings.HasPrefix(s, prefix) { | |
| 128 | + | return true | |
| 129 | + | } | |
| 130 | + | } | |
| 131 | + | return false | |
| 132 | + | } | |
| 133 | + | ||
| 116 | 134 | // AllowsRef reports whether p may write the ref (create, update or delete | |
| 117 | 135 | // it). Without branch prefixes every ref is allowed. With them, only a branch | |
| 118 | 136 | // "refs/heads/<name>" under one of the prefixes is: tags, every other ref and | |
| @@ -126,7 +144,7 @@ func (p Principal) BranchPrefixes() []string { return slices.Clone(p.branchPrefi | |||
| 126 | 144 | // is not "refs/heads/agent/x". On a case-insensitive file system the two could | |
| 127 | 145 | // name the same ref and the prefixes would not hold. | |
| 128 | 146 | func (p Principal) AllowsRef(ref string) bool { | |
| 129 | - | if len(p.branchPrefixes) == 0 { | |
| 147 | + | if !p.IsBranchLimited() { | |
| 130 | 148 | return true | |
| 131 | 149 | } | |
| 132 | 150 | name, ok := strings.CutPrefix(ref, "refs/heads/") | |
| @@ -136,14 +154,7 @@ func (p Principal) AllowsRef(ref string) bool { | |||
| 136 | 154 | if _, err := ParseBranchName(name); err != nil { | |
| 137 | 155 | return false | |
| 138 | 156 | } | |
| 139 | - | for _, prefix := range p.branchPrefixes { | |
| 140 | - | // An empty prefix, which ParseBranchPrefix never lets in, matches | |
| 141 | - | // nothing instead of everything: a limited token fails closed. | |
| 142 | - | if prefix != "" && strings.HasPrefix(name, prefix) { | |
| 143 | - | return true | |
| 144 | - | } | |
| 145 | - | } | |
| 146 | - | return false | |
| 157 | + | return p.CoversBranchPrefix(name) | |
| 147 | 158 | } | |
| 148 | 159 | ||
| 149 | 160 | // ExpiresAt returns the expiry of a stored token; zero means none. | |
| @@ -82,7 +82,7 @@ func (p Principal) HasOwnerRights() bool { | |||||
| 82 | if p.IsRoot() { | 82 | if p.IsRoot() { | ||
| 83 | return true | 83 | return true | ||
| 84 | } | 84 | } | ||
| 85 | - | if p.kind != kindToken || len(p.repos) > 0 || len(p.branchPrefixes) > 0 { | 85 | + | if p.kind != kindToken || len(p.repos) > 0 || p.IsBranchLimited() { |
| 86 | return false | 86 | return false | ||
| 87 | } | 87 | } | ||
| 88 | for _, s := range AllScopes() { | 88 | for _, s := range AllScopes() { | ||
| @@ -113,6 +113,24 @@ func (p Principal) Repos() []RepoName { return slices.Clone(p.repos) } | |||||
| 113 | // every branch. | 113 | // every branch. | ||
| 114 | func (p Principal) BranchPrefixes() []string { return slices.Clone(p.branchPrefixes) } | 114 | func (p Principal) BranchPrefixes() []string { return slices.Clone(p.branchPrefixes) } | ||
| 115 | 115 | ||||
| 116 | + | // IsBranchLimited reports whether p is limited to branch prefixes. A limited | |||
| 117 | + | // principal has no owner rights, creates no repository and administers none. | |||
| 118 | + | func (p Principal) IsBranchLimited() bool { return len(p.branchPrefixes) > 0 } | |||
| 119 | + | ||||
| 120 | + | // CoversBranchPrefix reports whether s lies under one of the branch prefixes of | |||
| 121 | + | // p: s starts with one of them. An empty prefix, which ParseBranchPrefix never | |||
| 122 | + | // lets in, covers nothing instead of everything, so that a corrupt record | |||
| 123 | + | // limits a token and never widens it. It is false without prefixes: callers | |||
| 124 | + | // test IsBranchLimited first. | |||
| 125 | + | func (p Principal) CoversBranchPrefix(s string) bool { | |||
| 126 | + | for _, prefix := range p.branchPrefixes { | |||
| 127 | + | if prefix != "" && strings.HasPrefix(s, prefix) { | |||
| 128 | + | return true | |||
| 129 | + | } | |||
| 130 | + | } | |||
| 131 | + | return false | |||
| 132 | + | } | |||
| 133 | + | ||||
| 116 | // AllowsRef reports whether p may write the ref (create, update or delete | 134 | // AllowsRef reports whether p may write the ref (create, update or delete | ||
| 117 | // it). Without branch prefixes every ref is allowed. With them, only a branch | 135 | // it). Without branch prefixes every ref is allowed. With them, only a branch | ||
| 118 | // "refs/heads/<name>" under one of the prefixes is: tags, every other ref and | 136 | // "refs/heads/<name>" under one of the prefixes is: tags, every other ref and | ||
| @@ -126,7 +144,7 @@ func (p Principal) BranchPrefixes() []string { return slices.Clone(p.branchPrefi | |||||
| 126 | // is not "refs/heads/agent/x". On a case-insensitive file system the two could | 144 | // is not "refs/heads/agent/x". On a case-insensitive file system the two could | ||
| 127 | // name the same ref and the prefixes would not hold. | 145 | // name the same ref and the prefixes would not hold. | ||
| 128 | func (p Principal) AllowsRef(ref string) bool { | 146 | func (p Principal) AllowsRef(ref string) bool { | ||
| 129 | - | if len(p.branchPrefixes) == 0 { | 147 | + | if !p.IsBranchLimited() { |
| 130 | return true | 148 | return true | ||
| 131 | } | 149 | } | ||
| 132 | name, ok := strings.CutPrefix(ref, "refs/heads/") | 150 | name, ok := strings.CutPrefix(ref, "refs/heads/") | ||
| @@ -136,14 +154,7 @@ func (p Principal) AllowsRef(ref string) bool { | |||||
| 136 | if _, err := ParseBranchName(name); err != nil { | 154 | if _, err := ParseBranchName(name); err != nil { | ||
| 137 | return false | 155 | return false | ||
| 138 | } | 156 | } | ||
| 139 | - | for _, prefix := range p.branchPrefixes { | 157 | + | return p.CoversBranchPrefix(name) |
| 140 | - | // An empty prefix, which ParseBranchPrefix never lets in, matches | |||
| 141 | - | // nothing instead of everything: a limited token fails closed. | |||
| 142 | - | if prefix != "" && strings.HasPrefix(name, prefix) { | |||
| 143 | - | return true | |||
| 144 | - | } | |||
| 145 | - | } | |||
| 146 | - | return false | |||
| 147 | } | 158 | } | ||
| 148 | 159 | ||||
| 149 | // ExpiresAt returns the expiry of a stored token; zero means none. | 160 | // ExpiresAt returns the expiry of a stored token; zero means none. | ||
M internal/domain/token_test.go +25 -0
| @@ -213,3 +213,28 @@ func TestBranchPrefixesOfPrincipal(t *testing.T) { | |||
| 213 | 213 | assert.Equal(t, []string{"a/"}, p.BranchPrefixes()) | |
| 214 | 214 | assert.Empty(t, domain.Root().BranchPrefixes()) | |
| 215 | 215 | } | |
| 216 | + | ||
| 217 | + | func TestBranchLimitedAndCoversBranchPrefix(t *testing.T) { | |
| 218 | + | t.Parallel() | |
| 219 | + | free := domain.TokenPrincipal(domain.Token{Scopes: domain.AllScopes()}) | |
| 220 | + | assert.False(t, free.IsBranchLimited()) | |
| 221 | + | assert.False(t, free.CoversBranchPrefix("agent/x"), "no prefixes cover nothing: callers test IsBranchLimited first") | |
| 222 | + | assert.False(t, domain.Root().IsBranchLimited()) | |
| 223 | + | assert.False(t, domain.Owner().IsBranchLimited()) | |
| 224 | + | assert.False(t, domain.Anonymous().IsBranchLimited()) | |
| 225 | + | ||
| 226 | + | limited := domain.TokenPrincipal(domain.Token{Scopes: domain.AllScopes(), BranchPrefixes: []string{"agent/claude/", "bot/"}}) | |
| 227 | + | assert.True(t, limited.IsBranchLimited()) | |
| 228 | + | for s, want := range map[string]bool{ | |
| 229 | + | "agent/claude/x": true, "agent/claude/": true, "bot/y": true, | |
| 230 | + | "agent/claude": false, "agent/claudex/y": false, "main": false, "": false, "xbot/y": false, | |
| 231 | + | } { | |
| 232 | + | assert.Equal(t, want, limited.CoversBranchPrefix(s), "%q", s) | |
| 233 | + | } | |
| 234 | + | ||
| 235 | + | // A corrupt empty prefix still limits, and covers nothing. | |
| 236 | + | corrupt := domain.TokenPrincipal(domain.Token{Scopes: domain.AllScopes(), BranchPrefixes: []string{""}}) | |
| 237 | + | assert.True(t, corrupt.IsBranchLimited()) | |
| 238 | + | assert.False(t, corrupt.CoversBranchPrefix("main")) | |
| 239 | + | assert.False(t, corrupt.CoversBranchPrefix("")) | |
| 240 | + | } | |
| @@ -213,3 +213,28 @@ func TestBranchPrefixesOfPrincipal(t *testing.T) { | |||||
| 213 | assert.Equal(t, []string{"a/"}, p.BranchPrefixes()) | 213 | assert.Equal(t, []string{"a/"}, p.BranchPrefixes()) | ||
| 214 | assert.Empty(t, domain.Root().BranchPrefixes()) | 214 | assert.Empty(t, domain.Root().BranchPrefixes()) | ||
| 215 | } | 215 | } | ||
| 216 | + | ||||
| 217 | + | func TestBranchLimitedAndCoversBranchPrefix(t *testing.T) { | |||
| 218 | + | t.Parallel() | |||
| 219 | + | free := domain.TokenPrincipal(domain.Token{Scopes: domain.AllScopes()}) | |||
| 220 | + | assert.False(t, free.IsBranchLimited()) | |||
| 221 | + | assert.False(t, free.CoversBranchPrefix("agent/x"), "no prefixes cover nothing: callers test IsBranchLimited first") | |||
| 222 | + | assert.False(t, domain.Root().IsBranchLimited()) | |||
| 223 | + | assert.False(t, domain.Owner().IsBranchLimited()) | |||
| 224 | + | assert.False(t, domain.Anonymous().IsBranchLimited()) | |||
| 225 | + | ||||
| 226 | + | limited := domain.TokenPrincipal(domain.Token{Scopes: domain.AllScopes(), BranchPrefixes: []string{"agent/claude/", "bot/"}}) | |||
| 227 | + | assert.True(t, limited.IsBranchLimited()) | |||
| 228 | + | for s, want := range map[string]bool{ | |||
| 229 | + | "agent/claude/x": true, "agent/claude/": true, "bot/y": true, | |||
| 230 | + | "agent/claude": false, "agent/claudex/y": false, "main": false, "": false, "xbot/y": false, | |||
| 231 | + | } { | |||
| 232 | + | assert.Equal(t, want, limited.CoversBranchPrefix(s), "%q", s) | |||
| 233 | + | } | |||
| 234 | + | ||||
| 235 | + | // A corrupt empty prefix still limits, and covers nothing. | |||
| 236 | + | corrupt := domain.TokenPrincipal(domain.Token{Scopes: domain.AllScopes(), BranchPrefixes: []string{""}}) | |||
| 237 | + | assert.True(t, corrupt.IsBranchLimited()) | |||
| 238 | + | assert.False(t, corrupt.CoversBranchPrefix("main")) | |||
| 239 | + | assert.False(t, corrupt.CoversBranchPrefix("")) | |||
| 240 | + | } | |||
M internal/usecase/access.go +12 -17
| @@ -26,12 +26,18 @@ func canReadAs(p domain.Principal, name domain.RepoName, vis domain.Visibility) | |||
| 26 | 26 | return p.Allows(domain.ScopeRepoRead, name) | |
| 27 | 27 | } | |
| 28 | 28 | ||
| 29 | - | // adminAccess is the access rule for changing or deleting a repository. A | |
| 30 | - | // caller that may not administer it learns nothing it could not already read: | |
| 31 | - | // not found when it cannot see the repository, unauthenticated when it is | |
| 32 | - | // anonymous, forbidden otherwise. | |
| 29 | + | // adminAccess is the access rule for changing or deleting a repository: | |
| 30 | + | // repo:admin on it, and no branch prefixes. A token limited to some branches | |
| 31 | + | // may not delete the repository (that deletes every branch, those outside its | |
| 32 | + | // prefixes included), nor change its default branch, visibility, description, | |
| 33 | + | // merge modes or failing-checks rule: these decide what happens to branches the | |
| 34 | + | // token may not write. A caller that may not administer it learns nothing it | |
| 35 | + | // could not already read: not found when it cannot see the repository, | |
| 36 | + | // unauthenticated when it is anonymous, forbidden otherwise. | |
| 33 | 37 | func adminAccess(p domain.Principal, repo domain.Repo) error { | |
| 34 | 38 | switch { | |
| 39 | + | case canAdmin(p, repo.Name) && p.IsBranchLimited(): | |
| 40 | + | return fmt.Errorf("%w: a token limited to branch prefixes may not administer a repository", domain.ErrForbidden) | |
| 35 | 41 | case canAdmin(p, repo.Name): | |
| 36 | 42 | return nil | |
| 37 | 43 | case !canRead(p, repo): | |
| @@ -42,8 +48,7 @@ func adminAccess(p domain.Principal, repo domain.Repo) error { | |||
| 42 | 48 | return domain.ErrForbidden | |
| 43 | 49 | } | |
| 44 | 50 | ||
| 45 | - | // canWrite is the access rule for pushing to a repository, or creating it on | |
| 46 | - | // first push: repo:write on it. | |
| 51 | + | // canWrite is the access rule for pushing to a repository: repo:write on it. | |
| 47 | 52 | func canWrite(p domain.Principal, name domain.RepoName) bool { | |
| 48 | 53 | return p.Allows(domain.ScopeRepoWrite, name) | |
| 49 | 54 | } | |
| @@ -52,17 +57,7 @@ func canWrite(p domain.Principal, name domain.RepoName) bool { | |||
| 52 | 57 | // first push: repo:write on the name, and no branch prefixes. A token limited | |
| 53 | 58 | // to some branches may push to a repository that exists; it never makes one. | |
| 54 | 59 | func canCreateRepo(p domain.Principal, name domain.RepoName) bool { | |
| 55 | - | return canWrite(p, name) && len(p.BranchPrefixes()) == 0 | |
| 56 | - | } | |
| 57 | - | ||
| 58 | - | // canAdminRepo is the access rule for changing the settings of a repository | |
| 59 | - | // or deleting it, on top of adminAccess: no branch prefixes. A token limited to | |
| 60 | - | // some branches may not delete the repository (that deletes every branch, those | |
| 61 | - | // outside its prefixes included), nor change its default branch, visibility, | |
| 62 | - | // description, merge modes or failing-checks rule: these settings decide what | |
| 63 | - | // happens to branches the token may not write. | |
| 64 | - | func canAdminRepo(p domain.Principal) bool { | |
| 65 | - | return len(p.BranchPrefixes()) == 0 | |
| 60 | + | return canWrite(p, name) && !p.IsBranchLimited() | |
| 66 | 61 | } | |
| 67 | 62 | ||
| 68 | 63 | // canManageTokens is the access rule for creating, listing and revoking | |
| @@ -26,12 +26,18 @@ func canReadAs(p domain.Principal, name domain.RepoName, vis domain.Visibility) | |||||
| 26 | return p.Allows(domain.ScopeRepoRead, name) | 26 | return p.Allows(domain.ScopeRepoRead, name) | ||
| 27 | } | 27 | } | ||
| 28 | 28 | ||||
| 29 | - | // adminAccess is the access rule for changing or deleting a repository. A | 29 | + | // adminAccess is the access rule for changing or deleting a repository: |
| 30 | - | // caller that may not administer it learns nothing it could not already read: | 30 | + | // repo:admin on it, and no branch prefixes. A token limited to some branches |
| 31 | - | // not found when it cannot see the repository, unauthenticated when it is | 31 | + | // may not delete the repository (that deletes every branch, those outside its |
| 32 | - | // anonymous, forbidden otherwise. | 32 | + | // prefixes included), nor change its default branch, visibility, description, |
| 33 | + | // merge modes or failing-checks rule: these decide what happens to branches the | |||
| 34 | + | // token may not write. A caller that may not administer it learns nothing it | |||
| 35 | + | // could not already read: not found when it cannot see the repository, | |||
| 36 | + | // unauthenticated when it is anonymous, forbidden otherwise. | |||
| 33 | func adminAccess(p domain.Principal, repo domain.Repo) error { | 37 | func adminAccess(p domain.Principal, repo domain.Repo) error { | ||
| 34 | switch { | 38 | switch { | ||
| 39 | + | case canAdmin(p, repo.Name) && p.IsBranchLimited(): | |||
| 40 | + | return fmt.Errorf("%w: a token limited to branch prefixes may not administer a repository", domain.ErrForbidden) | |||
| 35 | case canAdmin(p, repo.Name): | 41 | case canAdmin(p, repo.Name): | ||
| 36 | return nil | 42 | return nil | ||
| 37 | case !canRead(p, repo): | 43 | case !canRead(p, repo): | ||
| @@ -42,8 +48,7 @@ func adminAccess(p domain.Principal, repo domain.Repo) error { | |||||
| 42 | return domain.ErrForbidden | 48 | return domain.ErrForbidden | ||
| 43 | } | 49 | } | ||
| 44 | 50 | ||||
| 45 | - | // canWrite is the access rule for pushing to a repository, or creating it on | 51 | + | // canWrite is the access rule for pushing to a repository: repo:write on it. |
| 46 | - | // first push: repo:write on it. | |||
| 47 | func canWrite(p domain.Principal, name domain.RepoName) bool { | 52 | func canWrite(p domain.Principal, name domain.RepoName) bool { | ||
| 48 | return p.Allows(domain.ScopeRepoWrite, name) | 53 | return p.Allows(domain.ScopeRepoWrite, name) | ||
| 49 | } | 54 | } | ||
| @@ -52,17 +57,7 @@ func canWrite(p domain.Principal, name domain.RepoName) bool { | |||||
| 52 | // first push: repo:write on the name, and no branch prefixes. A token limited | 57 | // first push: repo:write on the name, and no branch prefixes. A token limited | ||
| 53 | // to some branches may push to a repository that exists; it never makes one. | 58 | // to some branches may push to a repository that exists; it never makes one. | ||
| 54 | func canCreateRepo(p domain.Principal, name domain.RepoName) bool { | 59 | func canCreateRepo(p domain.Principal, name domain.RepoName) bool { | ||
| 55 | - | return canWrite(p, name) && len(p.BranchPrefixes()) == 0 | 60 | + | return canWrite(p, name) && !p.IsBranchLimited() |
| 56 | - | } | |||
| 57 | - | ||||
| 58 | - | // canAdminRepo is the access rule for changing the settings of a repository | |||
| 59 | - | // or deleting it, on top of adminAccess: no branch prefixes. A token limited to | |||
| 60 | - | // some branches may not delete the repository (that deletes every branch, those | |||
| 61 | - | // outside its prefixes included), nor change its default branch, visibility, | |||
| 62 | - | // description, merge modes or failing-checks rule: these settings decide what | |||
| 63 | - | // happens to branches the token may not write. | |||
| 64 | - | func canAdminRepo(p domain.Principal) bool { | |||
| 65 | - | return len(p.BranchPrefixes()) == 0 | |||
| 66 | } | 61 | } | ||
| 67 | 62 | ||||
| 68 | // canManageTokens is the access rule for creating, listing and revoking | 63 | // canManageTokens is the access rule for creating, listing and revoking | ||
M internal/usecase/repos_write.go +0 -6
| @@ -60,9 +60,6 @@ func (r *Repos) Update(ctx context.Context, p domain.Principal, name domain.Repo | |||
| 60 | 60 | if err := adminAccess(p, repo); err != nil { | |
| 61 | 61 | return domain.Repo{}, err | |
| 62 | 62 | } | |
| 63 | - | if !canAdminRepo(p) { | |
| 64 | - | return domain.Repo{}, fmt.Errorf("%w: a token limited to branch prefixes may not change repository settings", domain.ErrForbidden) | |
| 65 | - | } | |
| 66 | 63 | before := repo | |
| 67 | 64 | if params.Description == nil && params.Visibility == nil && params.DefaultBranch == nil && | |
| 68 | 65 | params.AllowedMergeModes == nil && params.FailingChecksBlock == nil { | |
| @@ -187,9 +184,6 @@ func (r *Repos) Delete(ctx context.Context, p domain.Principal, name domain.Repo | |||
| 187 | 184 | if err := adminAccess(p, repo); err != nil { | |
| 188 | 185 | return err | |
| 189 | 186 | } | |
| 190 | - | if !canAdminRepo(p) { | |
| 191 | - | return fmt.Errorf("%w: a token limited to branch prefixes may not delete a repository", domain.ErrForbidden) | |
| 192 | - | } | |
| 193 | 187 | log := zerowrap.FromCtx(ctx) | |
| 194 | 188 | switch err := r.git.Trash(ctx, name); { | |
| 195 | 189 | case errors.Is(err, domain.ErrNotFound): | |
| @@ -60,9 +60,6 @@ func (r *Repos) Update(ctx context.Context, p domain.Principal, name domain.Repo | |||||
| 60 | if err := adminAccess(p, repo); err != nil { | 60 | if err := adminAccess(p, repo); err != nil { | ||
| 61 | return domain.Repo{}, err | 61 | return domain.Repo{}, err | ||
| 62 | } | 62 | } | ||
| 63 | - | if !canAdminRepo(p) { | |||
| 64 | - | return domain.Repo{}, fmt.Errorf("%w: a token limited to branch prefixes may not change repository settings", domain.ErrForbidden) | |||
| 65 | - | } | |||
| 66 | before := repo | 63 | before := repo | ||
| 67 | if params.Description == nil && params.Visibility == nil && params.DefaultBranch == nil && | 64 | if params.Description == nil && params.Visibility == nil && params.DefaultBranch == nil && | ||
| 68 | params.AllowedMergeModes == nil && params.FailingChecksBlock == nil { | 65 | params.AllowedMergeModes == nil && params.FailingChecksBlock == nil { | ||
| @@ -187,9 +184,6 @@ func (r *Repos) Delete(ctx context.Context, p domain.Principal, name domain.Repo | |||||
| 187 | if err := adminAccess(p, repo); err != nil { | 184 | if err := adminAccess(p, repo); err != nil { | ||
| 188 | return err | 185 | return err | ||
| 189 | } | 186 | } | ||
| 190 | - | if !canAdminRepo(p) { | |||
| 191 | - | return fmt.Errorf("%w: a token limited to branch prefixes may not delete a repository", domain.ErrForbidden) | |||
| 192 | - | } | |||
| 193 | log := zerowrap.FromCtx(ctx) | 187 | log := zerowrap.FromCtx(ctx) | ||
| 194 | switch err := r.git.Trash(ctx, name); { | 188 | switch err := r.git.Trash(ctx, name); { | ||
| 195 | case errors.Is(err, domain.ErrNotFound): | 189 | case errors.Is(err, domain.ErrNotFound): | ||
M internal/usecase/tokens.go +7 -6
| @@ -149,8 +149,9 @@ func buildToken(params ports.CreateTokenParams, now time.Time) (domain.Token, er | |||
| 149 | 149 | } | |
| 150 | 150 | ||
| 151 | 151 | // checkNoEscalation enforces that tok is within the rights of p: every scope | |
| 152 | - | // held by p, the repositories within its list and the expiry not beyond its | |
| 153 | - | // own. Root has no limits. | |
| 152 | + | // held by p, the repositories within its list, the branch prefixes within its | |
| 153 | + | // own (a limited p only makes tokens that are limited too, each prefix under | |
| 154 | + | // one of its own) and the expiry not beyond its own. Root has no limits. | |
| 154 | 155 | func checkNoEscalation(p domain.Principal, tok domain.Token) error { | |
| 155 | 156 | if p.IsRoot() { | |
| 156 | 157 | return nil | |
| @@ -170,12 +171,12 @@ func checkNoEscalation(p domain.Principal, tok domain.Token) error { | |||
| 170 | 171 | } | |
| 171 | 172 | } | |
| 172 | 173 | } | |
| 173 | - | if own := p.BranchPrefixes(); len(own) > 0 { | |
| 174 | + | if p.IsBranchLimited() { | |
| 174 | 175 | if len(tok.BranchPrefixes) == 0 { | |
| 175 | 176 | return fmt.Errorf("%w: the creating token is limited to branch prefixes, so the new token must be too", domain.ErrForbidden) | |
| 176 | 177 | } | |
| 177 | 178 | for _, bp := range tok.BranchPrefixes { | |
| 178 | - | if !slices.ContainsFunc(own, func(o string) bool { return o != "" && strings.HasPrefix(bp, o) }) { | |
| 179 | + | if !p.CoversBranchPrefix(bp) { | |
| 179 | 180 | return fmt.Errorf("%w: branch prefix %s is outside the creating token", domain.ErrForbidden, bp) | |
| 180 | 181 | } | |
| 181 | 182 | } | |
| @@ -189,7 +190,7 @@ func checkNoEscalation(p domain.Principal, tok domain.Token) error { | |||
| 189 | 190 | } | |
| 190 | 191 | ||
| 191 | 192 | // List returns the stored tokens the principal could have created, oldest | |
| 192 | - | // first: those whose scopes, repositories and expiry lie within its own (the | |
| 193 | + | // first: those whose scopes, repositories, branch prefixes and expiry lie within its own (the | |
| 193 | 194 | // no-escalation rule of Create). Root sees every token. It needs token:admin. | |
| 194 | 195 | func (t *Tokens) List(ctx context.Context, p domain.Principal) ([]domain.Token, error) { | |
| 195 | 196 | if err := requireTokenAdmin(p); err != nil { | |
| @@ -211,7 +212,7 @@ func (t *Tokens) List(ctx context.Context, p domain.Principal) ([]domain.Token, | |||
| 211 | 212 | // Create), so that it does not show itself. Root may revoke any token. | |
| 212 | 213 | // | |
| 213 | 214 | // A token that has owner rights (HasOwnerRights: every scope, no repository | |
| 214 | - | // list) could open web sessions, so revoking it also signs every browser out, | |
| 215 | + | // list and no branch prefixes) could open web sessions, so revoking it also signs every browser out, | |
| 215 | 216 | // as `ermite web logout --all` does: web sessions are not tied to the token | |
| 216 | 217 | // that opened them. The revocation holds even if signing out fails, which is | |
| 217 | 218 | // then reported as an error. | |
| @@ -149,8 +149,9 @@ func buildToken(params ports.CreateTokenParams, now time.Time) (domain.Token, er | |||||
| 149 | } | 149 | } | ||
| 150 | 150 | ||||
| 151 | // checkNoEscalation enforces that tok is within the rights of p: every scope | 151 | // checkNoEscalation enforces that tok is within the rights of p: every scope | ||
| 152 | - | // held by p, the repositories within its list and the expiry not beyond its | 152 | + | // held by p, the repositories within its list, the branch prefixes within its |
| 153 | - | // own. Root has no limits. | 153 | + | // own (a limited p only makes tokens that are limited too, each prefix under |
| 154 | + | // one of its own) and the expiry not beyond its own. Root has no limits. | |||
| 154 | func checkNoEscalation(p domain.Principal, tok domain.Token) error { | 155 | func checkNoEscalation(p domain.Principal, tok domain.Token) error { | ||
| 155 | if p.IsRoot() { | 156 | if p.IsRoot() { | ||
| 156 | return nil | 157 | return nil | ||
| @@ -170,12 +171,12 @@ func checkNoEscalation(p domain.Principal, tok domain.Token) error { | |||||
| 170 | } | 171 | } | ||
| 171 | } | 172 | } | ||
| 172 | } | 173 | } | ||
| 173 | - | if own := p.BranchPrefixes(); len(own) > 0 { | 174 | + | if p.IsBranchLimited() { |
| 174 | if len(tok.BranchPrefixes) == 0 { | 175 | if len(tok.BranchPrefixes) == 0 { | ||
| 175 | return fmt.Errorf("%w: the creating token is limited to branch prefixes, so the new token must be too", domain.ErrForbidden) | 176 | return fmt.Errorf("%w: the creating token is limited to branch prefixes, so the new token must be too", domain.ErrForbidden) | ||
| 176 | } | 177 | } | ||
| 177 | for _, bp := range tok.BranchPrefixes { | 178 | for _, bp := range tok.BranchPrefixes { | ||
| 178 | - | if !slices.ContainsFunc(own, func(o string) bool { return o != "" && strings.HasPrefix(bp, o) }) { | 179 | + | if !p.CoversBranchPrefix(bp) { |
| 179 | return fmt.Errorf("%w: branch prefix %s is outside the creating token", domain.ErrForbidden, bp) | 180 | return fmt.Errorf("%w: branch prefix %s is outside the creating token", domain.ErrForbidden, bp) | ||
| 180 | } | 181 | } | ||
| 181 | } | 182 | } | ||
| @@ -189,7 +190,7 @@ func checkNoEscalation(p domain.Principal, tok domain.Token) error { | |||||
| 189 | } | 190 | } | ||
| 190 | 191 | ||||
| 191 | // List returns the stored tokens the principal could have created, oldest | 192 | // List returns the stored tokens the principal could have created, oldest | ||
| 192 | - | // first: those whose scopes, repositories and expiry lie within its own (the | 193 | + | // first: those whose scopes, repositories, branch prefixes and expiry lie within its own (the |
| 193 | // no-escalation rule of Create). Root sees every token. It needs token:admin. | 194 | // no-escalation rule of Create). Root sees every token. It needs token:admin. | ||
| 194 | func (t *Tokens) List(ctx context.Context, p domain.Principal) ([]domain.Token, error) { | 195 | func (t *Tokens) List(ctx context.Context, p domain.Principal) ([]domain.Token, error) { | ||
| 195 | if err := requireTokenAdmin(p); err != nil { | 196 | if err := requireTokenAdmin(p); err != nil { | ||
| @@ -211,7 +212,7 @@ func (t *Tokens) List(ctx context.Context, p domain.Principal) ([]domain.Token, | |||||
| 211 | // Create), so that it does not show itself. Root may revoke any token. | 212 | // Create), so that it does not show itself. Root may revoke any token. | ||
| 212 | // | 213 | // | ||
| 213 | // A token that has owner rights (HasOwnerRights: every scope, no repository | 214 | // A token that has owner rights (HasOwnerRights: every scope, no repository | ||
| 214 | - | // list) could open web sessions, so revoking it also signs every browser out, | 215 | + | // list and no branch prefixes) could open web sessions, so revoking it also signs every browser out, |
| 215 | // as `ermite web logout --all` does: web sessions are not tied to the token | 216 | // as `ermite web logout --all` does: web sessions are not tied to the token | ||
| 216 | // that opened them. The revocation holds even if signing out fails, which is | 217 | // that opened them. The revocation holds even if signing out fails, which is | ||
| 217 | // then reported as an error. | 218 | // then reported as an error. | ||