[SCIM 3/4]: SCIM client token CRUD + Bearer auth #9180

jmpesp · 2025-10-09T02:56:25Z

Implement the CRUD routines for the tokens that will be used to authenticate SCIM clients for SamlScim Silos. Also implement the Bearer based authentication for SCIM clients.

Fill in the skeleton of CrdbScimProviderStore, which when implemented will complete the SCIM implementation in Nexus.

Implement the CRUD routines for the tokens that will be used to authenticate SCIM clients for SamlScim Silos. Also implement the Bearer based authentication for SCIM clients. Fill in the skeleton of CrdbScimProviderStore, which when implemented will complete the SCIM implementation in Nexus.

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

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

schema/crdb/dbinit.sql

Instead of silo Also grant silo admin role to USER_TEST_PRIVILEGED.id() instead of OpContext::for_tests in tests.

nexus/auth/src/authz/omicron.polar

nexus/src/app/scim.rs

david-crespo · 2025-10-16T19:03:15Z

I had a good conversation with the Codex CLI about the choice to effectively bypass the existing authn schemes setup here, treating these calls as "unauthed" from the point of view of the main authn system. The robot was pretty persuasive but I think the choice is worth pointing out and documenting in comments, probably on the scim_idp_get_provider.

https://gist.github.com/david-crespo/14d25cb770f9e10195f6d0ea45286874

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

nexus/src/app/scim.rs

papertigers · 2025-10-16T21:28:37Z

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

+            id: Uuid::new_v4(),
+            time_created: Utc::now(),
+            time_deleted: None,
+            // TODO: allow setting an expiry? have a silo default?


Do you mind filing a follow up ticket for this and noting it in the comment

opened #9260

papertigers · 2025-10-16T22:11:04Z

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

+        use nexus_db_schema::schema::scim_client_bearer_token::dsl;
+        let maybe_token = dsl::scim_client_bearer_token
+            .filter(dsl::bearer_token.eq(bearer_token))
+            .filter(dsl::time_deleted.is_null())


do we need filters for where we are not past the expired time here and other places?

nexus/src/app/scim.rs

papertigers · 2025-10-16T22:19:41Z

nexus/src/app/scim.rs

+
+        const BEARER: &str = "Bearer ";
+
+        if !token.starts_with(BEARER) {


I don't remember where we landed on this but assuming this logic is correct we are going to require operators to use Bearer xxx-xxx-xxx-xxx on the IdP side rather than just a raw header value?

Also the logic could simply become:

let Some(token) = token.strip_prefix(BEARER) else { return Err(Error::Unauthenticated { internal_message: "Invalid bearer token".to_string(), }); };

Then that could be passed to scim_idp_lookup_token_by_bearer() directly.

This changed to Bearer oxide-scim-token in 3b3b891 to fit with how other authn schemes work - we'll have to include some docs about it.

papertigers · 2025-10-16T22:37:12Z

nexus/tests/integration_tests/scim.rs

+
+    assert!(tokens.is_empty());
+
+    // Fleet admins can create SCIM client tokens


Just going off of the comment here, but can we add a test that a non fleet admin cannot create a SCIM client token?

Silo admins should be able to create SCIM tokens too. I can add a test to the follow up PR (that creates the silo admin users) to test this too.

schema/crdb/dbinit.sql

create opctx that uses that actor create new authn scheme for "Bearer oxide-scim-... make opctx.authorize calls make more sense remove external-scim create synthetic resource ScimClientBearerTokenList for tighter permission structure: - external-auth cannot list children on ScimClientBearerTokenList for example

david-crespo

Still looking through, but here is a pile of comments. I am much happier with this after adding a formal authn scheme for scim.

nexus/auth/src/authn/mod.rs

nexus/db-lookup/src/lookup.rs

david-crespo · 2025-10-20T19:28:35Z

nexus/auth/src/authn/mod.rs

    /// might use a Silo.  You usually want to use [`Context::silo_required()`]
    /// if you don't expect to be looking at a built-in user.
+    ///
+    /// Additionally, non-user Actors may also be associated with a Silo.


It would be good to say that by "non-user actor" you mean the SCIM arm of the match. It's obvious when you're looking at the diff but might not be so obvious later.

david-crespo · 2025-10-20T19:35:04Z

nexus/auth/src/authz/roles.rs

+            // XXX Ok, or an error?
+            return Ok(());
+        };
+


I don't think it should error, that might break everything for no reason. Interestingly, the SCIM actor technically does have a role on the fleet, it just happens to be hard-coded. It probably doesn't make sense for this method to load that pseudo-assignment into roleset, though it does make me wonder what is doing that if not this method. I will poke around — no change requested here.

I misunderstood this function - it's not inserting roles for a actor, it's looking them up, so this is a short circuit because SCIM actors won't ever have roles. 493af88 removes it

Might be good to comment // short circuit for actors that won't ever have roles

nexus/auth/src/authz/omicron.polar

david-crespo · 2025-10-20T20:24:04Z

nexus/auth/src/authz/actor.rs

+
+                    authn::Actor::Scim { .. } => false,
+                }
+            })


This makes me feel bad, but it might just be the name is_user, which, while it technically does divide the options the right way, doesn't feel like it names the meaning of the distinction. Or maybe the problem is that we're going to leave off is_user by mistake and cause a bug that way. If we see this extending to service accounts, are those going to also be non-users? Should they have read perms on their own silo? Maybe this is a question for later, but it illustrates the weirdness of this concept.

david-crespo · 2025-10-20T21:20:46Z

nexus/db-queries/tests/output/authz-roles.out

+  silo1-proj1-collaborator         ✘  ✘  ✘  ✘  ✘  ✘  ✘  ✘
+  silo1-proj1-viewer               ✘  ✘  ✘  ✘  ✘  ✘  ✘  ✘
+  unauthenticated                  !  !  !  !  !  !  !  !
+  scim                             ✘  ✘  ✘  ✘  ✘  ✘  ✘  ✘


david-crespo · 2025-10-20T21:25:53Z

nexus/src/app/scim.rs

+        let provider = self.scim_get_provider_from_opctx(opctx).await?;
+
+        let result = match provider.patch_group(&group_id, body).await {
+            Ok(response) => response.to_http_response(StatusCode::OK),


How come some of these take StatusCode::OK and some don't?

The scim2-rs::SingleResourceResponse is converted to a http::Response, but the call site needs to know the proper status code cause the conversion routine does not know for what HTTP method it's being called for. POST needs to be 201, and the rest need to be 200.

nexus/auth/src/authz/omicron.polar

jmpesp · 2025-10-21T19:42:33Z

@papertigers the change to have fn silo_id return a silo for an Actor::Scim caused the test_iam_roles_behavior test to fail. I think this can be addressed with the Polar changes we were talking about?

jmpesp requested a review from david-crespo October 9, 2025 02:56

jmpesp added 2 commits October 9, 2025 13:47

remove the "delete all tokens" endpoint

56d5288

remove delete all from VERIFY_ENDPOINTS

6e67ae9

david-crespo reviewed Oct 10, 2025

View reviewed changes

nexus/db-queries/src/db/datastore/scim.rs Outdated Show resolved Hide resolved

david-crespo reviewed Oct 10, 2025

View reviewed changes

nexus/db-queries/src/db/datastore/scim_provider_store.rs Outdated Show resolved Hide resolved

david-crespo reviewed Oct 10, 2025

View reviewed changes

schema/crdb/dbinit.sql Show resolved Hide resolved

jmpesp added 4 commits October 10, 2025 21:01

patterm

9bbb011

Use authz lookup on bearer token itself

ed37a89

Instead of silo Also grant silo admin role to USER_TEST_PRIVILEGED.id() instead of OpContext::for_tests in tests.

Merge branch 'main' into scim_token_crud_and_auth

88c03f6

Merge branch 'main' into scim_token_crud_and_auth

2b03a76

david-crespo reviewed Oct 16, 2025

View reviewed changes

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

david-crespo reviewed Oct 16, 2025

View reviewed changes

nexus/src/app/scim.rs Outdated Show resolved Hide resolved

david-crespo reviewed Oct 16, 2025

View reviewed changes

nexus/db-queries/src/db/datastore/scim.rs Outdated Show resolved Hide resolved

david-crespo reviewed Oct 16, 2025

View reviewed changes

nexus/src/app/scim.rs Outdated Show resolved Hide resolved

papertigers reviewed Oct 16, 2025

View reviewed changes

jmpesp added 7 commits October 17, 2025 16:40

rename to scim_get_provider_from_bearer_token

880fb63

remove list-children grant for external-scim, not necessary

76b452e

just fetch

5cebf49

test failures

456cdc0

test_iam_roles_behavior is a genius idea

3b52803

more comment

8995f83

david-crespo reviewed Oct 20, 2025

View reviewed changes

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

jmpesp added 4 commits October 21, 2025 16:16

Scim actor does have a silo!

723848d

fleet admins do need to create and retrieve scim client bearer tokens

84ae83b

remove wrong comment

493af88

Merge branch 'main' into scim_token_crud_and_auth

0ed8f13

actually commit stuff

5d4026b

jmpesp added 3 commits October 22, 2025 20:49

scim actors need to have a blank SiloAuthnPolicy

01520c8

good enough comment

bb98724

Merge branch 'main' into scim_token_crud_and_auth

3a90a98

david-crespo approved these changes Oct 22, 2025

View reviewed changes

jmpesp enabled auto-merge (squash) October 22, 2025 21:29

jmpesp merged commit af3b89f into oxidecomputer:main Oct 22, 2025
18 checks passed

jmpesp deleted the scim_token_crud_and_auth branch October 23, 2025 00:28


		const BEARER: &str = "Bearer ";

		if !token.starts_with(BEARER) {


		assert!(tokens.is_empty());

		// Fleet admins can create SCIM client tokens

[SCIM 3/4]: SCIM client token CRUD + Bearer auth #9180

[SCIM 3/4]: SCIM client token CRUD + Bearer auth #9180

Conversation

jmpesp commented Oct 9, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

david-crespo commented Oct 16, 2025

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!

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!

Uh oh!

david-crespo left a comment

Choose a reason for hiding this comment

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!

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!

Uh oh!

jmpesp commented Oct 21, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants