diff --git a/go.mod b/go.mod index 990a7bd73..4b29e0fd9 100644 --- a/go.mod +++ b/go.mod @@ -12,6 +12,7 @@ require ( github.com/antchfx/htmlquery v1.3.0 github.com/bradleyfalzon/ghinstallation/v2 v2.11.0 github.com/buildkite/terminal-to-html v3.2.0+incompatible + github.com/cenkalti/backoff v2.2.1+incompatible github.com/cenkalti/backoff/v4 v4.3.0 github.com/coreos/go-oidc/v3 v3.5.0 github.com/fatih/color v1.16.0 @@ -38,7 +39,7 @@ require ( github.com/lestrrat-go/jwx/v2 v2.1.1 github.com/mitchellh/iochan v1.0.0 github.com/pkg/errors v0.9.1 - github.com/playwright-community/playwright-go v0.4702.0 + github.com/playwright-community/playwright-go v0.4802.0 github.com/prometheus/client_golang v1.14.0 github.com/sdassow/atomic v0.0.1 github.com/spf13/cobra v1.8.0 @@ -66,7 +67,6 @@ require ( github.com/antchfx/xpath v1.2.3 // indirect github.com/apparentlymart/go-textseg/v13 v13.0.0 // indirect github.com/beorn7/perks v1.0.1 // indirect - github.com/cenkalti/backoff v2.2.1+incompatible // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/davecgh/go-spew v1.1.1 // indirect github.com/deckarep/golang-set/v2 v2.6.0 // indirect @@ -123,7 +123,6 @@ require ( go.opentelemetry.io/otel v1.30.0 // indirect go.opentelemetry.io/otel/metric v1.30.0 // indirect go.opentelemetry.io/otel/trace v1.30.0 // indirect - go.uber.org/multierr v1.11.0 // indirect golang.org/x/crypto v0.27.0 // indirect golang.org/x/sys v0.25.0 // indirect golang.org/x/text v0.18.0 // indirect diff --git a/go.sum b/go.sum index b5483abfd..c0f5091bd 100644 --- a/go.sum +++ b/go.sum @@ -88,8 +88,6 @@ github.com/buildkite/terminal-to-html v3.2.0+incompatible h1:WdXzl7ZmYzCAz4pElZo github.com/buildkite/terminal-to-html v3.2.0+incompatible/go.mod h1:BFFdFecOxCgjdcarqI+8izs6v85CU/1RA/4Bqh4GR7E= github.com/cenkalti/backoff v2.2.1+incompatible h1:tNowT99t7UNflLxfYYSlKYsBpXdEet03Pg2g16Swow4= github.com/cenkalti/backoff v2.2.1+incompatible/go.mod h1:90ReRw6GdpyfrHakVjL/QHaoyV4aDUVVkXQJJJ3NXXM= -github.com/cenkalti/backoff/v4 v4.2.1 h1:y4OZtCnogmCPw98Zjyt5a6+QwPLGkiQsYW5oUqylYbM= -github.com/cenkalti/backoff/v4 v4.2.1/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE= github.com/cenkalti/backoff/v4 v4.3.0 h1:MyRJ/UdXutAwSAT+s3wNd7MfTIcy71VQueUuFK343L8= github.com/cenkalti/backoff/v4 v4.3.0/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= @@ -364,8 +362,8 @@ github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINE github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= -github.com/playwright-community/playwright-go v0.4702.0 h1:3CwNpk4RoA42tyhmlgPDMxYEYtMydaeEqMYiW0RNlSY= -github.com/playwright-community/playwright-go v0.4702.0/go.mod h1:bpArn5TqNzmP0jroCgw4poSOG9gSeQg490iLqWAaa7w= +github.com/playwright-community/playwright-go v0.4802.0 h1:FSuvi5Pg/xp+n7vFpu2wGldwSQ3grsaDlHFRfHRQiy4= +github.com/playwright-community/playwright-go v0.4802.0/go.mod h1:kBNWs/w2aJ2ZUp1wEOOFLXgOqvppFngM5OS+qyhl+ZM= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/prometheus/client_golang v0.9.1/go.mod h1:7SWBe2y4D6OKWSNQJUaRYU/AaXPKyh/dDVn+NZz0KFw= @@ -472,8 +470,6 @@ go.opentelemetry.io/otel/sdk v1.29.0 h1:vkqKjk7gwhS8VaWb0POZKmIEDimRCMsopNYnriHy go.opentelemetry.io/otel/sdk v1.29.0/go.mod h1:pM8Dx5WKnvxLCb+8lG1PRNIDxu9g9b9g59Qr7hfAAok= go.opentelemetry.io/otel/trace v1.30.0 h1:7UBkkYzeg3C7kQX8VAidWh2biiQbtAKjyIML8dQ9wmc= go.opentelemetry.io/otel/trace v1.30.0/go.mod h1:5EyKqTzzmyqB9bwtCCq6pDLktPK6fmGf/Dph+8VI02o= -go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0= -go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y= golang.org/x/crypto v0.0.0-20180904163835-0709b304e793/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20190426145343-a29dc8fdc734/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= diff --git a/internal/authz/authorizer.go b/internal/authz/authorizer.go index a50544a0f..198a920cb 100644 --- a/internal/authz/authorizer.go +++ b/internal/authz/authorizer.go @@ -2,13 +2,204 @@ package authz import ( "context" + "errors" + "fmt" + "log/slog" + "github.com/go-logr/logr" + "github.com/leg100/otf/internal" "github.com/leg100/otf/internal/rbac" "github.com/leg100/otf/internal/resource" ) -// Authorizer is capable of granting or denying access to resources based on the -// subject contained within the context. -type Authorizer interface { - CanAccess(ctx context.Context, action rbac.Action, id resource.ID) (Subject, error) +// Authorizer intermediates authorization between subjects (entities requesting +// access) and resources (the entities to which access is being requested). +type Authorizer struct { + logr.Logger + WorkspacePolicyGetter + + organizationResolvers map[resource.Kind]OrganizationResolver + workspaceResolvers map[resource.Kind]WorkspaceResolver +} + +// Interface provides an interface for services to use to permit swapping out +// the authorizer for tests. +type Interface interface { + Authorize(ctx context.Context, action rbac.Action, req *AccessRequest, opts ...CanAccessOption) (Subject, error) + CanAccess(ctx context.Context, action rbac.Action, req *AccessRequest) bool +} + +func NewAuthorizer(logger logr.Logger) *Authorizer { + return &Authorizer{ + Logger: logger, + organizationResolvers: make(map[resource.Kind]OrganizationResolver), + workspaceResolvers: make(map[resource.Kind]WorkspaceResolver), + } +} + +type WorkspacePolicyGetter interface { + GetWorkspacePolicy(ctx context.Context, workspaceID resource.ID) (WorkspacePolicy, error) +} + +// OrganizationResolver takes the ID of a resource and returns the name of the +// organization it belongs to. +type OrganizationResolver func(ctx context.Context, id resource.ID) (string, error) + +// WorkspaceResolver takes the ID of a resource and returns the ID of the +// workspace it belongs to. +type WorkspaceResolver func(ctx context.Context, id resource.ID) (resource.ID, error) + +// RegisterOrganizationResolver registers with the authorizer the ability to +// resolve access requests for a specific resource kind to the name of the +// organization the resource belongs to. +// +// This is necessary because authorization is determined not only on resource ID +// but on the name of the organization the resource belongs to. +func (a *Authorizer) RegisterOrganizationResolver(kind resource.Kind, resolver OrganizationResolver) { + a.organizationResolvers[kind] = resolver +} + +// RegisterWorkspaceResolver registers with the authorizer the ability to +// resolve access requests for a specific resource kind to the workspace ID the +// resource belongs to. +// +// This is necessary because authorization is often determined based on +// workspace ID, and not the ID of a run, state version, etc. +func (a *Authorizer) RegisterWorkspaceResolver(kind resource.Kind, resolver WorkspaceResolver) { + a.workspaceResolvers[kind] = resolver +} + +// Options for configuring the individual calls of CanAccess. + +type CanAccessOption func(*canAccessConfig) + +// WithoutErrorLogging disables logging an unauthorized error. This can be +// useful if just checking if a user can do something. +func WithoutErrorLogging() CanAccessOption { + return func(cfg *canAccessConfig) { + cfg.disableLogs = true + } +} + +type canAccessConfig struct { + disableLogs bool +} + +// Authorize determines whether the subject can carry out an action on a +// resource. The subject is expected to be contained within the context. If the +// access request is nil then it's assumed the request is for access to the +// entire site (the highest level). +func (a *Authorizer) Authorize(ctx context.Context, action rbac.Action, req *AccessRequest, opts ...CanAccessOption) (Subject, error) { + var cfg canAccessConfig + for _, fn := range opts { + fn(&cfg) + } + subj, err := SubjectFromContext(ctx) + if err != nil { + return nil, err + } + // Allow context to contain specific instruction to skip authorization. + // Should only be used for testing purposes. + if SkipAuthz(ctx) { + return subj, nil + } + // Wrapped in function in order to log error messages uniformly. + err = func() error { + if req != nil && req.ID != nil { + // Check if resource kind is registered for its ID to be resolved to workspace + // ID. + if resolver, ok := a.workspaceResolvers[req.ID.Kind()]; ok { + workspaceID, err := resolver(ctx, *req.ID) + if err != nil { + return fmt.Errorf("resolving workspace ID: %w", err) + } + // Authorize workspace ID instead + req.ID = &workspaceID + } + // If the resource kind is a workspace, then fetch its policy. + if req.ID.Kind() == resource.WorkspaceKind { + policy, err := a.GetWorkspacePolicy(ctx, *req.ID) + if err != nil { + return fmt.Errorf("fetching workspace policy: %w", err) + } + req.WorkspacePolicy = &policy + } + // Resolve the organization if not already provided. Every resource + // belongs to an organization, so there should be a resolver for each + // resource kind to resolve the resource ID to the organization it + // belongs to. + if req.Organization == "" { + resolver, ok := a.organizationResolvers[req.ID.Kind()] + if !ok { + return errors.New("resource kind is missing organization resolver") + } + organization, err := resolver(ctx, *req.ID) + if err != nil { + return fmt.Errorf("resolving organization: %w", err) + } + req.Organization = organization + } + } + // Subject determines whether it is allowed to access resource. + if !subj.CanAccess(action, req) { + return internal.ErrAccessNotPermitted + } + return nil + }() + if err != nil && !cfg.disableLogs { + a.Error(err, "authorization failure", + "resource", req, + "action", action.String(), + "subject", subj, + ) + } + return subj, err +} + +// CanAccess is a helper to boil down an access request to a true/false +// decision, with any error encountered interpreted as false. +func (a *Authorizer) CanAccess(ctx context.Context, action rbac.Action, req *AccessRequest) bool { + _, err := a.Authorize(ctx, action, req, WithoutErrorLogging()) + return err == nil +} + +// AccessRequest is a request for access to either an organization or an +// individual resource. +type AccessRequest struct { + // Organization name to which access is being requested. + Organization string + // ID of resource to which access is being requested. If nil then the action + // is being requested on the organization. + ID *resource.ID + // WorkspacePolicy specifies workspace-specific permissions for the resource + // specified by ID. Only non-nil if ID refers to a workspace. + WorkspacePolicy *WorkspacePolicy +} + +// WorkspacePolicy binds workspace permissions to a workspace +type WorkspacePolicy struct { + Permissions []WorkspacePermission + // Whether workspace permits its state to be consumed by all workspaces in + // the organization. + GlobalRemoteState bool +} + +// WorkspacePermission binds a role to a team. +type WorkspacePermission struct { + TeamID resource.ID + Role rbac.Role +} + +func (r *AccessRequest) LogValue() slog.Value { + if r == nil { + return slog.StringValue("site") + } else { + attrs := []slog.Attr{ + slog.String("organization", r.Organization), + } + if r.ID != nil { + attrs = append(attrs, slog.String("resource_id", r.ID.String())) + } + return slog.GroupValue(attrs...) + } } diff --git a/internal/authz/authorizer_test.go b/internal/authz/authorizer_test.go new file mode 100644 index 000000000..47f19be30 --- /dev/null +++ b/internal/authz/authorizer_test.go @@ -0,0 +1,22 @@ +package authz + +import ( + "context" + "testing" + + "github.com/leg100/otf/internal/logr" + "github.com/leg100/otf/internal/rbac" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestAuthorizer(t *testing.T) { + authorizer := NewAuthorizer(logr.Discard()) + user := &Superuser{} + ctx := AddSubjectToContext(context.Background(), user) + + got, err := authorizer.Authorize(ctx, rbac.ListUsersAction, nil) + require.NoError(t, err) + + assert.Equal(t, user, got) +} diff --git a/internal/authz/authorizer_test_helper.go b/internal/authz/authorizer_test_helper.go index 22b56bc31..ef1489431 100644 --- a/internal/authz/authorizer_test_helper.go +++ b/internal/authz/authorizer_test_helper.go @@ -4,7 +4,6 @@ import ( "context" "github.com/leg100/otf/internal/rbac" - "github.com/leg100/otf/internal/resource" ) type allowAllAuthorizer struct { @@ -17,6 +16,10 @@ func NewAllowAllAuthorizer() *allowAllAuthorizer { } } -func (a *allowAllAuthorizer) CanAccess(context.Context, rbac.Action, resource.ID) (Subject, error) { +func (a *allowAllAuthorizer) Authorize(context.Context, rbac.Action, *AccessRequest, ...CanAccessOption) (Subject, error) { return a.User, nil } + +func (a *allowAllAuthorizer) CanAccess(context.Context, rbac.Action, *AccessRequest) bool { + return true +} diff --git a/internal/authz/authz.go b/internal/authz/authz.go index e64bf3b1b..2365f88a9 100644 --- a/internal/authz/authz.go +++ b/internal/authz/authz.go @@ -3,69 +3,17 @@ package authz import ( "context" - "fmt" - - "github.com/leg100/otf/internal/rbac" - "github.com/leg100/otf/internal/resource" ) // unexported key types prevents collisions type ( - subjectCtxKeyType string skipAuthzCtxKeyType string ) const ( - subjectCtxKey subjectCtxKeyType = "subject" skipAuthzCtxKey skipAuthzCtxKeyType = "skip_authz" ) -// Subject is an entity that carries out actions on resources. -type Subject interface { - CanAccessSite(action rbac.Action) bool - CanAccessTeam(action rbac.Action, id resource.ID) bool - CanAccessOrganization(action rbac.Action, name string) bool - CanAccessWorkspace(action rbac.Action, policy WorkspacePolicy) bool - - IsOwner(organization string) bool - IsSiteAdmin() bool - - Organizations() []string - - String() string -} - -// WorkspacePolicy binds workspace permissions to a workspace -type WorkspacePolicy struct { - Organization string - WorkspaceID resource.ID - Permissions []WorkspacePermission - - // Whether workspace permits its state to be consumed by all workspaces in - // the organization. - GlobalRemoteState bool -} - -// WorkspacePermission binds a role to a team. -type WorkspacePermission struct { - TeamID resource.ID - Role rbac.Role -} - -// AddSubjectToContext adds a subject to a context -func AddSubjectToContext(ctx context.Context, subj Subject) context.Context { - return context.WithValue(ctx, subjectCtxKey, subj) -} - -// SubjectFromContext retrieves a subject from a context -func SubjectFromContext(ctx context.Context) (Subject, error) { - subj, ok := ctx.Value(subjectCtxKey).(Subject) - if !ok { - return nil, fmt.Errorf("no subject in context") - } - return subj, nil -} - // AddSkipAuthz adds to the context an instruction to skip authorization. // Authorizers should obey this instruction using SkipAuthz func AddSkipAuthz(ctx context.Context) context.Context { @@ -80,33 +28,3 @@ func SkipAuthz(ctx context.Context) bool { } return false } - -// Superuser is a subject with unlimited privileges. -type Superuser struct { - Username string -} - -func (*Superuser) CanAccessSite(action rbac.Action) bool { return true } -func (*Superuser) CanAccessTeam(rbac.Action, resource.ID) bool { return true } -func (*Superuser) CanAccessOrganization(rbac.Action, string) bool { return true } -func (*Superuser) CanAccessWorkspace(rbac.Action, WorkspacePolicy) bool { return true } -func (s *Superuser) Organizations() []string { return nil } -func (s *Superuser) String() string { return s.Username } -func (s *Superuser) GetID() resource.ID { return resource.NewID(resource.UserKind) } -func (s *Superuser) IsSiteAdmin() bool { return true } -func (s *Superuser) IsOwner(string) bool { return true } - -// Nobody is a subject with no privileges. -type Nobody struct { - Username string -} - -func (*Nobody) CanAccessSite(action rbac.Action) bool { return false } -func (*Nobody) CanAccessTeam(rbac.Action, resource.ID) bool { return false } -func (*Nobody) CanAccessOrganization(rbac.Action, string) bool { return false } -func (*Nobody) CanAccessWorkspace(rbac.Action, WorkspacePolicy) bool { return false } -func (s *Nobody) Organizations() []string { return nil } -func (s *Nobody) String() string { return s.Username } -func (s *Nobody) ID() string { return s.Username } -func (s *Nobody) IsSiteAdmin() bool { return false } -func (s *Nobody) IsOwner(string) bool { return false } diff --git a/internal/authz/site_authorizer.go b/internal/authz/site_authorizer.go deleted file mode 100644 index 77107c6a4..000000000 --- a/internal/authz/site_authorizer.go +++ /dev/null @@ -1,27 +0,0 @@ -package authz - -import ( - "context" - - "github.com/go-logr/logr" - "github.com/leg100/otf/internal" - "github.com/leg100/otf/internal/rbac" - "github.com/leg100/otf/internal/resource" -) - -// SiteAuthorizer authorizes access to site-wide actions -type SiteAuthorizer struct { - logr.Logger -} - -func (a *SiteAuthorizer) CanAccess(ctx context.Context, action rbac.Action, _ resource.ID) (Subject, error) { - subj, err := SubjectFromContext(ctx) - if err != nil { - return nil, err - } - if subj.CanAccessSite(action) { - return subj, nil - } - a.Error(nil, "unauthorized action", "action", action, "subject", subj) - return nil, internal.ErrAccessNotPermitted -} diff --git a/internal/authz/subject.go b/internal/authz/subject.go new file mode 100644 index 000000000..cde794bee --- /dev/null +++ b/internal/authz/subject.go @@ -0,0 +1,37 @@ +package authz + +import ( + "context" + "fmt" + + "github.com/leg100/otf/internal/rbac" +) + +// unexported key types prevents collisions +type ( + subjectCtxKeyType string +) + +const ( + subjectCtxKey subjectCtxKeyType = "subject" +) + +// Subject is an entity that carries out actions on resources. +type Subject interface { + CanAccess(action rbac.Action, req *AccessRequest) bool + String() string +} + +// AddSubjectToContext adds a subject to a context +func AddSubjectToContext(ctx context.Context, subj Subject) context.Context { + return context.WithValue(ctx, subjectCtxKey, subj) +} + +// SubjectFromContext retrieves a subject from a context +func SubjectFromContext(ctx context.Context) (Subject, error) { + subj, ok := ctx.Value(subjectCtxKey).(Subject) + if !ok { + return nil, fmt.Errorf("no subject in context") + } + return subj, nil +} diff --git a/internal/authz/superuser.go b/internal/authz/superuser.go new file mode 100644 index 000000000..fd6be6d1c --- /dev/null +++ b/internal/authz/superuser.go @@ -0,0 +1,13 @@ +package authz + +import ( + "github.com/leg100/otf/internal/rbac" +) + +// Superuser is a subject with unlimited privileges. +type Superuser struct { + Username string +} + +func (*Superuser) CanAccess(rbac.Action, *AccessRequest) bool { return true } +func (s *Superuser) String() string { return s.Username } diff --git a/internal/configversion/configuration_version.go b/internal/configversion/configuration_version.go index 26599f8e8..df068f834 100644 --- a/internal/configversion/configuration_version.go +++ b/internal/configversion/configuration_version.go @@ -19,9 +19,6 @@ const ( // Default maximum config size is 10mb. DefaultConfigMaxSize int64 = 1024 * 1024 * 10 - - ConfigVersionKind = "cv" - IngressAttributesKind = "ia" ) type ( @@ -113,7 +110,7 @@ type ( // NewConfigurationVersion creates a ConfigurationVersion object from scratch func NewConfigurationVersion(workspaceID resource.ID, opts CreateOptions) (*ConfigurationVersion, error) { cv := ConfigurationVersion{ - ID: resource.NewID(ConfigVersionKind), + ID: resource.NewID(resource.ConfigVersionKind), CreatedAt: internal.CurrentTimestamp(nil), AutoQueueRuns: DefaultAutoQueueRuns, Source: DefaultSource, diff --git a/internal/configversion/service.go b/internal/configversion/service.go index beb98ce2e..d9c547bb1 100644 --- a/internal/configversion/service.go +++ b/internal/configversion/service.go @@ -17,8 +17,7 @@ import ( type ( Service struct { logr.Logger - - workspace authz.Authorizer + *authz.Authorizer db *pgdb cache internal.Cache @@ -29,8 +28,8 @@ type ( Options struct { logr.Logger - WorkspaceAuthorizer authz.Authorizer - MaxConfigSize int64 + MaxConfigSize int64 + Authorizer *authz.Authorizer internal.Cache *sql.DB @@ -41,11 +40,9 @@ type ( func NewService(opts Options) *Service { svc := Service{ - Logger: opts.Logger, + Logger: opts.Logger, + Authorizer: opts.Authorizer, } - - svc.workspace = opts.WorkspaceAuthorizer - svc.db = &pgdb{opts.DB} svc.cache = opts.Cache svc.tfeapi = &tfe{ @@ -66,6 +63,18 @@ func NewService(opts Options) *Service { // Fetch ingress attributes when API requests ingress attributes be included // in the response opts.Responder.Register(tfeapi.IncludeIngress, svc.tfeapi.includeIngressAttributes) + // Resolve authorization requests for config version IDs to a workspace IDs + opts.Authorizer.RegisterWorkspaceResolver(resource.ConfigVersionKind, + func(ctx context.Context, cvID resource.ID) (resource.ID, error) { + sv, err := svc.db.GetConfigurationVersion(ctx, ConfigurationVersionGetOptions{ + ID: &cvID, + }) + if err != nil { + return resource.ID{}, err + } + return sv.WorkspaceID, nil + }, + ) return &svc } @@ -76,7 +85,7 @@ func (s *Service) AddHandlers(r *mux.Router) { } func (s *Service) Create(ctx context.Context, workspaceID resource.ID, opts CreateOptions) (*ConfigurationVersion, error) { - subject, err := s.workspace.CanAccess(ctx, rbac.CreateConfigurationVersionAction, workspaceID) + subject, err := s.Authorize(ctx, rbac.CreateConfigurationVersionAction, &authz.AccessRequest{ID: &workspaceID}) if err != nil { return nil, err } @@ -95,7 +104,7 @@ func (s *Service) Create(ctx context.Context, workspaceID resource.ID, opts Crea } func (s *Service) List(ctx context.Context, workspaceID resource.ID, opts ListOptions) (*resource.Page[*ConfigurationVersion], error) { - subject, err := s.workspace.CanAccess(ctx, rbac.ListConfigurationVersionsAction, workspaceID) + subject, err := s.Authorize(ctx, rbac.ListConfigurationVersionsAction, &authz.AccessRequest{ID: &workspaceID}) if err != nil { return nil, err } @@ -111,7 +120,7 @@ func (s *Service) List(ctx context.Context, workspaceID resource.ID, opts ListOp } func (s *Service) Get(ctx context.Context, cvID resource.ID) (*ConfigurationVersion, error) { - subject, err := s.canAccess(ctx, rbac.GetConfigurationVersionAction, cvID) + subject, err := s.Authorize(ctx, rbac.GetConfigurationVersionAction, &authz.AccessRequest{ID: &cvID}) if err != nil { return nil, err } @@ -126,7 +135,7 @@ func (s *Service) Get(ctx context.Context, cvID resource.ID) (*ConfigurationVers } func (s *Service) GetLatest(ctx context.Context, workspaceID resource.ID) (*ConfigurationVersion, error) { - subject, err := s.workspace.CanAccess(ctx, rbac.GetConfigurationVersionAction, workspaceID) + subject, err := s.Authorize(ctx, rbac.GetConfigurationVersionAction, &authz.AccessRequest{ID: &workspaceID}) if err != nil { return nil, err } @@ -141,7 +150,7 @@ func (s *Service) GetLatest(ctx context.Context, workspaceID resource.ID) (*Conf } func (s *Service) Delete(ctx context.Context, cvID resource.ID) error { - subject, err := s.canAccess(ctx, rbac.DeleteConfigurationVersionAction, cvID) + subject, err := s.Authorize(ctx, rbac.DeleteConfigurationVersionAction, &authz.AccessRequest{ID: &cvID}) if err != nil { return err } @@ -154,11 +163,3 @@ func (s *Service) Delete(ctx context.Context, cvID resource.ID) error { s.V(2).Info("deleted configuration version", "id", cvID, "subject", subject) return nil } - -func (s *Service) canAccess(ctx context.Context, action rbac.Action, cvID resource.ID) (authz.Subject, error) { - cv, err := s.db.GetConfigurationVersion(ctx, ConfigurationVersionGetOptions{ID: &cvID}) - if err != nil { - return nil, err - } - return s.workspace.CanAccess(ctx, action, cv.WorkspaceID) -} diff --git a/internal/configversion/tarball_service.go b/internal/configversion/tarball_service.go index 8620eea99..b136d7e6c 100644 --- a/internal/configversion/tarball_service.go +++ b/internal/configversion/tarball_service.go @@ -4,6 +4,7 @@ import ( "context" "fmt" + "github.com/leg100/otf/internal/authz" "github.com/leg100/otf/internal/rbac" "github.com/leg100/otf/internal/resource" ) @@ -33,7 +34,7 @@ func (s *Service) UploadConfig(ctx context.Context, cvID resource.ID, config []b // DownloadConfig retrieves a tarball from the db func (s *Service) DownloadConfig(ctx context.Context, cvID resource.ID) ([]byte, error) { - subject, err := s.canAccess(ctx, rbac.DownloadConfigurationVersionAction, cvID) + subject, err := s.Authorize(ctx, rbac.DownloadConfigurationVersionAction, &authz.AccessRequest{ID: &cvID}) if err != nil { return nil, err } diff --git a/internal/configversion/tfe.go b/internal/configversion/tfe.go index b73839a7d..87a726185 100644 --- a/internal/configversion/tfe.go +++ b/internal/configversion/tfe.go @@ -226,7 +226,7 @@ func (a *tfe) includeIngressAttributes(ctx context.Context, v any) ([]any, error return nil, err } return []any{&types.IngressAttributes{ - ID: resource.ConvertID(cv.ID, IngressAttributesKind), + ID: resource.ConvertID(cv.ID, resource.IngressAttributesKind), CommitSHA: cv.IngressAttributes.CommitSHA, CommitURL: cv.IngressAttributes.CommitURL, }}, nil diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index dc7de5b99..7a245de14 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -12,6 +12,7 @@ import ( "github.com/leg100/otf/internal" "github.com/leg100/otf/internal/api" "github.com/leg100/otf/internal/authenticator" + "github.com/leg100/otf/internal/authz" "github.com/leg100/otf/internal/configversion" "github.com/leg100/otf/internal/connections" "github.com/leg100/otf/internal/disco" @@ -129,8 +130,11 @@ func New(ctx context.Context, logger logr.Logger, cfg Config) (*Daemon, error) { return nil, fmt.Errorf("setting up authentication middleware: %w", err) } + authorizer := authz.NewAuthorizer(logger) + orgService := organization.NewService(organization.Options{ Logger: logger, + Authorizer: authorizer, DB: db, Listener: listener, Renderer: renderer, @@ -141,6 +145,7 @@ func New(ctx context.Context, logger logr.Logger, cfg Config) (*Daemon, error) { teamService := team.NewService(team.Options{ Logger: logger, + Authorizer: authorizer, DB: db, Renderer: renderer, Responder: responder, @@ -149,6 +154,7 @@ func New(ctx context.Context, logger logr.Logger, cfg Config) (*Daemon, error) { }) userService := user.NewService(user.Options{ Logger: logger, + Authorizer: authorizer, DB: db, Renderer: renderer, Responder: responder, @@ -163,6 +169,7 @@ func New(ctx context.Context, logger logr.Logger, cfg Config) (*Daemon, error) { githubAppService := github.NewService(github.Options{ Logger: logger, + Authorizer: authorizer, DB: db, Renderer: renderer, HostnameService: hostnameService, @@ -174,6 +181,7 @@ func New(ctx context.Context, logger logr.Logger, cfg Config) (*Daemon, error) { vcsProviderService := vcsprovider.NewService(vcsprovider.Options{ Logger: logger, + Authorizer: authorizer, DB: db, Renderer: renderer, Responder: responder, @@ -211,6 +219,7 @@ func New(ctx context.Context, logger logr.Logger, cfg Config) (*Daemon, error) { } workspaceService := workspace.NewService(workspace.Options{ Logger: logger, + Authorizer: authorizer, DB: db, Listener: listener, Renderer: renderer, @@ -222,22 +231,22 @@ func New(ctx context.Context, logger logr.Logger, cfg Config) (*Daemon, error) { VCSProviderService: vcsProviderService, }) configService := configversion.NewService(configversion.Options{ - Logger: logger, - DB: db, - WorkspaceAuthorizer: workspaceService, - Responder: responder, - Cache: cache, - Signer: signer, - MaxConfigSize: cfg.MaxConfigSize, + Logger: logger, + Authorizer: authorizer, + DB: db, + Responder: responder, + Cache: cache, + Signer: signer, + MaxConfigSize: cfg.MaxConfigSize, }) runService := run.NewService(run.Options{ Logger: logger, + Authorizer: authorizer, DB: db, Listener: listener, Renderer: renderer, Responder: responder, - WorkspaceAuthorizer: workspaceService, OrganizationService: orgService, WorkspaceService: workspaceService, ConfigVersionService: configService, @@ -249,15 +258,16 @@ func New(ctx context.Context, logger logr.Logger, cfg Config) (*Daemon, error) { TokensService: tokensService, }) logsService := logs.NewService(logs.Options{ - Logger: logger, - DB: db, - RunAuthorizer: runService, - Cache: cache, - Listener: listener, - Verifier: signer, + Logger: logger, + Authorizer: authorizer, + DB: db, + Cache: cache, + Listener: listener, + Verifier: signer, }) moduleService := module.NewService(module.Options{ Logger: logger, + Authorizer: authorizer, DB: db, Renderer: renderer, HostnameService: hostnameService, @@ -269,6 +279,7 @@ func New(ctx context.Context, logger logr.Logger, cfg Config) (*Daemon, error) { }) stateService := state.NewService(state.Options{ Logger: logger, + Authorizer: authorizer, DB: db, WorkspaceService: workspaceService, Cache: cache, @@ -277,17 +288,18 @@ func New(ctx context.Context, logger logr.Logger, cfg Config) (*Daemon, error) { Signer: signer, }) variableService := variable.NewService(variable.Options{ - Logger: logger, - DB: db, - Renderer: renderer, - Responder: responder, - WorkspaceAuthorizer: workspaceService, - WorkspaceService: workspaceService, - RunClient: runService, + Logger: logger, + Authorizer: authorizer, + DB: db, + Renderer: renderer, + Responder: responder, + WorkspaceService: workspaceService, + RunClient: runService, }) runnerService := runner.NewService(runner.ServiceOptions{ Logger: logger, + Authorizer: authorizer, DB: db, Renderer: renderer, Responder: responder, @@ -352,11 +364,11 @@ func New(ctx context.Context, logger logr.Logger, cfg Config) (*Daemon, error) { } notificationService := notifications.NewService(notifications.Options{ - Logger: logger, - DB: db, - Listener: listener, - Responder: responder, - WorkspaceAuthorizer: workspaceService, + Logger: logger, + Authorizer: authorizer, + DB: db, + Listener: listener, + Responder: responder, }) handlers := []internal.Handlers{ diff --git a/internal/github/service.go b/internal/github/service.go index 50de8cf79..801f743a3 100644 --- a/internal/github/service.go +++ b/internal/github/service.go @@ -10,9 +10,7 @@ import ( "github.com/leg100/otf/internal" "github.com/leg100/otf/internal/authz" "github.com/leg100/otf/internal/http/html" - "github.com/leg100/otf/internal/organization" "github.com/leg100/otf/internal/rbac" - "github.com/leg100/otf/internal/resource" "github.com/leg100/otf/internal/sql" "github.com/leg100/otf/internal/vcs" ) @@ -21,13 +19,12 @@ type ( // Service is the service for github app management Service struct { logr.Logger + *authz.Authorizer GithubHostname string - site authz.Authorizer - organization *organization.Authorizer - db *pgdb - web *webHandlers + db *pgdb + web *webHandlers } Options struct { @@ -39,6 +36,7 @@ type ( GithubHostname string SkipTLSVerification bool + Authorizer *authz.Authorizer } ) @@ -46,8 +44,7 @@ func NewService(opts Options) *Service { svc := Service{ Logger: opts.Logger, GithubHostname: opts.GithubHostname, - site: &authz.SiteAuthorizer{Logger: opts.Logger}, - organization: &organization.Authorizer{Logger: opts.Logger}, + Authorizer: opts.Authorizer, db: &pgdb{opts.DB}, } svc.web = &webHandlers{ @@ -65,7 +62,7 @@ func (a *Service) AddHandlers(r *mux.Router) { } func (a *Service) CreateApp(ctx context.Context, opts CreateAppOptions) (*App, error) { - subject, err := a.site.CanAccess(ctx, rbac.CreateGithubAppAction, resource.ID{}) + subject, err := a.Authorize(ctx, rbac.CreateGithubAppAction, nil) if err != nil { return nil, err } @@ -81,7 +78,7 @@ func (a *Service) CreateApp(ctx context.Context, opts CreateAppOptions) (*App, e } func (a *Service) GetApp(ctx context.Context) (*App, error) { - subject, err := a.site.CanAccess(ctx, rbac.GetGithubAppAction, resource.ID{}) + subject, err := a.Authorize(ctx, rbac.GetGithubAppAction, nil) if err != nil { return nil, err } @@ -98,7 +95,7 @@ func (a *Service) GetApp(ctx context.Context) (*App, error) { } func (a *Service) DeleteApp(ctx context.Context) error { - subject, err := a.site.CanAccess(ctx, rbac.DeleteGithubAppAction, resource.ID{}) + subject, err := a.Authorize(ctx, rbac.DeleteGithubAppAction, nil) if err != nil { return err } diff --git a/internal/github/web.go b/internal/github/web.go index 319def137..1a28ba4ef 100644 --- a/internal/github/web.go +++ b/internal/github/web.go @@ -137,8 +137,8 @@ func (h *webHandlers) get(w http.ResponseWriter, r *http.Request) { App: app, Installations: installs, GithubHostname: h.GithubHostname, - CanCreateApp: user.CanAccessSite(rbac.CreateGithubAppAction), - CanDeleteApp: user.CanAccessSite(rbac.DeleteGithubAppAction), + CanCreateApp: user.CanAccess(rbac.CreateGithubAppAction, nil), + CanDeleteApp: user.CanAccess(rbac.DeleteGithubAppAction, nil), }) } diff --git a/internal/integration/agent_pools_ui_test.go b/internal/integration/agent_pools_ui_test.go index 4d67f52ff..2f3450234 100644 --- a/internal/integration/agent_pools_ui_test.go +++ b/internal/integration/agent_pools_ui_test.go @@ -93,7 +93,7 @@ func TestAgentPoolsUI(t *testing.T) { err = expect.Locator(page.Locator(fmt.Sprintf(`//div[@id='granted-workspaces']//a[text()='%s']`, ws1.Name))).ToBeVisible() require.NoError(t, err) - // go to workspaces + // go to workspace _, err = page.Goto(workspaceURL(daemon.System.Hostname(), org.Name, ws1.Name)) require.NoError(t, err) diff --git a/internal/integration/configuration_version_test.go b/internal/integration/configuration_version_test.go index d8fe7e510..5aa497882 100644 --- a/internal/integration/configuration_version_test.go +++ b/internal/integration/configuration_version_test.go @@ -109,7 +109,7 @@ func TestConfigurationVersion(t *testing.T) { name: "query non-existent workspace", workspaceID: resource.NewID(resource.WorkspaceKind), want: func(t *testing.T, got *resource.Page[*configversion.ConfigurationVersion], err error) { - assert.Equal(t, internal.ErrResourceNotFound, err) + require.ErrorIs(t, err, internal.ErrResourceNotFound) }, }, } diff --git a/internal/integration/state_service_test.go b/internal/integration/state_service_test.go index 320effca1..e7433dfa9 100644 --- a/internal/integration/state_service_test.go +++ b/internal/integration/state_service_test.go @@ -43,7 +43,7 @@ func TestIntegration_StateService(t *testing.T) { svc, _, ctx := setup(t, nil) _, err := svc.State.Get(ctx, resource.NewID(resource.StateVersionKind)) - require.Equal(t, internal.ErrResourceNotFound, err) + require.ErrorIs(t, err, internal.ErrResourceNotFound) }) // Get current creates two state versions and checks the second one is made @@ -64,7 +64,7 @@ func TestIntegration_StateService(t *testing.T) { svc, _, ctx := setup(t, nil) _, err := svc.State.GetCurrent(ctx, resource.NewID(resource.WorkspaceKind)) - assert.Equal(t, internal.ErrResourceNotFound, err) + require.ErrorIs(t, err, internal.ErrResourceNotFound) }) t.Run("list", func(t *testing.T) { @@ -85,7 +85,7 @@ func TestIntegration_StateService(t *testing.T) { svc, _, ctx := setup(t, nil) _, err := svc.State.List(ctx, resource.NewID(resource.WorkspaceKind), resource.PageOptions{}) - assert.Equal(t, internal.ErrResourceNotFound, err) + require.ErrorIs(t, err, internal.ErrResourceNotFound) }) t.Run("delete", func(t *testing.T) { @@ -98,7 +98,7 @@ func TestIntegration_StateService(t *testing.T) { require.NoError(t, err) _, err = svc.State.Get(ctx, want.ID) - assert.Equal(t, internal.ErrResourceNotFound, err) + require.ErrorIs(t, err, internal.ErrResourceNotFound) t.Run("deleting current version not allowed", func(t *testing.T) { err := svc.State.Delete(ctx, current.ID) diff --git a/internal/integration/workspace_permissions_service_test.go b/internal/integration/workspace_permissions_service_test.go index de7eba55a..83c7abfc6 100644 --- a/internal/integration/workspace_permissions_service_test.go +++ b/internal/integration/workspace_permissions_service_test.go @@ -33,7 +33,7 @@ func TestIntegration_WorkspacePermissionsService(t *testing.T) { err = svc.Workspaces.UnsetPermission(ctx, ws.ID, team.ID) require.NoError(t, err) - policy, err := svc.Workspaces.GetPolicy(ctx, ws.ID) + policy, err := svc.Workspaces.GetWorkspacePolicy(ctx, ws.ID) require.NoError(t, err) assert.Empty(t, policy.Permissions) }) @@ -50,11 +50,9 @@ func TestIntegration_WorkspacePermissionsService(t *testing.T) { err = svc.Workspaces.SetPermission(ctx, ws.ID, cherries.ID, rbac.WorkspacePlanRole) require.NoError(t, err) - got, err := svc.Workspaces.GetPolicy(ctx, ws.ID) + got, err := svc.Workspaces.GetWorkspacePolicy(ctx, ws.ID) require.NoError(t, err) - assert.Equal(t, org.Name, got.Organization) - assert.Equal(t, ws.ID, got.WorkspaceID) assert.Equal(t, 3, len(got.Permissions)) assert.Contains(t, got.Permissions, authz.WorkspacePermission{ TeamID: scum.ID, @@ -72,7 +70,7 @@ func TestIntegration_WorkspacePermissionsService(t *testing.T) { t.Run("workspace not found", func(t *testing.T) { nonExistentID := resource.NewID(resource.WorkspaceKind) - _, err := svc.Workspaces.GetPolicy(ctx, nonExistentID) + _, err := svc.Workspaces.GetWorkspacePolicy(ctx, nonExistentID) require.True(t, errors.Is(err, internal.ErrResourceNotFound)) }) } diff --git a/internal/logs/service.go b/internal/logs/service.go index b27d7464d..7cbdf7ffe 100644 --- a/internal/logs/service.go +++ b/internal/logs/service.go @@ -16,8 +16,7 @@ import ( type ( Service struct { logr.Logger - - run authz.Authorizer + authz.Interface api *api web *webHandlers @@ -39,15 +38,15 @@ type ( *sql.Listener internal.Verifier - RunAuthorizer authz.Authorizer + Authorizer *authz.Authorizer } ) func NewService(opts Options) *Service { db := &pgdb{opts.DB} svc := Service{ - Logger: opts.Logger, - run: opts.RunAuthorizer, + Logger: opts.Logger, + Interface: opts.Authorizer, } svc.api = &api{ Verifier: opts.Verifier, @@ -102,7 +101,7 @@ func (s *Service) GetChunk(ctx context.Context, opts GetChunkOptions) (Chunk, er // PutChunk writes a chunk of logs for a phase func (s *Service) PutChunk(ctx context.Context, opts PutChunkOptions) error { - _, err := s.run.CanAccess(ctx, rbac.PutChunkAction, opts.RunID) + _, err := s.Authorize(ctx, rbac.PutChunkAction, &authz.AccessRequest{ID: &opts.RunID}) if err != nil { return err } @@ -124,7 +123,7 @@ func (s *Service) PutChunk(ctx context.Context, opts PutChunkOptions) error { // Tail logs for a phase. Offset specifies the number of bytes into the logs // from which to start tailing. func (s *Service) Tail(ctx context.Context, opts GetChunkOptions) (<-chan Chunk, error) { - subject, err := s.run.CanAccess(ctx, rbac.TailLogsAction, opts.RunID) + subject, err := s.Authorize(ctx, rbac.TailLogsAction, &authz.AccessRequest{ID: &opts.RunID}) if err != nil { return nil, err } diff --git a/internal/logs/service_test.go b/internal/logs/service_test.go index 96af2819b..7562fc56d 100644 --- a/internal/logs/service_test.go +++ b/internal/logs/service_test.go @@ -21,7 +21,7 @@ func TestTail(t *testing.T) { chunkproxy: &fakeTailProxy{}, broker: &fakeSubService{stream: sub}, Logger: logr.Discard(), - run: &fakeAuthorizer{}, + Interface: &fakeAuthorizer{}, } stream, err := app.Tail(ctx, GetChunkOptions{ @@ -51,8 +51,8 @@ func TestTail(t *testing.T) { broker: &fakeSubService{ stream: make(chan pubsub.Event[Chunk]), }, - Logger: logr.Discard(), - run: &fakeAuthorizer{}, + Logger: logr.Discard(), + Interface: &fakeAuthorizer{}, } stream, err := svc.Tail(ctx, GetChunkOptions{ RunID: testutils.ParseID(t, "run-123"), @@ -74,7 +74,7 @@ func TestTail(t *testing.T) { chunkproxy: &fakeTailProxy{chunk: want}, broker: &fakeSubService{stream: sub}, Logger: logr.Discard(), - run: &fakeAuthorizer{}, + Interface: &fakeAuthorizer{}, } stream, err := svc.Tail(ctx, GetChunkOptions{ @@ -117,7 +117,7 @@ func TestTail(t *testing.T) { chunkproxy: &fakeTailProxy{chunk: want}, broker: &fakeSubService{stream: sub}, Logger: logr.Discard(), - run: &fakeAuthorizer{}, + Interface: &fakeAuthorizer{}, } stream, err := svc.Tail(ctx, GetChunkOptions{ @@ -156,7 +156,7 @@ func TestTail(t *testing.T) { chunkproxy: &fakeTailProxy{}, broker: &fakeSubService{stream: sub}, Logger: logr.Discard(), - run: &fakeAuthorizer{}, + Interface: &fakeAuthorizer{}, } stream, err := svc.Tail(ctx, GetChunkOptions{ diff --git a/internal/logs/test_helpers.go b/internal/logs/test_helpers.go index 41b514d2d..571c04bee 100644 --- a/internal/logs/test_helpers.go +++ b/internal/logs/test_helpers.go @@ -27,7 +27,9 @@ type ( chunkproxy } - fakeAuthorizer struct{} + fakeAuthorizer struct { + authz.Interface + } ) func newFakeCache(keyvalues ...string) *fakeCache { @@ -59,7 +61,7 @@ func (f *fakeTailProxy) get(ctx context.Context, opts GetChunkOptions) (Chunk, e return f.chunk, nil } -func (f *fakeAuthorizer) CanAccess(context.Context, rbac.Action, resource.ID) (authz.Subject, error) { +func (f *fakeAuthorizer) Authorize(context.Context, rbac.Action, *authz.AccessRequest, ...authz.CanAccessOption) (authz.Subject, error) { return &authz.Superuser{}, nil } diff --git a/internal/module/service.go b/internal/module/service.go index df3839f8f..84c853c66 100644 --- a/internal/module/service.go +++ b/internal/module/service.go @@ -8,9 +8,9 @@ import ( "github.com/go-logr/logr" "github.com/gorilla/mux" "github.com/leg100/otf/internal" + "github.com/leg100/otf/internal/authz" "github.com/leg100/otf/internal/connections" "github.com/leg100/otf/internal/http/html" - "github.com/leg100/otf/internal/organization" "github.com/leg100/otf/internal/rbac" "github.com/leg100/otf/internal/repohooks" "github.com/leg100/otf/internal/resource" @@ -26,11 +26,10 @@ type ( Service struct { logr.Logger *publisher + *authz.Authorizer db *pgdb - organization *organization.Authorizer - api *api web *webHandlers vcsproviders *vcsprovider.Service @@ -45,6 +44,7 @@ type ( *surl.Signer html.Renderer + Authorizer *authz.Authorizer RepohookService *repohooks.Service VCSProviderService *vcsprovider.Service ConnectionsService *connections.Service @@ -55,8 +55,8 @@ type ( func NewService(opts Options) *Service { svc := Service{ Logger: opts.Logger, + Authorizer: opts.Authorizer, connections: opts.ConnectionsService, - organization: &organization.Authorizer{Logger: opts.Logger}, db: &pgdb{opts.DB}, vcsproviders: opts.VCSProviderService, } @@ -66,6 +66,7 @@ func NewService(opts Options) *Service { } svc.web = &webHandlers{ Renderer: opts.Renderer, + authorizer: opts.Authorizer, client: &svc, vcsproviders: opts.VCSProviderService, system: opts.HostnameService, @@ -94,7 +95,7 @@ func (s *Service) PublishModule(ctx context.Context, opts PublishOptions) (*Modu return nil, err } - subject, err := s.organization.CanAccess(ctx, rbac.CreateModuleAction, vcsprov.Organization) + subject, err := s.Authorize(ctx, rbac.CreateModuleAction, &authz.AccessRequest{Organization: vcsprov.Organization}) if err != nil { return nil, err } @@ -213,7 +214,7 @@ func (s *Service) PublishVersion(ctx context.Context, opts PublishVersionOptions } func (s *Service) CreateModule(ctx context.Context, opts CreateOptions) (*Module, error) { - subject, err := s.organization.CanAccess(ctx, rbac.CreateModuleAction, opts.Organization) + subject, err := s.Authorize(ctx, rbac.CreateModuleAction, &authz.AccessRequest{Organization: opts.Organization}) if err != nil { return nil, err } @@ -229,7 +230,7 @@ func (s *Service) CreateModule(ctx context.Context, opts CreateOptions) (*Module } func (s *Service) ListModules(ctx context.Context, opts ListModulesOptions) ([]*Module, error) { - subject, err := s.organization.CanAccess(ctx, rbac.ListModulesAction, opts.Organization) + subject, err := s.Authorize(ctx, rbac.ListModulesAction, &authz.AccessRequest{Organization: opts.Organization}) if err != nil { return nil, err } @@ -244,7 +245,7 @@ func (s *Service) ListModules(ctx context.Context, opts ListModulesOptions) ([]* } func (s *Service) GetModule(ctx context.Context, opts GetModuleOptions) (*Module, error) { - subject, err := s.organization.CanAccess(ctx, rbac.GetModuleAction, opts.Organization) + subject, err := s.Authorize(ctx, rbac.GetModuleAction, &authz.AccessRequest{Organization: opts.Organization}) if err != nil { return nil, err } @@ -266,7 +267,7 @@ func (s *Service) GetModuleByID(ctx context.Context, id resource.ID) (*Module, e return nil, err } - subject, err := s.organization.CanAccess(ctx, rbac.GetModuleAction, module.Organization) + subject, err := s.Authorize(ctx, rbac.GetModuleAction, &authz.AccessRequest{Organization: module.Organization}) if err != nil { return nil, err } @@ -286,7 +287,7 @@ func (s *Service) DeleteModule(ctx context.Context, id resource.ID) (*Module, er return nil, err } - subject, err := s.organization.CanAccess(ctx, rbac.DeleteModuleAction, module.Organization) + subject, err := s.Authorize(ctx, rbac.DeleteModuleAction, &authz.AccessRequest{Organization: module.Organization}) if err != nil { return nil, err } @@ -317,7 +318,7 @@ func (s *Service) CreateVersion(ctx context.Context, opts CreateModuleVersionOpt return nil, err } - subject, err := s.organization.CanAccess(ctx, rbac.CreateModuleVersionAction, module.Organization) + subject, err := s.Authorize(ctx, rbac.CreateModuleVersionAction, &authz.AccessRequest{Organization: module.Organization}) if err != nil { return nil, err } @@ -415,7 +416,7 @@ func (s *Service) deleteVersion(ctx context.Context, versionID resource.ID) (*Mo return nil, err } - subject, err := s.organization.CanAccess(ctx, rbac.DeleteModuleVersionAction, module.Organization) + subject, err := s.Authorize(ctx, rbac.DeleteModuleVersionAction, &authz.AccessRequest{Organization: module.Organization}) if err != nil { return nil, err } diff --git a/internal/module/web.go b/internal/module/web.go index 00495b8ed..066987b02 100644 --- a/internal/module/web.go +++ b/internal/module/web.go @@ -34,6 +34,7 @@ type ( client webModulesClient vcsproviders vcsprovidersClient system webHostnameClient + authorizer webAuthorizer } webHostnameClient interface { @@ -49,6 +50,10 @@ type ( DeleteModule(ctx context.Context, id resource.ID) (*Module, error) } + webAuthorizer interface { + CanAccess(context.Context, rbac.Action, *authz.AccessRequest) bool + } + // vcsprovidersClient provides web handlers with access to vcs providers vcsprovidersClient interface { Get(context.Context, resource.ID) (*vcsprovider.VCSProvider, error) @@ -75,18 +80,11 @@ func (h *webHandlers) list(w http.ResponseWriter, r *http.Request) { h.Error(w, err.Error(), http.StatusUnprocessableEntity) return } - modules, err := h.client.ListModules(r.Context(), opts) if err != nil { h.Error(w, err.Error(), http.StatusInternalServerError) return } - subject, err := authz.SubjectFromContext(r.Context()) - if err != nil { - h.Error(w, err.Error(), http.StatusInternalServerError) - return - } - h.Render("module_list.tmpl", w, struct { organization.OrganizationPage Items []*Module @@ -94,7 +92,7 @@ func (h *webHandlers) list(w http.ResponseWriter, r *http.Request) { }{ OrganizationPage: organization.NewPage(r, "modules", opts.Organization), Items: modules, - CanPublishModule: subject.CanAccessOrganization(rbac.CreateModuleAction, opts.Organization), + CanPublishModule: h.authorizer.CanAccess(r.Context(), rbac.CreateModuleAction, &authz.AccessRequest{Organization: opts.Organization}), }) } diff --git a/internal/module/web_test.go b/internal/module/web_test.go index b6cf45fd1..58a50499b 100644 --- a/internal/module/web_test.go +++ b/internal/module/web_test.go @@ -163,6 +163,7 @@ func newTestWebHandlers(t *testing.T, opts ...testWebOption) *webHandlers { } return &webHandlers{ Renderer: testutils.NewRenderer(t), + authorizer: authz.NewAllowAllAuthorizer(), system: &svc, client: &svc, vcsproviders: &svc, diff --git a/internal/notifications/service.go b/internal/notifications/service.go index 73a2f7fd5..152a7865c 100644 --- a/internal/notifications/service.go +++ b/internal/notifications/service.go @@ -16,11 +16,11 @@ import ( type ( Service struct { logr.Logger + *authz.Authorizer - workspaceAuthorizer authz.Authorizer // authorize workspaces actions - db *pgdb - api *tfe - broker *pubsub.Broker[*Config] + db *pgdb + api *tfe + broker *pubsub.Broker[*Config] } Options struct { @@ -29,15 +29,15 @@ type ( *tfeapi.Responder logr.Logger - WorkspaceAuthorizer authz.Authorizer + Authorizer *authz.Authorizer } ) func NewService(opts Options) *Service { svc := Service{ - Logger: opts.Logger, - workspaceAuthorizer: opts.WorkspaceAuthorizer, - db: &pgdb{opts.DB}, + Logger: opts.Logger, + Authorizer: opts.Authorizer, + db: &pgdb{opts.DB}, } svc.api = &tfe{ Service: &svc, @@ -68,7 +68,7 @@ func (s *Service) Watch(ctx context.Context) (<-chan pubsub.Event[*Config], func } func (s *Service) Create(ctx context.Context, workspaceID resource.ID, opts CreateConfigOptions) (*Config, error) { - subject, err := s.workspaceAuthorizer.CanAccess(ctx, rbac.CreateNotificationConfigurationAction, workspaceID) + subject, err := s.Authorize(ctx, rbac.CreateNotificationConfigurationAction, &authz.AccessRequest{ID: &workspaceID}) if err != nil { return nil, err } @@ -88,7 +88,7 @@ func (s *Service) Create(ctx context.Context, workspaceID resource.ID, opts Crea func (s *Service) Update(ctx context.Context, id resource.ID, opts UpdateConfigOptions) (*Config, error) { var subject authz.Subject updated, err := s.db.update(ctx, id, func(nc *Config) (err error) { - subject, err = s.workspaceAuthorizer.CanAccess(ctx, rbac.UpdateNotificationConfigurationAction, nc.WorkspaceID) + subject, err = s.Authorize(ctx, rbac.UpdateNotificationConfigurationAction, &authz.AccessRequest{ID: &nc.WorkspaceID}) if err != nil { return err } @@ -108,7 +108,7 @@ func (s *Service) Get(ctx context.Context, id resource.ID) (*Config, error) { s.Error(err, "retrieving notification config", "id", id) return nil, err } - subject, err := s.workspaceAuthorizer.CanAccess(ctx, rbac.GetNotificationConfigurationAction, nc.WorkspaceID) + subject, err := s.Authorize(ctx, rbac.GetNotificationConfigurationAction, &authz.AccessRequest{ID: &nc.WorkspaceID}) if err != nil { return nil, err } @@ -117,7 +117,7 @@ func (s *Service) Get(ctx context.Context, id resource.ID) (*Config, error) { } func (s *Service) List(ctx context.Context, workspaceID resource.ID) ([]*Config, error) { - subject, err := s.workspaceAuthorizer.CanAccess(ctx, rbac.ListNotificationConfigurationsAction, workspaceID) + subject, err := s.Authorize(ctx, rbac.ListNotificationConfigurationsAction, &authz.AccessRequest{ID: &workspaceID}) if err != nil { return nil, err } @@ -136,7 +136,7 @@ func (s *Service) Delete(ctx context.Context, id resource.ID) error { s.Error(err, "retrieving notification config", "id", id) return err } - subject, err := s.workspaceAuthorizer.CanAccess(ctx, rbac.DeleteNotificationConfigurationAction, nc.WorkspaceID) + subject, err := s.Authorize(ctx, rbac.DeleteNotificationConfigurationAction, &authz.AccessRequest{ID: &nc.WorkspaceID}) if err != nil { return err } diff --git a/internal/organization/authorizer.go b/internal/organization/authorizer.go deleted file mode 100644 index 6627a6b33..000000000 --- a/internal/organization/authorizer.go +++ /dev/null @@ -1,30 +0,0 @@ -package organization - -import ( - "context" - - "github.com/go-logr/logr" - "github.com/leg100/otf/internal" - "github.com/leg100/otf/internal/authz" - "github.com/leg100/otf/internal/rbac" -) - -// Authorizer authorizes access to an organization -type Authorizer struct { - logr.Logger -} - -func (a *Authorizer) CanAccess(ctx context.Context, action rbac.Action, organizationName string) (authz.Subject, error) { - subj, err := authz.SubjectFromContext(ctx) - if err != nil { - return nil, err - } - if authz.SkipAuthz(ctx) { - return subj, nil - } - if subj.CanAccessOrganization(action, organizationName) { - return subj, nil - } - a.Error(nil, "unauthorized action", "organization", organizationName, "action", action.String(), "subject", subj) - return nil, internal.ErrAccessNotPermitted -} diff --git a/internal/organization/service.go b/internal/organization/service.go index 39233fbcd..0915739f1 100644 --- a/internal/organization/service.go +++ b/internal/organization/service.go @@ -2,6 +2,7 @@ package organization import ( "context" + "errors" "fmt" "github.com/go-logr/logr" @@ -22,11 +23,10 @@ type ( Service struct { RestrictOrganizationCreation bool - *Authorizer // authorize access to org + *authz.Authorizer logr.Logger db *pgdb - site authz.Authorizer // authorize access to site web *web tfeapi *tfe api *api @@ -40,6 +40,7 @@ type ( Options struct { RestrictOrganizationCreation bool TokensService *tokens.Service + Authorizer *authz.Authorizer *sql.DB *tfeapi.Responder @@ -56,11 +57,10 @@ type ( func NewService(opts Options) *Service { svc := Service{ - Authorizer: &Authorizer{opts.Logger}, + Authorizer: opts.Authorizer, Logger: opts.Logger, RestrictOrganizationCreation: opts.RestrictOrganizationCreation, db: &pgdb{opts.DB}, - site: &authz.SiteAuthorizer{Logger: opts.Logger}, tokenFactory: &tokenFactory{tokens: opts.TokensService}, } svc.web = &web{ @@ -115,16 +115,17 @@ func (s *Service) WatchOrganizations(ctx context.Context) (<-chan pubsub.Event[* // site admin can create organizations. Creating an organization automatically // creates an owners team and adds creator as an owner. func (s *Service) Create(ctx context.Context, opts CreateOptions) (*Organization, error) { - creator, err := s.restrictOrganizationCreation(ctx) + subject, err := s.Authorize(ctx, rbac.CreateOrganizationAction, &authz.AccessRequest{Organization: *opts.Name}) if err != nil { return nil, err } - + if err := s.restrictOrganizationCreation(subject); err != nil { + return nil, err + } org, err := NewOrganization(opts) if err != nil { return nil, fmt.Errorf("creating organization: %w", err) } - err = s.db.Tx(ctx, func(ctx context.Context, q *sqlc.Queries) error { if err := s.db.create(ctx, org); err != nil { return err @@ -137,24 +138,35 @@ func (s *Service) Create(ctx context.Context, opts CreateOptions) (*Organization return nil }) if err != nil { - s.Error(err, "creating organization", "id", org.ID, "subject", creator) + s.Error(err, "creating organization", "id", org.ID, "subject", subject) return nil, sql.Error(err) } - s.V(0).Info("created organization", "id", org.ID, "name", org.Name, "subject", creator) - + s.V(0).Info("created organization", "id", org.ID, "name", org.Name, "subject", subject) return org, nil } +func (s *Service) restrictOrganizationCreation(subject authz.Subject) error { + if s.RestrictOrganizationCreation { + type user interface { + IsSiteAdmin() bool + } + if user, ok := subject.(user); !ok || !user.IsSiteAdmin() { + s.Error(internal.ErrAccessNotPermitted, "cannot create organization because creation is restricted to site admins", "action", rbac.CreateOrganizationAction, "subject", subject) + return internal.ErrAccessNotPermitted + } + } + return nil +} + func (s *Service) AfterCreateOrganization(hook func(context.Context, *Organization) error) { s.afterCreateHooks = append(s.afterCreateHooks, hook) } func (s *Service) Update(ctx context.Context, name string, opts UpdateOptions) (*Organization, error) { - subject, err := s.CanAccess(ctx, rbac.UpdateOrganizationAction, name) + subject, err := s.Authorize(ctx, rbac.UpdateOrganizationAction, &authz.AccessRequest{Organization: name}) if err != nil { return nil, err } - org, err := s.db.update(ctx, name, func(org *Organization) error { return org.Update(opts) }) @@ -164,30 +176,40 @@ func (s *Service) Update(ctx context.Context, name string, opts UpdateOptions) ( } s.V(2).Info("updated organization", "name", name, "id", org.ID, "subject", subject) - return org, nil } -// List lists organizations according to the subject. If the -// subject has site-wide permission to list organizations then all organizations -// are listed. Otherwise: -// Subject is a user: list their organization memberships -// Subject is an agent: return its organization -// Subject is an organization token: return its organization -// Subject is a team: return its organization +// List organizations. If the subject lacks the ListOrganizationsAction +// permission then its organization memberships are listed instead. func (s *Service) List(ctx context.Context, opts ListOptions) (*resource.Page[*Organization], error) { - subject, err := authz.SubjectFromContext(ctx) + orgs, subject, err := func() (*resource.Page[*Organization], authz.Subject, error) { + var names []string + subject, err := s.Authorize(ctx, rbac.ListOrganizationsAction, nil, authz.WithoutErrorLogging()) + if errors.Is(err, internal.ErrAccessNotPermitted) { + // List subject's organization memberships instead. + type memberships interface { + Organizations() []string + } + user, ok := subject.(memberships) + if !ok { + return nil, subject, err + } + names = user.Organizations() + } else if err != nil { + return nil, subject, err + } + orgs, err := s.db.list(ctx, dbListOptions{PageOptions: opts.PageOptions, names: names}) + return orgs, subject, err + }() if err != nil { - return nil, err - } - if subject.CanAccessSite(rbac.ListOrganizationsAction) { - return s.db.list(ctx, dbListOptions{PageOptions: opts.PageOptions}) + s.Error(err, "listing organizations", "subject", subject) } - return s.db.list(ctx, dbListOptions{PageOptions: opts.PageOptions, names: subject.Organizations()}) + s.V(9).Info("listed organizations", "subject", subject) + return orgs, err } func (s *Service) Get(ctx context.Context, name string) (*Organization, error) { - subject, err := s.CanAccess(ctx, rbac.GetOrganizationAction, name) + subject, err := s.Authorize(ctx, rbac.GetOrganizationAction, &authz.AccessRequest{Organization: name}) if err != nil { return nil, err } @@ -204,7 +226,7 @@ func (s *Service) Get(ctx context.Context, name string) (*Organization, error) { } func (s *Service) Delete(ctx context.Context, name string) error { - subject, err := s.CanAccess(ctx, rbac.DeleteOrganizationAction, name) + subject, err := s.Authorize(ctx, rbac.DeleteOrganizationAction, &authz.AccessRequest{Organization: name}) if err != nil { return err } @@ -235,7 +257,7 @@ func (s *Service) BeforeDeleteOrganization(hook func(context.Context, *Organizat } func (s *Service) GetEntitlements(ctx context.Context, organization string) (Entitlements, error) { - _, err := s.CanAccess(ctx, rbac.GetEntitlementsAction, organization) + _, err := s.Authorize(ctx, rbac.GetEntitlementsAction, &authz.AccessRequest{Organization: organization}) if err != nil { return Entitlements{}, err } @@ -247,22 +269,10 @@ func (s *Service) GetEntitlements(ctx context.Context, organization string) (Ent return defaultEntitlements(org.ID), nil } -func (s *Service) restrictOrganizationCreation(ctx context.Context) (authz.Subject, error) { - subject, err := authz.SubjectFromContext(ctx) - if err != nil { - return nil, err - } - if s.RestrictOrganizationCreation && !subject.IsSiteAdmin() { - s.Error(nil, "unauthorized action", "action", rbac.CreateOrganizationAction, "subject", subject) - return subject, internal.ErrAccessNotPermitted - } - return subject, nil -} - // CreateToken creates an organization token. If an organization // token already exists it is replaced. func (s *Service) CreateToken(ctx context.Context, opts CreateOrganizationTokenOptions) (*OrganizationToken, []byte, error) { - _, err := s.CanAccess(ctx, rbac.CreateOrganizationTokenAction, opts.Organization) + _, err := s.Authorize(ctx, rbac.CreateOrganizationTokenAction, &authz.AccessRequest{Organization: opts.Organization}) if err != nil { return nil, nil, err } @@ -314,7 +324,7 @@ func (s *Service) ListTokens(ctx context.Context, organization string) ([]*Organ } func (s *Service) DeleteToken(ctx context.Context, organization string) error { - _, err := s.CanAccess(ctx, rbac.CreateOrganizationTokenAction, organization) + _, err := s.Authorize(ctx, rbac.CreateOrganizationTokenAction, &authz.AccessRequest{Organization: organization}) if err != nil { return err } diff --git a/internal/organization/service_test.go b/internal/organization/service_test.go index 95c936b92..a27aba42b 100644 --- a/internal/organization/service_test.go +++ b/internal/organization/service_test.go @@ -1,7 +1,6 @@ package organization import ( - "context" "testing" "github.com/go-logr/logr" @@ -17,18 +16,32 @@ func TestAuthorize(t *testing.T) { restrict bool want error }{ - {"site admin", &authz.Superuser{}, false, nil}, - {"restrict to site admin - site admin", &authz.Superuser{}, true, nil}, - {"restrict to site admin - user", &unprivUser{}, true, internal.ErrAccessNotPermitted}, + { + "anyone can create an organization", + &unprivUser{}, + false, + nil, + }, + { + "site admin can create an organization when creation is restricted", + &siteAdmin{}, + true, + nil, + }, + { + "normal users cannot create an organization when creation is restricted", + &unprivUser{}, + true, + internal.ErrAccessNotPermitted, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ctx := authz.AddSubjectToContext(context.Background(), tt.subject) svc := &Service{ Logger: logr.Discard(), RestrictOrganizationCreation: tt.restrict, } - _, err := svc.restrictOrganizationCreation(ctx) + err := svc.restrictOrganizationCreation(tt.subject) assert.Equal(t, tt.want, err) }) } @@ -39,3 +52,9 @@ type unprivUser struct { } func (s *unprivUser) IsSiteAdmin() bool { return false } + +type siteAdmin struct { + authz.Subject +} + +func (s *siteAdmin) IsSiteAdmin() bool { return true } diff --git a/internal/organization/token.go b/internal/organization/token.go index 7c43779aa..bf1941a68 100644 --- a/internal/organization/token.go +++ b/internal/organization/token.go @@ -53,18 +53,13 @@ func (f *tokenFactory) NewOrganizationToken(opts CreateOrganizationTokenOptions) return &ot, token, nil } -func (u *OrganizationToken) CanAccessSite(action rbac.Action) bool { - // only be used for organization-scoped resources. - return false -} - -func (u *OrganizationToken) CanAccessTeam(rbac.Action, resource.ID) bool { - // only be used for organization-scoped resources. - return false -} - -func (u *OrganizationToken) CanAccessOrganization(action rbac.Action, org string) bool { - if u.Organization != org { +func (u *OrganizationToken) CanAccess(action rbac.Action, req *authz.AccessRequest) bool { + if req == nil { + // Organization token cannot take action on site-level resources + return false + } + if req.Organization != u.Organization { + // Organization token cannot take action on other organizations return false } // can perform most actions in an organization, so it is easier to first refuse @@ -76,18 +71,6 @@ func (u *OrganizationToken) CanAccessOrganization(action rbac.Action, org string return true } -func (u *OrganizationToken) CanAccessWorkspace(action rbac.Action, policy authz.WorkspacePolicy) bool { - return u.CanAccessOrganization(action, policy.Organization) -} - -func (u *OrganizationToken) IsOwner(organization string) bool { - // an owner would give perms to all actions in org whereas an org token - // cannot perform certain actions, so org token is not an owner. - return false -} - -func (u *OrganizationToken) IsSiteAdmin() bool { return false } - func (u *OrganizationToken) Organizations() []string { return []string{u.Organization} } diff --git a/internal/organization/web.go b/internal/organization/web.go index 507c347a6..5f5647034 100644 --- a/internal/organization/web.go +++ b/internal/organization/web.go @@ -125,10 +125,7 @@ func (a *web) list(w http.ResponseWriter, r *http.Request) { a.Error(w, err.Error(), http.StatusInternalServerError) return } - var canCreate bool - if !a.RestrictCreation || subject.CanAccessSite(rbac.CreateOrganizationAction) { - canCreate = true - } + canCreate := !a.RestrictCreation || subject.CanAccess(rbac.CreateOrganizationAction, nil) a.Render("organization_list.tmpl", w, struct { html.SitePage diff --git a/internal/organization/web_test.go b/internal/organization/web_test.go index 08cc066ce..c78baa633 100644 --- a/internal/organization/web_test.go +++ b/internal/organization/web_test.go @@ -179,6 +179,6 @@ func (f *fakeWebService) Delete(context.Context, string) error { return nil } -func (s *unprivilegedSubject) CanAccessSite(_ rbac.Action) bool { +func (s *unprivilegedSubject) CanAccess(rbac.Action, *authz.AccessRequest) bool { return false } diff --git a/internal/resource/kind.go b/internal/resource/kind.go index 4eb1922c9..82f524df7 100644 --- a/internal/resource/kind.go +++ b/internal/resource/kind.go @@ -10,6 +10,8 @@ const ( OrganizationKind Kind = "org" WorkspaceKind Kind = "ws" RunKind Kind = "run" + ConfigVersionKind Kind = "cv" + IngressAttributesKind Kind = "ia" JobKind Kind = "job" ChunkKind Kind = "chunk" UserKind Kind = "user" diff --git a/internal/run/authorizer.go b/internal/run/authorizer.go deleted file mode 100644 index b26b14d10..000000000 --- a/internal/run/authorizer.go +++ /dev/null @@ -1,23 +0,0 @@ -package run - -import ( - "context" - - "github.com/leg100/otf/internal/authz" - "github.com/leg100/otf/internal/rbac" - "github.com/leg100/otf/internal/resource" -) - -// authorizer authorizes access to a run -type authorizer struct { - db *pgdb - workspace authz.Authorizer -} - -func (a *authorizer) CanAccess(ctx context.Context, action rbac.Action, runID resource.ID) (authz.Subject, error) { - run, err := a.db.GetRun(ctx, runID) - if err != nil { - return nil, err - } - return a.workspace.CanAccess(ctx, action, run.WorkspaceID) -} diff --git a/internal/run/lock_file_service.go b/internal/run/lock_file_service.go index 99e97b2f5..072eac634 100644 --- a/internal/run/lock_file_service.go +++ b/internal/run/lock_file_service.go @@ -4,6 +4,7 @@ import ( "context" "fmt" + "github.com/leg100/otf/internal/authz" "github.com/leg100/otf/internal/rbac" "github.com/leg100/otf/internal/resource" ) @@ -14,7 +15,7 @@ func lockFileCacheKey(runID resource.ID) string { // GetLockFile returns the lock file for the run. func (s *Service) GetLockFile(ctx context.Context, runID resource.ID) ([]byte, error) { - subject, err := s.CanAccess(ctx, rbac.GetLockFileAction, runID) + subject, err := s.Authorize(ctx, rbac.GetLockFileAction, &authz.AccessRequest{ID: &runID}) if err != nil { return nil, err } @@ -38,7 +39,7 @@ func (s *Service) GetLockFile(ctx context.Context, runID resource.ID) ([]byte, e // UploadLockFile persists the lock file for a run. func (s *Service) UploadLockFile(ctx context.Context, runID resource.ID, file []byte) error { - subject, err := s.CanAccess(ctx, rbac.UploadLockFileAction, runID) + subject, err := s.Authorize(ctx, rbac.UploadLockFileAction, &authz.AccessRequest{ID: &runID}) if err != nil { return err } diff --git a/internal/run/run.go b/internal/run/run.go index 3395ea872..d609778fa 100644 --- a/internal/run/run.go +++ b/internal/run/run.go @@ -9,10 +9,8 @@ import ( "time" "github.com/leg100/otf/internal" - "github.com/leg100/otf/internal/authz" "github.com/leg100/otf/internal/configversion" "github.com/leg100/otf/internal/organization" - "github.com/leg100/otf/internal/rbac" "github.com/leg100/otf/internal/resource" "github.com/leg100/otf/internal/user" "github.com/leg100/otf/internal/workspace" @@ -448,21 +446,6 @@ func (r *Run) EnqueuePlan() error { return nil } -func (*Run) CanAccessSite(action rbac.Action) bool { - // run cannot carry out site-level actions - return false -} - -func (r *Run) CanAccessOrganization(action rbac.Action, name string) bool { - // run cannot access organization-level resources - return false -} - -func (r *Run) CanAccessWorkspace(action rbac.Action, policy *authz.WorkspacePolicy) bool { - // run can access anything within its workspace - return r.WorkspaceID == policy.WorkspaceID -} - func (r *Run) EnqueueApply() error { switch r.Status { case RunPlanned, RunCostEstimated: diff --git a/internal/run/service.go b/internal/run/service.go index ec519830d..09d190f86 100644 --- a/internal/run/service.go +++ b/internal/run/service.go @@ -35,14 +35,9 @@ type ( Service struct { logr.Logger + authz.Interface - site authz.Authorizer - organization *organization.Authorizer - workspaceAuthorizer authz.Authorizer - *authorizer - - workspaces *workspace.Service - + workspaces *workspace.Service cache internal.Cache db *pgdb tfeapi *tfe @@ -58,8 +53,8 @@ type ( } Options struct { - WorkspaceAuthorizer authz.Authorizer - VCSEventSubscriber vcs.Subscriber + Authorizer *authz.Authorizer + VCSEventSubscriber vcs.Subscriber WorkspaceService *workspace.Service OrganizationService *organization.Service @@ -81,14 +76,11 @@ type ( func NewService(opts Options) *Service { db := &pgdb{opts.DB} svc := Service{ - Logger: opts.Logger, - workspaces: opts.WorkspaceService, - db: db, - cache: opts.Cache, - site: &authz.SiteAuthorizer{Logger: opts.Logger}, - organization: &organization.Authorizer{Logger: opts.Logger}, - workspaceAuthorizer: opts.WorkspaceAuthorizer, - authorizer: &authorizer{db, opts.WorkspaceAuthorizer}, + Logger: opts.Logger, + workspaces: opts.WorkspaceService, + db: db, + cache: opts.Cache, + Interface: opts.Authorizer, } svc.factory = &factory{ organizations: opts.OrganizationService, @@ -99,6 +91,7 @@ func NewService(opts Options) *Service { } svc.web = &webHandlers{ Renderer: opts.Renderer, + authorizer: opts.Authorizer, logger: opts.Logger, runs: &svc, workspaces: opts.WorkspaceService, @@ -108,6 +101,7 @@ func NewService(opts Options) *Service { workspaces: opts.WorkspaceService, Responder: opts.Responder, Signer: opts.Signer, + authorizer: opts.Authorizer, } svc.api = &api{ Service: &svc, @@ -138,6 +132,17 @@ func NewService(opts Options) *Service { opts.Responder.Register(tfeapi.IncludeCreatedBy, svc.tfeapi.includeCreatedBy) opts.Responder.Register(tfeapi.IncludeCurrentRun, svc.tfeapi.includeCurrentRun) + // Resolve authorization requests for run IDs to a workspace IDs + opts.Authorizer.RegisterWorkspaceResolver(resource.RunKind, + func(ctx context.Context, runID resource.ID) (resource.ID, error) { + run, err := db.GetRun(ctx, runID) + if err != nil { + return resource.ID{}, err + } + return run.WorkspaceID, nil + }, + ) + // Subscribe run spawner to incoming vcs events opts.VCSEventSubscriber.Subscribe(spawner.handle) @@ -155,7 +160,7 @@ func (s *Service) AddHandlers(r *mux.Router) { } func (s *Service) Create(ctx context.Context, workspaceID resource.ID, opts CreateOptions) (*Run, error) { - subject, err := s.workspaceAuthorizer.CanAccess(ctx, rbac.CreateRunAction, workspaceID) + subject, err := s.Authorize(ctx, rbac.CreateRunAction, &authz.AccessRequest{ID: &workspaceID}) if err != nil { return nil, err } @@ -177,7 +182,7 @@ func (s *Service) Create(ctx context.Context, workspaceID resource.ID, opts Crea // Get retrieves a run from the db. func (s *Service) Get(ctx context.Context, runID resource.ID) (*Run, error) { - subject, err := s.CanAccess(ctx, rbac.GetRunAction, runID) + subject, err := s.CanAccessRun(ctx, rbac.GetRunAction, runID) if err != nil { return nil, err } @@ -205,16 +210,16 @@ func (s *Service) List(ctx context.Context, opts ListOptions) (*resource.Page[*R return nil, err } // subject needs perms on workspace to list runs in workspace - subject, authErr = s.workspaceAuthorizer.CanAccess(ctx, rbac.GetWorkspaceAction, workspace.ID) + subject, authErr = s.Authorize(ctx, rbac.GetWorkspaceAction, &authz.AccessRequest{ID: &workspace.ID}) } else if opts.WorkspaceID != nil { // subject needs perms on workspace to list runs in workspace - subject, authErr = s.workspaceAuthorizer.CanAccess(ctx, rbac.GetWorkspaceAction, *opts.WorkspaceID) + subject, authErr = s.Authorize(ctx, rbac.GetWorkspaceAction, &authz.AccessRequest{ID: opts.WorkspaceID}) } else if opts.Organization != nil { // subject needs perms on org to list runs in org - subject, authErr = s.organization.CanAccess(ctx, rbac.ListRunsAction, *opts.Organization) + subject, authErr = s.Authorize(ctx, rbac.ListRunsAction, &authz.AccessRequest{Organization: *opts.Organization}) } else { // subject needs to be site admin to list runs across site - subject, authErr = s.site.CanAccess(ctx, rbac.ListRunsAction, resource.ID{}) + subject, authErr = s.Authorize(ctx, rbac.ListRunsAction, nil) } if authErr != nil { return nil, authErr @@ -237,7 +242,7 @@ func (s *Service) List(ctx context.Context, opts ListOptions) (*resource.Page[*R func (s *Service) EnqueuePlan(ctx context.Context, runID resource.ID) (run *Run, err error) { err = s.db.Tx(ctx, func(ctx context.Context, q *sqlc.Queries) error { // TODO: this does not need to be part of the tx - subject, err := s.CanAccess(ctx, rbac.EnqueuePlanAction, runID) + subject, err := s.CanAccessRun(ctx, rbac.EnqueuePlanAction, runID) if err != nil { return err } @@ -266,12 +271,7 @@ func (s *Service) AfterEnqueuePlan(hook func(context.Context, *Run) error) { } func (s *Service) Delete(ctx context.Context, runID resource.ID) error { - run, err := s.db.GetRun(ctx, runID) - if err != nil { - return err - } - - subject, err := s.workspaceAuthorizer.CanAccess(ctx, rbac.DeleteRunAction, run.WorkspaceID) + subject, err := s.CanAccessRun(ctx, rbac.DeleteRunAction, runID) if err != nil { return err } @@ -349,13 +349,13 @@ func (s *Service) watchWithOptions(ctx context.Context, opts WatchOptions) (<-ch var err error if opts.WorkspaceID != nil { // caller must have workspace-level read permissions - _, err = s.workspaceAuthorizer.CanAccess(ctx, rbac.WatchAction, *opts.WorkspaceID) + _, err = s.Authorize(ctx, rbac.WatchAction, &authz.AccessRequest{ID: opts.WorkspaceID}) } else if opts.Organization != nil { // caller must have organization-level read permissions - _, err = s.organization.CanAccess(ctx, rbac.WatchAction, *opts.Organization) + _, err = s.Authorize(ctx, rbac.WatchAction, &authz.AccessRequest{Organization: *opts.Organization}) } else { // caller must have site-level read permissions - _, err = s.site.CanAccess(ctx, rbac.WatchAction, resource.ID{}) + _, err = s.Authorize(ctx, rbac.WatchAction, nil) } if err != nil { return nil, err @@ -390,7 +390,7 @@ func (s *Service) watchWithOptions(ctx context.Context, opts WatchOptions) (<-ch func (s *Service) Apply(ctx context.Context, runID resource.ID) error { return s.db.Tx(ctx, func(ctx context.Context, q *sqlc.Queries) error { // TODO: this does not need to be part of the tx - subject, err := s.CanAccess(ctx, rbac.ApplyRunAction, runID) + subject, err := s.CanAccessRun(ctx, rbac.ApplyRunAction, runID) if err != nil { return err } @@ -420,7 +420,7 @@ func (s *Service) AfterEnqueueApply(hook func(context.Context, *Run) error) { // Discard discards the run. func (s *Service) Discard(ctx context.Context, runID resource.ID) error { - subject, err := s.CanAccess(ctx, rbac.DiscardRunAction, runID) + subject, err := s.CanAccessRun(ctx, rbac.DiscardRunAction, runID) if err != nil { return err } @@ -440,7 +440,7 @@ func (s *Service) Discard(ctx context.Context, runID resource.ID) error { func (s *Service) Cancel(ctx context.Context, runID resource.ID) error { return s.db.Tx(ctx, func(ctx context.Context, q *sqlc.Queries) error { - subject, err := s.CanAccess(ctx, rbac.CancelRunAction, runID) + subject, err := s.CanAccessRun(ctx, rbac.CancelRunAction, runID) if err != nil { return err } @@ -476,7 +476,7 @@ func (s *Service) AfterCancelRun(hook func(context.Context, *Run) error) { // ForceCancel forcefully cancels a run. func (s *Service) ForceCancel(ctx context.Context, runID resource.ID) error { return s.db.Tx(ctx, func(ctx context.Context, q *sqlc.Queries) error { - subject, err := s.CanAccess(ctx, rbac.ForceCancelRunAction, runID) + subject, err := s.CanAccessRun(ctx, rbac.ForceCancelRunAction, runID) if err != nil { return err } @@ -509,7 +509,7 @@ func planFileCacheKey(f PlanFormat, id resource.ID) string { // GetPlanFile returns the plan file for the run. func (s *Service) GetPlanFile(ctx context.Context, runID resource.ID, format PlanFormat) ([]byte, error) { - subject, err := s.CanAccess(ctx, rbac.GetPlanFileAction, runID) + subject, err := s.CanAccessRun(ctx, rbac.GetPlanFileAction, runID) if err != nil { return nil, err } @@ -533,7 +533,7 @@ func (s *Service) GetPlanFile(ctx context.Context, runID resource.ID, format Pla // UploadPlanFile persists a run's plan file. The plan format should be either // be binary or json. func (s *Service) UploadPlanFile(ctx context.Context, runID resource.ID, plan []byte, format PlanFormat) error { - subject, err := s.CanAccess(ctx, rbac.UploadPlanFileAction, runID) + subject, err := s.CanAccessRun(ctx, rbac.UploadPlanFileAction, runID) if err != nil { return err } @@ -622,3 +622,11 @@ func (s *Service) autoQueueRun(ctx context.Context, ws *workspace.Workspace) err } return nil } + +func (s *Service) CanAccessRun(ctx context.Context, action rbac.Action, runID resource.ID) (authz.Subject, error) { + run, err := s.db.GetRun(ctx, runID) + if err != nil { + return nil, err + } + return s.Authorize(ctx, action, &authz.AccessRequest{ID: &run.WorkspaceID}) +} diff --git a/internal/run/service_test.go b/internal/run/service_test.go index fa0752216..e7bde07de 100644 --- a/internal/run/service_test.go +++ b/internal/run/service_test.go @@ -16,9 +16,9 @@ func TestService_Watch(t *testing.T) { in := make(chan pubsub.Event[*Run], 1) svc := &Service{ - site: authz.NewAllowAllAuthorizer(), - Logger: logr.Discard(), - broker: &fakeSubService{ch: in}, + Interface: authz.NewAllowAllAuthorizer(), + Logger: logr.Discard(), + broker: &fakeSubService{ch: in}, } // inject input event diff --git a/internal/run/test_helpers.go b/internal/run/test_helpers.go index 91c9ac562..fab5911ad 100644 --- a/internal/run/test_helpers.go +++ b/internal/run/test_helpers.go @@ -55,7 +55,8 @@ func newTestWebHandlers(t *testing.T, opts ...fakeWebServiceOption) *webHandlers } return &webHandlers{ - Renderer: renderer, + Renderer: renderer, + authorizer: authz.NewAllowAllAuthorizer(), workspaces: &workspace.FakeService{ Workspaces: []*workspace.Workspace{svc.ws}, }, diff --git a/internal/run/tfe.go b/internal/run/tfe.go index 94cd30ffb..74c901a1b 100644 --- a/internal/run/tfe.go +++ b/internal/run/tfe.go @@ -27,6 +27,7 @@ type tfe struct { *tfeapi.Responder workspaces *workspace.Service + authorizer *authz.Authorizer } func (a *tfe) addHandlers(r *mux.Router) { @@ -354,20 +355,13 @@ func (a *tfe) includeCreatedBy(ctx context.Context, v any) ([]any, error) { // toRun converts a run into its equivalent json:api struct func (a *tfe) toRun(from *Run, ctx context.Context) (*types.Run, error) { - subject, err := authz.SubjectFromContext(ctx) - if err != nil { - return nil, err - } - policy, err := a.workspaces.GetPolicy(ctx, from.WorkspaceID) - if err != nil { - return nil, err - } + accessRequest := &authz.AccessRequest{ID: &from.ID} perms := &types.RunPermissions{ - CanDiscard: subject.CanAccessWorkspace(rbac.DiscardRunAction, policy), - CanForceExecute: subject.CanAccessWorkspace(rbac.ApplyRunAction, policy), - CanForceCancel: subject.CanAccessWorkspace(rbac.ForceCancelRunAction, policy), - CanCancel: subject.CanAccessWorkspace(rbac.CancelRunAction, policy), - CanApply: subject.CanAccessWorkspace(rbac.ApplyRunAction, policy), + CanDiscard: a.authorizer.CanAccess(ctx, rbac.DiscardRunAction, accessRequest), + CanForceExecute: a.authorizer.CanAccess(ctx, rbac.ApplyRunAction, accessRequest), + CanForceCancel: a.authorizer.CanAccess(ctx, rbac.ForceCancelRunAction, accessRequest), + CanCancel: a.authorizer.CanAccess(ctx, rbac.CancelRunAction, accessRequest), + CanApply: a.authorizer.CanAccess(ctx, rbac.ApplyRunAction, accessRequest), } var timestamps types.RunStatusTimestamps diff --git a/internal/run/web.go b/internal/run/web.go index b5ee1d095..dd11e4c43 100644 --- a/internal/run/web.go +++ b/internal/run/web.go @@ -26,6 +26,7 @@ type ( logger logr.Logger runs webRunClient workspaces webWorkspaceClient + authorizer webAuthorizer } webRunClient interface { @@ -44,7 +45,11 @@ type ( webWorkspaceClient interface { Get(ctx context.Context, workspaceID resource.ID) (*workspace.Workspace, error) - GetPolicy(ctx context.Context, workspaceID resource.ID) (authz.WorkspacePolicy, error) + GetWorkspacePolicy(ctx context.Context, workspaceID resource.ID) (authz.WorkspacePolicy, error) + } + + webAuthorizer interface { + CanAccess(context.Context, rbac.Action, *authz.AccessRequest) bool } ) @@ -106,11 +111,6 @@ func (h *webHandlers) list(w http.ResponseWriter, r *http.Request) { h.Error(w, err.Error(), http.StatusInternalServerError) return } - policy, err := h.workspaces.GetPolicy(r.Context(), ws.ID) - if err != nil { - h.Error(w, err.Error(), http.StatusInternalServerError) - return - } runs, err := h.runs.List(r.Context(), ListOptions{ WorkspaceID: ¶ms.WorkspaceID, PageOptions: resource.PageOptions{ @@ -122,11 +122,8 @@ func (h *webHandlers) list(w http.ResponseWriter, r *http.Request) { h.Error(w, err.Error(), http.StatusInternalServerError) return } - user, err := authz.SubjectFromContext(r.Context()) - if err != nil { - h.Error(w, err.Error(), http.StatusInternalServerError) - return - } + + canUpdateWorkspace := h.authorizer.CanAccess(r.Context(), rbac.UpdateWorkspaceAction, &authz.AccessRequest{ID: &ws.ID}) response := struct { workspace.WorkspacePage @@ -135,7 +132,7 @@ func (h *webHandlers) list(w http.ResponseWriter, r *http.Request) { }{ WorkspacePage: workspace.NewPage(r, "runs", ws), Page: runs, - CanUpdateWorkspace: user.CanAccessWorkspace(rbac.UpdateWorkspaceAction, policy), + CanUpdateWorkspace: canUpdateWorkspace, } if isHTMX := r.Header.Get("HX-Request"); isHTMX == "true" { diff --git a/internal/runner/job.go b/internal/runner/job.go index 133d10657..bf9099610 100644 --- a/internal/runner/job.go +++ b/internal/runner/job.go @@ -66,7 +66,10 @@ func newJob(run *otfrun.Run) *Job { func (j *Job) LogValue() slog.Value { attrs := []slog.Attr{ + slog.String("job_id", j.ID.String()), slog.String("run_id", j.RunID.String()), + slog.String("workspace_id", j.WorkspaceID.String()), + slog.String("organization", j.Organization), slog.String("phase", string(j.Phase)), slog.String("status", string(j.Status)), } @@ -80,63 +83,59 @@ func (j *Job) LogValue() slog.Value { return slog.GroupValue(attrs...) } -func (j *Job) Organizations() []string { return nil } +func (j *Job) String() string { return j.ID.String() } -func (j *Job) IsSiteAdmin() bool { return false } -func (j *Job) IsOwner(string) bool { return false } -func (j *Job) String() string { return j.ID.String() } - -func (j *Job) CanAccessSite(action rbac.Action) bool { - return false -} - -func (j *Job) CanAccessOrganization(action rbac.Action, name string) bool { +func (j *Job) CanAccess(action rbac.Action, req *authz.AccessRequest) bool { + if req == nil { + // Job cannot carry out site-wide actions + return false + } + if req.Organization != j.Organization { + // Job cannot carry out actions on other organizations + return false + } + // Permissible organization actions on same organization switch action { case rbac.GetOrganizationAction, rbac.GetEntitlementsAction, rbac.GetModuleAction, rbac.ListModulesAction: - return j.Organization == name - default: - return false + return true } -} - -func (j *Job) CanAccessWorkspace(action rbac.Action, policy authz.WorkspacePolicy) bool { - if policy.WorkspaceID != j.WorkspaceID { - // job is allowed the retrieve the state of *another* workspace only if: - // (a) workspace is in the same organization as job, or - // (b) workspace has enabled global remote state (permitting organization-wide - // state sharing). + // Permissible workspace actions on same workspace. + if req.ID != nil && *req.ID == j.WorkspaceID { + // Allow actions on same workspace as job depending on run phase switch action { - case rbac.GetStateVersionAction, rbac.GetWorkspaceAction, rbac.DownloadStateAction: - if j.Organization == policy.Organization && policy.GlobalRemoteState { + case rbac.DownloadStateAction, rbac.GetStateVersionAction, rbac.GetWorkspaceAction, rbac.GetRunAction, rbac.ListVariableSetsAction, rbac.ListWorkspaceVariablesAction, rbac.PutChunkAction, rbac.DownloadConfigurationVersionAction, rbac.GetPlanFileAction, rbac.CancelRunAction: + // any phase + return true + case rbac.UploadLockFileAction, rbac.UploadPlanFileAction, rbac.ApplyRunAction: + // plan phase + if j.Phase == internal.PlanPhase { + return true + } + case rbac.GetLockFileAction, rbac.CreateStateVersionAction: + // apply phase + if j.Phase == internal.ApplyPhase { return true } } return false } - // allow actions on same workspace as job depending on run phase - switch action { - case rbac.DownloadStateAction, rbac.GetStateVersionAction, rbac.GetWorkspaceAction, rbac.GetRunAction, rbac.ListVariableSetsAction, rbac.ListWorkspaceVariablesAction, rbac.PutChunkAction, rbac.DownloadConfigurationVersionAction, rbac.GetPlanFileAction, rbac.CancelRunAction: - // any phase - return true - case rbac.UploadLockFileAction, rbac.UploadPlanFileAction, rbac.ApplyRunAction: - // plan phase - if j.Phase == internal.PlanPhase { - return true - } - case rbac.GetLockFileAction, rbac.CreateStateVersionAction: - // apply phase - if j.Phase == internal.ApplyPhase { - return true + // If workspace policy is non-nil then that means the job is trying to + // access *another* workspace. Check the policy to determine if it is + // allowed to do so. + if req.WorkspacePolicy != nil { + switch action { + case rbac.GetStateVersionAction, rbac.GetWorkspaceAction, rbac.DownloadStateAction: + if req.WorkspacePolicy.GlobalRemoteState { + // Job is allowed to retrieve the state of this workspace + // because the workspace has allowed global remote state + // sharing. + return true + } } } return false } -func (j *Job) CanAccessTeam(rbac.Action, resource.ID) bool { - // Can't access team level actions - return false -} - func (j *Job) allocate(runnerID resource.ID) error { if err := j.updateStatus(JobAllocated); err != nil { return err diff --git a/internal/runner/meta.go b/internal/runner/meta.go index 31cb777b8..43e604b67 100644 --- a/internal/runner/meta.go +++ b/internal/runner/meta.go @@ -144,39 +144,23 @@ func (m *RunnerMetaAgentPool) LogValue() slog.Value { ) } -func (m *RunnerMeta) IsSiteAdmin() bool { return true } -func (m *RunnerMeta) IsOwner(string) bool { return true } +func (m *RunnerMeta) String() string { return m.ID.String() } -func (m *RunnerMeta) Organizations() []string { return nil } -func (m *RunnerMeta) String() string { return m.ID.String() } - -func (*RunnerMeta) CanAccessSite(action rbac.Action) bool { - return false -} - -func (*RunnerMeta) CanAccessTeam(rbac.Action, resource.ID) bool { - return false -} - -func (m *RunnerMeta) CanAccessOrganization(action rbac.Action, name string) bool { - // TODO: permit only those actions that an agent needs to carry out (get - // agent jobs, etc). - if m.AgentPool != nil { - return m.AgentPool.OrganizationName == name +func (m *RunnerMeta) CanAccess(action rbac.Action, req *authz.AccessRequest) bool { + if req == nil { + // Don't permit runners to carry out site-level actions + return false } - return true -} - -func (m *RunnerMeta) CanAccessWorkspace(action rbac.Action, policy authz.WorkspacePolicy) bool { - // only a server-based agent can authenticate as an Agent, and if that is - // so, then it can carry out all workspace-based actions. - // // TODO: permit only those actions that an agent needs to carry out (get // agent jobs, etc). - if m.AgentPool != nil { - return m.AgentPool.OrganizationName == policy.Organization + if m.IsAgent() { + // Agents can only carry out actions on the organization their pool + // belongs to. + return m.AgentPool.OrganizationName == req.Organization + } else { + // Server runners can carry out actions on all organizations. + return true } - return true } func runnerFromContext(ctx context.Context) (*RunnerMeta, error) { diff --git a/internal/runner/service.go b/internal/runner/service.go index 58b4637ec..8fa9dd99c 100644 --- a/internal/runner/service.go +++ b/internal/runner/service.go @@ -12,7 +12,6 @@ import ( otfhttp "github.com/leg100/otf/internal/http" "github.com/leg100/otf/internal/http/html" "github.com/leg100/otf/internal/logr" - "github.com/leg100/otf/internal/organization" "github.com/leg100/otf/internal/pubsub" "github.com/leg100/otf/internal/rbac" "github.com/leg100/otf/internal/resource" @@ -29,8 +28,7 @@ var ErrInvalidStateTransition = errors.New("invalid runner state transition") type ( Service struct { logr.Logger - - organization *organization.Authorizer + *authz.Authorizer tfeapi *tfe api *api @@ -54,6 +52,7 @@ type ( RunService *otfrun.Service WorkspaceService *workspace.Service TokensService *tokens.Service + Authorizer *authz.Authorizer } phaseClient interface { @@ -65,9 +64,9 @@ type ( func NewService(opts ServiceOptions) *Service { svc := &Service{ - Logger: opts.Logger, - db: &db{DB: opts.DB}, - organization: &organization.Authorizer{Logger: opts.Logger}, + Logger: opts.Logger, + Authorizer: opts.Authorizer, + db: &db{DB: opts.DB}, tokenFactory: &tokenFactory{ tokens: opts.TokensService, }, @@ -83,6 +82,7 @@ func NewService(opts ServiceOptions) *Service { } svc.web = &webHandlers{ Renderer: opts.Renderer, + authorizer: opts.Authorizer, logger: opts.Logger, svc: svc, workspaces: opts.WorkspaceService, @@ -288,7 +288,7 @@ func (s *Service) listServerRunners(ctx context.Context) ([]*RunnerMeta, error) } func (s *Service) listRunnersByOrganization(ctx context.Context, organization string) ([]*RunnerMeta, error) { - _, err := s.organization.CanAccess(ctx, rbac.ListRunnersAction, organization) + _, err := s.Authorize(ctx, rbac.ListRunnersAction, &authz.AccessRequest{Organization: organization}) if err != nil { return nil, err } @@ -509,7 +509,7 @@ func (s *Service) CreateAgentToken(ctx context.Context, poolID resource.ID, opts if err != nil { return nil, nil, nil, err } - subject, err := s.organization.CanAccess(ctx, rbac.CreateAgentTokenAction, pool.Organization) + subject, err := s.Authorize(ctx, rbac.CreateAgentTokenAction, &authz.AccessRequest{Organization: pool.Organization}) if err != nil { return nil, nil, nil, err } @@ -541,7 +541,7 @@ func (s *Service) GetAgentToken(ctx context.Context, tokenID resource.ID) (*agen if err != nil { return nil, nil, err } - subject, err := s.organization.CanAccess(ctx, rbac.GetAgentTokenAction, pool.Organization) + subject, err := s.Authorize(ctx, rbac.GetAgentTokenAction, &authz.AccessRequest{Organization: pool.Organization}) if err != nil { return nil, nil, err } @@ -560,7 +560,7 @@ func (s *Service) ListAgentTokens(ctx context.Context, poolID resource.ID) ([]*a if err != nil { return nil, err } - subject, err := s.organization.CanAccess(ctx, rbac.ListAgentTokensAction, pool.Organization) + subject, err := s.Authorize(ctx, rbac.ListAgentTokensAction, &authz.AccessRequest{Organization: pool.Organization}) if err != nil { return nil, err } @@ -585,7 +585,7 @@ func (s *Service) DeleteAgentToken(ctx context.Context, tokenID resource.ID) (*a if err != nil { return nil, nil, err } - subject, err := s.organization.CanAccess(ctx, rbac.DeleteAgentTokenAction, pool.Organization) + subject, err := s.Authorize(ctx, rbac.DeleteAgentTokenAction, &authz.AccessRequest{Organization: pool.Organization}) if err != nil { return nil, nil, err } @@ -627,7 +627,7 @@ func (s *Service) checkWorkspacePoolAccess(ctx context.Context, ws *workspace.Wo } func (s *Service) CreateAgentPool(ctx context.Context, opts CreateAgentPoolOptions) (*Pool, error) { - subject, err := s.organization.CanAccess(ctx, rbac.CreateAgentPoolAction, opts.Organization) + subject, err := s.Authorize(ctx, rbac.CreateAgentPoolAction, &authz.AccessRequest{Organization: opts.Organization}) if err != nil { return nil, err } @@ -659,7 +659,7 @@ func (s *Service) updateAgentPool(ctx context.Context, poolID resource.ID, opts if err != nil { return err } - subject, err = s.organization.CanAccess(ctx, rbac.UpdateAgentPoolAction, pool.Organization) + subject, err = s.Authorize(ctx, rbac.UpdateAgentPoolAction, &authz.AccessRequest{Organization: pool.Organization}) if err != nil { return err } @@ -700,7 +700,7 @@ func (s *Service) GetAgentPool(ctx context.Context, poolID resource.ID) (*Pool, s.Error(err, "retrieving agent pool", "agent_pool_id", poolID) return nil, err } - subject, err := s.organization.CanAccess(ctx, rbac.GetAgentPoolAction, pool.Organization) + subject, err := s.Authorize(ctx, rbac.GetAgentPoolAction, &authz.AccessRequest{Organization: pool.Organization}) if err != nil { return nil, err } @@ -709,7 +709,7 @@ func (s *Service) GetAgentPool(ctx context.Context, poolID resource.ID) (*Pool, } func (s *Service) listAgentPoolsByOrganization(ctx context.Context, organization string, opts listPoolOptions) ([]*Pool, error) { - subject, err := s.organization.CanAccess(ctx, rbac.ListAgentPoolsAction, organization) + subject, err := s.Authorize(ctx, rbac.ListAgentPoolsAction, &authz.AccessRequest{Organization: organization}) if err != nil { return nil, err } @@ -729,7 +729,7 @@ func (s *Service) deleteAgentPool(ctx context.Context, poolID resource.ID) (*Poo if err != nil { return nil, nil, err } - subject, err := s.organization.CanAccess(ctx, rbac.DeleteAgentPoolAction, pool.Organization) + subject, err := s.Authorize(ctx, rbac.DeleteAgentPoolAction, &authz.AccessRequest{Organization: pool.Organization}) if err != nil { return nil, nil, err } diff --git a/internal/runner/web.go b/internal/runner/web.go index 7b42283de..21e1774d5 100644 --- a/internal/runner/web.go +++ b/internal/runner/web.go @@ -27,6 +27,7 @@ type webHandlers struct { svc webClient workspaces *workspacepkg.Service logger logr.Logger + authorizer webAuthorizer } // webClient gives web handlers access to the agents service endpoints @@ -49,6 +50,10 @@ type webClient interface { DeleteAgentToken(ctx context.Context, tokenID resource.ID) (*agentToken, error) } +type webAuthorizer interface { + CanAccess(context.Context, rbac.Action, *authz.AccessRequest) bool +} + type ( // templates may serialize hundreds of workspaces to JSON, so a struct with // only the fields needed is used rather than the full *workspace.Workspace @@ -227,12 +232,6 @@ func (h *webHandlers) getAgentPool(w http.ResponseWriter, r *http.Request) { return } - subject, err := authz.SubjectFromContext(r.Context()) - if err != nil { - h.Error(w, err.Error(), http.StatusInternalServerError) - return - } - // template requires three sets of workspaces: // // (a) assigned workspaces @@ -295,7 +294,7 @@ func (h *webHandlers) getAgentPool(w http.ResponseWriter, r *http.Request) { }{ OrganizationPage: organization.NewPage(r, pool.Name, pool.Organization), Pool: pool, - CanDeleteAgentPool: subject.CanAccessOrganization(rbac.DeleteAgentPoolAction, pool.Organization), + CanDeleteAgentPool: h.authorizer.CanAccess(r.Context(), rbac.DeleteAgentPoolAction, &authz.AccessRequest{Organization: pool.Organization}), AllowedButUnassignedWorkspaces: allowedButUnassignedWorkspaces, AssignedWorkspaces: assignedWorkspaces, AvailableWorkspaces: availableWorkspaces, diff --git a/internal/sql/queries/workspace_permission.sql b/internal/sql/queries/workspace_permission.sql index 51c9786d2..e73d43737 100644 --- a/internal/sql/queries/workspace_permission.sql +++ b/internal/sql/queries/workspace_permission.sql @@ -7,15 +7,24 @@ INSERT INTO workspace_permissions ( sqlc.arg('workspace_id'), sqlc.arg('team_id'), sqlc.arg('role') -) ON CONFLICT (workspace_id, team_id) DO UPDATE SET role = sqlc.arg('role'); +) ON CONFLICT (workspace_id, team_id) DO UPDATE SET role = sqlc.arg('role') +; --- name: FindWorkspacePermissionsByWorkspaceID :many -SELECT * -FROM workspace_permissions -WHERE workspace_id = sqlc.arg('workspace_id'); +-- name: FindWorkspacePermissionsAndGlobalRemoteState :one +SELECT + w.global_remote_state, + ( + SELECT array_agg(wp.*)::workspace_permissions[] + FROM workspace_permissions wp + WHERE wp.workspace_id = w.workspace_id + ) AS workspace_permissions +FROM workspaces w +WHERE w.workspace_id = sqlc.arg('workspace_id') +; -- name: DeleteWorkspacePermissionByID :exec DELETE FROM workspace_permissions WHERE workspace_id = sqlc.arg('workspace_id') -AND team_id = sqlc.arg('team_id'); +AND team_id = sqlc.arg('team_id') +; diff --git a/internal/sql/sqlc/workspace_permission.sql.go b/internal/sql/sqlc/workspace_permission.sql.go index fd2f54337..f62ac251b 100644 --- a/internal/sql/sqlc/workspace_permission.sql.go +++ b/internal/sql/sqlc/workspace_permission.sql.go @@ -29,30 +29,28 @@ func (q *Queries) DeleteWorkspacePermissionByID(ctx context.Context, arg DeleteW return err } -const findWorkspacePermissionsByWorkspaceID = `-- name: FindWorkspacePermissionsByWorkspaceID :many -SELECT workspace_id, team_id, role -FROM workspace_permissions -WHERE workspace_id = $1 +const findWorkspacePermissionsAndGlobalRemoteState = `-- name: FindWorkspacePermissionsAndGlobalRemoteState :one +SELECT + w.global_remote_state, + ( + SELECT array_agg(wp.*)::workspace_permissions[] + FROM workspace_permissions wp + WHERE wp.workspace_id = w.workspace_id + ) AS workspace_permissions +FROM workspaces w +WHERE w.workspace_id = $1 ` -func (q *Queries) FindWorkspacePermissionsByWorkspaceID(ctx context.Context, workspaceID resource.ID) ([]WorkspacePermission, error) { - rows, err := q.db.Query(ctx, findWorkspacePermissionsByWorkspaceID, workspaceID) - if err != nil { - return nil, err - } - defer rows.Close() - var items []WorkspacePermission - for rows.Next() { - var i WorkspacePermission - if err := rows.Scan(&i.WorkspaceID, &i.TeamID, &i.Role); err != nil { - return nil, err - } - items = append(items, i) - } - if err := rows.Err(); err != nil { - return nil, err - } - return items, nil +type FindWorkspacePermissionsAndGlobalRemoteStateRow struct { + GlobalRemoteState pgtype.Bool + WorkspacePermissions []WorkspacePermission +} + +func (q *Queries) FindWorkspacePermissionsAndGlobalRemoteState(ctx context.Context, workspaceID resource.ID) (FindWorkspacePermissionsAndGlobalRemoteStateRow, error) { + row := q.db.QueryRow(ctx, findWorkspacePermissionsAndGlobalRemoteState, workspaceID) + var i FindWorkspacePermissionsAndGlobalRemoteStateRow + err := row.Scan(&i.GlobalRemoteState, &i.WorkspacePermissions) + return i, err } const upsertWorkspacePermission = `-- name: UpsertWorkspacePermission :exec diff --git a/internal/sql/types.go b/internal/sql/types.go index e79b7a31a..051a4c159 100644 --- a/internal/sql/types.go +++ b/internal/sql/types.go @@ -29,4 +29,6 @@ var tableTypes = []string{ "state_version_outputs[]", "agent_pools", "agent_pools[]", + "workspace_permissions", + "workspace_permissions[]", } diff --git a/internal/state/service.go b/internal/state/service.go index 9bf51600d..a9dbead74 100644 --- a/internal/state/service.go +++ b/internal/state/service.go @@ -29,14 +29,14 @@ type ( Service struct { logr.Logger - db *pgdb - cache internal.Cache // cache state file - workspace authz.Authorizer - web *webHandlers - tfeapi *tfe - api *api + db *pgdb + cache internal.Cache // cache state file + web *webHandlers + tfeapi *tfe + api *api *factory // for creating state versions + *authz.Authorizer } Options struct { @@ -48,6 +48,7 @@ type ( *surl.Signer WorkspaceService *workspace.Service + Authorizer *authz.Authorizer } // StateVersionListOptions represents the options for listing state versions. @@ -61,11 +62,11 @@ type ( func NewService(opts Options) *Service { db := &pgdb{opts.DB} svc := Service{ - Logger: opts.Logger, - cache: opts.Cache, - db: db, - workspace: opts.WorkspaceService, - factory: &factory{db}, + Logger: opts.Logger, + Authorizer: opts.Authorizer, + cache: opts.Cache, + db: db, + factory: &factory{db}, } svc.web = &webHandlers{ Renderer: opts.Renderer, @@ -85,6 +86,17 @@ func NewService(opts Options) *Service { // include state version outputs in api responses when requested. opts.Responder.Register(tfeapi.IncludeOutputs, svc.tfeapi.includeOutputs) opts.Responder.Register(tfeapi.IncludeOutputs, svc.tfeapi.includeWorkspaceCurrentOutputs) + + // Resolve authorization requests for state version IDs to a workspace IDs + opts.Authorizer.RegisterWorkspaceResolver(resource.StateVersionKind, + func(ctx context.Context, svID resource.ID) (resource.ID, error) { + sv, err := db.getVersion(ctx, svID) + if err != nil { + return resource.ID{}, err + } + return sv.WorkspaceID, nil + }, + ) return &svc } @@ -95,7 +107,7 @@ func (a *Service) AddHandlers(r *mux.Router) { } func (a *Service) Create(ctx context.Context, opts CreateStateVersionOptions) (*Version, error) { - subject, err := a.workspace.CanAccess(ctx, rbac.CreateStateVersionAction, opts.WorkspaceID) + subject, err := a.Authorize(ctx, rbac.CreateStateVersionAction, &authz.AccessRequest{ID: &opts.WorkspaceID}) if err != nil { return nil, err } @@ -123,7 +135,7 @@ func (a *Service) DownloadCurrent(ctx context.Context, workspaceID resource.ID) } func (a *Service) List(ctx context.Context, workspaceID resource.ID, opts resource.PageOptions) (*resource.Page[*Version], error) { - subject, err := a.workspace.CanAccess(ctx, rbac.ListStateVersionsAction, workspaceID) + subject, err := a.Authorize(ctx, rbac.ListStateVersionsAction, &authz.AccessRequest{ID: &workspaceID}) if err != nil { return nil, err } @@ -138,7 +150,7 @@ func (a *Service) List(ctx context.Context, workspaceID resource.ID, opts resour } func (a *Service) GetCurrent(ctx context.Context, workspaceID resource.ID) (*Version, error) { - subject, err := a.workspace.CanAccess(ctx, rbac.GetStateVersionAction, workspaceID) + subject, err := a.Authorize(ctx, rbac.GetStateVersionAction, &authz.AccessRequest{ID: &workspaceID}) if err != nil { return nil, err } @@ -158,7 +170,7 @@ func (a *Service) GetCurrent(ctx context.Context, workspaceID resource.ID) (*Ver } func (a *Service) Get(ctx context.Context, versionID resource.ID) (*Version, error) { - subject, err := a.CanAccess(ctx, rbac.GetStateVersionAction, versionID) + subject, err := a.Authorize(ctx, rbac.GetStateVersionAction, &authz.AccessRequest{ID: &versionID}) if err != nil { return nil, err } @@ -173,7 +185,7 @@ func (a *Service) Get(ctx context.Context, versionID resource.ID) (*Version, err } func (a *Service) Delete(ctx context.Context, versionID resource.ID) error { - subject, err := a.CanAccess(ctx, rbac.DeleteStateVersionAction, versionID) + subject, err := a.Authorize(ctx, rbac.DeleteStateVersionAction, &authz.AccessRequest{ID: &versionID}) if err != nil { return err } @@ -187,7 +199,7 @@ func (a *Service) Delete(ctx context.Context, versionID resource.ID) error { } func (a *Service) Rollback(ctx context.Context, versionID resource.ID) (*Version, error) { - subject, err := a.CanAccess(ctx, rbac.RollbackStateVersionAction, versionID) + subject, err := a.Authorize(ctx, rbac.RollbackStateVersionAction, &authz.AccessRequest{ID: &versionID}) if err != nil { return nil, err } @@ -227,7 +239,7 @@ func (a *Service) Upload(ctx context.Context, svID resource.ID, state []byte) er } func (a *Service) Download(ctx context.Context, svID resource.ID) ([]byte, error) { - subject, err := a.CanAccess(ctx, rbac.DownloadStateAction, svID) + subject, err := a.Authorize(ctx, rbac.DownloadStateAction, &authz.AccessRequest{ID: &svID}) if err != nil { return nil, err } @@ -254,7 +266,7 @@ func (a *Service) GetOutput(ctx context.Context, outputID resource.ID) (*Output, return nil, err } - subject, err := a.CanAccess(ctx, rbac.GetStateVersionOutputAction, out.StateVersionID) + subject, err := a.Authorize(ctx, rbac.GetStateVersionOutputAction, &authz.AccessRequest{ID: &out.StateVersionID}) if err != nil { return nil, err } @@ -262,11 +274,3 @@ func (a *Service) GetOutput(ctx context.Context, outputID resource.ID) (*Output, a.V(9).Info("retrieved state version output", "id", outputID, "subject", subject) return out, nil } - -func (a *Service) CanAccess(ctx context.Context, action rbac.Action, svID resource.ID) (authz.Subject, error) { - sv, err := a.db.getVersion(ctx, svID) - if err != nil { - return nil, err - } - return a.workspace.CanAccess(ctx, action, sv.WorkspaceID) -} diff --git a/internal/team/authorizer.go b/internal/team/authorizer.go deleted file mode 100644 index 2d7c18609..000000000 --- a/internal/team/authorizer.go +++ /dev/null @@ -1,31 +0,0 @@ -package team - -import ( - "context" - - "github.com/go-logr/logr" - "github.com/leg100/otf/internal" - "github.com/leg100/otf/internal/authz" - "github.com/leg100/otf/internal/rbac" - "github.com/leg100/otf/internal/resource" -) - -// authorizer authorizes access to a team -type authorizer struct { - logr.Logger -} - -func (a *authorizer) CanAccess(ctx context.Context, action rbac.Action, teamID resource.ID) (authz.Subject, error) { - subj, err := authz.SubjectFromContext(ctx) - if err != nil { - return nil, err - } - if authz.SkipAuthz(ctx) { - return subj, nil - } - if subj.CanAccessTeam(action, teamID) { - return subj, nil - } - a.Error(nil, "unauthorized action", "team_id", teamID, "action", action.String(), "subject", subj) - return nil, internal.ErrAccessNotPermitted -} diff --git a/internal/team/service.go b/internal/team/service.go index 24f4d0725..80888c4bf 100644 --- a/internal/team/service.go +++ b/internal/team/service.go @@ -24,9 +24,7 @@ var ErrRemovingOwnersTeamNotPermitted = errors.New("the owners team cannot be de type ( Service struct { logr.Logger - - organization *organization.Authorizer // authorizes org access - team authz.Authorizer // authorizes team access + *authz.Authorizer db *pgdb web *webHandlers @@ -46,15 +44,15 @@ type ( OrganizationService *organization.Service TokensService *tokens.Service + Authorizer *authz.Authorizer } ) func NewService(opts Options) *Service { svc := Service{ - Logger: opts.Logger, - organization: &organization.Authorizer{Logger: opts.Logger}, - team: &authorizer{Logger: opts.Logger}, - db: &pgdb{opts.DB, opts.Logger}, + Logger: opts.Logger, + Authorizer: opts.Authorizer, + db: &pgdb{opts.DB, opts.Logger}, teamTokenFactory: &teamTokenFactory{ tokens: opts.TokensService, }, @@ -93,6 +91,13 @@ func NewService(opts Options) *Service { opts.TokensService.RegisterKind(TeamTokenKind, func(ctx context.Context, tokenID resource.ID) (authz.Subject, error) { return svc.GetTeamByTokenID(ctx, tokenID) }) + opts.Authorizer.RegisterOrganizationResolver(resource.TeamKind, func(ctx context.Context, id resource.ID) (string, error) { + team, err := svc.db.getTeamByID(ctx, id) + if err != nil { + return "", err + } + return team.Organization, nil + }) return &svc } @@ -104,7 +109,7 @@ func (a *Service) AddHandlers(r *mux.Router) { } func (a *Service) Create(ctx context.Context, organization string, opts CreateTeamOptions) (*Team, error) { - subject, err := a.organization.CanAccess(ctx, rbac.CreateTeamAction, organization) + subject, err := a.Authorize(ctx, rbac.CreateTeamAction, &authz.AccessRequest{Organization: organization}) if err != nil { return nil, err } @@ -144,7 +149,7 @@ func (a *Service) Update(ctx context.Context, teamID resource.ID, opts UpdateTea a.Error(err, "retrieving team", "team_id", teamID) return nil, err } - subject, err := a.organization.CanAccess(ctx, rbac.UpdateTeamAction, team.Organization) + subject, err := a.Authorize(ctx, rbac.UpdateTeamAction, &authz.AccessRequest{Organization: team.Organization}) if err != nil { return nil, err } @@ -164,7 +169,7 @@ func (a *Service) Update(ctx context.Context, teamID resource.ID, opts UpdateTea // List lists teams in the organization. func (a *Service) List(ctx context.Context, organization string) ([]*Team, error) { - subject, err := a.organization.CanAccess(ctx, rbac.ListTeamsAction, organization) + subject, err := a.Authorize(ctx, rbac.ListTeamsAction, &authz.AccessRequest{Organization: organization}) if err != nil { return nil, err } @@ -180,7 +185,7 @@ func (a *Service) List(ctx context.Context, organization string) ([]*Team, error } func (a *Service) Get(ctx context.Context, organization, name string) (*Team, error) { - subject, err := a.organization.CanAccess(ctx, rbac.GetTeamAction, organization) + subject, err := a.Authorize(ctx, rbac.GetTeamAction, &authz.AccessRequest{Organization: organization}) if err != nil { return nil, err } @@ -203,7 +208,7 @@ func (a *Service) GetByID(ctx context.Context, teamID resource.ID) (*Team, error return nil, err } - subject, err := a.organization.CanAccess(ctx, rbac.GetTeamAction, team.Organization) + subject, err := a.Authorize(ctx, rbac.GetTeamAction, &authz.AccessRequest{Organization: team.Organization}) if err != nil { return nil, err } @@ -220,7 +225,7 @@ func (a *Service) Delete(ctx context.Context, teamID resource.ID) error { return err } - subject, err := a.organization.CanAccess(ctx, rbac.DeleteTeamAction, team.Organization) + subject, err := a.Authorize(ctx, rbac.DeleteTeamAction, &authz.AccessRequest{Organization: team.Organization}) if err != nil { return err } @@ -247,7 +252,7 @@ func (a *Service) GetTeamByTokenID(ctx context.Context, tokenID resource.ID) (*T return nil, err } - subject, err := a.organization.CanAccess(ctx, rbac.GetTeamAction, team.Organization) + subject, err := a.Authorize(ctx, rbac.GetTeamAction, &authz.AccessRequest{Organization: team.Organization}) if err != nil { return nil, err } diff --git a/internal/team/team.go b/internal/team/team.go index b100709bc..780b3fa2f 100644 --- a/internal/team/team.go +++ b/internal/team/team.go @@ -119,8 +119,6 @@ func newTeam(organization string, opts CreateTeamOptions) (*Team, error) { func (t *Team) String() string { return t.Name } func (t *Team) OrganizationAccess() OrganizationAccess { return t.Access } -func (t *Team) IsSiteAdmin() bool { return false } - func (t *Team) IsOwners() bool { return t.Name == "owners" } @@ -129,55 +127,48 @@ func (t *Team) IsOwner(organization string) bool { return t.Organization == organization && t.IsOwners() } -func (t *Team) CanAccessSite(action rbac.Action) bool { - return false -} - -func (t *Team) CanAccessTeam(action rbac.Action, id resource.ID) bool { - // team can access self - return t.ID == id -} - -func (t *Team) CanAccessOrganization(action rbac.Action, org string) bool { - if t.Organization == org { - if t.IsOwners() { - // owner team can perform all actions on organization +func (t *Team) CanAccess(action rbac.Action, req *authz.AccessRequest) bool { + if req == nil { + // Deny all site-level access + return false + } + if req.Organization != t.Organization { + // Deny access to other organizations + return false + } + if t.IsOwners() { + // owner team can perform all actions on organization + return true + } + if rbac.OrganizationMinPermissions.IsAllowed(action) { + return true + } + if t.Access.ManageWorkspaces { + if rbac.WorkspaceManagerRole.IsAllowed(action) { return true } - if rbac.OrganizationMinPermissions.IsAllowed(action) { + } + if t.Access.ManageVCS { + if rbac.VCSManagerRole.IsAllowed(action) { return true } - if t.Access.ManageWorkspaces { - if rbac.WorkspaceManagerRole.IsAllowed(action) { - return true - } - } - if t.Access.ManageVCS { - if rbac.VCSManagerRole.IsAllowed(action) { - return true - } - } - if t.Access.ManageModules { - if rbac.VCSManagerRole.IsAllowed(action) { - return true - } - } } - return false -} - -func (t *Team) CanAccessWorkspace(action rbac.Action, policy authz.WorkspacePolicy) bool { - // coarser-grained organization perms take precedence. - if t.CanAccessOrganization(action, policy.Organization) { - return true + if t.Access.ManageModules { + if rbac.RegistryManagerRole.IsAllowed(action) { + return true + } } - // fallback to checking finer-grained workspace perms - if t.Organization != policy.Organization { - return false + if req.ID != nil && req.ID.Kind() == resource.TeamKind { + // team can access self + return t.ID == *req.ID } - for _, perm := range policy.Permissions { - if t.ID == perm.TeamID { - return perm.Role.IsAllowed(action) + if req.WorkspacePolicy != nil { + // Team can only access workspace if a specific permission has been + // assigned to the team. + for _, perm := range req.WorkspacePolicy.Permissions { + if t.ID == perm.TeamID { + return perm.Role.IsAllowed(action) + } } } return false diff --git a/internal/team/token.go b/internal/team/token.go index c076960d7..bf88b09f8 100644 --- a/internal/team/token.go +++ b/internal/team/token.go @@ -6,6 +6,7 @@ import ( "time" "github.com/leg100/otf/internal" + "github.com/leg100/otf/internal/authz" "github.com/leg100/otf/internal/rbac" "github.com/leg100/otf/internal/resource" "github.com/leg100/otf/internal/tokens" @@ -67,7 +68,7 @@ func (t *Token) LogValue() slog.Value { } func (a *Service) CreateTeamToken(ctx context.Context, opts CreateTokenOptions) (*Token, []byte, error) { - _, err := a.team.CanAccess(ctx, rbac.CreateTeamTokenAction, opts.TeamID) + _, err := a.Authorize(ctx, rbac.CreateTeamTokenAction, &authz.AccessRequest{ID: &opts.TeamID}) if err != nil { return nil, nil, err } @@ -89,7 +90,7 @@ func (a *Service) CreateTeamToken(ctx context.Context, opts CreateTokenOptions) } func (a *Service) GetTeamToken(ctx context.Context, teamID resource.ID) (*Token, error) { - _, err := a.team.CanAccess(ctx, rbac.GetTeamTokenAction, teamID) + _, err := a.Authorize(ctx, rbac.GetTeamTokenAction, &authz.AccessRequest{ID: &teamID}) if err != nil { return nil, err } @@ -97,7 +98,7 @@ func (a *Service) GetTeamToken(ctx context.Context, teamID resource.ID) (*Token, } func (a *Service) DeleteTeamToken(ctx context.Context, teamID resource.ID) error { - _, err := a.team.CanAccess(ctx, rbac.DeleteTeamTokenAction, teamID) + _, err := a.Authorize(ctx, rbac.DeleteTeamTokenAction, &authz.AccessRequest{ID: &teamID}) if err != nil { return err } diff --git a/internal/team/web.go b/internal/team/web.go index a66c38352..ae1cbb291 100644 --- a/internal/team/web.go +++ b/internal/team/web.go @@ -133,7 +133,7 @@ func (h *webHandlers) listTeams(w http.ResponseWriter, r *http.Request) { }{ OrganizationPage: organization.NewPage(r, "teams", org), Teams: teams, - CanCreateTeam: subject.CanAccessOrganization(rbac.CreateTeamAction, org), + CanCreateTeam: subject.CanAccess(rbac.CreateTeamAction, &authz.AccessRequest{Organization: org}), }) } diff --git a/internal/tokens/service.go b/internal/tokens/service.go index a17c5111d..7973c72e6 100644 --- a/internal/tokens/service.go +++ b/internal/tokens/service.go @@ -3,7 +3,6 @@ package tokens import ( "github.com/go-logr/logr" "github.com/gorilla/mux" - "github.com/leg100/otf/internal/authz" "github.com/leg100/otf/internal/resource" "github.com/lestrrat-go/jwx/v2/jwk" ) @@ -11,11 +10,10 @@ import ( type ( Service struct { logr.Logger + *tokenFactory *registry - site authz.Authorizer // authorizes site access - middleware mux.MiddlewareFunc } @@ -30,7 +28,6 @@ type ( func NewService(opts Options) (*Service, error) { svc := Service{ Logger: opts.Logger, - site: &authz.SiteAuthorizer{Logger: opts.Logger}, } key, err := jwk.FromRaw([]byte(opts.Secret)) if err != nil { diff --git a/internal/user/service.go b/internal/user/service.go index 906eb0d26..9e1fc9a79 100644 --- a/internal/user/service.go +++ b/internal/user/service.go @@ -10,7 +10,6 @@ import ( "github.com/leg100/otf/internal/authz" "github.com/leg100/otf/internal/http/html" "github.com/leg100/otf/internal/logr" - "github.com/leg100/otf/internal/organization" "github.com/leg100/otf/internal/rbac" "github.com/leg100/otf/internal/resource" "github.com/leg100/otf/internal/sql" @@ -25,11 +24,9 @@ var ErrCannotDeleteOnlyOwner = errors.New("cannot remove the last owner") type ( Service struct { logr.Logger + *authz.Authorizer - site authz.Authorizer // authorizes site access - organization *organization.Authorizer // authorizes org access - teams *team.Service - + teams *team.Service db *pgdb web *webHandlers tfeapi *tfe @@ -42,6 +39,7 @@ type ( SiteToken string TokensService *tokens.Service TeamService *team.Service + Authorizer *authz.Authorizer *sql.DB *tfeapi.Responder @@ -52,10 +50,9 @@ type ( func NewService(opts Options) *Service { svc := Service{ - Logger: opts.Logger, - organization: &organization.Authorizer{Logger: opts.Logger}, - site: &authz.SiteAuthorizer{Logger: opts.Logger}, - db: &pgdb{opts.DB, opts.Logger}, + Logger: opts.Logger, + Authorizer: opts.Authorizer, + db: &pgdb{opts.DB, opts.Logger}, userTokenFactory: &userTokenFactory{ tokens: opts.TokensService, }, @@ -130,7 +127,7 @@ func (a *Service) AddHandlers(r *mux.Router) { } func (a *Service) Create(ctx context.Context, username string, opts ...NewUserOption) (*User, error) { - subject, err := a.site.CanAccess(ctx, rbac.CreateUserAction, resource.ID{}) + subject, err := a.Authorize(ctx, rbac.CreateUserAction, nil) if err != nil { return nil, err } @@ -148,7 +145,7 @@ func (a *Service) Create(ctx context.Context, username string, opts ...NewUserOp } func (a *Service) GetUser(ctx context.Context, spec UserSpec) (*User, error) { - subject, err := a.site.CanAccess(ctx, rbac.GetUserAction, resource.ID{}) + subject, err := a.Authorize(ctx, rbac.GetUserAction, nil) if err != nil { return nil, err } @@ -166,7 +163,7 @@ func (a *Service) GetUser(ctx context.Context, spec UserSpec) (*User, error) { // List lists all users. func (a *Service) List(ctx context.Context) ([]*User, error) { - _, err := a.site.CanAccess(ctx, rbac.ListUsersAction, resource.ID{}) + _, err := a.Authorize(ctx, rbac.ListUsersAction, nil) if err != nil { return nil, err } @@ -176,7 +173,7 @@ func (a *Service) List(ctx context.Context) ([]*User, error) { // ListOrganizationUsers lists an organization's users func (a *Service) ListOrganizationUsers(ctx context.Context, organization string) ([]*User, error) { - _, err := a.organization.CanAccess(ctx, rbac.ListUsersAction, organization) + _, err := a.Authorize(ctx, rbac.ListUsersAction, &authz.AccessRequest{Organization: organization}) if err != nil { return nil, err } @@ -193,7 +190,7 @@ func (a *Service) ListTeamUsers(ctx context.Context, teamID resource.ID) ([]*Use return nil, err } - subject, err := a.organization.CanAccess(ctx, rbac.ListUsersAction, team.Organization) + subject, err := a.Authorize(ctx, rbac.ListUsersAction, &authz.AccessRequest{Organization: team.Organization}) if err != nil { return nil, err } @@ -210,7 +207,7 @@ func (a *Service) ListTeamUsers(ctx context.Context, teamID resource.ID) ([]*Use } func (a *Service) Delete(ctx context.Context, username string) error { - subject, err := a.site.CanAccess(ctx, rbac.DeleteUserAction, resource.ID{}) + subject, err := a.Authorize(ctx, rbac.DeleteUserAction, nil) if err != nil { return err } @@ -234,7 +231,7 @@ func (a *Service) AddTeamMembership(ctx context.Context, teamID resource.ID, use return err } - subject, err := a.organization.CanAccess(ctx, rbac.AddTeamMembershipAction, team.Organization) + subject, err := a.Authorize(ctx, rbac.AddTeamMembershipAction, &authz.AccessRequest{Organization: team.Organization}) if err != nil { return err } @@ -273,7 +270,7 @@ func (a *Service) RemoveTeamMembership(ctx context.Context, teamID resource.ID, return err } - subject, err := a.organization.CanAccess(ctx, rbac.RemoveTeamMembershipAction, team.Organization) + subject, err := a.Authorize(ctx, rbac.RemoveTeamMembershipAction, &authz.AccessRequest{Organization: team.Organization}) if err != nil { return err } diff --git a/internal/user/user.go b/internal/user/user.go index 250b6b85b..2a6222bc9 100644 --- a/internal/user/user.go +++ b/internal/user/user.go @@ -123,9 +123,14 @@ func (u *User) IsSiteAdmin() bool { return u.SiteAdmin || u.ID == SiteAdminID } -func (u *User) CanAccessSite(action rbac.Action) bool { +func (u *User) CanAccess(action rbac.Action, req *authz.AccessRequest) bool { + // Site admin can do whatever it wants + if u.IsSiteAdmin() { + return true + } switch action { - case rbac.GetGithubAppAction: + case rbac.CreateOrganizationAction, rbac.GetGithubAppAction: + // These actions are available to any user. return true case rbac.CreateUserAction, rbac.ListUsersAction: // A user can perform these actions only if they are an owner of at @@ -137,45 +142,14 @@ func (u *User) CanAccessSite(action rbac.Action) bool { } } } - // Otherwise only the site admin can perform site actions. - return u.IsSiteAdmin() -} - -func (u *User) CanAccessTeam(action rbac.Action, teamID resource.ID) bool { - // coarser-grained site-level perms take precedence - if u.CanAccessSite(action) { - return true - } - for _, team := range u.Teams { - if team.ID == teamID { - return true - } - } - return false -} - -func (u *User) CanAccessOrganization(action rbac.Action, org string) bool { - // coarser-grained site-level perms take precedence - if u.CanAccessSite(action) { - return true - } - // fallback to finer-grained organization-level perms - for _, team := range u.Teams { - if team.CanAccessOrganization(action, org) { - return true - } - } - return false -} - -func (u *User) CanAccessWorkspace(action rbac.Action, policy authz.WorkspacePolicy) bool { - // coarser-grained organization perms take precedence. - if u.CanAccessOrganization(action, policy.Organization) { - return true + if req == nil { + // nil req means site-level access is being requested and there are no + // further allowed actions that are available to user at the site-level. + return false } - // fallback to checking finer-grained workspace perms + // All other user perms are inherited from team memberships. for _, team := range u.Teams { - if team.CanAccessWorkspace(action, policy) { + if team.CanAccess(action, req) { return true } } diff --git a/internal/user/user_test.go b/internal/user/user_test.go index e2a08df25..bca502dee 100644 --- a/internal/user/user_test.go +++ b/internal/user/user_test.go @@ -3,6 +3,7 @@ package user import ( "testing" + "github.com/leg100/otf/internal/authz" "github.com/leg100/otf/internal/rbac" "github.com/leg100/otf/internal/team" "github.com/stretchr/testify/assert" @@ -12,7 +13,7 @@ func TestSiteAdminCanAccessOrganization(t *testing.T) { u := User{ ID: SiteAdminID, } - assert.True(t, u.CanAccessOrganization(rbac.ListRunsAction, "acme-corp")) + assert.True(t, u.CanAccess(rbac.ListRunsAction, &authz.AccessRequest{Organization: "acme-corp"})) } func TestOwnerCanAccessOrganization(t *testing.T) { @@ -24,7 +25,7 @@ func TestOwnerCanAccessOrganization(t *testing.T) { }, }, } - assert.True(t, u.CanAccessOrganization(rbac.ListRunsAction, "acme-corp")) + assert.True(t, u.CanAccess(rbac.ListRunsAction, &authz.AccessRequest{Organization: "acme-corp"})) } func TestUser_Organizations(t *testing.T) { diff --git a/internal/user/web.go b/internal/user/web.go index 36485ca8f..e4c86e604 100644 --- a/internal/user/web.go +++ b/internal/user/web.go @@ -233,7 +233,7 @@ func (h *webHandlers) getTeam(w http.ResponseWriter, r *http.Request) { // Retrieve full list of users for populating a select form from which new // team members can be chosen. Only do this if the subject has perms to // retrieve the list. - user, err := authz.SubjectFromContext(r.Context()) + user, err := UserFromContext(r.Context()) if err != nil { h.Error(w, err.Error(), http.StatusInternalServerError) return @@ -241,7 +241,7 @@ func (h *webHandlers) getTeam(w http.ResponseWriter, r *http.Request) { // get usernames of non-members var nonMemberUsernames []string - if user.CanAccessSite(rbac.ListUsersAction) { + if user.CanAccess(rbac.ListUsersAction, nil) { users, err := h.users.List(r.Context()) if err != nil { h.Error(w, err.Error(), http.StatusInternalServerError) @@ -269,11 +269,11 @@ func (h *webHandlers) getTeam(w http.ResponseWriter, r *http.Request) { OrganizationPage: organization.NewPage(r, team.ID.String(), team.Organization), Team: team, Members: members, - CanUpdateTeam: user.CanAccessOrganization(rbac.UpdateTeamAction, team.Organization), - CanDeleteTeam: user.CanAccessOrganization(rbac.DeleteTeamAction, team.Organization), - CanAddMember: user.CanAccessOrganization(rbac.AddTeamMembershipAction, team.Organization), - CanRemoveMember: user.CanAccessOrganization(rbac.RemoveTeamMembershipAction, team.Organization), - CanDelete: user.CanAccessOrganization(rbac.DeleteTeamAction, team.Organization), + CanUpdateTeam: user.CanAccess(rbac.UpdateTeamAction, &authz.AccessRequest{Organization: team.Organization}), + CanDeleteTeam: user.CanAccess(rbac.DeleteTeamAction, &authz.AccessRequest{Organization: team.Organization}), + CanAddMember: user.CanAccess(rbac.AddTeamMembershipAction, &authz.AccessRequest{Organization: team.Organization}), + CanRemoveMember: user.CanAccess(rbac.RemoveTeamMembershipAction, &authz.AccessRequest{Organization: team.Organization}), + CanDelete: user.CanAccess(rbac.DeleteTeamAction, &authz.AccessRequest{Organization: team.Organization}), IsOwner: user.IsOwner(team.Organization), AddMemberDropdown: html.DropdownUI{ Name: "username", diff --git a/internal/variable/service.go b/internal/variable/service.go index 068daf1fb..3c990e607 100644 --- a/internal/variable/service.go +++ b/internal/variable/service.go @@ -7,7 +7,6 @@ import ( "github.com/gorilla/mux" "github.com/leg100/otf/internal/authz" "github.com/leg100/otf/internal/http/html" - "github.com/leg100/otf/internal/organization" "github.com/leg100/otf/internal/rbac" "github.com/leg100/otf/internal/resource" "github.com/leg100/otf/internal/run" @@ -20,20 +19,19 @@ import ( type ( Service struct { logr.Logger + *authz.Authorizer - db *pgdb - web *web - tfeapi *tfe - api *api - workspace authz.Authorizer - organization *organization.Authorizer - runs runClient + db *pgdb + web *web + tfeapi *tfe + api *api + runs runClient } Options struct { - WorkspaceAuthorizer authz.Authorizer - WorkspaceService *workspace.Service - RunClient runClient + WorkspaceService *workspace.Service + RunClient runClient + Authorizer *authz.Authorizer *sql.DB *tfeapi.Responder @@ -48,15 +46,15 @@ type ( func NewService(opts Options) *Service { svc := Service{ - Logger: opts.Logger, - db: &pgdb{opts.DB}, - workspace: opts.WorkspaceAuthorizer, - organization: &organization.Authorizer{Logger: opts.Logger}, - runs: opts.RunClient, + Logger: opts.Logger, + Authorizer: opts.Authorizer, + db: &pgdb{opts.DB}, + runs: opts.RunClient, } svc.web = &web{ Renderer: opts.Renderer, + authorizer: opts.Authorizer, workspaces: opts.WorkspaceService, variables: &svc, } @@ -95,7 +93,7 @@ func (s *Service) ListEffectiveVariables(ctx context.Context, runID resource.ID) } func (s *Service) CreateWorkspaceVariable(ctx context.Context, workspaceID resource.ID, opts CreateVariableOptions) (*Variable, error) { - subject, err := s.workspace.CanAccess(ctx, rbac.CreateWorkspaceVariableAction, workspaceID) + subject, err := s.Authorize(ctx, rbac.CreateWorkspaceVariableAction, &authz.AccessRequest{ID: &workspaceID}) if err != nil { return nil, err } @@ -139,7 +137,7 @@ func (s *Service) UpdateWorkspaceVariable(ctx context.Context, variableID resour return err } - subject, err = s.workspace.CanAccess(ctx, rbac.UpdateWorkspaceVariableAction, before.WorkspaceID) + subject, err = s.Authorize(ctx, rbac.UpdateWorkspaceVariableAction, &authz.AccessRequest{ID: &before.WorkspaceID}) if err != nil { return err } @@ -170,7 +168,7 @@ func (s *Service) UpdateWorkspaceVariable(ctx context.Context, variableID resour } func (s *Service) ListWorkspaceVariables(ctx context.Context, workspaceID resource.ID) ([]*Variable, error) { - subject, err := s.workspace.CanAccess(ctx, rbac.ListWorkspaceVariablesAction, workspaceID) + subject, err := s.Authorize(ctx, rbac.ListWorkspaceVariablesAction, &authz.AccessRequest{ID: &workspaceID}) if err != nil { return nil, err } @@ -193,7 +191,7 @@ func (s *Service) GetWorkspaceVariable(ctx context.Context, variableID resource. return nil, err } - subject, err := s.workspace.CanAccess(ctx, rbac.ListWorkspaceVariablesAction, wv.WorkspaceID) + subject, err := s.Authorize(ctx, rbac.ListWorkspaceVariablesAction, &authz.AccessRequest{ID: &wv.WorkspaceID}) if err != nil { return nil, err } @@ -214,7 +212,7 @@ func (s *Service) DeleteWorkspaceVariable(ctx context.Context, variableID resour return err } - subject, err = s.workspace.CanAccess(ctx, rbac.DeleteWorkspaceVariableAction, wv.WorkspaceID) + subject, err = s.Authorize(ctx, rbac.DeleteWorkspaceVariableAction, &authz.AccessRequest{ID: &wv.WorkspaceID}) if err != nil { return err } @@ -230,7 +228,7 @@ func (s *Service) DeleteWorkspaceVariable(ctx context.Context, variableID resour } func (s *Service) createVariableSet(ctx context.Context, organization string, opts CreateVariableSetOptions) (*VariableSet, error) { - subject, err := s.organization.CanAccess(ctx, rbac.CreateVariableSetAction, organization) + subject, err := s.Authorize(ctx, rbac.CreateVariableSetAction, &authz.AccessRequest{Organization: organization}) if err != nil { return nil, err } @@ -272,7 +270,7 @@ func (s *Service) updateVariableSet(ctx context.Context, setID resource.ID, opts return err } - subject, err = s.organization.CanAccess(ctx, rbac.UpdateVariableSetAction, before.Organization) + subject, err = s.Authorize(ctx, rbac.UpdateVariableSetAction, &authz.AccessRequest{Organization: before.Organization}) if err != nil { return err } @@ -300,7 +298,7 @@ func (s *Service) updateVariableSet(ctx context.Context, setID resource.ID, opts } func (s *Service) listVariableSets(ctx context.Context, organization string) ([]*VariableSet, error) { - subject, err := s.organization.CanAccess(ctx, rbac.ListVariableSetsAction, organization) + subject, err := s.Authorize(ctx, rbac.ListVariableSetsAction, &authz.AccessRequest{Organization: organization}) if err != nil { return nil, err } @@ -316,7 +314,7 @@ func (s *Service) listVariableSets(ctx context.Context, organization string) ([] } func (s *Service) listWorkspaceVariableSets(ctx context.Context, workspaceID resource.ID) ([]*VariableSet, error) { - subject, err := s.workspace.CanAccess(ctx, rbac.ListVariableSetsAction, workspaceID) + subject, err := s.Authorize(ctx, rbac.ListVariableSetsAction, &authz.AccessRequest{ID: &workspaceID}) if err != nil { return nil, err } @@ -338,7 +336,7 @@ func (s *Service) getVariableSet(ctx context.Context, setID resource.ID) (*Varia return nil, err } - subject, err := s.organization.CanAccess(ctx, rbac.GetVariableSetAction, set.Organization) + subject, err := s.Authorize(ctx, rbac.GetVariableSetAction, &authz.AccessRequest{Organization: set.Organization}) if err != nil { s.Error(err, "retrieving variable set", "subject", subject, "set", set) return nil, err @@ -355,7 +353,7 @@ func (s *Service) getVariableSetByVariableID(ctx context.Context, variableID res return nil, err } - subject, err := s.organization.CanAccess(ctx, rbac.GetVariableSetVariableAction, set.Organization) + subject, err := s.Authorize(ctx, rbac.GetVariableSetVariableAction, &authz.AccessRequest{Organization: set.Organization}) if err != nil { return nil, err } @@ -372,7 +370,7 @@ func (s *Service) deleteVariableSet(ctx context.Context, setID resource.ID) (*Va return nil, err } - subject, err := s.organization.CanAccess(ctx, rbac.DeleteVariableSetAction, set.Organization) + subject, err := s.Authorize(ctx, rbac.DeleteVariableSetAction, &authz.AccessRequest{Organization: set.Organization}) if err != nil { return nil, err } @@ -398,7 +396,7 @@ func (s *Service) createVariableSetVariable(ctx context.Context, setID resource. return err } - subject, err = s.organization.CanAccess(ctx, rbac.AddVariableToSetAction, set.Organization) + subject, err = s.Authorize(ctx, rbac.AddVariableToSetAction, &authz.AccessRequest{Organization: set.Organization}) if err != nil { return err } @@ -440,7 +438,7 @@ func (s *Service) updateVariableSetVariable(ctx context.Context, variableID reso if err != nil { return err } - subject, err = s.organization.CanAccess(ctx, rbac.UpdateVariableSetAction, set.Organization) + subject, err = s.Authorize(ctx, rbac.UpdateVariableSetAction, &authz.AccessRequest{Organization: set.Organization}) if err != nil { return err } @@ -477,7 +475,7 @@ func (s *Service) deleteVariableSetVariable(ctx context.Context, variableID reso return nil, err } - subject, err := s.organization.CanAccess(ctx, rbac.RemoveVariableFromSetAction, set.Organization) + subject, err := s.Authorize(ctx, rbac.RemoveVariableFromSetAction, &authz.AccessRequest{Organization: set.Organization}) if err != nil { return nil, err } @@ -500,7 +498,7 @@ func (s *Service) applySetToWorkspaces(ctx context.Context, setID resource.ID, w return err } - subject, err := s.organization.CanAccess(ctx, rbac.ApplyVariableSetToWorkspacesAction, set.Organization) + subject, err := s.Authorize(ctx, rbac.ApplyVariableSetToWorkspacesAction, &authz.AccessRequest{Organization: set.Organization}) if err != nil { return err } @@ -521,7 +519,7 @@ func (s *Service) deleteSetFromWorkspaces(ctx context.Context, setID resource.ID return err } - subject, err := s.organization.CanAccess(ctx, rbac.DeleteVariableSetFromWorkspacesAction, set.Organization) + subject, err := s.Authorize(ctx, rbac.DeleteVariableSetFromWorkspacesAction, &authz.AccessRequest{Organization: set.Organization}) if err != nil { return err } diff --git a/internal/variable/web.go b/internal/variable/web.go index a1533317d..514bdd588 100644 --- a/internal/variable/web.go +++ b/internal/variable/web.go @@ -21,7 +21,8 @@ type ( html.Renderer workspaces webWorkspaceClient - variables webVariablesClient + variables webVariablesClient + authorizer webAuthorizer } // webVariablesClient provides web handlers with access to variables @@ -48,7 +49,11 @@ type ( webWorkspaceClient interface { Get(ctx context.Context, workspaceID resource.ID) (*workspace.Workspace, error) List(ctx context.Context, opts workspace.ListOptions) (*resource.Page[*workspace.Workspace], error) - GetPolicy(ctx context.Context, workspaceID resource.ID) (authz.WorkspacePolicy, error) + GetWorkspacePolicy(ctx context.Context, workspaceID resource.ID) (authz.WorkspacePolicy, error) + } + + webAuthorizer interface { + CanAccess(context.Context, rbac.Action, *authz.AccessRequest) bool } workspaceInfo struct { @@ -182,16 +187,6 @@ func (h *web) listWorkspaceVariables(w http.ResponseWriter, r *http.Request) { h.Error(w, err.Error(), http.StatusInternalServerError) return } - policy, err := h.workspaces.GetPolicy(r.Context(), ws.ID) - if err != nil { - h.Error(w, err.Error(), http.StatusInternalServerError) - return - } - user, err := authz.SubjectFromContext(r.Context()) - if err != nil { - h.Error(w, err.Error(), http.StatusInternalServerError) - return - } sets, err := h.variables.listWorkspaceVariableSets(r.Context(), workspaceID) if err != nil { h.Error(w, err.Error(), http.StatusInternalServerError) @@ -211,7 +206,6 @@ func (h *web) listWorkspaceVariables(w http.ResponseWriter, r *http.Request) { workspace.WorkspacePage WorkspaceVariableTable workspaceVariableTable VariableSetTables []setVariableTable - Policy authz.WorkspacePolicy CanCreateVariable bool CanDeleteVariable bool CanUpdateWorkspace bool @@ -219,13 +213,12 @@ func (h *web) listWorkspaceVariables(w http.ResponseWriter, r *http.Request) { WorkspacePage: workspace.NewPage(r, "variables", ws), WorkspaceVariableTable: workspaceVariableTable{ Variables: variables, - CanDeleteVariable: user.CanAccessWorkspace(rbac.DeleteWorkspaceVariableAction, policy), + CanDeleteVariable: h.authorizer.CanAccess(r.Context(), rbac.DeleteWorkspaceVariableAction, &authz.AccessRequest{ID: &ws.ID}), }, VariableSetTables: setVariableTables, - Policy: policy, - CanCreateVariable: user.CanAccessWorkspace(rbac.CreateWorkspaceVariableAction, policy), - CanDeleteVariable: user.CanAccessWorkspace(rbac.DeleteWorkspaceVariableAction, policy), - CanUpdateWorkspace: user.CanAccessWorkspace(rbac.UpdateWorkspaceAction, policy), + CanCreateVariable: h.authorizer.CanAccess(r.Context(), rbac.CreateWorkspaceVariableAction, &authz.AccessRequest{ID: &ws.ID}), + CanDeleteVariable: h.authorizer.CanAccess(r.Context(), rbac.DeleteWorkspaceVariableAction, &authz.AccessRequest{ID: &ws.ID}), + CanUpdateWorkspace: h.authorizer.CanAccess(r.Context(), rbac.UpdateWorkspaceAction, &authz.AccessRequest{ID: &ws.ID}), }) } @@ -315,12 +308,6 @@ func (h *web) listVariableSets(w http.ResponseWriter, r *http.Request) { return } - user, err := authz.SubjectFromContext(r.Context()) - if err != nil { - h.Error(w, err.Error(), http.StatusInternalServerError) - return - } - h.Render("variable_set_list.tmpl", w, struct { organization.OrganizationPage VariableSets []*VariableSet @@ -328,7 +315,7 @@ func (h *web) listVariableSets(w http.ResponseWriter, r *http.Request) { }{ OrganizationPage: organization.NewPage(r, "variable sets", org), VariableSets: sets, - CanCreate: user.CanAccessOrganization(rbac.CreateVariableSetAction, org), + CanCreate: h.authorizer.CanAccess(r.Context(), rbac.CreateVariableSetAction, &authz.AccessRequest{Organization: org}), }) } @@ -438,12 +425,6 @@ func (h *web) editVariableSet(w http.ResponseWriter, r *http.Request) { } } - user, err := authz.SubjectFromContext(r.Context()) - if err != nil { - h.Error(w, err.Error(), http.StatusInternalServerError) - return - } - h.Render("variable_set_edit.tmpl", w, struct { organization.OrganizationPage *VariableSet @@ -461,11 +442,11 @@ func (h *web) editVariableSet(w http.ResponseWriter, r *http.Request) { FormAction: paths.UpdateVariableSet(set.ID.String()), AvailableWorkspaces: availableWorkspaces, ExistingWorkspaces: existingWorkspaces, - CanCreateVariable: user.CanAccessOrganization(rbac.CreateWorkspaceVariableAction, set.Organization), - CanDeleteVariable: user.CanAccessOrganization(rbac.DeleteWorkspaceVariableAction, set.Organization), + CanCreateVariable: h.authorizer.CanAccess(r.Context(), rbac.CreateWorkspaceVariableAction, &authz.AccessRequest{Organization: set.Organization}), + CanDeleteVariable: h.authorizer.CanAccess(r.Context(), rbac.DeleteWorkspaceVariableAction, &authz.AccessRequest{Organization: set.Organization}), VariableTable: setVariableTable{ VariableSet: set, - CanDeleteVariable: user.CanAccessOrganization(rbac.DeleteWorkspaceVariableAction, set.Organization), + CanDeleteVariable: h.authorizer.CanAccess(r.Context(), rbac.DeleteWorkspaceVariableAction, &authz.AccessRequest{Organization: set.Organization}), }, }) } diff --git a/internal/vcsprovider/service.go b/internal/vcsprovider/service.go index af17a09b5..93676e172 100644 --- a/internal/vcsprovider/service.go +++ b/internal/vcsprovider/service.go @@ -9,7 +9,6 @@ import ( "github.com/leg100/otf/internal/authz" "github.com/leg100/otf/internal/github" "github.com/leg100/otf/internal/http/html" - "github.com/leg100/otf/internal/organization" "github.com/leg100/otf/internal/rbac" "github.com/leg100/otf/internal/resource" "github.com/leg100/otf/internal/sql" @@ -21,9 +20,8 @@ import ( type ( Service struct { logr.Logger + *authz.Authorizer - site authz.Authorizer - organization *organization.Authorizer db *pgdb web *webHandlers api *tfe @@ -46,6 +44,7 @@ type ( GithubHostname string GitlabHostname string SkipTLSVerification bool + Authorizer *authz.Authorizer } ) @@ -59,9 +58,8 @@ func NewService(opts Options) *Service { svc := Service{ Logger: opts.Logger, HostnameService: opts.HostnameService, + Authorizer: opts.Authorizer, githubapps: opts.GithubAppService, - site: &authz.SiteAuthorizer{Logger: opts.Logger}, - organization: &organization.Authorizer{Logger: opts.Logger}, factory: &factory, db: &pgdb{ DB: opts.DB, @@ -110,7 +108,7 @@ func (a *Service) AddHandlers(r *mux.Router) { } func (a *Service) Create(ctx context.Context, opts CreateOptions) (*VCSProvider, error) { - subject, err := a.organization.CanAccess(ctx, rbac.CreateVCSProviderAction, opts.Organization) + subject, err := a.Authorize(ctx, rbac.CreateVCSProviderAction, &authz.AccessRequest{Organization: opts.Organization}) if err != nil { return nil, err } @@ -135,7 +133,7 @@ func (a *Service) Update(ctx context.Context, id resource.ID, opts UpdateOptions after *VCSProvider ) err := a.db.update(ctx, id, func(provider *VCSProvider) (err error) { - subject, err = a.organization.CanAccess(ctx, rbac.UpdateVariableSetAction, provider.Organization) + subject, err = a.Authorize(ctx, rbac.UpdateVariableSetAction, &authz.AccessRequest{Organization: provider.Organization}) if err != nil { return err } @@ -156,7 +154,7 @@ func (a *Service) Update(ctx context.Context, id resource.ID, opts UpdateOptions } func (a *Service) List(ctx context.Context, organization string) ([]*VCSProvider, error) { - subject, err := a.organization.CanAccess(ctx, rbac.ListVCSProvidersAction, organization) + subject, err := a.Authorize(ctx, rbac.ListVCSProvidersAction, &authz.AccessRequest{Organization: organization}) if err != nil { return nil, err } @@ -171,7 +169,7 @@ func (a *Service) List(ctx context.Context, organization string) ([]*VCSProvider } func (a *Service) ListAllVCSProviders(ctx context.Context) ([]*VCSProvider, error) { - subject, err := a.site.CanAccess(ctx, rbac.ListVCSProvidersAction, resource.ID{}) + subject, err := a.Authorize(ctx, rbac.ListVCSProvidersAction, nil) if err != nil { return nil, err } @@ -210,7 +208,7 @@ func (a *Service) Get(ctx context.Context, id resource.ID) (*VCSProvider, error) return nil, err } - subject, err := a.organization.CanAccess(ctx, rbac.GetVCSProviderAction, provider.Organization) + subject, err := a.Authorize(ctx, rbac.GetVCSProviderAction, &authz.AccessRequest{Organization: provider.Organization}) if err != nil { return nil, err } @@ -240,7 +238,7 @@ func (a *Service) Delete(ctx context.Context, id resource.ID) (*VCSProvider, err return err } - subject, err = a.organization.CanAccess(ctx, rbac.DeleteVCSProviderAction, provider.Organization) + subject, err = a.Authorize(ctx, rbac.DeleteVCSProviderAction, &authz.AccessRequest{Organization: provider.Organization}) if err != nil { return err } diff --git a/internal/workspace/authorizer.go b/internal/workspace/authorizer.go deleted file mode 100644 index 7913ddd1a..000000000 --- a/internal/workspace/authorizer.go +++ /dev/null @@ -1,37 +0,0 @@ -package workspace - -import ( - "context" - - "github.com/go-logr/logr" - "github.com/leg100/otf/internal" - "github.com/leg100/otf/internal/authz" - "github.com/leg100/otf/internal/rbac" - "github.com/leg100/otf/internal/resource" -) - -// authorizer authorizes access to a workspace -type authorizer struct { - logr.Logger - - db *pgdb -} - -func (a *authorizer) CanAccess(ctx context.Context, action rbac.Action, workspaceID resource.ID) (authz.Subject, error) { - subj, err := authz.SubjectFromContext(ctx) - if err != nil { - return nil, err - } - if authz.SkipAuthz(ctx) { - return subj, nil - } - policy, err := a.db.GetWorkspacePolicy(ctx, workspaceID) - if err != nil { - return nil, internal.ErrResourceNotFound - } - if subj.CanAccessWorkspace(action, policy) { - return subj, nil - } - a.Error(nil, "unauthorized action", "workspace_id", workspaceID, "organization", policy.Organization, "action", action.String(), "subject", subj) - return nil, internal.ErrAccessNotPermitted -} diff --git a/internal/workspace/db.go b/internal/workspace/db.go index 2452008a3..c912238f1 100644 --- a/internal/workspace/db.go +++ b/internal/workspace/db.go @@ -5,6 +5,8 @@ import ( "github.com/jackc/pgx/v5/pgtype" "github.com/leg100/otf/internal" + "github.com/leg100/otf/internal/authz" + "github.com/leg100/otf/internal/rbac" "github.com/leg100/otf/internal/resource" "github.com/leg100/otf/internal/sql" "github.com/leg100/otf/internal/sql/sqlc" @@ -341,3 +343,48 @@ func (db *pgdb) delete(ctx context.Context, workspaceID resource.ID) error { } return nil } + +func (db *pgdb) SetWorkspacePermission(ctx context.Context, workspaceID, teamID resource.ID, role rbac.Role) error { + err := db.Querier(ctx).UpsertWorkspacePermission(ctx, sqlc.UpsertWorkspacePermissionParams{ + WorkspaceID: workspaceID, + TeamID: teamID, + Role: sql.String(role.String()), + }) + if err != nil { + return sql.Error(err) + } + return nil +} + +func (db *pgdb) UnsetWorkspacePermission(ctx context.Context, workspaceID, teamID resource.ID) error { + err := db.Querier(ctx).DeleteWorkspacePermissionByID(ctx, sqlc.DeleteWorkspacePermissionByIDParams{ + WorkspaceID: workspaceID, + TeamID: teamID, + }) + if err != nil { + return sql.Error(err) + } + return nil +} + +func (db *pgdb) GetWorkspacePolicy(ctx context.Context, workspaceID resource.ID) (authz.WorkspacePolicy, error) { + perms, err := db.Querier(ctx).FindWorkspacePermissionsAndGlobalRemoteState(ctx, workspaceID) + if err != nil { + return authz.WorkspacePolicy{}, sql.Error(err) + } + p := authz.WorkspacePolicy{ + GlobalRemoteState: perms.GlobalRemoteState.Bool, + Permissions: make([]authz.WorkspacePermission, len(perms.WorkspacePermissions)), + } + for i, perm := range perms.WorkspacePermissions { + role, err := rbac.WorkspaceRoleFromString(perm.Role.String) + if err != nil { + return authz.WorkspacePolicy{}, err + } + p.Permissions[i] = authz.WorkspacePermission{ + TeamID: perm.TeamID, + Role: role, + } + } + return p, nil +} diff --git a/internal/workspace/lock_service.go b/internal/workspace/lock_service.go index 92273dc43..fd33057dd 100644 --- a/internal/workspace/lock_service.go +++ b/internal/workspace/lock_service.go @@ -4,6 +4,7 @@ import ( "context" "fmt" + "github.com/leg100/otf/internal/authz" "github.com/leg100/otf/internal/rbac" "github.com/leg100/otf/internal/resource" "github.com/leg100/otf/internal/user" @@ -17,7 +18,7 @@ func (s *Service) Lock(ctx context.Context, workspaceID resource.ID, runID *reso if runID != nil { id = *runID } else { - subject, err := s.CanAccess(ctx, rbac.LockWorkspaceAction, workspaceID) + subject, err := s.Authorize(ctx, rbac.LockWorkspaceAction, &authz.AccessRequest{ID: &workspaceID}) if err != nil { return nil, err } @@ -53,7 +54,7 @@ func (s *Service) Unlock(ctx context.Context, workspaceID resource.ID, runID *re } else { action = rbac.UnlockWorkspaceAction } - subject, err := s.CanAccess(ctx, action, workspaceID) + subject, err := s.Authorize(ctx, action, &authz.AccessRequest{ID: &workspaceID}) if err != nil { return nil, err } diff --git a/internal/workspace/permissions_db.go b/internal/workspace/permissions_db.go deleted file mode 100644 index de1ffba93..000000000 --- a/internal/workspace/permissions_db.go +++ /dev/null @@ -1,67 +0,0 @@ -package workspace - -import ( - "context" - - "github.com/leg100/otf/internal/authz" - "github.com/leg100/otf/internal/rbac" - "github.com/leg100/otf/internal/resource" - "github.com/leg100/otf/internal/sql" - "github.com/leg100/otf/internal/sql/sqlc" -) - -func (db *pgdb) SetWorkspacePermission(ctx context.Context, workspaceID, teamID resource.ID, role rbac.Role) error { - err := db.Querier(ctx).UpsertWorkspacePermission(ctx, sqlc.UpsertWorkspacePermissionParams{ - WorkspaceID: workspaceID, - TeamID: teamID, - Role: sql.String(role.String()), - }) - if err != nil { - return sql.Error(err) - } - return nil -} - -func (db *pgdb) GetWorkspacePolicy(ctx context.Context, workspaceID resource.ID) (authz.WorkspacePolicy, error) { - q := db.Querier(ctx) - - // Retrieve not only permissions but the workspace too, so that: - // (1) we ensure that workspace exists and return not found if not - // (2) we retrieve the name of the organization, which is part of a policy - ws, err := q.FindWorkspaceByID(ctx, workspaceID) - if err != nil { - return authz.WorkspacePolicy{}, sql.Error(err) - } - perms, err := q.FindWorkspacePermissionsByWorkspaceID(ctx, workspaceID) - if err != nil { - return authz.WorkspacePolicy{}, sql.Error(err) - } - - policy := authz.WorkspacePolicy{ - Organization: ws.OrganizationName.String, - WorkspaceID: workspaceID, - GlobalRemoteState: ws.GlobalRemoteState.Bool, - } - for _, perm := range perms { - role, err := rbac.WorkspaceRoleFromString(perm.Role.String) - if err != nil { - return authz.WorkspacePolicy{}, err - } - policy.Permissions = append(policy.Permissions, authz.WorkspacePermission{ - TeamID: perm.TeamID, - Role: role, - }) - } - return policy, nil -} - -func (db *pgdb) UnsetWorkspacePermission(ctx context.Context, workspaceID, teamID resource.ID) error { - err := db.Querier(ctx).DeleteWorkspacePermissionByID(ctx, sqlc.DeleteWorkspacePermissionByIDParams{ - WorkspaceID: workspaceID, - TeamID: teamID, - }) - if err != nil { - return sql.Error(err) - } - return nil -} diff --git a/internal/workspace/permissions_service.go b/internal/workspace/permissions_service.go deleted file mode 100644 index 975b137c2..000000000 --- a/internal/workspace/permissions_service.go +++ /dev/null @@ -1,47 +0,0 @@ -package workspace - -import ( - "context" - - "github.com/leg100/otf/internal/authz" - "github.com/leg100/otf/internal/rbac" - "github.com/leg100/otf/internal/resource" -) - -// GetPolicy retrieves a workspace policy. -// -// NOTE: no authz protects this endpoint because it's used in the process of making -// authz decisions. -func (s *Service) GetPolicy(ctx context.Context, workspaceID resource.ID) (authz.WorkspacePolicy, error) { - return s.db.GetWorkspacePolicy(ctx, workspaceID) -} - -func (s *Service) SetPermission(ctx context.Context, workspaceID, teamID resource.ID, role rbac.Role) error { - subject, err := s.CanAccess(ctx, rbac.SetWorkspacePermissionAction, workspaceID) - if err != nil { - return err - } - - if err := s.db.SetWorkspacePermission(ctx, workspaceID, teamID, role); err != nil { - s.Error(err, "setting workspace permission", "subject", subject, "workspace", workspaceID) - return err - } - - s.V(0).Info("set workspace permission", "team_id", teamID, "role", role, "subject", subject, "workspace", workspaceID) - - // TODO: publish event - - return nil -} - -func (s *Service) UnsetPermission(ctx context.Context, workspaceID, teamID resource.ID) error { - subject, err := s.CanAccess(ctx, rbac.UnsetWorkspacePermissionAction, workspaceID) - if err != nil { - s.Error(err, "unsetting workspace permission", "team_id", teamID, "subject", subject, "workspace", workspaceID) - return err - } - - s.V(0).Info("unset workspace permission", "team_id", teamID, "subject", subject, "workspace", workspaceID) - // TODO: publish event - return s.db.UnsetWorkspacePermission(ctx, workspaceID, teamID) -} diff --git a/internal/workspace/service.go b/internal/workspace/service.go index 121b21e5e..7f06330cd 100644 --- a/internal/workspace/service.go +++ b/internal/workspace/service.go @@ -24,10 +24,7 @@ import ( type ( Service struct { logr.Logger - - site authz.Authorizer - organization *organization.Authorizer - authz.Authorizer // workspace authorizer + *authz.Authorizer db *pgdb web *webHandlers @@ -45,6 +42,7 @@ type ( *sql.DB *sql.Listener *tfeapi.Responder + *authz.Authorizer html.Renderer logr.Logger @@ -60,28 +58,26 @@ type ( func NewService(opts Options) *Service { db := &pgdb{opts.DB} svc := Service{ - Logger: opts.Logger, - Authorizer: &authorizer{ - Logger: opts.Logger, - db: db, - }, - db: db, - connections: opts.ConnectionService, - organization: &organization.Authorizer{Logger: opts.Logger}, - site: &authz.SiteAuthorizer{Logger: opts.Logger}, + Logger: opts.Logger, + Authorizer: opts.Authorizer, + db: db, + connections: opts.ConnectionService, } svc.web = &webHandlers{ Renderer: opts.Renderer, + authorizer: opts.Authorizer, teams: opts.TeamService, vcsproviders: opts.VCSProviderService, client: &svc, uiHelpers: &uiHelpers{ - service: opts.UserService, + service: opts.UserService, + authorizer: opts.Authorizer, }, } svc.tfeapi = &tfe{ - Service: &svc, - Responder: opts.Responder, + Service: &svc, + Responder: opts.Responder, + Authorizer: opts.Authorizer, } svc.api = &api{ Service: &svc, @@ -103,6 +99,16 @@ func NewService(opts Options) *Service { // response opts.Responder.Register(tfeapi.IncludeWorkspace, svc.tfeapi.include) opts.Responder.Register(tfeapi.IncludeWorkspaces, svc.tfeapi.includeMany) + // Instruct the authorizer to resolve workspace IDs to organization names. + opts.Authorizer.RegisterOrganizationResolver(resource.WorkspaceKind, func(ctx context.Context, id resource.ID) (string, error) { + ws, err := svc.db.get(ctx, id) + if err != nil { + return "", err + } + return ws.Organization, nil + }) + // Provide the authorizer with the ability to retrieve workspace policies. + opts.Authorizer.WorkspacePolicyGetter = &svc return &svc } @@ -125,7 +131,7 @@ func (s *Service) Create(ctx context.Context, opts CreateOptions) (*Workspace, e return nil, err } - subject, err := s.organization.CanAccess(ctx, rbac.CreateWorkspaceAction, ws.Organization) + subject, err := s.Authorize(ctx, rbac.CreateWorkspaceAction, &authz.AccessRequest{Organization: ws.Organization}) if err != nil { return nil, err } @@ -179,7 +185,7 @@ func (s *Service) AfterCreateWorkspace(hook func(context.Context, *Workspace) er } func (s *Service) Get(ctx context.Context, workspaceID resource.ID) (*Workspace, error) { - subject, err := s.CanAccess(ctx, rbac.GetWorkspaceAction, workspaceID) + subject, err := s.Authorize(ctx, rbac.GetWorkspaceAction, &authz.AccessRequest{ID: &workspaceID}) if err != nil { return nil, err } @@ -202,7 +208,7 @@ func (s *Service) GetByName(ctx context.Context, organization, workspace string) return nil, err } - subject, err := s.CanAccess(ctx, rbac.GetWorkspaceAction, ws.ID) + subject, err := s.Authorize(ctx, rbac.GetWorkspaceAction, &authz.AccessRequest{ID: &ws.ID}) if err != nil { return nil, err } @@ -215,13 +221,13 @@ func (s *Service) GetByName(ctx context.Context, organization, workspace string) func (s *Service) List(ctx context.Context, opts ListOptions) (*resource.Page[*Workspace], error) { if opts.Organization == nil { // subject needs perms on site to list workspaces across site - _, err := s.site.CanAccess(ctx, rbac.ListWorkspacesAction, resource.ID{}) + _, err := s.Authorize(ctx, rbac.ListWorkspacesAction, nil) if err != nil { return nil, err } } else { // check if subject has perms to list workspaces in organization - _, err := s.organization.CanAccess(ctx, rbac.ListWorkspacesAction, *opts.Organization) + _, err := s.Authorize(ctx, rbac.ListWorkspacesAction, &authz.AccessRequest{Organization: *opts.Organization}) if err == internal.ErrAccessNotPermitted { // user does not have org-wide perms; fallback to listing workspaces // for which they have workspace-level perms. @@ -249,7 +255,7 @@ func (s *Service) BeforeUpdateWorkspace(hook func(context.Context, *Workspace) e } func (s *Service) Update(ctx context.Context, workspaceID resource.ID, opts UpdateOptions) (*Workspace, error) { - subject, err := s.CanAccess(ctx, rbac.UpdateWorkspaceAction, workspaceID) + subject, err := s.Authorize(ctx, rbac.UpdateWorkspaceAction, &authz.AccessRequest{ID: &workspaceID}) if err != nil { return nil, err } @@ -294,7 +300,7 @@ func (s *Service) Update(ctx context.Context, workspaceID resource.ID, opts Upda } func (s *Service) Delete(ctx context.Context, workspaceID resource.ID) (*Workspace, error) { - subject, err := s.CanAccess(ctx, rbac.DeleteWorkspaceAction, workspaceID) + subject, err := s.Authorize(ctx, rbac.DeleteWorkspaceAction, &authz.AccessRequest{ID: &workspaceID}) if err != nil { return nil, err } @@ -321,6 +327,44 @@ func (s *Service) Delete(ctx context.Context, workspaceID resource.ID) (*Workspa return ws, nil } +func (s *Service) SetPermission(ctx context.Context, workspaceID, teamID resource.ID, role rbac.Role) error { + subject, err := s.Authorize(ctx, rbac.SetWorkspacePermissionAction, &authz.AccessRequest{ID: &workspaceID}) + if err != nil { + return err + } + + if err := s.db.SetWorkspacePermission(ctx, workspaceID, teamID, role); err != nil { + s.Error(err, "setting workspace permission", "subject", subject, "workspace", workspaceID) + return err + } + + s.V(0).Info("set workspace permission", "team_id", teamID, "role", role, "subject", subject, "workspace", workspaceID) + + // TODO: publish event + + return nil +} + +func (s *Service) UnsetPermission(ctx context.Context, workspaceID, teamID resource.ID) error { + subject, err := s.Authorize(ctx, rbac.UnsetWorkspacePermissionAction, &authz.AccessRequest{ID: &workspaceID}) + if err != nil { + s.Error(err, "unsetting workspace permission", "team_id", teamID, "subject", subject, "workspace", workspaceID) + return err + } + + s.V(0).Info("unset workspace permission", "team_id", teamID, "subject", subject, "workspace", workspaceID) + // TODO: publish event + return s.db.UnsetWorkspacePermission(ctx, workspaceID, teamID) +} + +// GetWorkspacePolicy retrieves the authorization policy for a workspace. +// +// NOTE: there is no auth because it is used in the process of making an auth +// decision. +func (s *Service) GetWorkspacePolicy(ctx context.Context, workspaceID resource.ID) (authz.WorkspacePolicy, error) { + return s.db.GetWorkspacePolicy(ctx, workspaceID) +} + // connect connects the workspace to a repo. func (s *Service) connect(ctx context.Context, workspaceID resource.ID, connection *Connection) error { subject, err := authz.SubjectFromContext(ctx) diff --git a/internal/workspace/tag_service.go b/internal/workspace/tag_service.go index ce22955f2..c7121045c 100644 --- a/internal/workspace/tag_service.go +++ b/internal/workspace/tag_service.go @@ -27,7 +27,7 @@ type ( ) func (s *Service) ListTags(ctx context.Context, organization string, opts ListTagsOptions) (*resource.Page[*Tag], error) { - subject, err := s.organization.CanAccess(ctx, rbac.ListTagsAction, organization) + subject, err := s.Authorize(ctx, rbac.ListTagsAction, &authz.AccessRequest{Organization: organization}) if err != nil { return nil, err } @@ -41,7 +41,7 @@ func (s *Service) ListTags(ctx context.Context, organization string, opts ListTa } func (s *Service) DeleteTags(ctx context.Context, organization string, tagIDs []resource.ID) error { - subject, err := s.organization.CanAccess(ctx, rbac.DeleteTagsAction, organization) + subject, err := s.Authorize(ctx, rbac.DeleteTagsAction, &authz.AccessRequest{Organization: organization}) if err != nil { return err } @@ -62,7 +62,7 @@ func (s *Service) TagWorkspaces(ctx context.Context, tagID resource.ID, workspac err = s.db.Tx(ctx, func(ctx context.Context, _ *sqlc.Queries) error { for _, wid := range workspaceIDs { - _, err := s.CanAccess(ctx, rbac.TagWorkspacesAction, wid) + _, err := s.Authorize(ctx, rbac.TagWorkspacesAction, &authz.AccessRequest{ID: &wid}) if err != nil { return err } @@ -81,7 +81,7 @@ func (s *Service) TagWorkspaces(ctx context.Context, tagID resource.ID, workspac } func (s *Service) AddTags(ctx context.Context, workspaceID resource.ID, tags []TagSpec) error { - subject, err := s.CanAccess(ctx, rbac.AddTagsAction, workspaceID) + subject, err := s.Authorize(ctx, rbac.AddTagsAction, &authz.AccessRequest{ID: &workspaceID}) if err != nil { return err } @@ -101,7 +101,7 @@ func (s *Service) AddTags(ctx context.Context, workspaceID resource.ID, tags []T } func (s *Service) RemoveTags(ctx context.Context, workspaceID resource.ID, tags []TagSpec) error { - subject, err := s.CanAccess(ctx, rbac.RemoveTagsAction, workspaceID) + subject, err := s.Authorize(ctx, rbac.RemoveTagsAction, &authz.AccessRequest{ID: &workspaceID}) if err != nil { return err } @@ -150,7 +150,7 @@ func (s *Service) RemoveTags(ctx context.Context, workspaceID resource.ID, tags } func (s *Service) ListWorkspaceTags(ctx context.Context, workspaceID resource.ID, opts ListWorkspaceTagsOptions) (*resource.Page[*Tag], error) { - subject, err := s.CanAccess(ctx, rbac.ListWorkspaceTags, workspaceID) + subject, err := s.Authorize(ctx, rbac.ListWorkspaceTags, &authz.AccessRequest{ID: &workspaceID}) if err != nil { return nil, err } diff --git a/internal/workspace/test_helpers.go b/internal/workspace/test_helpers.go index b1dbc1ede..079f9655c 100644 --- a/internal/workspace/test_helpers.go +++ b/internal/workspace/test_helpers.go @@ -57,7 +57,7 @@ func (f *FakeService) ListTags(context.Context, string, ListTagsOptions) (*resou return nil, nil } -func (f *FakeService) GetPolicy(context.Context, resource.ID) (authz.WorkspacePolicy, error) { +func (f *FakeService) GetWorkspacePolicy(context.Context, resource.ID) (authz.WorkspacePolicy, error) { return f.Policy, nil } diff --git a/internal/workspace/tfe.go b/internal/workspace/tfe.go index 01eff07d0..6bebb7036 100644 --- a/internal/workspace/tfe.go +++ b/internal/workspace/tfe.go @@ -31,6 +31,7 @@ type ( tfe struct { *Service *tfeapi.Responder + *authz.Authorizer } ) @@ -411,25 +412,19 @@ func (a *tfe) updateWorkspace(w http.ResponseWriter, r *http.Request, workspaceI } func (a *tfe) convert(from *Workspace, r *http.Request) (*types.Workspace, error) { - subject, err := authz.SubjectFromContext(r.Context()) - if err != nil { - return nil, err - } - policy, err := a.GetPolicy(r.Context(), from.ID) - if err != nil { - return nil, err - } + ctx := r.Context() + accessRequest := &authz.AccessRequest{ID: &from.ID} perms := &types.WorkspacePermissions{ - CanLock: subject.CanAccessWorkspace(rbac.LockWorkspaceAction, policy), - CanUnlock: subject.CanAccessWorkspace(rbac.UnlockWorkspaceAction, policy), - CanForceUnlock: subject.CanAccessWorkspace(rbac.UnlockWorkspaceAction, policy), - CanQueueApply: subject.CanAccessWorkspace(rbac.ApplyRunAction, policy), - CanQueueDestroy: subject.CanAccessWorkspace(rbac.ApplyRunAction, policy), - CanQueueRun: subject.CanAccessWorkspace(rbac.CreateRunAction, policy), - CanDestroy: subject.CanAccessWorkspace(rbac.DeleteWorkspaceAction, policy), - CanReadSettings: subject.CanAccessWorkspace(rbac.GetWorkspaceAction, policy), - CanUpdate: subject.CanAccessWorkspace(rbac.UpdateWorkspaceAction, policy), - CanUpdateVariable: subject.CanAccessWorkspace(rbac.UpdateWorkspaceAction, policy), + CanLock: a.CanAccess(ctx, rbac.LockWorkspaceAction, accessRequest), + CanUnlock: a.CanAccess(ctx, rbac.UnlockWorkspaceAction, accessRequest), + CanForceUnlock: a.CanAccess(ctx, rbac.UnlockWorkspaceAction, accessRequest), + CanQueueApply: a.CanAccess(ctx, rbac.ApplyRunAction, accessRequest), + CanQueueDestroy: a.CanAccess(ctx, rbac.ApplyRunAction, accessRequest), + CanQueueRun: a.CanAccess(ctx, rbac.CreateRunAction, accessRequest), + CanDestroy: a.CanAccess(ctx, rbac.DeleteWorkspaceAction, accessRequest), + CanReadSettings: a.CanAccess(ctx, rbac.GetWorkspaceAction, accessRequest), + CanUpdate: a.CanAccess(ctx, rbac.UpdateWorkspaceAction, accessRequest), + CanUpdateVariable: a.CanAccess(ctx, rbac.UpdateWorkspaceAction, accessRequest), } to := &types.Workspace{ diff --git a/internal/workspace/ui_helpers.go b/internal/workspace/ui_helpers.go index fde9e7d76..2cb7250d3 100644 --- a/internal/workspace/ui_helpers.go +++ b/internal/workspace/ui_helpers.go @@ -12,13 +12,18 @@ import ( ) type uiHelpers struct { - service uiHelpersService + service uiHelpersService + authorizer uiHelpersAuthorizer } type uiHelpersService interface { GetUser(context.Context, userpkg.UserSpec) (*userpkg.User, error) } +type uiHelpersAuthorizer interface { + CanAccess(context.Context, rbac.Action, *authz.AccessRequest) bool +} + type LockButton struct { State string // locked or unlocked Text string // button text @@ -33,7 +38,6 @@ type LockButton struct { func (h *uiHelpers) lockButtonHelper( ctx context.Context, ws *Workspace, - policy authz.WorkspacePolicy, user *userpkg.User, ) (LockButton, error) { var btn LockButton @@ -43,7 +47,7 @@ func (h *uiHelpers) lockButtonHelper( btn.Text = "Unlock" btn.Action = paths.UnlockWorkspace(ws.ID.String()) // A user needs at least the unlock permission - if !user.CanAccessWorkspace(rbac.UnlockWorkspaceAction, policy) { + if !h.authorizer.CanAccess(ctx, rbac.UnlockWorkspaceAction, &authz.AccessRequest{ID: &ws.ID}) { btn.Tooltip = "insufficient permissions" btn.Disabled = true return btn, nil @@ -68,7 +72,7 @@ func (h *uiHelpers) lockButtonHelper( return btn, nil } // User is going to need the force unlock permission - if user.CanAccessWorkspace(rbac.ForceUnlockWorkspaceAction, policy) { + if h.authorizer.CanAccess(ctx, rbac.ForceUnlockWorkspaceAction, &authz.AccessRequest{ID: &ws.ID}) { btn.Text = "Force unlock" btn.Action = paths.ForceUnlockWorkspace(ws.ID.String()) return btn, nil @@ -81,7 +85,7 @@ func (h *uiHelpers) lockButtonHelper( btn.Text = "Lock" btn.Action = paths.LockWorkspace(ws.ID.String()) // User needs at least the lock permission - if !user.CanAccessWorkspace(rbac.LockWorkspaceAction, policy) { + if !h.authorizer.CanAccess(ctx, rbac.LockWorkspaceAction, &authz.AccessRequest{ID: &ws.ID}) { btn.Disabled = true btn.Tooltip = "insufficient permissions" } diff --git a/internal/workspace/ui_helpers_test.go b/internal/workspace/ui_helpers_test.go index c65c2ce41..f5adeeb4d 100644 --- a/internal/workspace/ui_helpers_test.go +++ b/internal/workspace/ui_helpers_test.go @@ -2,9 +2,11 @@ package workspace import ( "context" + "slices" "testing" "github.com/leg100/otf/internal/authz" + "github.com/leg100/otf/internal/rbac" "github.com/leg100/otf/internal/resource" "github.com/leg100/otf/internal/testutils" "github.com/leg100/otf/internal/user" @@ -13,21 +15,21 @@ import ( ) func TestWorkspace_LockButtonHelper(t *testing.T) { - wsID := testutils.ParseID(t, "ws-123") - privilegedUser := &user.User{ID: resource.NewID(resource.UserKind), SiteAdmin: true} - privilegedUser2 := &user.User{ID: resource.NewID(resource.UserKind), SiteAdmin: true} - unprivilegedUser := &user.User{ID: resource.NewID(resource.UserKind), SiteAdmin: false} + bobby := &user.User{ID: resource.NewID(resource.UserKind), Username: "bobby"} + annie := &user.User{ID: resource.NewID(resource.UserKind), Username: "annie"} tests := []struct { - name string - ws *Workspace - user *user.User - want LockButton + name string + lockedBy *user.User + currentUser *user.User + currentUserPermissions []rbac.Action + want LockButton }{ { "unlocked state", - &Workspace{ID: wsID}, - privilegedUser, + nil, + nil, + []rbac.Action{rbac.LockWorkspaceAction}, LockButton{ State: "unlocked", Text: "Lock", @@ -36,8 +38,9 @@ func TestWorkspace_LockButtonHelper(t *testing.T) { }, { "insufficient permissions to lock", - &Workspace{ID: wsID}, - unprivilegedUser, + nil, + nil, + []rbac.Action{}, LockButton{ State: "unlocked", Text: "Lock", @@ -48,8 +51,9 @@ func TestWorkspace_LockButtonHelper(t *testing.T) { }, { "insufficient permissions to unlock", - &Workspace{ID: wsID, Lock: &privilegedUser.ID}, - unprivilegedUser, + bobby, + nil, + []rbac.Action{}, LockButton{ State: "locked", Text: "Unlock", @@ -60,43 +64,74 @@ func TestWorkspace_LockButtonHelper(t *testing.T) { }, { "user can unlock their own lock", - &Workspace{ID: wsID, Lock: &privilegedUser.ID}, - privilegedUser, + bobby, + bobby, + []rbac.Action{rbac.UnlockWorkspaceAction}, LockButton{ State: "locked", Text: "Unlock", - Message: "locked by: janitor", - Tooltip: "locked by: janitor", + Message: "locked by: bobby", + Tooltip: "locked by: bobby", Action: "/app/workspaces/ws-123/unlock", }, }, { - "user can force unlock lock held by different user", - &Workspace{ID: wsID, Lock: &privilegedUser.ID}, - privilegedUser2, + "user without force-unlock permission cannot force-unlock lock held by different user", + bobby, + annie, + []rbac.Action{rbac.UnlockWorkspaceAction}, + LockButton{ + State: "locked", + Text: "Unlock", + Message: "locked by: bobby", + Tooltip: "locked by: bobby", + Disabled: true, + Action: "/app/workspaces/ws-123/unlock", + }, + }, + { + "user with force-unlock permission can force-unlock lock held by different user", + bobby, + annie, + []rbac.Action{rbac.UnlockWorkspaceAction, rbac.ForceUnlockWorkspaceAction}, LockButton{ State: "locked", Text: "Force unlock", Action: "/app/workspaces/ws-123/force-unlock", - Message: "locked by: janitor", - Tooltip: "locked by: janitor", + Message: "locked by: bobby", + Tooltip: "locked by: bobby", }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { helpers := &uiHelpers{ - service: &fakeUIHelpersService{}, + service: &fakeUIHelpersService{lockedBy: tt.lockedBy}, + authorizer: &fakeLockButtonAuthorizer{perms: tt.currentUserPermissions}, + } + ws := &Workspace{ID: testutils.ParseID(t, "ws-123")} + if tt.lockedBy != nil { + ws.Lock = &tt.lockedBy.ID } - got, err := helpers.lockButtonHelper(context.Background(), tt.ws, authz.WorkspacePolicy{}, tt.user) + got, err := helpers.lockButtonHelper(context.Background(), ws, tt.currentUser) require.NoError(t, err) assert.Equal(t, tt.want, got) }) } } -type fakeUIHelpersService struct{} +type fakeUIHelpersService struct { + lockedBy *user.User +} func (f *fakeUIHelpersService) GetUser(context.Context, user.UserSpec) (*user.User, error) { - return &user.User{Username: "janitor"}, nil + return f.lockedBy, nil +} + +type fakeLockButtonAuthorizer struct { + perms []rbac.Action +} + +func (f *fakeLockButtonAuthorizer) CanAccess(ctx context.Context, action rbac.Action, _ *authz.AccessRequest) bool { + return slices.Contains(f.perms, action) } diff --git a/internal/workspace/web.go b/internal/workspace/web.go index 5a88353eb..a19f8740c 100644 --- a/internal/workspace/web.go +++ b/internal/workspace/web.go @@ -49,6 +49,7 @@ type ( teams webTeamClient vcsproviders webVCSProvidersClient client webClient + authorizer webAuthorizer } webTeamClient interface { @@ -62,6 +63,10 @@ type ( GetVCSClient(ctx context.Context, providerID resource.ID) (vcs.Client, error) } + webAuthorizer interface { + CanAccess(context.Context, rbac.Action, *authz.AccessRequest) bool + } + // webClient provides web handlers with access to the workspace service webClient interface { Create(ctx context.Context, opts CreateOptions) (*Workspace, error) @@ -77,7 +82,7 @@ type ( RemoveTags(ctx context.Context, workspaceID resource.ID, tags []TagSpec) error ListTags(ctx context.Context, organization string, opts ListTagsOptions) (*resource.Page[*Tag], error) - GetPolicy(ctx context.Context, workspaceID resource.ID) (authz.WorkspacePolicy, error) + GetWorkspacePolicy(ctx context.Context, workspaceID resource.ID) (authz.WorkspacePolicy, error) SetPermission(ctx context.Context, workspaceID, teamID resource.ID, role rbac.Role) error UnsetPermission(ctx context.Context, workspaceID, teamID resource.ID) error } @@ -171,11 +176,10 @@ func (h *webHandlers) listWorkspaces(w http.ResponseWriter, r *http.Request) { return m } - user, err := authz.SubjectFromContext(r.Context()) - if err != nil { - h.Error(w, err.Error(), http.StatusInternalServerError) - return - } + canCreateWorkspace := h.authorizer.CanAccess( + r.Context(), + rbac.CreateTeamAction, + &authz.AccessRequest{Organization: *params.Organization}) response := struct { organization.OrganizationPage @@ -185,7 +189,7 @@ func (h *webHandlers) listWorkspaces(w http.ResponseWriter, r *http.Request) { CanCreateWorkspace bool }{ OrganizationPage: organization.NewPage(r, "workspaces", *params.Organization), - CanCreateWorkspace: user.CanAccessOrganization(rbac.CreateTeamAction, *params.Organization), + CanCreateWorkspace: canCreateWorkspace, Page: workspaces, TagFilters: tagfilters(), Search: params.Search, @@ -254,11 +258,6 @@ func (h *webHandlers) getWorkspace(w http.ResponseWriter, r *http.Request) { h.Error(w, err.Error(), http.StatusInternalServerError) return } - policy, err := h.client.GetPolicy(r.Context(), id) - if err != nil { - h.Error(w, err.Error(), http.StatusInternalServerError) - return - } user, err := user.UserFromContext(r.Context()) if err != nil { h.Error(w, err.Error(), http.StatusInternalServerError) @@ -290,7 +289,7 @@ func (h *webHandlers) getWorkspace(w http.ResponseWriter, r *http.Request) { return } - lockButton, err := h.lockButtonHelper(r.Context(), ws, policy, user) + lockButton, err := h.lockButtonHelper(r.Context(), ws, user) if err != nil { h.Error(w, err.Error(), http.StatusInternalServerError) return @@ -313,13 +312,13 @@ func (h *webHandlers) getWorkspace(w http.ResponseWriter, r *http.Request) { WorkspacePage: NewPage(r, ws.Name, ws), LockButton: lockButton, VCSProvider: provider, - CanApply: user.CanAccessWorkspace(rbac.ApplyRunAction, policy), - CanAddTags: user.CanAccessWorkspace(rbac.AddTagsAction, policy), - CanRemoveTags: user.CanAccessWorkspace(rbac.RemoveTagsAction, policy), - CanCreateRun: user.CanAccessWorkspace(rbac.CreateRunAction, policy), - CanLockWorkspace: user.CanAccessWorkspace(rbac.LockWorkspaceAction, policy), - CanUnlockWorkspace: user.CanAccessWorkspace(rbac.UnlockWorkspaceAction, policy), - CanUpdateWorkspace: user.CanAccessWorkspace(rbac.UpdateWorkspaceAction, policy), + CanApply: h.authorizer.CanAccess(r.Context(), rbac.ApplyRunAction, &authz.AccessRequest{ID: &ws.ID}), + CanAddTags: h.authorizer.CanAccess(r.Context(), rbac.AddTagsAction, &authz.AccessRequest{ID: &ws.ID}), + CanRemoveTags: h.authorizer.CanAccess(r.Context(), rbac.RemoveTagsAction, &authz.AccessRequest{ID: &ws.ID}), + CanCreateRun: h.authorizer.CanAccess(r.Context(), rbac.CreateRunAction, &authz.AccessRequest{ID: &ws.ID}), + CanLockWorkspace: h.authorizer.CanAccess(r.Context(), rbac.LockWorkspaceAction, &authz.AccessRequest{ID: &ws.ID}), + CanUnlockWorkspace: h.authorizer.CanAccess(r.Context(), rbac.UnlockWorkspaceAction, &authz.AccessRequest{ID: &ws.ID}), + CanUpdateWorkspace: h.authorizer.CanAccess(r.Context(), rbac.UpdateWorkspaceAction, &authz.AccessRequest{ID: &ws.ID}), TagsDropdown: html.DropdownUI{ Name: "tag_name", Available: internal.Diff(getTagNames(), ws.Tags), @@ -363,12 +362,7 @@ func (h *webHandlers) editWorkspace(w http.ResponseWriter, r *http.Request) { return } - policy, err := h.client.GetPolicy(r.Context(), workspaceID) - if err != nil { - h.Error(w, err.Error(), http.StatusInternalServerError) - return - } - user, err := authz.SubjectFromContext(r.Context()) + policy, err := h.client.GetWorkspacePolicy(r.Context(), workspaceID) if err != nil { h.Error(w, err.Error(), http.StatusInternalServerError) return @@ -467,8 +461,8 @@ func (h *webHandlers) editWorkspace(w http.ResponseWriter, r *http.Request) { VCSTriggerAlways: VCSTriggerAlways, VCSTriggerPatterns: VCSTriggerPatterns, VCSTriggerTags: VCSTriggerTags, - CanUpdateWorkspace: user.CanAccessWorkspace(rbac.UpdateWorkspaceAction, policy), - CanDeleteWorkspace: user.CanAccessWorkspace(rbac.DeleteWorkspaceAction, policy), + CanUpdateWorkspace: h.authorizer.CanAccess(r.Context(), rbac.UpdateWorkspaceAction, &authz.AccessRequest{ID: &workspace.ID}), + CanDeleteWorkspace: h.authorizer.CanAccess(r.Context(), rbac.DeleteWorkspaceAction, &authz.AccessRequest{ID: &workspace.ID}), PoolsURL: poolsURL, }) } diff --git a/internal/workspace/web_test.go b/internal/workspace/web_test.go index 435bd68b3..e2c086f4e 100644 --- a/internal/workspace/web_test.go +++ b/internal/workspace/web_test.go @@ -55,7 +55,7 @@ func TestWorkspace_Create(t *testing.T) { } func TestGetWorkspaceHandler(t *testing.T) { - privilegedUser = resource.NewID(resource.UserKind) + bobby := &user.User{ID: resource.NewID(resource.UserKind)} tests := []struct { name string @@ -65,14 +65,19 @@ func TestGetWorkspaceHandler(t *testing.T) { "unlocked", &Workspace{ID: testutils.ParseID(t, "ws-unlocked"), Lock: nil}, }, { - "locked", &Workspace{ID: testutils.ParseID(t, "ws-locked"), Lock: &privilegedUser}, + "locked", &Workspace{ID: testutils.ParseID(t, "ws-locked"), Lock: &bobby.ID}, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { app := &webHandlers{ Renderer: testutils.NewRenderer(t), - client: &FakeService{Workspaces: []*Workspace{tt.workspace}}, + uiHelpers: &uiHelpers{ + service: &fakeUIHelpersService{lockedBy: bobby}, + authorizer: authz.NewAllowAllAuthorizer(), + }, + authorizer: authz.NewAllowAllAuthorizer(), + client: &FakeService{Workspaces: []*Workspace{tt.workspace}}, } q := "/?workspace_id=ws-123" @@ -176,7 +181,8 @@ func TestEditWorkspaceHandler(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { app := &webHandlers{ - Renderer: testutils.NewRenderer(t), + Renderer: testutils.NewRenderer(t), + authorizer: authz.NewAllowAllAuthorizer(), client: &FakeService{ Policy: tt.policy, Workspaces: []*Workspace{tt.ws}, @@ -231,8 +237,9 @@ func TestListWorkspacesHandler(t *testing.T) { workspaces[i-1] = &Workspace{ID: testutils.ParseID(t, fmt.Sprintf("ws-%d", i))} } app := &webHandlers{ - Renderer: testutils.NewRenderer(t), - client: &FakeService{Workspaces: workspaces}, + Renderer: testutils.NewRenderer(t), + authorizer: authz.NewAllowAllAuthorizer(), + client: &FakeService{Workspaces: workspaces}, } t.Run("first page", func(t *testing.T) { @@ -269,8 +276,9 @@ func TestListWorkspacesHandler(t *testing.T) { func TestListWorkspacesHandler_WithLatestRun(t *testing.T) { ws := &Workspace{ID: testutils.ParseID(t, "ws-foo"), LatestRun: &LatestRun{Status: "applied", ID: testutils.ParseID(t, "run-123")}} app := &webHandlers{ - Renderer: testutils.NewRenderer(t), - client: &FakeService{Workspaces: []*Workspace{ws}}, + Renderer: testutils.NewRenderer(t), + authorizer: authz.NewAllowAllAuthorizer(), + client: &FakeService{Workspaces: []*Workspace{ws}}, } r := httptest.NewRequest("GET", "/?organization_name=acme", nil) diff --git a/internal/workspace/workspace_test.go b/internal/workspace/workspace_test.go index 760a16fa8..4f46856f7 100644 --- a/internal/workspace/workspace_test.go +++ b/internal/workspace/workspace_test.go @@ -437,17 +437,17 @@ func TestWorkspace_UpdateConnection(t *testing.T) { } var ( - privilegedUser = resource.NewID(resource.UserKind) - burglarTestID = resource.NewID(resource.UserKind) - runTestID1 = resource.NewID(resource.RunKind) - runTestID2 = resource.NewID(resource.RunKind) + bobby = resource.NewID(resource.UserKind) + burglarTestID = resource.NewID(resource.UserKind) + runTestID1 = resource.NewID(resource.RunKind) + runTestID2 = resource.NewID(resource.RunKind) ) func TestWorkspace_Lock(t *testing.T) { t.Run("lock an unlocked lock", func(t *testing.T) { ws := &Workspace{} assert.False(t, ws.Locked()) - err := ws.Enlock(privilegedUser) + err := ws.Enlock(bobby) require.NoError(t, err) assert.True(t, ws.Locked()) }) @@ -459,29 +459,29 @@ func TestWorkspace_Lock(t *testing.T) { }) t.Run("user cannot lock a locked workspace", func(t *testing.T) { ws := &Workspace{Lock: &runTestID1} - err := ws.Enlock(privilegedUser) + err := ws.Enlock(bobby) require.Equal(t, ErrWorkspaceAlreadyLocked, err) }) } func TestWorkspace_Unlock(t *testing.T) { t.Run("cannot unlock workspace already unlocked", func(t *testing.T) { - err := (&Workspace{}).Unlock(privilegedUser, false) + err := (&Workspace{}).Unlock(bobby, false) require.Equal(t, ErrWorkspaceAlreadyUnlocked, err) }) t.Run("user can unlock their own lock", func(t *testing.T) { - ws := &Workspace{Lock: &privilegedUser} - err := ws.Unlock(privilegedUser, false) + ws := &Workspace{Lock: &bobby} + err := ws.Unlock(bobby, false) require.NoError(t, err) assert.False(t, ws.Locked()) }) t.Run("user cannot unlock another user's lock", func(t *testing.T) { - ws := &Workspace{Lock: &privilegedUser} + ws := &Workspace{Lock: &bobby} err := ws.Unlock(burglarTestID, false) require.Equal(t, ErrWorkspaceLockedByDifferentUser, err) }) t.Run("user can unlock a lock by force", func(t *testing.T) { - ws := &Workspace{Lock: &privilegedUser} + ws := &Workspace{Lock: &bobby} err := ws.Unlock(burglarTestID, true) require.NoError(t, err) assert.False(t, ws.Locked()) diff --git a/sqlc.yaml b/sqlc.yaml index 73f7adbe8..d05bb6326 100644 --- a/sqlc.yaml +++ b/sqlc.yaml @@ -73,6 +73,9 @@ sql: go_type: type: "AgentPool" pointer: true + - db_type: "workspace_permissions" + go_type: + type: "WorkspacePermission" - column: "agent_pools.agent_pool_id" go_type: import: "github.com/leg100/otf/internal/resource"