[1/n] Add id column on tokens and sessions
#8137
Merged
Conversation
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
david-crespo
commented
May 12, 2025
david-crespo
force-pushed
the
token-session-ids
branch
from
48c4c68 to
7f377d8
Compare
david-crespo
commented
May 13, 2025
| @@ -957,7 +957,7 @@ authz_resource! { | |||
| authz_resource! { | |||
| name = "DeviceAccessToken", | |||
| parent = "Fleet", | |||
| primary_key = String, // token | |||
| primary_key = { uuid_kind = AccessTokenKind }, | |||
| roles_allowed = false, | |||
| polar_snippet = FleetChild, | |||
There was a problem hiding this comment.
I'm realizing FleetChild will probably have to change.
david-crespo
commented
May 20, 2025
david-crespo
force-pushed
the
token-session-ids
branch
from
ce269ce to
6fbe688
Compare
7 tasks
david-crespo
changed the title
id column on tokens and sessionsid column on tokens and sessions
This was referenced May 27, 2025
david-crespo
force-pushed
the
token-session-ids
branch
from
6fbe688 to
a4d0d74
Compare
david-crespo
force-pushed
the
token-session-ids
branch
from
a4d0d74 to
6f16cfe
Compare
hawkw
reviewed
Jun 2, 2025
hawkw
approved these changes
Jun 2, 2025
david-crespo
added a commit
that referenced
this pull request
Jun 4, 2025
Closes #2302. Built on top of #8137, but doesn't really rely on much in there. Implementing [RFD 570](https://rfd.shared.oxide.computer/rfd/0570). - [x] Add `silo_settings` table with nullable device token expiration (seconds) column. Null setting means no max - Also populate this table with null maxes for all existing silos - [x] Follow behavior of quotas table: create silo settings entry on silo create, delete it on silo delete - [x] View and update settings endpoints at `/v1/settings` with all necessary plumbing - Following RFD 570, these are silo-scoped endpoints, only accessible to users inside the silo, as opposed to, for example, fleet operators being able to set it for any silo - [x] Users don't set token expiration directly yet — at token create time, if the current silo has its max set, the token gets an expiration timestamp based on that max. Otherwise the token does not expire - [x] Test that trying to set a negative or 0 TTL 400s - [x] Fix authz tests and verify endpoint tests - [x] Bikeshed endpoint paths, operation IDs, property names, etc
david-crespo
added a commit
that referenced
this pull request
Jun 4, 2025
Built on top of #8137 and #8214. This is only for a user to list and delete their own tokens. It doesn't quite match RFD 570, which says `/v1/device-tokens` instead of `/v1/me/access-tokens`, but it feels good under `/v1/me`, and after trying to make the UI too, I think "access tokens" is much more intuitive. If I stick with this, I will update RFD 570 to match. ~~I'm not sure about the path `/v1/device-tokens` — in the API we call them `Device Access Tokens`. I think `/v1/access-tokens` might be more intuitive because the `device` is sort of an implementation detail, it refers to the OAuth device auth flow, which we are using. In practice, the user just gets a token with the CLI and pastes a code into the web UI and they don't have to think too much about it, so exposing that detail in the name might not be worth it.~~ Went with `/v1/me/access-tokens`. - [x] Basic token list and delete - [x] Basic integration tests - [x] Finalize endpoint paths - [x] Figure out authz story - Went with restricting datastore functions to current actor for now
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.
Closes #8139. Token IDs are used in #8227 and #8231 for token CRUD.
WIP. Everything compiles and existing tests work, but we are not yet testing retrieval by ID and we need to think about authz checks on the datastore functions for retrieving tokens and sessions by token string.
This PR would be a lot shorter if not for the fact that the
tokencolumn was the primary key on both tables, so adding theidrequires a bunch of migration steps to change the primary key. There were also a lot of changes to code and tests around making things ID-centric instead of token-centric.idcol and changing primary key to thatTypedUuidto session and token DB modelsauthz_resource!andlookup_resource!calls for new primary keySessiontrait methods to useidinstead oftokenFakeSessionimplementations (one for unit tests, one for integration tests) to a) track sessions in aVecinstead of aHashMapwith token keys