Merge remote-tracking branch 'origin/authzquerier_layer' into authzquerier_layer

johnstcn · johnstcn · commit b96bb216a628 · 2023-02-07T15:30:39.000Z
diff --git a/coderd/activitybump.go b/coderd/activitybump.go
@@ -10,19 +10,15 @@ import (
 	"golang.org/x/xerrors"
 
 	"cdr.dev/slog"
-	"github.com/coder/coder/coderd/authzquery"
 	"github.com/coder/coder/coderd/database"
-	"github.com/coder/coder/coderd/rbac"
 )
 
 // activityBumpWorkspace automatically bumps the workspace's auto-off timer
 // if it is set to expire soon.
-func activityBumpWorkspace(log slog.Logger, db database.Store, workspaceID uuid.UUID) {
+func activityBumpWorkspace(ctx context.Context, log slog.Logger, db database.Store, workspaceID uuid.UUID) {
 	// We set a short timeout so if the app is under load, these
 	// low priority operations fail first.
-	ctx, cancel := context.WithTimeout(context.Background(), time.Second*15)
-	// We always want to use the **system** authz context for this.
-	ctx = authzquery.WithAuthorizeSystemContext(ctx, rbac.RolesAdminSystem())
+	ctx, cancel := context.WithTimeout(ctx, time.Second*15)
 	defer cancel()
 
 	err := db.InTx(func(s database.Store) error {
diff --git a/coderd/authzquery/context.go b/coderd/authzquery/context.go
@@ -30,47 +30,6 @@ func WithAuthorizeContext(ctx context.Context, actor rbac.Subject) context.Conte
 	return context.WithValue(ctx, authContextKey{}, actor)
 }
 
-// WithWorkspaceAgentTokenContext returns a context with a workspace agent token
-// authorization subject. A workspace agent authorization subject is the
-// workspace owner's authorization subject + a workspace agent scope.
-//
-// TODO: The arguments and usage of this function are not finalized. It might
-// be a bit awkward to use at present. The arguments are required to build the
-// required authorization context. The arguments should be the owner of the
-// workspace authorization roles.
-func WithWorkspaceAgentTokenContext(ctx context.Context, workspaceID uuid.UUID, actorID uuid.UUID, roles rbac.ExpandableRoles, groups []string) context.Context {
-	// TODO: This workspace ID should be applied in the scope.
-	var _ = workspaceID
-	return context.WithValue(ctx, authContextKey{}, rbac.Subject{
-		ID:    actorID.String(),
-		Roles: roles,
-		Scope: rbac.Scope{
-			Role: rbac.Role{
-				Name:        "workspace-agent-scope",
-				DisplayName: "Workspace Agent Scope",
-				// TODO: More permissions are needed for the agent to work.
-				Site: []rbac.Permission{
-					{
-						ResourceType: rbac.ResourceWorkspace.Type,
-						Action:       rbac.ActionRead,
-					},
-					{
-						ResourceType: rbac.ResourceWorkspace.Type,
-						Action:       rbac.ActionRead,
-					},
-					// TODO: Read the workspace owner user.
-				},
-				Org:  map[string][]rbac.Permission{},
-				User: []rbac.Permission{},
-			},
-			// TODO: We need to whitelist more resources such as the workspace
-			// owner.
-			AllowIDList: []string{workspaceID.String()},
-		},
-		Groups: groups,
-	})
-}
-
 // ActorFromContext returns the authorization subject from the context.
 // All authentication flows should set the authorization subject in the context.
 // If no actor is present, the function returns false.
diff --git a/coderd/rbac/scopes.go b/coderd/rbac/scopes.go
@@ -43,6 +43,9 @@ func (s Scope) Name() string {
 	return s.Role.Name
 }
 
+// WorkspaceAgentScope returns a scope that is the same as ScopeAll but can only
+// affect resources in the allow list. Only a scope is returned as the roles
+// should come from the workspace owner.
 func WorkspaceAgentScope(workspaceID, ownerID uuid.UUID) Scope {
 	allScope, err := ScopeAll.Expand()
 	if err != nil {
@@ -58,6 +61,7 @@ func WorkspaceAgentScope(workspaceID, ownerID uuid.UUID) Scope {
 		AllowIDList: []string{
 			workspaceID.String(),
 			ownerID.String(),
+			// TODO: Might want to include the template the workspace uses too?
 		},
 	}
 }
diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go
@@ -897,7 +897,7 @@ func (api *API) workspaceAgentReportStats(rw http.ResponseWriter, r *http.Reques
 		slog.F("payload", req),
 	)
 
-	activityBumpWorkspace(api.Logger.Named("activity_bump"), api.Database, workspace.ID)
+	activityBumpWorkspace(ctx, api.Logger.Named("activity_bump"), api.Database, workspace.ID)
 
 	payload, err := json.Marshal(req)
 	if err != nil {

Original file line number	Diff line number	Diff line change
`@@ -43,6 +43,9 @@ func (s Scope) Name() string {`
`43`	`43`	`return s.Role.Name`
`44`	`44`	`}`
`45`	`45`
	`46`	`+// WorkspaceAgentScope returns a scope that is the same as ScopeAll but can only`
	`47`	`+// affect resources in the allow list. Only a scope is returned as the roles`
	`48`	`+// should come from the workspace owner.`
`46`	`49`	`func WorkspaceAgentScope(workspaceID, ownerID uuid.UUID) Scope {`
`47`	`50`	`allScope, err := ScopeAll.Expand()`
`48`	`51`	`if err != nil {`
`@@ -58,6 +61,7 @@ func WorkspaceAgentScope(workspaceID, ownerID uuid.UUID) Scope {`
`58`	`61`	`AllowIDList: []string{`
`59`	`62`	`workspaceID.String(),`
`60`	`63`	`ownerID.String(),`
	`64`	`+ // TODO: Might want to include the template the workspace uses too?`
`61`	`65`	`},`
`62`	`66`	`}`
`63`	`67`	`}`
Original file line number	Diff line number	Diff line change
`@@ -897,7 +897,7 @@ func (api API) workspaceAgentReportStats(rw http.ResponseWriter, r http.Reques`
`897`	`897`	`slog.F("payload", req),`
`898`	`898`	`)`
`899`	`899`
`900`		`- activityBumpWorkspace(api.Logger.Named("activity_bump"), api.Database, workspace.ID)`
	`900`	`+ activityBumpWorkspace(ctx, api.Logger.Named("activity_bump"), api.Database, workspace.ID)`
`901`	`901`
`902`	`902`	`payload, err := json.Marshal(req)`
`903`	`903`	`if err != nil {`