+16 -13

knotserver/backfill.go

Reviewed

··· 4 4 "context" 5 5 "fmt" 6 6 "log/slog" 7 7 + "maps" 7 8 "slices" 8 9 9 10 "github.com/bluesky-social/indigo/atproto/syntax" ··· 86 87 return nil 87 88 } 88 89 89 89 - repoDids, err := d.ListRepoDids() 90 90 + byRepo, err := e.GetCollaboratorsByRepo(rbac.ThisServer) 90 91 if err != nil { 91 91 - return fmt.Errorf("list repos: %w", err) 92 92 + return fmt.Errorf("list collaborators: %w", err) 92 93 } 93 94 94 95 var rows []db.Collaborator 95 95 - for _, repoDid := range repoDids { 96 96 + var skipped int 97 97 + for _, repoDid := range slices.Sorted(maps.Keys(byRepo)) { 98 98 + candidates := byRepo[repoDid] 99 99 + 96 100 ownerDid, _, err := d.GetRepoKeyOwner(repoDid) 97 101 if err != nil { 98 98 - l.Warn("skipping repo during collaborator backfill", "repoDid", repoDid, "err", err) 102 102 + l.Warn("skipping collaborators for unresolvable repo", "repoDid", repoDid, "collaborators", len(candidates), "err", err) 103 103 + skipped += len(candidates) 99 104 continue 100 105 } 101 106 102 107 repo, err := syntax.ParseDID(repoDid) 103 108 if err != nil { 104 104 - l.Warn("skipping repo with invalid DID", "repoDid", repoDid, "err", err) 109 109 + l.Warn("skipping collaborators for repo with invalid DID", "repoDid", repoDid, "collaborators", len(candidates), "err", err) 110 110 + skipped += len(candidates) 105 111 continue 106 112 } 107 113 owner, err := syntax.ParseDID(ownerDid) 108 114 if err != nil { 109 109 - l.Warn("skipping repo with invalid owner DID", "repoDid", repoDid, "owner", ownerDid, "err", err) 115 115 + l.Warn("skipping collaborators for repo with invalid owner DID", "repoDid", repoDid, "owner", ownerDid, "collaborators", len(candidates), "err", err) 116 116 + skipped += len(candidates) 110 117 continue 111 118 } 112 119 113 113 - collaborators, err := e.GetUserByRoleInRepo("repo:collaborator", rbac.ThisServer, repoDid) 114 114 - if err != nil { 115 115 - return fmt.Errorf("list collaborators for %s: %w", repoDid, err) 116 116 - } 117 117 - 118 118 - for _, candidate := range collaborators { 120 120 + for _, candidate := range candidates { 119 121 subject, err := syntax.ParseDID(candidate) 120 122 if err != nil { 121 123 l.Warn("skipping collaborator with invalid DID", "repoDid", repoDid, "candidate", candidate, "err", err) 124 124 + skipped++ 122 125 continue 123 126 } 124 127 rows = append(rows, db.Collaborator{ ··· 133 136 return fmt.Errorf("apply backfill: %w", err) 134 137 } 135 138 136 136 - l.Info("backfilled collaborators from casbin", "count", len(rows), "repos", len(repoDids), "marked", markApplied) 139 139 + l.Info("backfilled collaborators from casbin", "count", len(rows), "repos", len(byRepo), "skipped", skipped, "marked", markApplied) 137 140 return nil 138 141 }

+12 -14

rbac/rbac.go

Reviewed

··· 263 263 return e.GetUserByRole(role, intoSpindle(domain)) 264 264 } 265 265 266 266 - func (e *Enforcer) GetUserByRoleInRepo(role, domain, repo string) ([]string, error) { 267 267 - policies, err := e.E.GetImplicitUsersForResourceByDomain(repo, domain) 266 266 + func (e *Enforcer) GetCollaboratorsByRepo(domain string) (map[string][]string, error) { 267 267 + policies, err := e.E.GetFilteredNamedPolicy("p", 3, "repo:collaborator") 268 268 if err != nil { 269 269 return nil, err 270 270 } 271 271 272 272 - var users []string 272 272 + byRepo := make(map[string][]string) 273 273 for _, p := range policies { 274 274 - user := p[0] 275 275 - if !strings.HasPrefix(user, "did:") { 274 274 + subject, dom, repo := p[0], p[1], p[2] 275 275 + if dom != domain || !strings.HasPrefix(subject, "did:") { 276 276 continue 277 277 } 278 278 - ok, err := e.E.Enforce(user, domain, repo, role) 279 279 - if err != nil { 280 280 - return nil, err 281 281 - } 282 282 - if ok { 283 283 - users = append(users, user) 284 284 - } 278 278 + byRepo[repo] = append(byRepo[repo], subject) 279 279 + } 280 280 + 281 281 + for repo, users := range byRepo { 282 282 + slices.Sort(users) 283 283 + byRepo[repo] = slices.Compact(users) 285 284 } 286 285 287 287 - slices.Sort(users) 288 288 - return slices.Compact(users), nil 286 286 + return byRepo, nil 289 287 } 290 288 291 289 func (e *Enforcer) IsKnotOwner(user, domain string) (bool, error) {

+7 -4

rbac/rbac_test.go

Reviewed

··· 151 151 assert.ElementsMatch(t, []string{}, perms) 152 152 } 153 153 154 154 - func TestGetByRole(t *testing.T) { 154 154 + func TestGetCollaboratorsByRepo(t *testing.T) { 155 155 e := setup(t) 156 156 157 157 knot := "example.com" 158 158 repo := "did:plc:foo/my-repo" 159 159 + otherRepo := "did:plc:foo/other-repo" 159 160 owner := "did:plc:foo" 160 161 collaborator1 := "did:plc:bar" 161 162 collaborator2 := "did:plc:baz" 162 163 163 164 _ = e.AddKnot(knot) 164 165 _ = e.AddRepo(owner, knot, repo) 166 166 + _ = e.AddRepo(owner, knot, otherRepo) 165 167 166 168 err := e.AddCollaborator(collaborator1, knot, repo) 167 169 assert.NoError(t, err) ··· 169 171 err = e.AddCollaborator(collaborator2, knot, repo) 170 172 assert.NoError(t, err) 171 173 172 172 - collaborators, err := e.GetUserByRoleInRepo("repo:collaborator", knot, repo) 174 174 + byRepo, err := e.GetCollaboratorsByRepo(knot) 173 175 assert.NoError(t, err) 174 176 assert.ElementsMatch(t, []string{ 175 177 "did:plc:bar", // collaborator1 176 178 "did:plc:baz", // collaborator2 177 177 - }, collaborators) 178 178 - assert.NotContains(t, collaborators, owner, "owner does not hold repo:collaborator and must not be listed") 179 179 + }, byRepo[repo]) 180 180 + assert.NotContains(t, byRepo[repo], owner, "owner does not hold repo:collaborator and must not be listed") 181 181 + assert.Empty(t, byRepo[otherRepo], "a repo without collaborators must not appear") 179 182 } 180 183 181 184 func TestGetPermissionsInRepo(t *testing.T) {