Silo admin endpoints for user logout + listing tokens and sessions #8479

david-crespo · 2025-06-30T19:31:35Z

The idea here is that to disable a user's access to the system, admins first disable that user's ability to log in on the IdP side and then hit this endpoint to remove all of their existing credentials on our end. The centerpiece is the logout endpoint, but I added the endpoints for listing sessions and tokens because someone pointed out you really want to see those come back empty after logout. They're also kind of useful anyway. Then I added user_view just because it wouldn't make sense to have token and session list endpoints hanging off /v1/users/{user_id} without having that defined.

Add /v1/users/{user_id}/logout that deletes all of the user's tokens and sessions
Add new authz resource SiloUserAuthnList letting us authorize that action for silo admins specifically (can't use silo modify because fleet collaborator and admin get that on all silos)
Update IAM policy test
Test that logout deletes tokens and the right perms are enforced
Test that logout deletes sessions and the right perms are enforced
Add user_view and user_token_list and user_session_list endpoints for symmetry and to give the admin a warm fuzzy feeling when they see that the tokens and sessions are in fact gone (also makes testing a little cleaner)
Fix session list including expired sessions (and test it)
Think about whether we need to do something about dueling admins issues, i.e., what if the person you're trying to disable are themselves a silo admin and they log everyone else out of the silo. The only solution I can think of off the top of my head is an operator-level version of this endpoint that can be used by a user outside of the silo in question.

nexus/auth/src/authz/omicron.polar

david-crespo · 2025-07-15T17:04:05Z

nexus/db-queries/src/db/datastore/console_session.rs

+            // TODO: unlike with tokens, we do not have expiration time here,
+            // so we can't filter out expired sessions by comparing to now. In
+            // the authn code, this works by dynamically comparing the created
+            // and last used times against now + idle/absolute TTL. We may
+            // have to do that here but it's kind of sad. It might be nicer to
+            // make sessions work more like tokens and put idle and absolute
+            // expiration time right there in the table at session create time.


This is the major outstanding issue. I will probably make the fix a separate PR on top of this one because it's an actual change, not just an addition.

Mm, I like the idea of the proposed refactor --- generally it seems nicer to represent things as deadlines, I think. I agree that it's a big enough change that it could be done subsequently. In the meantime, might we want to cruft together a thing that filters such sessions out of the returned Vec in Rust code?

Do you think it's worth opening an issue to track that, in addition to/instead of the TODO comment?

I don't think we need to filter them out of the Vec for the quick fix — we can get the expiration cutoffs based on the idle and absolute session timeouts in the Nexus config and stick them right in the SQL query.

I'm going to attempt the proper fix in a separate PR and if it's easy enough I'll put it underneath this one. If it's too hard, I'll make the quick fix here and make and issue for the real fix.

I did the hack fix in 13af0ac, complete with beautiful test. Issue for real fix in #8625 — I intend to get that in for v16, so the hack will not have to actually be in the wild. But I think it is fine if it has to wait for v17.

hawkw · 2025-07-15T21:11:45Z

nexus/auth/src/authz/api_resources.rs

+// TODO: does it make sense to use a single resource to represent both user
+// sessions and tokens? it seems silly to have two identical ones


Hmm, how would that work? Is that a question you want to answer before merging this branch?

Status quo is the single resource version (I meant resource from the POV of authz). I think it would be simple to have two instead, I would just make a second one and name one tokens and one sessions, and use each as appropriate. Since that can be done any time we need to distinguish the two (we may never have to) I am inclined to solve this by deleting the comment.

In c3c070e and 0e5a179 I ended up going the other way, namely splitting the authz resource into two: one for sessions and one for tokens. While doing that I realized that there is existing authz resource for the session list, which we use to give the external authenticator user permission to create sessions for users. Tokens, work a little differently -- apparently tokens are children of the user, so we check create_child on the user (which seems a bit weird). Made #8628 about merging the two session list concepts.

omicron/nexus/db-queries/src/db/datastore/device_auth.rs

Lines 96 to 97 in 464e168

opctx.authorize(authz::Action::Delete, authz_request).await?;

opctx.authorize(authz::Action::CreateChild, authz_user).await?;

The session and token lists are identical for now, but they would probably diverge if I eventually did the work to merge them with the existing session and token list authz concepts.

nexus/auth/src/authz/omicron.polar

hawkw · 2025-07-15T21:24:38Z

nexus/db-queries/src/db/datastore/console_session.rs

+            // TODO: unlike with tokens, we do not have expiration time here,
+            // so we can't filter out expired sessions by comparing to now. In
+            // the authn code, this works by dynamically comparing the created
+            // and last used times against now + idle/absolute TTL. We may
+            // have to do that here but it's kind of sad. It might be nicer to
+            // make sessions work more like tokens and put idle and absolute
+            // expiration time right there in the table at session create time.


Mm, I like the idea of the proposed refactor --- generally it seems nicer to represent things as deadlines, I think. I agree that it's a big enough change that it could be done subsequently. In the meantime, might we want to cruft together a thing that filters such sessions out of the returned Vec in Rust code?

Do you think it's worth opening an issue to track that, in addition to/instead of the TODO comment?

nexus/db-queries/src/db/datastore/device_auth.rs

nexus/external-api/src/lib.rs

nexus/src/app/silo.rs

nexus/tests/integration_tests/device_auth.rs

hawkw

I had one last nitpick about positional args, which you are welcome to ignore --- i know that's kind of pedantic. Otherwise, this looks good to me, regardless of whether you end up changing that bit or not.

hawkw · 2025-07-17T21:43:15Z

nexus/db-queries/src/db/datastore/console_session.rs

+        idle_ttl: TimeDelta,
+        abs_ttl: TimeDelta,


nitpicky, sorry: I really don't like when a function has two or more positional arguments of the same type, because when you call the function, you have to remember which is which, and it's easy to transpose them etc by mistake. It might be nicer to have a quick

#[derive(Copy, Clone)] pub struct SessionTtls { pub idle: TimeDelta, pub abs: TimeDelta, }

so they're distinguished by name at the call site instead of by which order they're passed in?

Yeah, I thought about doing this to make it all less sketchy-feeling. Will do (maybe in a followup PR so I can get this merged ASAP).

nexus/auth/src/authz/omicron.polar

nexus/src/app/silo.rs

hawkw · 2025-07-17T21:59:45Z

nexus/external-api/src/lib.rs

+    /// Log user out
+    ///
+    /// Silo admins can use this endpoint to log the specified user out by
+    /// deleting all of their tokens AND sessions. This cannot be undone.


The new comment makes me less sad about the POST, thank you!

david-crespo force-pushed the silo-user-logout branch 2 times, most recently from 109676e to 276db6b Compare July 1, 2025 22:30

david-crespo changed the title ~~Silo admin endpoint for user logout~~ Silo admin endpoint for user logout + listing tokens and sessions Jul 2, 2025

david-crespo changed the title ~~Silo admin endpoint for user logout + listing tokens and sessions~~ Silo admin endpoints for user logout + listing tokens and sessions Jul 2, 2025

david-crespo commented Jul 2, 2025

View reviewed changes

nexus/auth/src/authz/omicron.polar Show resolved Hide resolved

david-crespo marked this pull request as ready for review July 2, 2025 23:11

david-crespo mentioned this pull request Jul 3, 2025

Add effective fleet and silo role to /v1/me response #8515

Open

david-crespo force-pushed the silo-user-logout branch from c97c76d to 10f014e Compare July 15, 2025 16:55

ahl requested a review from hawkw July 15, 2025 17:02

david-crespo commented Jul 15, 2025

View reviewed changes

hawkw reviewed Jul 15, 2025

View reviewed changes

david-crespo added 18 commits July 17, 2025 10:42

endpoint for silo admin to expire all user's tokens and sessions

94874a9

basic authz allowing silo admin and self to hit logout endpoint

2a47746

update policy test for session list

d7e8a3b

UserSessions -> SiloUserAuthnList

de80558

do authz checks inside datastore delete functions

b5f6b5d

user_view endpoint

c82b96d

user token list endpoint

4b8c0c6

use token list endpoint in integration test

e9d8d9b

session list endpoint

b35de2d

also test sessions in the device_auth logout test

b526489

cargo fmt

06b263c

self-review tweaks

301ef0d

tweak endpoint summaries

a2d27a7

fix iam policy snapshot test failure

b153a23

add authz::SiloUserTokenList

f1b11c4

rename authn list to session list

a775b45

comment fixes

e8b561c

better doc comment on logout endpoint

f497556

david-crespo mentioned this pull request Jul 17, 2025

[nexus] Store expiration times on session records #8625

Open

hack solution to filtering out expired sessions

13af0ac

david-crespo force-pushed the silo-user-logout branch from 56e19da to 13af0ac Compare July 17, 2025 18:12

david-crespo mentioned this pull request Jul 17, 2025

[nexus] Merge SiloUserSessionList and ConsoleSessionList #8628

Open

david-crespo added this to the 16 milestone Jul 17, 2025

hawkw approved these changes Jul 17, 2025

View reviewed changes

david-crespo merged commit 31b49ec into main Jul 17, 2025
17 checks passed

david-crespo deleted the silo-user-logout branch July 17, 2025 22:45

david-crespo mentioned this pull request Jul 18, 2025

Administrator API for disabling/removing device tokens #3892

Closed

		// TODO: does it make sense to use a single resource to represent both user
		// sessions and tokens? it seems silly to have two identical ones

	opctx.authorize(authz::Action::Delete, authz_request).await?;
	opctx.authorize(authz::Action::CreateChild, authz_user).await?;

Silo admin endpoints for user logout + listing tokens and sessions #8479

Silo admin endpoints for user logout + listing tokens and sessions #8479

Uh oh!

Conversation

david-crespo commented Jun 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

david-crespo Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

david-crespo Jul 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hawkw left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

david-crespo Jul 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

david-crespo commented Jun 30, 2025 •

edited

Loading

david-crespo Jul 15, 2025 •

edited

Loading

david-crespo Jul 17, 2025 •

edited

Loading

david-crespo Jul 17, 2025 •

edited

Loading