Skip to content

Conversation

@ThomasK33
Copy link
Member

Fix API key scope authorization for workspace creation

This PR fixes an issue with API key scopes and workspace creation. Previously, the RBAC policy allowed creation of resources with an empty ID in the allow list, but this approach was inconsistent with how other permissions work.

The changes:

  1. Update the Rego policy to properly handle "create" actions by checking if the resource type is in the allow list
  2. Add tests to verify that workspace creation requires a matching type entry in the allow list
  3. Add tests for scope filtering to ensure proper behavior
  4. Add a test to verify that authorization requires a scope
  5. Add a test to ensure workspace agent scope allow lists contain the correct elements

These changes ensure that API key scopes are properly enforced for creation operations while maintaining backward compatibility.

Copy link
Member Author

ThomasK33 commented Sep 29, 2025

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@ThomasK33 ThomasK33 linked an issue Sep 29, 2025 that may be closed by this pull request
@ThomasK33 ThomasK33 force-pushed the thomask33/09-28-add_api_key_audit_metadata branch from 5ac8d9c to bd1ff54 Compare September 29, 2025 13:25
@ThomasK33 ThomasK33 force-pushed the thomask33/09-29-feat_typed_rbac_allow_list branch from f53f9aa to 7a79289 Compare September 29, 2025 13:25
@ThomasK33 ThomasK33 marked this pull request as ready for review September 29, 2025 16:12
@ThomasK33 ThomasK33 requested a review from Emyrk as a code owner September 29, 2025 16:12
@ThomasK33 ThomasK33 requested a review from johnstcn September 29, 2025 16:12
@ThomasK33 ThomasK33 force-pushed the thomask33/09-29-feat_typed_rbac_allow_list branch from 7a79289 to f38d137 Compare September 29, 2025 16:15
@ThomasK33 ThomasK33 force-pushed the thomask33/09-28-add_api_key_audit_metadata branch from bd1ff54 to eedeed8 Compare September 29, 2025 16:15
mafredri and others added 26 commits October 23, 2025 19:29
This PR uses the same sha256 hashing technique as we use for APIKeys. So
now all randomly generated secrets will be hashed with sha256 for
consistency.

This is a breaking change for the oauth tokens. Since oauth is only
allowed for dev builds and experimental, this is ok.
Closes coder/internal#921

The flake in the linked issue was caused by the startup script taking longer than 1 second in CI. The existing conditional, that the startup script duration was under a second, was incorrect; the correct conditional is that the metric exists with the `success` label set to `true`.
Second flake for this test today 😮‍💨.

Flake seen here, though I couldn't replicate this locally, some CI
exclusive networking issue.

https://github.com/coder/coder/actions/runs/18770305895/job/53553517887?pr=20448
```
    agent_test.go:3619: 
        	Error Trace:	/home/runner/work/coder/coder/agent/agent_test.go:3619
        	Error:      	Received unexpected error:
        	            	expected 1, got 0.000000:
        	            	    github.com/coder/coder/v2/agent_test.TestAgent_Metrics_SSH.func7
        	            	        /home/runner/work/coder/coder/agent/agent_test.go:3557
        	Test:       	TestAgent_Metrics_SSH
        	Messages:   	check fn for coderd_agentstats_currently_reachable_peers failed
```
This value is incremented by a successful ping to the peer from the
agent, which is dependent on all the networking code, which I think is
definitely out of scope of this test for agent metrics. So, we'll just
assert that the metrics exist with the correct labels (`derp`, `p2p`)
Pipes through the Task's ID and prompt into the provisioner. This is
required to support the new `coder_ai_task.prompt` field and modified
`coder_ai_task.id` field.
…20336)

As we're moving away from the SidebarAppID nomenclature, this PR
introduces a new `TaskAppID` field to `codersdk.WorkspaceBuild` and
deprecates the `AITaskSidebarAppID` field. They both contain the same
value.
…20384)

Co-authored-by: github-actions[bot]
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: M Atif Ali <atif@coder.com>
Co-authored-by: Ethan Dickson <ethan@coder.com>
Add API key allow list to the SDK

This PR adds an allow list to API keys in the SDK. The allow list is a list of targets that the API key is allowed to access. If the allow list is empty, a default allow list with a single entry that allows access to all resources is created.

The changes include:

- Adding a default allow list when generating an API key if none is provided
- Adding allow list to the API key response in the SDK
- Converting database allow list entries to SDK format in the API response
- Adding tests to verify the default allow list behavior



Fixes #19854
Updates the UI to use the new API endpoints for tasks and use its new
data model.

Disclaimer: Since the base data model for tasks changed, we had to do a
quite large refactor and I'm sorry for that 🙏, but you'll notice most of
the changes are to adjust the types.

Closes coder/internal#976

---------

Co-authored-by: Bruno Quaresma <bruno_nonato_quaresma@hotmail.com>
Relates to coder/internal#1093

This is the first of N pull requests to allow coder script ordering.
It introduces what is for now dead code, but paves the way for various
interfaces that allow coder scripts and other processes to depend on one
another via CLI commands and terraform configurations.

The next step is to add reactivity to the graph, such that changes in
the status of one vertex will propagate and allow other vertices to
change their own statuses.

Concurrency and stress testing yield the following:

CPU Profile:
<img width="1512" height="862" alt="Screenshot 2025-10-17 at 10 38 52"
src="https://github.com/user-attachments/assets/f46cf1a2-a0b2-4c02-81a0-069798108ee5"
/>

Mem Profile:
<img width="1512" height="862" alt="Screenshot 2025-10-17 at 10 38 01"
src="https://github.com/user-attachments/assets/45be1235-fff6-45ba-a50d-db9880377bd0"
/>

Predictably, lock contention and memory allocation are the largest
components of this system under stress. Nothing seems untoward.
This commit adds comprehensive support for token scoping and 
allow-listing in the CLI token management commands:

- Add --scope flag to create scoped tokens with specific 
  permissions
- Add --allow flag to create tokens restricted to specific 
  resources
- Display scopes and allow-list in token list/view commands
- Add tokens view subcommand for detailed token inspection
- Update help text and documentation with scoping examples
- Add comprehensive test coverage for new functionality
Ensure composite scopes retain required defaults when merging API keys.
Add MergeDefaultsForMissingTypes to union composite defaults safely.
Cover API key scope expansion and allow list merging with unit tests.
Expose SafeAllowList in Subject logging and register user:read scope.
Implements RFC-compliant PATCH /users/{user}/keys/tokens/{keyname}
endpoint enabling updates to API key authorization parameters
including scopes, allow lists, and expiration times.

Key changes:
- Add patchToken handler with comprehensive validation
- Extract scope and allow list normalization into reusable functions  
- Implement authorization checks preventing non-owners from minting
  elevated scopes with wildcard allow lists
- Add UpdateAPIKeyAuthorization database query with audit support
- Generate OpenAPI documentation and TypeScript client types

The endpoint supports partial updates with proper authorization
context validation and maintains backward compatibility with
existing token creation patterns.
The allow_list for RBAC scopes has been updated to use typed elements
of the form `{type: string, id: string}` instead of raw string IDs.

This change enables more granular authorization policies. Specifically, it
modifies the behavior for "create" actions. A create operation is now
permitted if the scope's allow_list contains an entry matching the
resource type, even without a specific ID. This is useful for scenarios
like workspace agent tokens which need to create resources but cannot
know the ID ahead of time.

For all other actions (e.g., read, update, delete), the allow_list
must still contain an entry that matches both the type and the specific
ID of the resource.

The Rego policy, relevant Go code, and tests have been updated to
implement and verify this new typed allow_list behavior.
@ThomasK33 ThomasK33 force-pushed the thomask33/09-29-feat_typed_rbac_allow_list branch from 4bdf346 to d94abdc Compare October 24, 2025 14:20
@ThomasK33
Copy link
Member Author

Superseded by #20412.

@ThomasK33 ThomasK33 closed this Oct 24, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Oct 24, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OPA/Policy: ensure scope and allow-list checks