fix authorization with pk given

f0ssel · f0ssel · commit c7c00e70e758 · 2024-07-24T17:42:22.000Z
diff --git a/enterprise/coderd/provisionerdaemons.go b/enterprise/coderd/provisionerdaemons.go
@@ -83,41 +83,53 @@ type provisionerDaemonAuth struct {
 	authorizer rbac.Authorizer
 }
 
-// authorize returns mutated tags and true if the given HTTP request is authorized to access the provisioner daemon
-// protobuf API, and returns nil, false otherwise.
-func (p *provisionerDaemonAuth) authorize(r *http.Request, orgID uuid.UUID, tags map[string]string) (map[string]string, bool) {
+// authorize returns mutated tags if the given HTTP request is authorized to access the provisioner daemon
+// protobuf API, and returns nil, err otherwise.
+func (p *provisionerDaemonAuth) authorize(r *http.Request, orgID uuid.UUID, tags map[string]string) (map[string]string, error) {
 	ctx := r.Context()
-	apiKey, ok := httpmw.APIKeyOptional(r)
-	if ok {
+	apiKey, apiKeyOK := httpmw.APIKeyOptional(r)
+	pk, pkOK := httpmw.ProvisionerKeyAuthOptional(r)
+	provAuth := httpmw.ProvisionerDaemonAuthenticated(r)
+	if !provAuth && !apiKeyOK {
+		return nil, xerrors.New("no API key or provisioner key provided")
+	}
+	if apiKeyOK && pkOK {
+		return nil, xerrors.New("Both API key and provisioner key authentication provided. Only one is allowed.")
+	}
+
+	if apiKeyOK {
 		tags = provisionersdk.MutateTags(apiKey.UserID, tags)
 		if tags[provisionersdk.TagScope] == provisionersdk.ScopeUser {
 			// Any authenticated user can create provisioner daemons scoped
 			// for jobs that they own,
-			return tags, true
+			return tags, nil
 		}
 		ua := httpmw.UserAuthorization(r)
-		if err := p.authorizer.Authorize(ctx, ua, policy.ActionCreate, rbac.ResourceProvisionerDaemon.InOrg(orgID)); err == nil {
-			// User is allowed to create provisioner daemons
-			return tags, true
+		err := p.authorizer.Authorize(ctx, ua, policy.ActionCreate, rbac.ResourceProvisionerDaemon.InOrg(orgID))
+		if err != nil {
+			if !provAuth {
+				return nil, xerrors.New("user unauthorized")
+			}
+
+			// If using provisioner key / PSK auth, the daemon is, by definition, scoped to the organization.
+			tags = provisionersdk.MutateTags(uuid.Nil, tags)
+			return tags, nil
 		}
-	}
 
-	// Check for provisioner key or PSK auth.
-	provAuth := httpmw.ProvisionerDaemonAuthenticated(r)
-	if !provAuth {
-		return nil, false
+		// User is allowed to create provisioner daemons
+		return tags, nil
 	}
 
 	pk, ok := httpmw.ProvisionerKeyAuthOptional(r)
 	if ok {
 		if pk.OrganizationID != orgID {
-			return nil, false
+			return nil, xerrors.New("provisioner key unauthorized")
 		}
 	}
 
 	// If using provisioner key / PSK auth, the daemon is, by definition, scoped to the organization.
 	tags = provisionersdk.MutateTags(uuid.Nil, tags)
-	return tags, true
+	return tags, nil
 }
 
 // Serves the provisioner daemon protobuf API over a WebSocket.
@@ -180,12 +192,13 @@ func (api *API) provisionerDaemonServe(rw http.ResponseWriter, r *http.Request)
 		api.Logger.Warn(ctx, "unnamed provisioner daemon")
 	}
 
-	tags, authorized := api.provisionerDaemonAuth.authorize(r, organization.ID, tags)
-	if !authorized {
-		api.Logger.Warn(ctx, "unauthorized provisioner daemon serve request", slog.F("tags", tags))
+	tags, err := api.provisionerDaemonAuth.authorize(r, organization.ID, tags)
+	if err != nil {
+		api.Logger.Warn(ctx, "unauthorized provisioner daemon serve request", slog.F("tags", tags), slog.Error(err))
 		httpapi.Write(ctx, rw, http.StatusForbidden,
 			codersdk.Response{
 				Message: fmt.Sprintf("You aren't allowed to create provisioner daemons with scope %q", tags[provisionersdk.TagScope]),
+				Detail:  err.Error(),
 			},
 		)
 		return
