From bc598317ecb56cea9614d8f3dfc379167feede34 Mon Sep 17 00:00:00 2001 From: Lewis Date: Fri, 5 Jun 2026 09:32:53 +0300 Subject: [PATCH] appview: read permissions thru knotacl service Lewis: May this revision serve well! --- appview/ingester.go | 26 ++- appview/issues/issues.go | 13 +- appview/knots/knots.go | 69 ++++++- appview/labels/labels.go | 2 +- appview/middleware/middleware.go | 8 +- appview/oauth/oauth.go | 19 +- appview/oauth/scopes.go | 4 + .../pages/templates/repo/settings/access.html | 13 ++ appview/pulls/compose.go | 3 +- appview/pulls/create.go | 6 +- appview/pulls/labels.go | 2 +- appview/pulls/lifecycle.go | 5 +- appview/pulls/pulls.go | 8 +- appview/pulls/resubmit.go | 11 +- appview/pulls/single.go | 4 +- appview/repo/repo.go | 195 +++++++++++++++++- appview/repo/router.go | 1 + appview/repo/settings.go | 36 +--- appview/reporesolver/resolver.go | 16 +- appview/state/knotstream.go | 16 +- appview/state/router.go | 9 +- appview/state/state.go | 23 ++- appview/validator/label.go | 24 +-- appview/validator/label_test.go | 87 ++++++++ appview/validator/validator.go | 8 +- nix/modules/appview.nix | 1 + 26 files changed, 470 insertions(+), 139 deletions(-) create mode 100644 appview/validator/label_test.go diff --git a/appview/ingester.go b/appview/ingester.go index 7ae78a14..4b79658f 100644 --- a/appview/ingester.go +++ b/appview/ingester.go @@ -27,6 +27,7 @@ import ( "tangled.org/core/appview/cache" "tangled.org/core/appview/config" "tangled.org/core/appview/db" + "tangled.org/core/appview/knotacl" "tangled.org/core/appview/mentions" "tangled.org/core/appview/models" "tangled.org/core/appview/notify" @@ -42,6 +43,7 @@ type Ingester struct { Ctx context.Context Db *db.DB Enforcer *rbac.Enforcer + Acl *knotacl.Service IdResolver *idresolver.Resolver Cache *cache.Cache Config *config.Config @@ -116,7 +118,7 @@ func (i *Ingester) Ingest() processFunc { case tangled.LabelDefinitionNSID: err = i.ingestLabelDefinition(e, l) case tangled.LabelOpNSID: - err = i.ingestLabelOp(e, l) + err = i.ingestLabelOp(ctx, e, l) case tangled.RepoNSID: err = i.ingestRepo(ctx, e, l) } @@ -458,9 +460,12 @@ func (i *Ingester) ingestArtifact(ctx context.Context, e *jmodels.Event, l *slog return fmt.Errorf("artifact record has neither valid repoDid nor repo field") } - ok, err := i.Enforcer.E.Enforce(did, repo.Knot, repo.RepoIdentifier(), "repo:push") - if err != nil || !ok { - return err + allowed, permErr := i.Acl.HasRepoPermissionErr(ctx, repo, did, "repo:push") + if permErr != nil { + l.Warn("ingesting artifact without permission check", "did", did, "repo", repo.RepoIdentifier(), "err", permErr) + } else if !allowed { + l.Info("skipping unauthorized artifact", "did", did, "repo", repo.RepoIdentifier()) + return nil } repoDid := repo.RepoDid @@ -473,8 +478,8 @@ func (i *Ingester) ingestArtifact(ctx context.Context, e *jmodels.Event, l *slog } } - createdAt, err := time.Parse(time.RFC3339, record.CreatedAt) - if err != nil { + createdAt, parseErr := time.Parse(time.RFC3339, record.CreatedAt) + if parseErr != nil { createdAt = time.Now() } @@ -1778,7 +1783,7 @@ func (i *Ingester) ingestLabelDefinition(e *jmodels.Event, l *slog.Logger) error return nil } -func (i *Ingester) ingestLabelOp(e *jmodels.Event, l *slog.Logger) error { +func (i *Ingester) ingestLabelOp(ctx context.Context, e *jmodels.Event, l *slog.Logger) error { did := e.Did rkey := e.Commit.RKey @@ -1828,8 +1833,11 @@ func (i *Ingester) ingestLabelOp(e *jmodels.Event, l *slog.Logger) error { if !ok { return fmt.Errorf("failed to find label def for key: %s, expected: %q", o.OperandKey, slices.Collect(maps.Keys(actx.Defs))) } - if err := i.Validator.ValidateLabelOp(def, repo, &o); err != nil { - return fmt.Errorf("failed to validate labelop: %w", err) + if err := i.Validator.ValidateLabelOp(ctx, def, repo, &o); err != nil { + if !errors.Is(err, knotacl.ErrKnotUnreachable) { + return fmt.Errorf("failed to validate labelop: %w", err) + } + l.Warn("ingesting labelop without permission check", "did", o.Did, "err", err) } } diff --git a/appview/issues/issues.go b/appview/issues/issues.go index c67e6ba1..e4ecb107 100644 --- a/appview/issues/issues.go +++ b/appview/issues/issues.go @@ -19,12 +19,12 @@ import ( "tangled.org/core/appview/config" "tangled.org/core/appview/db" issues_indexer "tangled.org/core/appview/indexer/issues" + "tangled.org/core/appview/knotacl" "tangled.org/core/appview/mentions" "tangled.org/core/appview/models" "tangled.org/core/appview/notify" "tangled.org/core/appview/oauth" "tangled.org/core/appview/pages" - "tangled.org/core/appview/pages/repoinfo" "tangled.org/core/appview/pagination" "tangled.org/core/appview/reporesolver" "tangled.org/core/appview/searchquery" @@ -32,14 +32,13 @@ import ( "tangled.org/core/idresolver" "tangled.org/core/ogre" "tangled.org/core/orm" - "tangled.org/core/rbac" "tangled.org/core/tid" ) type Issues struct { oauth *oauth.OAuth repoResolver *reporesolver.RepoResolver - enforcer *rbac.Enforcer + acl *knotacl.Service pages *pages.Pages idResolver *idresolver.Resolver mentionsResolver *mentions.Resolver @@ -55,7 +54,7 @@ type Issues struct { func New( oauth *oauth.OAuth, repoResolver *reporesolver.RepoResolver, - enforcer *rbac.Enforcer, + acl *knotacl.Service, pages *pages.Pages, idResolver *idresolver.Resolver, mentionsResolver *mentions.Resolver, @@ -69,7 +68,7 @@ func New( return &Issues{ oauth: oauth, repoResolver: repoResolver, - enforcer: enforcer, + acl: acl, pages: pages, idResolver: idResolver, mentionsResolver: mentionsResolver, @@ -340,7 +339,7 @@ func (rp *Issues) CloseIssue(w http.ResponseWriter, r *http.Request) { return } - roles := repoinfo.RolesInRepo{Roles: rp.enforcer.GetPermissionsInRepo(user.Did, f.Knot, f.RepoIdentifier())} + roles := rp.acl.RolesInRepo(r.Context(), f, user.Did) isRepoOwner := roles.IsOwner() isCollaborator := roles.IsCollaborator() isIssueOwner := user.Did == issue.Did @@ -388,7 +387,7 @@ func (rp *Issues) ReopenIssue(w http.ResponseWriter, r *http.Request) { return } - roles := repoinfo.RolesInRepo{Roles: rp.enforcer.GetPermissionsInRepo(user.Did, f.Knot, f.RepoIdentifier())} + roles := rp.acl.RolesInRepo(r.Context(), f, user.Did) isRepoOwner := roles.IsOwner() isCollaborator := roles.IsCollaborator() isIssueOwner := user.Did == issue.Did diff --git a/appview/knots/knots.go b/appview/knots/knots.go index 4cc3de49..5d802868 100644 --- a/appview/knots/knots.go +++ b/appview/knots/knots.go @@ -6,7 +6,6 @@ import ( "fmt" "log/slog" "net/http" - "slices" "strings" "time" @@ -14,12 +13,15 @@ import ( "tangled.org/core/api/tangled" "tangled.org/core/appview/config" "tangled.org/core/appview/db" + "tangled.org/core/appview/knotacl" + "tangled.org/core/appview/knotcompat" "tangled.org/core/appview/middleware" "tangled.org/core/appview/models" "tangled.org/core/appview/oauth" "tangled.org/core/appview/pages" "tangled.org/core/appview/serververify" "tangled.org/core/appview/xrpcclient" + "tangled.org/core/consts" "tangled.org/core/eventconsumer" "tangled.org/core/idresolver" "tangled.org/core/orm" @@ -37,6 +39,7 @@ type Knots struct { Pages *pages.Pages Config *config.Config Enforcer *rbac.Enforcer + Acl *knotacl.Service IdResolver *idresolver.Resolver Logger *slog.Logger Knotstream *eventconsumer.Consumer @@ -119,13 +122,7 @@ func (k *Knots) dashboard(w http.ResponseWriter, r *http.Request) { } registration := registrations[0] - members, err := k.Enforcer.GetUserByRole("server:member", domain) - if err != nil { - l.Error("failed to get knot members", "err", err) - http.Error(w, "Not found", http.StatusInternalServerError) - return - } - slices.Sort(members) + members := k.Acl.KnotMembers(r.Context(), domain) repos, err := db.GetRepos( k.Db, @@ -556,6 +553,34 @@ func (k *Knots) addMember(w http.ResponseWriter, r *http.Request) { return } + if knotcompat.KnotHasCapability(r.Context(), domain, k.Config.Core.Dev, consts.CapKnotACL) { + client, err := k.OAuth.ServiceClient( + r, + oauth.WithService(domain), + oauth.WithLxm(tangled.KnotAddMemberNSID), + oauth.WithDev(k.Config.Core.Dev), + ) + if err != nil { + l.Error("failed to create knot service client", "err", err) + fail() + return + } + + err = tangled.KnotAddMember(r.Context(), client, &tangled.KnotAddMember_Input{ + Subject: memberId.DID.String(), + }) + if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil { + l.Error("failed to call XRPC knot.addMember", "xrpcerr", xrpcerr, "err", err) + k.Pages.Notice(w, noticeId, xrpcerr.Error()) + return + } + + k.Acl.InvalidateMembers(domain) + + k.Pages.HxRedirect(w, fmt.Sprintf("/settings/knots/%s", domain)) + return + } + client, err := k.OAuth.AuthorizedClient(r) if err != nil { l.Error("failed to authorize client", "err", err) @@ -635,6 +660,34 @@ func (k *Knots) removeMember(w http.ResponseWriter, r *http.Request) { return } + if knotcompat.KnotHasCapability(r.Context(), domain, k.Config.Core.Dev, consts.CapKnotACL) { + client, err := k.OAuth.ServiceClient( + r, + oauth.WithService(domain), + oauth.WithLxm(tangled.KnotRemoveMemberNSID), + oauth.WithDev(k.Config.Core.Dev), + ) + if err != nil { + l.Error("failed to create knot service client", "err", err) + fail() + return + } + + err = tangled.KnotRemoveMember(r.Context(), client, &tangled.KnotRemoveMember_Input{ + Subject: memberId.DID.String(), + }) + if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil { + l.Error("failed to call XRPC knot.removeMember", "xrpcerr", xrpcerr, "err", err) + k.Pages.Notice(w, noticeId, xrpcerr.Error()) + return + } + + k.Acl.InvalidateMembers(domain) + + k.Pages.HxRefresh(w) + return + } + client, err := k.OAuth.AuthorizedClient(r) if err != nil { l.Error("failed to authorize client", "err", err) diff --git a/appview/labels/labels.go b/appview/labels/labels.go index 1f1ef71e..64abfedc 100644 --- a/appview/labels/labels.go +++ b/appview/labels/labels.go @@ -167,7 +167,7 @@ func (l *Labels) PerformLabelOp(w http.ResponseWriter, r *http.Request) { for i := range labelOps { def := actx.Defs[labelOps[i].OperandKey] - if err := l.validator.ValidateLabelOp(def, repo, &labelOps[i]); err != nil { + if err := l.validator.ValidateLabelOp(r.Context(), def, repo, &labelOps[i]); err != nil { fail(fmt.Sprintf("Invalid form data: %s", err), err) return } diff --git a/appview/middleware/middleware.go b/appview/middleware/middleware.go index 207b764d..6eb2350b 100644 --- a/appview/middleware/middleware.go +++ b/appview/middleware/middleware.go @@ -17,6 +17,7 @@ import ( "github.com/go-chi/chi/v5" "tangled.org/core/appview/cache" "tangled.org/core/appview/db" + "tangled.org/core/appview/knotacl" "tangled.org/core/appview/models" "tangled.org/core/appview/oauth" "tangled.org/core/appview/pages" @@ -32,6 +33,7 @@ type Middleware struct { oauth *oauth.OAuth db *db.DB enforcer *rbac.Enforcer + acl *knotacl.Service repoResolver *reporesolver.RepoResolver idResolver *idresolver.Resolver pages *pages.Pages @@ -39,11 +41,12 @@ type Middleware struct { logger *slog.Logger } -func New(oauth *oauth.OAuth, db *db.DB, enforcer *rbac.Enforcer, repoResolver *reporesolver.RepoResolver, idResolver *idresolver.Resolver, pages *pages.Pages, rdb *cache.Cache, logger *slog.Logger) Middleware { +func New(oauth *oauth.OAuth, db *db.DB, enforcer *rbac.Enforcer, acl *knotacl.Service, repoResolver *reporesolver.RepoResolver, idResolver *idresolver.Resolver, pages *pages.Pages, rdb *cache.Cache, logger *slog.Logger) Middleware { return Middleware{ oauth: oauth, db: db, enforcer: enforcer, + acl: acl, repoResolver: repoResolver, idResolver: idResolver, pages: pages, @@ -173,8 +176,7 @@ func (mw Middleware) RepoPermissionMiddleware(requiredPerm string) middlewareFun return } - ok, err := mw.enforcer.E.Enforce(actor.Did, f.Knot, f.RepoIdentifier(), requiredPerm) - if err != nil || !ok { + if !mw.acl.HasRepoPermission(r.Context(), f, actor.Did, requiredPerm) { l.Warn("permission denied", "did", actor.Did, "perm", requiredPerm, "repo", f.RepoIdentifier()) http.Error(w, "Forbidden", http.StatusUnauthorized) return diff --git a/appview/oauth/oauth.go b/appview/oauth/oauth.go index 74af1873..eddd001c 100644 --- a/appview/oauth/oauth.go +++ b/appview/oauth/oauth.go @@ -31,6 +31,11 @@ const ( sessionCacheTTL = time.Hour ) +type KnotMembership interface { + IsKnotMember(ctx context.Context, host, userDid string) bool + InvalidateMembers(host string) +} + type OAuth struct { ClientApp *oauth.ClientApp SessStore *sessions.CookieStore @@ -41,6 +46,7 @@ type OAuth struct { Posthog posthog.Client Db *db.DB Enforcer *rbac.Enforcer + Acl KnotMembership IdResolver *idresolver.Resolver Logger *slog.Logger @@ -92,7 +98,7 @@ func (o *OAuth) HandlePermanentAuthErr(ctx context.Context, did syntax.DID, sess return true } -func New(config *config.Config, ph posthog.Client, db *db.DB, enforcer *rbac.Enforcer, res *idresolver.Resolver, logger *slog.Logger) (*OAuth, error) { +func New(config *config.Config, ph posthog.Client, db *db.DB, enforcer *rbac.Enforcer, acl KnotMembership, res *idresolver.Resolver, logger *slog.Logger) (*OAuth, error) { var oauthConfig oauth.ClientConfig var clientUri string if config.Core.Dev { @@ -152,6 +158,7 @@ func New(config *config.Config, ph posthog.Client, db *db.DB, enforcer *rbac.Enf Posthog: ph, Db: db, Enforcer: enforcer, + Acl: acl, IdResolver: res, Logger: logger, sessionCache: expirable.NewLRU[string, *oauth.ClientSession](sessionCacheSize, nil, sessionCacheTTL), @@ -405,16 +412,16 @@ func (s *ServiceClientOpts) Host() string { } func (o *OAuth) ServiceClient(r *http.Request, os ...ServiceClientOpt) (*xrpc.Client, error) { - opts := DefaultServiceClientOpts() - for _, o := range os { - o(&opts) - } - client, err := o.AuthorizedClient(r) if err != nil { return nil, err } + opts := DefaultServiceClientOpts() + for _, o := range os { + o(&opts) + } + // force expiry to atleast 60 seconds in the future sixty := time.Now().Unix() + 60 if opts.exp < sixty { diff --git a/appview/oauth/scopes.go b/appview/oauth/scopes.go index 808c7298..b00aa131 100644 --- a/appview/oauth/scopes.go +++ b/appview/oauth/scopes.go @@ -27,7 +27,10 @@ var TangledScopes = []string{ "blob:*/*", + "rpc:sh.tangled.knot.addMember?aud=*", + "rpc:sh.tangled.knot.removeMember?aud=*", "rpc:sh.tangled.pipeline.cancelPipeline?aud=*", + "rpc:sh.tangled.repo.addCollaborator?aud=*", "rpc:sh.tangled.repo.addSecret?aud=*", "rpc:sh.tangled.repo.create?aud=*", "rpc:sh.tangled.repo.delete?aud=*", @@ -38,6 +41,7 @@ var TangledScopes = []string{ "rpc:sh.tangled.repo.listSecrets?aud=*", "rpc:sh.tangled.repo.merge?aud=*", "rpc:sh.tangled.repo.mergeCheck?aud=*", + "rpc:sh.tangled.repo.removeCollaborator?aud=*", "rpc:sh.tangled.repo.removeSecret?aud=*", "rpc:sh.tangled.repo.setDefaultBranch?aud=*", } diff --git a/appview/pages/templates/repo/settings/access.html b/appview/pages/templates/repo/settings/access.html index 66b12da7..060c4ec2 100644 --- a/appview/pages/templates/repo/settings/access.html +++ b/appview/pages/templates/repo/settings/access.html @@ -20,6 +20,7 @@

{{ template "collaboratorsGrid" . }} +
{{ end }} @@ -40,6 +41,18 @@

{{ .Role }}

+ + {{ if and (eq .Role "collaborator") $.RepoInfo.Roles.CollaboratorInviteAllowed }} + + {{ end }} {{ end }} diff --git a/appview/pulls/compose.go b/appview/pulls/compose.go index 518a2fed..6217058a 100644 --- a/appview/pulls/compose.go +++ b/appview/pulls/compose.go @@ -18,7 +18,6 @@ import ( "tangled.org/core/appview/oauth" "tangled.org/core/appview/pages" "tangled.org/core/appview/pages/markup" - "tangled.org/core/appview/pages/repoinfo" "tangled.org/core/appview/xrpcclient" "tangled.org/core/patchutil" "tangled.org/core/types" @@ -67,7 +66,7 @@ func (s *Pulls) NewPull(w http.ResponseWriter, r *http.Request) { } // Determine PR type based on input parameters - roles := repoinfo.RolesInRepo{Roles: s.enforcer.GetPermissionsInRepo(userDid.String(), f.Knot, f.RepoIdentifier())} + roles := s.acl.RolesInRepo(r.Context(), f, userDid.String()) isPushAllowed := roles.IsPushAllowed() isBranchBased := isPushAllowed && sourceBranch != "" && fromFork == "" isForkBased := fromFork != "" && sourceBranch != "" diff --git a/appview/pulls/create.go b/appview/pulls/create.go index 690c71ff..8c55d1b0 100644 --- a/appview/pulls/create.go +++ b/appview/pulls/create.go @@ -11,8 +11,8 @@ import ( "time" "tangled.org/core/api/tangled" - "tangled.org/core/appview/compat113" "tangled.org/core/appview/db" + "tangled.org/core/appview/knotcompat" "tangled.org/core/appview/models" "tangled.org/core/appview/oauth" "tangled.org/core/appview/reporesolver" @@ -303,7 +303,7 @@ func (s *Pulls) createPullRequest( Collection: tangled.RepoPullNSID, Repo: userDid.String(), Rkey: rkey, - Record: compat113.Pull(&record), + Record: knotcompat.Pull(&record), }) if err != nil { l.Error("failed to create pull request", "err", err) @@ -403,7 +403,7 @@ func (s *Pulls) createStackedPullRequest( RepoApplyWrites_Create: &comatproto.RepoApplyWrites_Create{ Collection: tangled.RepoPullNSID, Rkey: &p.Rkey, - Value: compat113.Pull(&record), + Value: knotcompat.Pull(&record), }, }) } diff --git a/appview/pulls/labels.go b/appview/pulls/labels.go index 15e42696..0efaf72d 100644 --- a/appview/pulls/labels.go +++ b/appview/pulls/labels.go @@ -140,7 +140,7 @@ func (s *Pulls) applyCreationLabels( valid := make([]models.LabelOp, 0, len(raw)) for _, op := range raw { def := defs[op.OperandKey] - if err := s.validator.ValidateLabelOp(def, repo, &op); err != nil { + if err := s.validator.ValidateLabelOp(ctx, def, repo, &op); err != nil { l.Warn("invalid label op", "err", err, "subject", op.Subject, "key", op.OperandKey) continue } diff --git a/appview/pulls/lifecycle.go b/appview/pulls/lifecycle.go index 317c0d09..04ad8a9e 100644 --- a/appview/pulls/lifecycle.go +++ b/appview/pulls/lifecycle.go @@ -6,7 +6,6 @@ import ( "tangled.org/core/appview/db" "tangled.org/core/appview/models" - "tangled.org/core/appview/pages/repoinfo" "tangled.org/core/appview/reporesolver" "tangled.org/core/orm" @@ -36,7 +35,7 @@ func (s *Pulls) ClosePull(w http.ResponseWriter, r *http.Request) { l = l.With("pull_id", pull.PullId, "pull_owner", pull.OwnerDid) // auth filter: only owner or collaborators can close - roles := repoinfo.RolesInRepo{Roles: s.enforcer.GetPermissionsInRepo(user.Did, f.Knot, f.RepoIdentifier())} + roles := s.acl.RolesInRepo(r.Context(), f, user.Did) isOwner := roles.IsOwner() isCollaborator := roles.IsCollaborator() isPullAuthor := user.Did == pull.OwnerDid @@ -113,7 +112,7 @@ func (s *Pulls) ReopenPull(w http.ResponseWriter, r *http.Request) { l = l.With("pull_id", pull.PullId, "pull_owner", pull.OwnerDid, "state", pull.State) // auth filter: only owner or collaborators can close - roles := repoinfo.RolesInRepo{Roles: s.enforcer.GetPermissionsInRepo(user.Did, f.Knot, f.RepoIdentifier())} + roles := s.acl.RolesInRepo(r.Context(), f, user.Did) isOwner := roles.IsOwner() isCollaborator := roles.IsCollaborator() isPullAuthor := user.Did == pull.OwnerDid diff --git a/appview/pulls/pulls.go b/appview/pulls/pulls.go index 4a2ae025..7ac5dcb3 100644 --- a/appview/pulls/pulls.go +++ b/appview/pulls/pulls.go @@ -10,6 +10,7 @@ import ( "tangled.org/core/appview/config" "tangled.org/core/appview/db" pulls_indexer "tangled.org/core/appview/indexer/pulls" + "tangled.org/core/appview/knotacl" "tangled.org/core/appview/mentions" "tangled.org/core/appview/models" "tangled.org/core/appview/notify" @@ -19,7 +20,6 @@ import ( "tangled.org/core/appview/validator" "tangled.org/core/idresolver" "tangled.org/core/ogre" - "tangled.org/core/rbac" indigoxrpc "github.com/bluesky-social/indigo/xrpc" ) @@ -35,7 +35,7 @@ type Pulls struct { db *db.DB config *config.Config notifier notify.Notifier - enforcer *rbac.Enforcer + acl *knotacl.Service logger *slog.Logger validator *validator.Validator indexer *pulls_indexer.Indexer @@ -51,7 +51,7 @@ func New( db *db.DB, config *config.Config, notifier notify.Notifier, - enforcer *rbac.Enforcer, + acl *knotacl.Service, validator *validator.Validator, indexer *pulls_indexer.Indexer, logger *slog.Logger, @@ -65,7 +65,7 @@ func New( db: db, config: config, notifier: notifier, - enforcer: enforcer, + acl: acl, logger: logger, validator: validator, indexer: indexer, diff --git a/appview/pulls/resubmit.go b/appview/pulls/resubmit.go index cffe3f07..22ebe443 100644 --- a/appview/pulls/resubmit.go +++ b/appview/pulls/resubmit.go @@ -7,12 +7,11 @@ import ( "time" "tangled.org/core/api/tangled" - "tangled.org/core/appview/compat113" "tangled.org/core/appview/db" + "tangled.org/core/appview/knotcompat" "tangled.org/core/appview/models" "tangled.org/core/appview/oauth" "tangled.org/core/appview/pages" - "tangled.org/core/appview/pages/repoinfo" "tangled.org/core/appview/reporesolver" "tangled.org/core/appview/xrpcclient" "tangled.org/core/orm" @@ -123,7 +122,7 @@ func (s *Pulls) resubmitBranch(w http.ResponseWriter, r *http.Request) { return } - roles := repoinfo.RolesInRepo{Roles: s.enforcer.GetPermissionsInRepo(user.Did, f.Knot, f.RepoIdentifier())} + roles := s.acl.RolesInRepo(r.Context(), f, user.Did) if !roles.IsPushAllowed() { l.Warn("unauthorized user - no push permission") w.WriteHeader(http.StatusUnauthorized) @@ -331,7 +330,7 @@ func (s *Pulls) resubmitPullHelper( Repo: userDid.String(), Rkey: pull.Rkey, SwapRecord: ex.Cid, - Record: compat113.Pull(&record), + Record: knotcompat.Pull(&record), }) if err != nil { l.Error("failed to update record on PDS", "err", err, "rkey", pull.Rkey) @@ -520,7 +519,7 @@ func (s *Pulls) resubmitStackedPullHelper( RepoApplyWrites_Create: &comatproto.RepoApplyWrites_Create{ Collection: tangled.RepoPullNSID, Rkey: &p.Rkey, - Value: compat113.Pull(&record), + Value: knotcompat.Pull(&record), }, }) } @@ -578,7 +577,7 @@ func (s *Pulls) resubmitStackedPullHelper( RepoApplyWrites_Update: &comatproto.RepoApplyWrites_Update{ Collection: tangled.RepoPullNSID, Rkey: op.Rkey, - Value: compat113.Pull(&record), + Value: knotcompat.Pull(&record), }, }) } diff --git a/appview/pulls/single.go b/appview/pulls/single.go index 629a59fc..2d0d1451 100644 --- a/appview/pulls/single.go +++ b/appview/pulls/single.go @@ -3,7 +3,6 @@ package pulls import ( "fmt" "net/http" - "slices" "strconv" "tangled.org/core/api/tangled" @@ -364,8 +363,7 @@ func (s *Pulls) branchDeleteStatus(r *http.Request, repo *models.Repo, pull *mod } // user can only delete branch if they are a collaborator in the repo that the branch belongs to - perms := s.enforcer.GetPermissionsInRepo(user.Did, repo.Knot, repo.RepoIdentifier()) - if !slices.Contains(perms, "repo:push") { + if !s.acl.HasRepoPermission(r.Context(), repo, user.Did, "repo:push") { return nil } diff --git a/appview/repo/repo.go b/appview/repo/repo.go index 85f74d10..53de76dc 100644 --- a/appview/repo/repo.go +++ b/appview/repo/repo.go @@ -15,9 +15,10 @@ import ( "tangled.org/core/appview/cloudflare" "tangled.org/core/api/tangled" - "tangled.org/core/appview/compat113" "tangled.org/core/appview/config" "tangled.org/core/appview/db" + "tangled.org/core/appview/knotacl" + "tangled.org/core/appview/knotcompat" "tangled.org/core/appview/models" "tangled.org/core/appview/notify" "tangled.org/core/appview/oauth" @@ -27,6 +28,7 @@ import ( "tangled.org/core/appview/sites" "tangled.org/core/appview/validator" xrpcclient "tangled.org/core/appview/xrpcclient" + "tangled.org/core/consts" "tangled.org/core/eventconsumer" "tangled.org/core/idresolver" "tangled.org/core/ogre" @@ -52,6 +54,7 @@ type Repo struct { spindlestream *eventconsumer.Consumer db *db.DB enforcer *rbac.Enforcer + acl *knotacl.Service notifier notify.Notifier logger *slog.Logger serviceAuth *serviceauth.ServiceAuth @@ -70,6 +73,7 @@ func New( config *config.Config, notifier notify.Notifier, enforcer *rbac.Enforcer, + acl *knotacl.Service, logger *slog.Logger, validator *validator.Validator, cfClient *cloudflare.Client, @@ -84,6 +88,7 @@ func New( db: db, notifier: notifier, enforcer: enforcer, + acl: acl, logger: logger, validator: validator, cfClient: cfClient, @@ -754,6 +759,39 @@ func (rp *Repo) AddCollaborator(w http.ResponseWriter, r *http.Request) { l = l.With("collaborator", collaboratorIdent.Handle) l = l.With("knot", f.Knot) + if knotcompat.KnotHasCapability(r.Context(), f.Knot, rp.config.Core.Dev, consts.CapKnotACL) { + if f.RepoDid == "" { + fail("This repository is missing its DID and cannot manage collaborators.", nil) + return + } + + client, err := rp.oauth.ServiceClient( + r, + oauth.WithService(f.Knot), + oauth.WithLxm(tangled.RepoAddCollaboratorNSID), + oauth.WithDev(rp.config.Core.Dev), + ) + if err != nil { + fail("Failed to connect to knot server.", err) + return + } + + err = tangled.RepoAddCollaborator(r.Context(), client, &tangled.RepoAddCollaborator_Input{ + Repo: f.RepoDid, + Subject: collaboratorIdent.DID.String(), + }) + if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil { + l.Error("failed to call XRPC repo.addCollaborator", "xrpcerr", xrpcerr, "err", err) + rp.pages.Notice(w, errorId, xrpcerr.Error()) + return + } + + rp.acl.InvalidateCollaborators(f.Knot, f.RepoDid) + + rp.pages.HxRefresh(w) + return + } + existing, err := db.GetCollaborators(rp.db, orm.FilterEq("repo_did", f.RepoDid), orm.FilterEq("subject_did", collaboratorIdent.DID.String()), @@ -782,7 +820,7 @@ func (rp *Repo) AddCollaborator(w http.ResponseWriter, r *http.Request) { Collection: tangled.RepoCollaboratorNSID, Repo: currentUser.Did, Rkey: rkey, - Record: compat113.Collaborator(repoCollaboratorRecord(f, collaboratorIdent.DID.String(), createdAt)), + Record: knotcompat.Collaborator(repoCollaboratorRecord(f, collaboratorIdent.DID.String(), createdAt)), }) // invalid record if err != nil { @@ -853,6 +891,148 @@ func (rp *Repo) AddCollaborator(w http.ResponseWriter, r *http.Request) { rp.pages.HxRefresh(w) } +func (rp *Repo) RemoveCollaborator(w http.ResponseWriter, r *http.Request) { + user := rp.oauth.GetMultiAccountUser(r) + l := rp.logger.With("handler", "RemoveCollaborator") + l = l.With("did", user.Did) + + f, err := rp.repoResolver.Resolve(r) + if err != nil { + l.Error("failed to get repo and knot", "err", err) + return + } + + errorId := "collaborator-error" + fail := func(msg string, err error) { + l.Error(msg, "err", err) + rp.pages.Notice(w, errorId, msg) + } + + collaborator := r.FormValue("collaborator") + if collaborator == "" { + fail("Invalid form.", nil) + return + } + collaborator = strings.TrimPrefix(collaborator, "@") + + collaboratorIdent, err := rp.idResolver.ResolveIdent(r.Context(), collaborator) + if err != nil { + fail(fmt.Sprintf("'%s' is not a valid DID/handle.", collaborator), err) + return + } + l = l.With("collaborator", collaboratorIdent.Handle, "knot", f.Knot) + + if collaboratorIdent.DID.String() == f.Did { + fail("Cannot remove the repository owner.", nil) + return + } + + if knotcompat.KnotHasCapability(r.Context(), f.Knot, rp.config.Core.Dev, consts.CapKnotACL) { + if f.RepoDid == "" { + fail("This repository is missing its DID and cannot manage collaborators.", nil) + return + } + + client, err := rp.oauth.ServiceClient( + r, + oauth.WithService(f.Knot), + oauth.WithLxm(tangled.RepoRemoveCollaboratorNSID), + oauth.WithDev(rp.config.Core.Dev), + ) + if err != nil { + fail("Failed to connect to knot server.", err) + return + } + + err = tangled.RepoRemoveCollaborator(r.Context(), client, &tangled.RepoRemoveCollaborator_Input{ + Repo: f.RepoDid, + Subject: collaboratorIdent.DID.String(), + }) + if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil { + l.Error("failed to call XRPC repo.removeCollaborator", "xrpcerr", xrpcerr, "err", err) + rp.pages.Notice(w, errorId, xrpcerr.Error()) + return + } + + rp.acl.InvalidateCollaborators(f.Knot, f.RepoDid) + + rp.pages.HxRefresh(w) + return + } + + existing, err := db.GetCollaborators(rp.db, + orm.FilterEq("repo_did", f.RepoDid), + orm.FilterEq("subject_did", collaboratorIdent.DID.String()), + ) + if err != nil { + fail("Failed to look up collaborator.", err) + return + } + if len(existing) == 0 { + fail(fmt.Sprintf("%s is not a collaborator.", collaboratorIdent.Handle), nil) + return + } + row := existing[0] + + client, err := rp.oauth.AuthorizedClient(r) + if err != nil { + fail("Failed to write to PDS.", err) + return + } + + tx, err := rp.db.BeginTx(r.Context(), nil) + if err != nil { + fail("Failed to remove collaborator.", err) + return + } + committed := false + defer func() { + if !committed { + tx.Rollback() + if err := rp.enforcer.E.LoadPolicy(); err != nil { + l.Error("failed to reload policy after rollback", "err", err) + } + } + }() + + if err := rp.enforcer.RemoveCollaborator(collaboratorIdent.DID.String(), f.Knot, f.RepoIdentifier()); err != nil { + fail("Failed to remove collaborator permissions.", err) + return + } + + if err := db.DeleteCollaborator(tx, + orm.FilterEq("repo_did", f.RepoDid), + orm.FilterEq("subject_did", collaboratorIdent.DID.String()), + ); err != nil { + fail("Failed to remove collaborator.", err) + return + } + + if row.Rkey.Valid && row.Rkey.String != "" { + if _, err := comatproto.RepoDeleteRecord(r.Context(), client, &comatproto.RepoDeleteRecord_Input{ + Collection: tangled.RepoCollaboratorNSID, + Repo: row.Did.String(), + Rkey: row.Rkey.String, + }); err != nil { + fail("Failed to delete collaborator record from PDS.", err) + return + } + } + + if err := tx.Commit(); err != nil { + fail("Failed to remove collaborator.", err) + return + } + committed = true + + if err := rp.enforcer.E.SavePolicy(); err != nil { + fail("Failed to update collaborator permissions.", err) + return + } + + rp.pages.HxRefresh(w) +} + func (rp *Repo) RenameRepo(w http.ResponseWriter, r *http.Request) { l := rp.logger.With("handler", "RenameRepo") noticeId := "rename-repo-error" @@ -871,7 +1051,7 @@ func (rp *Repo) RenameRepo(w http.ResponseWriter, r *http.Request) { return } - if !compat113.KnotSupports114(r.Context(), f.Knot, rp.config.Core.Dev) { + if !knotcompat.KnotSupports114(r.Context(), f.Knot, rp.config.Core.Dev) { rp.pages.Notice(w, noticeId, "This repository's knot is below v1.14 and does not yet support renames. Ask the knot operator to upgrade.") return } @@ -1242,11 +1422,7 @@ func (rp *Repo) ForkRepo(w http.ResponseWriter, r *http.Request) { switch r.Method { case http.MethodGet: user := rp.oauth.GetMultiAccountUser(r) - knots, err := rp.enforcer.GetKnotsForUser(user.Did) - if err != nil { - rp.pages.Notice(w, "repo", "Invalid user account.") - return - } + knots := rp.acl.KnotsForUser(r.Context(), user.Did) rp.pages.ForkRepo(w, pages.ForkRepoParams{ LoggedInUser: user, @@ -1264,8 +1440,7 @@ func (rp *Repo) ForkRepo(w http.ResponseWriter, r *http.Request) { } l = l.With("targetKnot", targetKnot) - ok, err := rp.enforcer.E.Enforce(user.Did, targetKnot, targetKnot, "repo:create") - if err != nil || !ok { + if !rp.acl.IsRepoCreateAllowed(r.Context(), targetKnot, user.Did) { rp.pages.Notice(w, "repo", "You do not have permission to create a repo in this knot.") return } diff --git a/appview/repo/router.go b/appview/repo/router.go index 82e229a1..606010d0 100644 --- a/appview/repo/router.go +++ b/appview/repo/router.go @@ -88,6 +88,7 @@ func (rp *Repo) Router(mw *middleware.Middleware) http.Handler { r.With(mw.RepoPermissionMiddleware("repo:owner")).Post("/label/subscribe", rp.SubscribeLabel) r.With(mw.RepoPermissionMiddleware("repo:owner")).Post("/label/unsubscribe", rp.UnsubscribeLabel) r.With(mw.RepoPermissionMiddleware("repo:invite")).Put("/collaborator", rp.AddCollaborator) + r.With(mw.RepoPermissionMiddleware("repo:invite")).Delete("/collaborator", rp.RemoveCollaborator) r.With(mw.RepoPermissionMiddleware("repo:delete")).Delete("/delete", rp.DeleteRepo) r.With(mw.RepoPermissionMiddleware("repo:owner")).Post("/rename", rp.RenameRepo) r.Put("/branches/default", rp.SetDefaultBranch) diff --git a/appview/repo/settings.go b/appview/repo/settings.go index baabef38..11532df9 100644 --- a/appview/repo/settings.go +++ b/appview/repo/settings.go @@ -448,39 +448,13 @@ func (rp *Repo) accessSettings(w http.ResponseWriter, r *http.Request) { l := rp.logger.With("handler", "accessSettings") f, err := rp.repoResolver.Resolve(r) - user := rp.oauth.GetMultiAccountUser(r) - - collaborators, err := func(repo *models.Repo) ([]pages.Collaborator, error) { - repoCollaborators, err := rp.enforcer.E.GetImplicitUsersForResourceByDomain(repo.RepoIdentifier(), repo.Knot) - if err != nil { - return nil, err - } - var collaborators []pages.Collaborator - for _, item := range repoCollaborators { - // currently only two roles: owner and member - var role string - switch item[3] { - case "repo:owner": - role = "owner" - case "repo:collaborator": - role = "collaborator" - default: - continue - } - - did := item[0] - - c := pages.Collaborator{ - Did: did, - Role: role, - } - collaborators = append(collaborators, c) - } - return collaborators, nil - }(f) if err != nil { - l.Error("failed to get collaborators", "err", err) + l.Error("failed to resolve repo", "err", err) + return } + user := rp.oauth.GetMultiAccountUser(r) + + collaborators := rp.acl.Collaborators(r.Context(), f) rp.pages.RepoAccessSettings(w, pages.RepoAccessSettingsParams{ LoggedInUser: user, diff --git a/appview/reporesolver/resolver.go b/appview/reporesolver/resolver.go index a728bf24..1d0e91ee 100644 --- a/appview/reporesolver/resolver.go +++ b/appview/reporesolver/resolver.go @@ -13,10 +13,10 @@ import ( "tangled.org/core/appview/cache" "tangled.org/core/appview/config" "tangled.org/core/appview/db" + "tangled.org/core/appview/knotacl" "tangled.org/core/appview/models" "tangled.org/core/appview/oauth" "tangled.org/core/appview/pages/repoinfo" - "tangled.org/core/rbac" ) var ( @@ -25,14 +25,14 @@ var ( ) type RepoResolver struct { - config *config.Config - enforcer *rbac.Enforcer - execer db.Execer - rdb *cache.Cache + config *config.Config + acl *knotacl.Service + execer db.Execer + rdb *cache.Cache } -func New(config *config.Config, enforcer *rbac.Enforcer, execer db.Execer, rdb *cache.Cache) *RepoResolver { - return &RepoResolver{config: config, enforcer: enforcer, execer: execer, rdb: rdb} +func New(config *config.Config, acl *knotacl.Service, execer db.Execer, rdb *cache.Cache) *RepoResolver { + return &RepoResolver{config: config, acl: acl, execer: execer, rdb: rdb} } func CanonicalRepoPath(handle string, repo *models.Repo) string { @@ -102,7 +102,7 @@ func (rr *RepoResolver) GetRepoInfo(r *http.Request, user *oauth.MultiAccountUse roles := repoinfo.RolesInRepo{} if user != nil { isStarred = db.GetStarStatus(rr.execer, user.Did, repoDid) - roles.Roles = rr.enforcer.GetPermissionsInRepo(user.Did, repo.Knot, repo.RepoIdentifier()) + roles = rr.acl.RolesInRepo(r.Context(), repo, user.Did) } stats := repo.RepoStats diff --git a/appview/state/knotstream.go b/appview/state/knotstream.go index 29292ebb..17352dec 100644 --- a/appview/state/knotstream.go +++ b/appview/state/knotstream.go @@ -16,8 +16,10 @@ import ( "tangled.org/core/api/tangled" "tangled.org/core/appview/config" "tangled.org/core/appview/db" + "tangled.org/core/appview/knotcompat" "tangled.org/core/appview/models" "tangled.org/core/appview/sites" + "tangled.org/core/consts" ec "tangled.org/core/eventconsumer" "tangled.org/core/eventstream" knotdb "tangled.org/core/knotserver/db" @@ -88,12 +90,14 @@ func ingestRefUpdate(ctx context.Context, d *db.DB, enforcer *rbac.Enforcer, pc return err } - knownKnots, err := enforcer.GetKnotsForUser(record.CommitterDid) - if err != nil { - return err - } - if !slices.Contains(knownKnots, source.Host) { - return fmt.Errorf("%s does not belong to %s, something is fishy", record.CommitterDid, source.Host) + if !knotcompat.KnotHasCapability(ctx, source.Host, dev, consts.CapKnotACL) { + knownKnots, err := enforcer.GetKnotsForUser(record.CommitterDid) + switch { + case err != nil: + logger.Warn("gitRefUpdate membership lookup failed, ingesting without the sanity check", "committer", record.CommitterDid, "knot", source.Host, "err", err) + case !slices.Contains(knownKnots, source.Host): + logger.Warn("gitRefUpdate committer is not a known member of the knot, ingesting anyway", "committer", record.CommitterDid, "knot", source.Host) + } } if record.Repo == "" { diff --git a/appview/state/router.go b/appview/state/router.go index 2fc0d8cc..e9cef4ea 100644 --- a/appview/state/router.go +++ b/appview/state/router.go @@ -10,6 +10,7 @@ import ( "github.com/go-chi/chi/v5" "tangled.org/core/appview/db" "tangled.org/core/appview/issues" + "tangled.org/core/appview/knotacl" "tangled.org/core/appview/knots" "tangled.org/core/appview/labels" "tangled.org/core/appview/metrics" @@ -33,6 +34,7 @@ func (s *State) Router() http.Handler { s.oauth, s.db, s.enforcer, + s.aclService, s.repoResolver, s.idResolver, s.pages, @@ -41,6 +43,7 @@ func (s *State) Router() http.Handler { ) router.Use(metrics.Middleware) + router.Use(knotacl.MemoMiddleware) if err := db.ReapStaleRunningMigrations(context.Background(), s.db); err != nil { s.logger.Warn("failed to reap stale running migrations", "err", err) @@ -311,6 +314,7 @@ func (s *State) KnotsRouter() http.Handler { Pages: s.pages, Config: s.config, Enforcer: s.enforcer, + Acl: s.aclService, IdResolver: s.idResolver, Knotstream: s.knotstream, Logger: logger, @@ -338,7 +342,7 @@ func (s *State) IssuesRouter(mw *middleware.Middleware) http.Handler { issues := issues.New( s.oauth, s.repoResolver, - s.enforcer, + s.aclService, s.pages, s.idResolver, s.mentionsResolver, @@ -362,7 +366,7 @@ func (s *State) PullsRouter(mw *middleware.Middleware) http.Handler { s.db, s.config, s.notifier, - s.enforcer, + s.aclService, s.validator, s.indexer.Pulls, log.SubLogger(s.logger, "pulls"), @@ -381,6 +385,7 @@ func (s *State) RepoRouter(mw *middleware.Middleware) http.Handler { s.config, s.notifier, s.enforcer, + s.aclService, log.SubLogger(s.logger, "repo"), s.validator, s.cfClient, diff --git a/appview/state/state.go b/appview/state/state.go index b9da5ea2..fa910482 100644 --- a/appview/state/state.go +++ b/appview/state/state.go @@ -19,6 +19,8 @@ import ( "tangled.org/core/appview/db" "tangled.org/core/appview/email" "tangled.org/core/appview/indexer" + "tangled.org/core/appview/knotacl" + "tangled.org/core/appview/knotcompat" "tangled.org/core/appview/mentions" "tangled.org/core/appview/models" "tangled.org/core/appview/notify" @@ -67,6 +69,7 @@ type State struct { jc *jetstream.JetstreamClient config *config.Config repoResolver *reporesolver.RepoResolver + aclService *knotacl.Service knotstream *eventconsumer.Consumer spindlestream *eventconsumer.Consumer pipelineNotifier *pipelines.StatusNotifier @@ -111,13 +114,16 @@ func Make(ctx context.Context, config *config.Config) (*State, error) { } pages := pages.NewPages(config, res, d, rdb, log.SubLogger(logger, "pages")) - oauth, err := oauth.New(config, posthog, d, enforcer, res, log.SubLogger(logger, "oauth")) + knotcompat.UseNativeLatch(knotacl.NewLatch(d, log.SubLogger(logger, "knotacl-latch"))) + aclService := knotacl.NewService(enforcer, d, config.Core.Dev, log.SubLogger(logger, "knotacl")) + oauth, err := oauth.New(config, posthog, d, enforcer, aclService, res, log.SubLogger(logger, "oauth")) if err != nil { return nil, fmt.Errorf("failed to start oauth handler: %w", err) } - validator := validator.New(d, res, enforcer) - repoResolver := reporesolver.New(config, enforcer, d, rdb) + validator := validator.New(d, res, aclService) + + repoResolver := reporesolver.New(config, aclService, d, rdb) mentionsResolver := mentions.New(config, res, d, log.SubLogger(logger, "mentionsResolver")) @@ -183,6 +189,7 @@ func Make(ctx context.Context, config *config.Config) (*State, error) { Ctx: ctx, Db: d, Enforcer: enforcer, + Acl: aclService, IdResolver: res, Cache: rdb, Config: config, @@ -236,6 +243,7 @@ func Make(ctx context.Context, config *config.Config) (*State, error) { jc: jc, config: config, repoResolver: repoResolver, + aclService: aclService, knotstream: knotstream, spindlestream: spindlestream, pipelineNotifier: pipelineNotifier, @@ -447,11 +455,7 @@ func (s *State) NewRepo(w http.ResponseWriter, r *http.Request) { switch r.Method { case http.MethodGet: user := s.oauth.GetMultiAccountUser(r) - knots, err := s.enforcer.GetKnotsForUser(user.Did) - if err != nil { - s.pages.Notice(w, "repo", "Invalid user account.") - return - } + knots := s.aclService.KnotsForUser(r.Context(), user.Did) s.pages.NewRepo(w, pages.NewRepoParams{ LoggedInUser: user, @@ -499,8 +503,7 @@ func (s *State) NewRepo(w http.ResponseWriter, r *http.Request) { } // ACL validation - ok, err := s.enforcer.E.Enforce(user.Did, domain, domain, "repo:create") - if err != nil || !ok { + if !s.aclService.IsRepoCreateAllowed(r.Context(), domain, user.Did) { l.Info("unauthorized") s.pages.Notice(w, "repo", "You do not have permission to create a repo in this knot.") return diff --git a/appview/validator/label.go b/appview/validator/label.go index 6d46aba2..2fbbce07 100644 --- a/appview/validator/label.go +++ b/appview/validator/label.go @@ -95,7 +95,7 @@ func (v *Validator) ValidateLabelDefinition(label *models.LabelDefinition) error return nil } -func (v *Validator) ValidateLabelOp(labelDef *models.LabelDefinition, repo *models.Repo, labelOp *models.LabelOp) error { +func (v *Validator) ValidateLabelOp(ctx context.Context, labelDef *models.LabelDefinition, repo *models.Repo, labelOp *models.LabelOp) error { if labelDef == nil { return fmt.Errorf("label definition is required") } @@ -106,17 +106,6 @@ func (v *Validator) ValidateLabelOp(labelDef *models.LabelDefinition, repo *mode return fmt.Errorf("label operation is required") } - // validate permissions: only collaborators can apply labels currently - // - // TODO: introduce a repo:triage permission - ok, err := v.enforcer.IsPushAllowed(labelOp.Did, repo.Knot, repo.RepoIdentifier()) - if err != nil { - return fmt.Errorf("failed to enforce permissions: %w", err) - } - if !ok { - return fmt.Errorf("unauhtorized label operation") - } - expectedKey := labelDef.AtUri().String() if labelOp.OperandKey != expectedKey { return fmt.Errorf("operand key %q does not match label definition URI %q", labelOp.OperandKey, expectedKey) @@ -142,6 +131,17 @@ func (v *Validator) ValidateLabelOp(labelDef *models.LabelDefinition, repo *mode return fmt.Errorf("performed_at timestamp is required") } + // validate permissions: only collaborators can apply labels currently + // + // TODO: introduce a repo:triage permission + ok, err := v.acl.HasRepoPermissionErr(ctx, repo, labelOp.Did, "repo:push") + if err != nil { + return err + } + if !ok { + return fmt.Errorf("unauthorized label operation") + } + return nil } diff --git a/appview/validator/label_test.go b/appview/validator/label_test.go new file mode 100644 index 00000000..2f8f43d8 --- /dev/null +++ b/appview/validator/label_test.go @@ -0,0 +1,87 @@ +package validator + +import ( + "context" + "encoding/json" + "errors" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "path/filepath" + "strings" + "testing" + "time" + + "tangled.org/core/api/tangled" + "tangled.org/core/appview/db" + "tangled.org/core/appview/knotacl" + "tangled.org/core/appview/models" + "tangled.org/core/consts" + "tangled.org/core/rbac" +) + +func unreachableListValidator(t *testing.T) (*Validator, string) { + t.Helper() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case strings.HasSuffix(r.URL.Path, tangled.KnotVersionNSID): + json.NewEncoder(w).Encode(tangled.KnotVersion_Output{Version: "v1.15.0", Capabilities: []string{string(consts.CapKnotACL)}}) + case strings.HasSuffix(r.URL.Path, tangled.RepoListCollaboratorsNSID): + http.Error(w, "list down", http.StatusInternalServerError) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(srv.Close) + host := strings.TrimPrefix(srv.URL, "http://") + + dir := t.TempDir() + enforcer, err := rbac.NewEnforcer(filepath.Join(dir, "rbac.db")) + if err != nil { + t.Fatalf("NewEnforcer: %v", err) + } + d, err := db.Make(context.Background(), filepath.Join(dir, "appview.db")) + if err != nil { + t.Fatalf("db.Make: %v", err) + } + svc := knotacl.NewService(enforcer, d, true, slog.New(slog.NewTextHandler(io.Discard, nil))) + return &Validator{acl: svc}, host +} + +func TestValidateLabelOp_MalformedRejectedBeforePermCheck(t *testing.T) { + v, host := unreachableListValidator(t) + def := &models.LabelDefinition{Did: "did:plc:akshay", Rkey: "deadbeef"} + repo := &models.Repo{Did: "did:plc:akshay", Knot: host, RepoDid: "did:plc:limpet"} + + op := &models.LabelOp{ + Did: "did:plc:scallop", + OperandKey: "does-not-match-the-def-aturi", + Operation: "garbage", + } + err := v.ValidateLabelOp(context.Background(), def, repo, op) + if errors.Is(err, knotacl.ErrKnotUnreachable) { + t.Fatalf("malformed op returned ErrKnotUnreachable; structural validation did not run before the perm check") + } + if err == nil || !strings.Contains(err.Error(), "operand key") { + t.Fatalf("want a structural operand-key error, got %v", err) + } +} + +func TestValidateLabelOp_WellFormedFailsOpenWhenKnotUnreachable(t *testing.T) { + v, host := unreachableListValidator(t) + def := &models.LabelDefinition{Did: "did:plc:akshay", Rkey: "deadbeef"} + repo := &models.Repo{Did: "did:plc:akshay", Knot: host, RepoDid: "did:plc:limpet"} + + op := &models.LabelOp{ + Did: "did:plc:scallop", + OperandKey: def.AtUri().String(), + Operation: models.LabelOperationAdd, + Subject: "at://did:plc:limpet/sh.tangled.repo.issue/abc123", + PerformedAt: time.Now(), + } + err := v.ValidateLabelOp(context.Background(), def, repo, op) + if !errors.Is(err, knotacl.ErrKnotUnreachable) { + t.Fatalf("well-formed op against an unreachable knot = %v, want ErrKnotUnreachable so the ingester fails open", err) + } +} diff --git a/appview/validator/validator.go b/appview/validator/validator.go index 1fcf5888..434a6c1e 100644 --- a/appview/validator/validator.go +++ b/appview/validator/validator.go @@ -2,23 +2,23 @@ package validator import ( "tangled.org/core/appview/db" + "tangled.org/core/appview/knotacl" "tangled.org/core/appview/pages/markup" "tangled.org/core/idresolver" - "tangled.org/core/rbac" ) type Validator struct { db *db.DB sanitizer markup.Sanitizer resolver *idresolver.Resolver - enforcer *rbac.Enforcer + acl *knotacl.Service } -func New(db *db.DB, res *idresolver.Resolver, enforcer *rbac.Enforcer) *Validator { +func New(db *db.DB, res *idresolver.Resolver, acl *knotacl.Service) *Validator { return &Validator{ db: db, sanitizer: markup.NewSanitizer(), resolver: res, - enforcer: enforcer, + acl: acl, } } diff --git a/nix/modules/appview.nix b/nix/modules/appview.nix index 037de80d..01f87bcc 100644 --- a/nix/modules/appview.nix +++ b/nix/modules/appview.nix @@ -270,6 +270,7 @@ in {env}`TANGLED_OAUTH_CLIENT_SECRET`, {env}`TANGLED_RESEND_API_KEY`, {env}`TANGLED_CAMO_SHARED_SECRET`, {env}`TANGLED_AVATAR_SHARED_SECRET`, {env}`TANGLED_REDIS_PASS`, {env}`TANGLED_PDS_ADMIN_SECRET`, + {env}`TANGLED_KNOT_ADMIN_SECRET`, {env}`TANGLED_CLOUDFLARE_API_TOKEN`, {env}`TANGLED_CLOUDFLARE_ZONE_ID`, {env}`TANGLED_CLOUDFLARE_TURNSTILE_SITE_KEY`, {env}`TANGLED_CLOUDFLARE_TURNSTILE_SECRET_KEY`, -- 2.51.2