fix(coderd): enforce required external auth on workspace create#26314
Draft
dylanhuff-at-coder wants to merge 1 commit into
Draft
fix(coderd): enforce required external auth on workspace create#26314dylanhuff-at-coder wants to merge 1 commit into
dylanhuff-at-coder wants to merge 1 commit into
Conversation
Required external auth was only enforced by client-side preflight checks in the CLI and UI, so creating a workspace directly through the REST API succeeded even when the owner had not authenticated with a required provider. The build then ran with an empty token and produced a broken workspace. Workspace creation now validates the workspace owner's external auth links against the template version's non-optional providers before any workspace row is inserted or prebuilt workspace is claimed, returning 403 with one validation entry per missing provider. The owner is the subject of the check, not the initiator, because build-time token injection uses the owner's links.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Required external auth (
coder_external_authwithoptional = false) is only enforced by client-side preflight checks in the CLI and dashboard. Creating a workspace directly through the REST API (POST /api/v2/users/{user}/workspaces) succeeds even when the workspace owner has never authenticated with a required provider. Token injection at job acquisition silently skips missing or invalid tokens, so the build "succeeds" with an empty token and produces a broken workspace.Fix
Workspace creation now validates the workspace owner's external auth server-side:
GET /templateversions/{id}/external-authhandler logic is refactored into a reusabletemplateVersionExternalAuthForUserhelper that accepts an explicit user ID. Endpoint behavior is unchanged.createWorkspaceresolves the exact template version in use (req.TemplateVersionID, falling back totemplate.ActiveVersionID) and rejects the request with 403 when any non-optional provider is unauthenticated for the owner. The check runs before any workspace row is inserted and before any prebuilt workspace is claimed, and outside any retrying DB transaction (token refresh makes remote OAuth calls and must not be replayed).dbauthz).validationsentry per missing provider (field: "external_auth",detail: <provider-id>), with display names in the human-readable detail (falling back to the provider ID), so API clients can react programmatically and fetch authenticate URLs from the existing preflight endpoint.Customer-visible behavior (release note worthy)
optional = truein the template.Follow-ups (intentionally not in this PR)
Tests
TestCreateWorkspaceExternalAuthincoderd/workspaces_test.go: missing required auth returns 403 with structured validations and no workspace row; owner-vs-initiator (authenticated admin creating for an unauthenticated member is rejected, succeeds after the member authenticates); optional providers do not block; invalid/revoked tokens (via a failingValidateURL) are treated as unauthenticated; display-name fallback to provider ID.TestWorkspaceDeleteSuspendedUserexpectations updated: creation now performs one additional token validation.createWorkspaceand are gated by the same check, but exercising a real claim requires the enterprise prebuilds harness, which is disproportionate for this PR.go test ./coderd -run 'TestCreateWorkspaceExternalAuth|TestTemplateVersionsExternalAuth|TestWorkspaceDeleteSuspendedUser|TestPostWorkspacesByOrganization|TestWorkspace$|TestWorkspacesByUser'(also with-race),go test ./cli -run TestCreateWithGitAuth,make lint/go,make lint/emdash, full pre-commit hooks.Research and implementation decisions (subagent findings)
Three parallel research passes validated the diagnosis and compared three candidate layers before implementation:
createWorkspace(chosen). Only layer that can return a 4xx with no workspace row. Covers allcreateWorkspacecallers (direct API, org-member endpoint, tasks/chats creation, prebuild claims) while leaving system paths (prebuild provisioning, autostart lifecycle executor) exempt by construction.wsbuilder(rejected for this PR).Build()executes inside a RepeatableRead transaction with up to 5 serialization retries, and both HTTP paths wrap it in an outer transaction. Remote OAuth refresh inside a retried transaction can replay refreshes; GitHub rotates refresh tokens on use, so a replay can permanently invalidate the link. A DB-only variant remains viable for future start-transition enforcement.Implementation details that came out of review:
provisionerdserverinjects the owner's tokens at job time; the initiator's auth state is irrelevant to what terraform receives.dbauthz.AsSystemRestrictedis required to read another user's links (policy.ActionReadPersonal) and carries the mandatory//nolint:gocriticjustification enforced byscripts/rules.go.RefreshTokenperforms the same remote validation the preflight endpoint already does for the dashboard's 1s polling; provider outages fail closed, consistent with job-time behavior.ExternalAuthProvidersNamestreat all providers as required (Optional=false), consistent with existing client-side preflight semantics.Fixes PLAT-241.
Note
This PR was generated by Coder Agents on behalf of @dylanhuff-at-coder.