diff --git a/apps/api/internal/service/access.go b/apps/api/internal/service/access.go new file mode 100644 index 00000000..2d526132 --- /dev/null +++ b/apps/api/internal/service/access.go @@ -0,0 +1,49 @@ +package service + +import ( + "context" + + "github.com/google/uuid" + + "github.com/Devlaner/devlane/api/internal/model" + "github.com/Devlaner/devlane/api/internal/store" +) + +// enforceProjectVisibility rejects access to a project whose visibility +// (`network`) is not public, unless the caller is a workspace admin/owner or a +// member of the project itself. Public projects are visible to any workspace +// member. +// +// It mirrors the gate in ProjectService.GetByID so that project sub-resources +// (issues, states, labels, cycles, modules, estimates, intake, comments, pages, +// views, attachments, …) respect the same rules — otherwise a plain workspace +// member could reach a secret project's data through its sub-resource routes +// even though the project itself returns 404. +// +// It returns ErrProjectNotFound when the project is hidden from the caller, to +// match GetByID's 404 behaviour and avoid leaking the project's existence. +// Callers should invoke it only after they have already confirmed workspace +// membership and that the project belongs to the workspace. +func enforceProjectVisibility( + ctx context.Context, + ps *store.ProjectStore, + ws *store.WorkspaceStore, + workspaceID, projectID, userID uuid.UUID, +) error { + p, err := ps.GetByID(ctx, projectID) + if err != nil { + return ErrProjectNotFound + } + if p.Network == model.NetworkPublic { + return nil + } + // Secret project: workspace admins/owners can always see it. + if wm, err := ws.GetMember(ctx, workspaceID, userID); err == nil && wm != nil && wm.Role >= model.RoleAdmin { + return nil + } + // Otherwise the caller must be a member of the project. + if pm, err := ps.GetProjectMember(ctx, projectID, userID); err == nil && pm != nil { + return nil + } + return ErrProjectNotFound +} diff --git a/apps/api/internal/service/access_test.go b/apps/api/internal/service/access_test.go new file mode 100644 index 00000000..a7167127 --- /dev/null +++ b/apps/api/internal/service/access_test.go @@ -0,0 +1,109 @@ +package service_test + +import ( + "context" + "errors" + "testing" + + "github.com/google/uuid" + "gorm.io/gorm" + + "github.com/Devlaner/devlane/api/internal/model" + "github.com/Devlaner/devlane/api/internal/service" + "github.com/Devlaner/devlane/api/internal/store" + "github.com/Devlaner/devlane/api/internal/testutil" +) + +func makeProjectSecret(t *testing.T, db *gorm.DB, projectID uuid.UUID) { + t.Helper() + if err := db.Model(&model.Project{}).Where("id = ?", projectID). + Update("network", model.NetworkSecret).Error; err != nil { + t.Fatalf("make project secret: %v", err) + } +} + +// A secret (non-public) project's sub-resources must respect the same +// visibility rule as ProjectService.GetByID: reachable only by a workspace +// admin/owner or a member of the project — never by a plain workspace member +// who was never added to the project, even though workspace membership and +// project-in-workspace both pass. Exercised through real service methods so the +// gate is verified end-to-end for every sub-resource that shares +// ensureProjectAccess. +func TestSecretProjectVisibility(t *testing.T) { + ts := testutil.NewTestServer(t) + db := ts.DB + ctx := context.Background() + + owner := testutil.CreateUser(t, db) + wrk := testutil.CreateWorkspace(t, db, owner.ID) // workspace owner + auto project lead + + admin := testutil.CreateUser(t, db) + testutil.AddWorkspaceMember(t, db, wrk.ID, admin.ID, model.RoleAdmin) + + // Plain workspace member, not added to the project. + outsider := testutil.CreateUser(t, db) + testutil.AddWorkspaceMember(t, db, wrk.ID, outsider.ID, model.RoleMember) + + // Plain workspace member who IS a member of the project. + insider := testutil.CreateUser(t, db) + testutil.AddWorkspaceMember(t, db, wrk.ID, insider.ID, model.RoleMember) + + proj := testutil.CreateProject(t, db, wrk.ID, owner.ID) + testutil.AddProjectMember(t, db, proj.ID, wrk.ID, insider.ID, model.RoleMember) + + intakeSvc := service.NewIntakeService( + store.NewIntakeStore(db), + store.NewIssueStore(db), + store.NewProjectStore(db), + store.NewWorkspaceStore(db), + ) + stateSvc := service.NewStateService( + store.NewStateStore(db), + store.NewProjectStore(db), + store.NewWorkspaceStore(db), + ) + + // listErr returns the access error surfaced by two independent + // sub-resource services (intake and states), so the assertion isn't tied + // to one code path. + listErr := func(userID uuid.UUID) (error, error) { + _, e1 := intakeSvc.List(ctx, wrk.Slug, proj.ID, userID, nil) + _, e2 := stateSvc.List(ctx, wrk.Slug, proj.ID, userID) + return e1, e2 + } + + // While public, every workspace member can read the sub-resources. + if e1, e2 := listErr(outsider.ID); e1 != nil || e2 != nil { + t.Fatalf("public project should be readable by a workspace member: intake=%v state=%v", e1, e2) + } + + makeProjectSecret(t, db, proj.ID) + + t.Run("outsider-denied", func(t *testing.T) { + e1, e2 := listErr(outsider.ID) + if !errors.Is(e1, service.ErrProjectNotFound) { + t.Errorf("intake: want ErrProjectNotFound, got %v", e1) + } + if !errors.Is(e2, service.ErrProjectNotFound) { + t.Errorf("states: want ErrProjectNotFound, got %v", e2) + } + }) + + t.Run("workspace-admin-allowed", func(t *testing.T) { + if e1, e2 := listErr(admin.ID); e1 != nil || e2 != nil { + t.Errorf("workspace admin should see the secret project: intake=%v state=%v", e1, e2) + } + }) + + t.Run("project-member-allowed", func(t *testing.T) { + if e1, e2 := listErr(insider.ID); e1 != nil || e2 != nil { + t.Errorf("project member should see the secret project: intake=%v state=%v", e1, e2) + } + }) + + t.Run("project-lead-allowed", func(t *testing.T) { + if e1, e2 := listErr(owner.ID); e1 != nil || e2 != nil { + t.Errorf("project lead should see the secret project: intake=%v state=%v", e1, e2) + } + }) +} diff --git a/apps/api/internal/service/attachment.go b/apps/api/internal/service/attachment.go index 94e083d4..543a7a23 100644 --- a/apps/api/internal/service/attachment.go +++ b/apps/api/internal/service/attachment.go @@ -81,6 +81,9 @@ func (s *AttachmentService) ensureProjectAccess(ctx context.Context, workspaceSl if !inWorkspace { return ErrProjectNotFound } + if err := enforceProjectVisibility(ctx, s.ps, s.ws, wrk.ID, projectID, userID); err != nil { + return err + } return nil } @@ -118,6 +121,9 @@ func (s *AttachmentService) AuthorizeDownload(ctx context.Context, issueID, asse if !ok { return ErrProjectForbidden } + if err := enforceProjectVisibility(ctx, s.ps, s.ws, att.WorkspaceID, att.ProjectID, userID); err != nil { + return err + } return nil } diff --git a/apps/api/internal/service/comment.go b/apps/api/internal/service/comment.go index 0ad7a6a8..c1e347d7 100644 --- a/apps/api/internal/service/comment.go +++ b/apps/api/internal/service/comment.go @@ -74,6 +74,9 @@ func (s *CommentService) ensureProjectAccess(ctx context.Context, workspaceSlug if !inWorkspace { return ErrProjectNotFound } + if err := enforceProjectVisibility(ctx, s.ps, s.ws, wrk.ID, projectID, userID); err != nil { + return err + } return nil } diff --git a/apps/api/internal/service/cycle.go b/apps/api/internal/service/cycle.go index 04ee1a58..76e0d573 100644 --- a/apps/api/internal/service/cycle.go +++ b/apps/api/internal/service/cycle.go @@ -97,6 +97,9 @@ func (s *CycleService) ensureProjectAccess(ctx context.Context, workspaceSlug st if !inWorkspace { return ErrProjectNotFound } + if err := enforceProjectVisibility(ctx, s.ps, s.ws, wrk.ID, projectID, userID); err != nil { + return err + } return nil } diff --git a/apps/api/internal/service/estimate.go b/apps/api/internal/service/estimate.go index 4078e941..386a5672 100644 --- a/apps/api/internal/service/estimate.go +++ b/apps/api/internal/service/estimate.go @@ -42,6 +42,9 @@ func (s *EstimateService) ensureProjectAccess(ctx context.Context, workspaceSlug if !inWorkspace { return nil, ErrProjectNotFound } + if err := enforceProjectVisibility(ctx, s.ps, s.ws, wrk.ID, projectID, userID); err != nil { + return nil, err + } return wrk, nil } diff --git a/apps/api/internal/service/export.go b/apps/api/internal/service/export.go index d73af7b8..26568c1a 100644 --- a/apps/api/internal/service/export.go +++ b/apps/api/internal/service/export.go @@ -58,6 +58,9 @@ func (s *ExportService) ExportIssues(ctx context.Context, slug string, userID uu if !in { return "", nil, ErrProjectNotFound } + if err := enforceProjectVisibility(ctx, s.ps, s.ws, wrk.ID, pid, userID); err != nil { + return "", nil, err + } } data, err := s.buildWorkbook(ctx, projectIDs) diff --git a/apps/api/internal/service/intake.go b/apps/api/internal/service/intake.go index dae90d97..94a0487f 100644 --- a/apps/api/internal/service/intake.go +++ b/apps/api/internal/service/intake.go @@ -58,6 +58,9 @@ func (s *IntakeService) ensureProjectAccess(ctx context.Context, workspaceSlug s if !inWorkspace { return nil, ErrProjectNotFound } + if err := enforceProjectVisibility(ctx, s.ps, s.ws, wrk.ID, projectID, userID); err != nil { + return nil, err + } return wrk, nil } diff --git a/apps/api/internal/service/issue.go b/apps/api/internal/service/issue.go index 5d651e82..bdbdb25d 100644 --- a/apps/api/internal/service/issue.go +++ b/apps/api/internal/service/issue.go @@ -226,6 +226,9 @@ func (s *IssueService) ensureProjectAccess(ctx context.Context, workspaceSlug st if !inWorkspace { return ErrProjectNotFound } + if err := enforceProjectVisibility(ctx, s.ps, s.ws, wrk.ID, projectID, userID); err != nil { + return err + } return nil } diff --git a/apps/api/internal/service/issue_view.go b/apps/api/internal/service/issue_view.go index 9f1e10cc..68f72dec 100644 --- a/apps/api/internal/service/issue_view.go +++ b/apps/api/internal/service/issue_view.go @@ -48,6 +48,9 @@ func (s *IssueViewService) ensureProjectAccess(ctx context.Context, workspaceSlu if !inWorkspace { return ErrProjectNotFound } + if err := enforceProjectVisibility(ctx, s.ps, s.ws, wrk.ID, projectID, userID); err != nil { + return err + } return nil } diff --git a/apps/api/internal/service/label.go b/apps/api/internal/service/label.go index 9a3ec9a5..607de470 100644 --- a/apps/api/internal/service/label.go +++ b/apps/api/internal/service/label.go @@ -35,6 +35,9 @@ func (s *LabelService) ensureProjectAccess(ctx context.Context, workspaceSlug st if !inWorkspace { return uuid.Nil, ErrProjectNotFound } + if err := enforceProjectVisibility(ctx, s.ps, s.ws, wrk.ID, projectID, userID); err != nil { + return uuid.Nil, err + } return wrk.ID, nil } diff --git a/apps/api/internal/service/module.go b/apps/api/internal/service/module.go index 1b6223f4..9cda28d9 100644 --- a/apps/api/internal/service/module.go +++ b/apps/api/internal/service/module.go @@ -55,6 +55,9 @@ func (s *ModuleService) ensureProjectAccess(ctx context.Context, workspaceSlug s if !inWorkspace { return ErrProjectNotFound } + if err := enforceProjectVisibility(ctx, s.ps, s.ws, wrk.ID, projectID, userID); err != nil { + return err + } return nil } diff --git a/apps/api/internal/service/page.go b/apps/api/internal/service/page.go index 2be5bb43..d665096e 100644 --- a/apps/api/internal/service/page.go +++ b/apps/api/internal/service/page.go @@ -123,6 +123,9 @@ func (s *PageService) ensureProjectAccess(ctx context.Context, workspaceSlug str if !inWorkspace { return ErrProjectNotFound } + if err := enforceProjectVisibility(ctx, s.projectStore, s.ws, wrk.ID, projectID, userID); err != nil { + return err + } return nil } diff --git a/apps/api/internal/service/state.go b/apps/api/internal/service/state.go index 3543417b..27f61632 100644 --- a/apps/api/internal/service/state.go +++ b/apps/api/internal/service/state.go @@ -47,6 +47,9 @@ func (s *StateService) ensureProjectAccess(ctx context.Context, workspaceSlug st if !inWorkspace { return uuid.Nil, ErrProjectNotFound } + if err := enforceProjectVisibility(ctx, s.ps, s.ws, wrk.ID, projectID, userID); err != nil { + return uuid.Nil, err + } return wrk.ID, nil }