wip

internal/authz/authorizer.go

@@ -37,8 +37,18 @@ func (a *Authorizer) RegisterAuthorizer(kind resource.Kind, authorizer ResourceA
 	a.resourceAuthorizers[kind] = authorizer
 }
 
+// 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 to a specific resource kind to the workspace ID the
+// 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
@@ -82,6 +92,24 @@ func (a *Authorizer) CanAccess(ctx context.Context, action rbac.Action, req *Acc
 			// Authorize workspace ID instead
 			req.ID = &workspaceID
 		}
+		// Then check if the resource kind - including the case where the
+		// resource kind has been resolved to a workspace - is registered for
+		// its ID to be resolved to an oranization name. This is ony necessary
+		// if the organization has not been specified in the access request.
+		if req.Organization == "" {
+			if resolver, ok := a.organizationResolvers[req.ID.Kind()]; ok {
+				organization, err := resolver(ctx, *req.ID)
+				if err != nil {
+					a.Error(err, "authorization failure",
+						"resource", req,
+						"action", action.String(),
+						"subject", subj,
+					)
+					return nil, err
+				}
+				req.Organization = organization
+			}
+		}
 	}
 	// Allow subject to determine whether it is allowed to access resource.
 	if subj.CanAccess(action, req) {
@@ -113,6 +141,8 @@ func (a *Authorizer) CanAccess(ctx context.Context, action rbac.Action, req *Acc
 	return nil, internal.ErrAccessNotPermitted
 }
 
+// CanAccessDecision is a helper to boil down an access request to a true/false
+// decision, with any error encountered interpreted as false.
 func (a *Authorizer) CanAccessDecision(ctx context.Context, action rbac.Action, req *AccessRequest) bool {
 	_, err := a.CanAccess(ctx, action, req)
 	return err != nil
@@ -129,14 +159,15 @@ type AccessRequest struct {
 }
 
 func (r *AccessRequest) LogValue() slog.Value {
-	// Compose error message
-	var resource string
 	if r == nil {
-		resource = "site"
-	} else if r.Organization != nil {
-		resource = *r.Organization
-	} else if r.ID != nil {
-		resource = r.ID.String()
+		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...)
 	}
-	return slog.GroupValue(slog.String("resource", resource))
 }

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,

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
 }
@@ -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.CanAccess(ctx, rbac.GetConfigurationVersionAction, &authz.AccessRequest{ID: &cvID})
 	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.CanAccess(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)
-}

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.CanAccess(ctx, rbac.DownloadConfigurationVersionAction, &authz.AccessRequest{ID: &cvID})
 	if err != nil {
 		return nil, err
 	}

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

internal/github/service.go

@@ -10,7 +10,6 @@ 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/sql"
 	"github.com/leg100/otf/internal/vcs"
@@ -20,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 {
@@ -38,15 +36,15 @@ type (
 
 		GithubHostname      string
 		SkipTLSVerification bool
+		Authorizer          *authz.Authorizer
 	}
 )
 
 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{

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),
 	})
 }
 

internal/logs/service.go

@@ -16,8 +16,7 @@ import (
 type (
 	Service struct {
 		logr.Logger
-
-		run authz.Authorizer
+		*authz.Authorizer
 
 		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,
+		Authorizer: 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.CanAccess(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.CanAccess(ctx, rbac.TailLogsAction, &authz.AccessRequest{ID: &opts.RunID})
 	if err != nil {
 		return nil, err
 	}