mirror of
https://github.com/coder/coder.git
synced 2026-10-31 19:36:41 +08:00
cbb36e613d29930aedf45b69ec9e10c35b69655a
3
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Bobby Ho
|
663f41ffa9 |
feat: derive OAuth2 client type from token_endpoint_auth_method (#28043)
Adds an OAuth2 client type (public vs confidential, RFC 7591 §2) derived from the requested auth method instead of hardcoded confidential. The type is stored and guarded here, but no endpoint enforces on it yet; public behavior at the token endpoint follows in the next PR in the stack. - Client type is derived once and reused by both registration and redirect URI validation, so they can't disagree - IsPublic() fails closed: an unrecognized or missing value reads as confidential - RFC 7592 update (PUT) now rejects moving a client between public and confidential (400) instead of silently flipping it when the auth method is omitted - Discovery still doesn't advertise "none"; follows once the token endpoint honors it ### Behavior by client shape `client_type` is derived from `token_endpoint_auth_method` at POST and pinned at PUT. RFC 7592 GET/PUT authenticate with the registration access token, not the client secret, so neither endpoint reads a secret. | Registered with | Stored `client_type` / method | GET reports | PUT that flips the method | |------------------------------------|---------------------------------------|-----------------------|-----------------------------------------| | omitted, or `client_secret_basic` | `confidential` / `client_secret_basic` | `client_secret_basic` | `none` → 400 `invalid_client_metadata` | | `none` (new) | `public` / `none` | `none` | `client_secret_*` → 400 `invalid_client_metadata` | | `none` (before this PR) | `confidential` / `none` | `none` | either → 200, type stays `confidential` | - PUT still replaces every other RFC 7591 field. `client_type` is the only pinned one; the method may move within a type (`client_secret_basic` ↔ `client_secret_post`). - Row 3 is the only shape where the two columns disagree. The guard fires only on a method change that crosses the type line, so those clients keep managing themselves instead of being locked out of their own configuration endpoint. - The token endpoint does not consult `client_type` yet, so every client still authenticates with a secret and registration still issues one. Split out of #27873, second in the stack (on top of #28041). Refs https://linear.app/codercom/issue/ENG-3029/oauth2-support-public-client |
||
|
Kyle Carberry
|
aaa0dacdb3 |
fix: infer workspace claim time from build history for /agents delete dialog (#25057)
Closes [CODAGT-317](https://linear.app/codercom/issue/CODAGT-317/pr-workspaces-sometimes-require-name-confirmation-to-delete). ## Problem The `/agents` archive-and-delete molly-guard (typing the workspace name) was firing for chats that had clearly created their own workspace. The heuristic in `resolveArchiveAndDeleteAction` decides whether confirmation is needed by comparing the workspace's `created_at` against the chat's `created_at`: ```ts return new Date(workspaceCreatedAt) >= new Date(chatCreatedAt); ``` That assumption breaks for **prebuilt workspaces**. `ClaimPrebuiltWorkspace` rewrites `owner_id`, `name`, `updated_at`, `last_used_at`, etc., but **never touches `created_at`**, which still reflects when the prebuild was provisioned by the reconciler, often hours before the chat exists. Result: every prebuild-claimed workspace looks pre-existing, so the molly-guard fires. Concrete example from a real chat: | Field | Value | |---|---| | `chat.created_at` | `2026-05-07T15:12:23Z` | | `workspace.created_at` (provision) | `2026-05-07T14:22:24Z` | | `latest_build.created_at` (claim) | `2026-05-07T15:19:09Z` | `14:22:24 < 15:12:23` so `isWorkspaceAutoCreated` returned false even though the chat issued the claim. ## Fix (frontend-only) Derive the moment a workspace was acquired from existing build history rather than relying on `workspace.created_at`: - Build #1 initiator = prebuilds system user → workspace was a prebuild → use `build_2.created_at` (the claim build) as the acquisition time. - Build #1 initiator = real user → workspace was created from scratch → use `workspace.created_at` (unchanged behavior). - Unclaimed prebuild or no build history → return `null` (force confirmation; safe degradation for a destructive flow). The resolver fetches the build list via the existing `getWorkspaceBuilds` endpoint when the dialog might fire. No new column, no migration, no schema change. Works retroactively for all existing claimed prebuilds; no backfill needed. The prebuilds system user UUID is exposed via `codersdk.PrebuildsSystemUserID` and typegen'd to `typesGenerated.ts`. `coderd/database.PrebuildsSystemUserID` parses that constant via `uuid.MustParse` so the two cannot drift; if the codersdk literal ever changes, package init fails fast. ## History The first draft of this PR added a `workspaces.claimed_at` column populated by `ClaimPrebuiltWorkspace`. After review feedback from @johnstcn pointing out that the same fact is already implicit in build history, I pivoted to the frontend-only approach. Subsequent review notes consolidated the prebuilds system user UUID into a single typegen'd constant. ## Why not the other open PRs - **#25055** (`chatKey` cache fallback) only fixes a different cache-miss path; it explicitly notes it does not address `created_at < chat.created_at`. - **#25053** (`chats.workspace_auto_created` boolean) puts the truth on the wrong side of the schema: "this workspace was claimed at time T" is a property of the workspace, not the chat. The MCP plumbing it adds is also unnecessary now that the same answer is available from build history. ## Test plan - `pnpm vitest run --project=unit src/pages/AgentsPage/utils/agentWorkspaceUtils.test.ts` — 40/40 pass; new cases cover prebuild claim before/after chat, unclaimed prebuild, missing-build-history fallback, and the fetch-skip when the chat is not in cache. - `pnpm lint:types`, `pnpm check`, `make pre-commit`. <details> <summary>Disclosure</summary> Opened on behalf of @kylecarbs by [Coder Agents](https://coder.com/coder-agents). </details> |
||
|
Susana Ferreira
|
72f7d70bab |
feat: allow TemplateAdmin to delete prebuilds via auth layer (#18333)
## Description This PR adds support for deleting prebuilt workspaces via the authorization layer. It introduces special-case handling to ensure that `prebuilt_workspace` permissions are evaluated when attempting to delete a prebuilt workspace, falling back to the standard `workspace` resource as needed. Prebuilt workspaces are a subset of workspaces, identified by having `owner_id` set to `PREBUILD_SYSTEM_USER`. This means: * A user with `prebuilt_workspace.delete` permission is allowed to **delete only prebuilt workspaces**. * A user with `workspace.delete` permission can **delete both normal and prebuilt workspaces**. ⚠️ This implementation is scoped to **deletion operations only**. No other operations are currently supported for the `prebuilt_workspace` resource. To delete a workspace, users must have the following permissions: * `workspace.read`: to read the current workspace state * `update`: to modify workspace metadata and related resources during deletion (e.g., updating the `deleted` field in the database) * `delete`: to perform the actual deletion of the workspace ## Changes * Introduced `authorizeWorkspace()` helper to handle prebuilt workspace authorization logic. * Ensured both `prebuilt_workspace` and `workspace` permissions are checked. * Added comments to clarify the current behavior and limitations. * Moved `SystemUserID` constant from the `prebuilds` package to the `database` package `PrebuildsSystemUserID` to resolve an import cycle (commit https://github.com/coder/coder/pull/18333/commits/f24e4ab4b6f0a56726fd04be2d7302c9fdb52d53). * Update middleware `ExtractOrganizationMember` to include system user members. |