[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.

david-crespo · 2025-10-16T17:22:29Z

nexus/auth/src/authz/omicron.polar

 	"create_child" if "external-authenticator" on "parent_fleet";
+
+	# external scim has to be able to read SCIM tokens
+	"list_children" if "external-scim" on "parent_fleet";


Why is this needed?

Yeah, I can't find a spot where the external-scim opctx needs to do this. Is it intended for the unimplemented list users operation etc?

david-crespo · 2025-10-16T18:43:28Z

nexus/src/app/scim.rs

+
+    /// Authenticate a SCIM client based on a bearer token, and return a SCIM
+    /// provider store implementation that is scoped to a Silo.
+    pub(crate) async fn scim_idp_get_provider(


I think it would be good for this name to make clearer that it handles token authentication. scim_idp_get_provider sounds like a lookup. It might even make sense to split it into two pieces, one that is the authn and one that gets the provider based on the token. But that's silly if they're always called together, and you don't really want to return the token back to the call site either. scim_token_auth_and_get_provider is a bit much I guess.

This might be more relevant in the next PR, when you implement the actual SCIM operations, but it could make sense to mirror op_context_for_external_api and wrap up the authentication and the creation of the op context into one operation. Right now it looks like the unimplemented SCIM things don't take and OpContext but they will need to in order to do DB operations. Or at least, that's how I think it should work: op context created at top level at authn time and then passed into the provider methods.

omicron/nexus/src/context.rs

Lines 335 to 359 in ef318b3

/// Authenticates an incoming request to the external API and produces a new

/// operation context for it

pub(crate) async fn op_context_for_external_api(

rqctx: &dropshot::RequestContext<ApiContext>,

) -> Result<OpContext, dropshot::HttpError> {

let apictx = rqctx.context();

OpContext::new_async(

&rqctx.log,

async {

let authn = Arc::new(

apictx.context.external_authn.authn_request(rqctx).await?,

);

let datastore = Arc::clone(apictx.context.nexus.datastore());

let authz = authz::Context::new(

Arc::clone(&authn),

Arc::clone(&apictx.context.authz),

datastore,

);

Ok((authn, authz))

},

|metadata| OpContext::load_request_metadata(rqctx, metadata),

OpKind::ExternalApiRequest,

)

.await

}

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

david-crespo · 2025-10-16T19:32:31Z

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

+        opctx: &OpContext,
+        authz_silo: &authz::Silo,
+    ) -> ListResultVec<ScimClientBearerToken> {
+        opctx.authorize(authz::Action::ListChildren, authz_silo).await?;


This seems to me too lenient of a check. Any silo viewer has ListChildren on the silo. Maybe that's fine considering the list of tokens doesn't actually tell you anything. But it's something to think about.

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

Lines 183 to 195 in ef318b3

resource: Silo "silo1"

USER Q R LC RP M MP CC D

fleet-admin ✘ ✔ ✘ ✔ ✔ ✔ ✘ ✔

fleet-collaborator ✘ ✔ ✘ ✔ ✔ ✔ ✘ ✔

fleet-viewer ✘ ✔ ✘ ✔ ✘ ✘ ✘ ✘

silo1-admin ✘ ✔ ✔ ✔ ✔ ✔ ✔ ✔

silo1-collaborator ✘ ✔ ✔ ✔ ✘ ✘ ✔ ✘

silo1-viewer ✘ ✔ ✔ ✔ ✘ ✘ ✘ ✘

silo1-proj1-admin ✘ ✔ ✘ ✔ ✘ ✘ ✘ ✘

silo1-proj1-collaborator ✘ ✔ ✘ ✔ ✘ ✘ ✘ ✘

silo1-proj1-viewer ✘ ✔ ✘ ✔ ✘ ✘ ✘ ✘

unauthenticated ! ! ! ! ! ! ! !

The only way I know of to make this more restrictive is to create a "synthetic" authz resource representing the list of scim tokens for a given silo and then do the authz checks against that. Here's an example of a synthetic authz list resource with its own permissions distinct from silo ListChildren:

omicron/nexus/auth/src/authz/api_resources.rs

Lines 672 to 695 in ef318b3

/// Synthetic resource describing the list of Identity Providers associated with

/// a Silo

#[derive(Clone, Debug, Eq, PartialEq)]

pub struct SiloIdentityProviderList(Silo);

impl SiloIdentityProviderList {

pub fn new(silo: Silo) -> SiloIdentityProviderList {

SiloIdentityProviderList(silo)

}

pub fn silo(&self) -> &Silo {

&self.0

}

}

impl oso::PolarClass for SiloIdentityProviderList {

fn get_polar_class_builder() -> oso::ClassBuilder<Self> {

oso::Class::builder()

.with_equality_check()

.add_attribute_getter("silo", |list: &SiloIdentityProviderList| {

list.0.clone()

})

}

}

david-crespo · 2025-10-16T20:22:16Z

nexus/src/app/scim.rs

    ) -> ListResultVec<views::ScimClientBearerToken> {
-        let (.., _authz_silo, _) =
+        let (.., authz_silo, _) =
            silo_lookup.fetch_for(authz::Action::ListChildren).await?;


I don't think you need to specify ListChildren here, you can just do .fetch() and let the datastore method handle the permission relevant on the token list.

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

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?

papertigers · 2025-10-16T22:17:29Z

nexus/src/app/scim.rs

+        let token = match header.to_str() {
+            Ok(v) => v,
+            Err(_) => {
+                return Err(Error::Unauthenticated {
+                    internal_message: "Invalid bearer token".to_string(),
+                });
+            }
+        };


Suggested change

let token = match header.to_str() {

Ok(v) => v,

Err(_) => {

return Err(Error::Unauthenticated {

internal_message: "Invalid bearer token".to_string(),

});

}

};

let Ok(token) = header.to_str() else {

return Err(Error::Unauthenticated {

internal_message: "Invalid bearer token".to_string(),

});

};

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.

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?

schema/crdb/dbinit.sql

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

papertigers reviewed Oct 16, 2025

View reviewed changes

	/// Authenticates an incoming request to the external API and produces a new
	/// operation context for it
	pub(crate) async fn op_context_for_external_api(
	rqctx: &dropshot::RequestContext<ApiContext>,
	) -> Result<OpContext, dropshot::HttpError> {
	let apictx = rqctx.context();
	OpContext::new_async(
	&rqctx.log,
	async {
	let authn = Arc::new(
	apictx.context.external_authn.authn_request(rqctx).await?,
	);
	let datastore = Arc::clone(apictx.context.nexus.datastore());
	let authz = authz::Context::new(
	Arc::clone(&authn),
	Arc::clone(&apictx.context.authz),
	datastore,
	);
	Ok((authn, authz))
	},
	\|metadata\| OpContext::load_request_metadata(rqctx, metadata),
	OpKind::ExternalApiRequest,
	)
	.await
	}

	resource: Silo "silo1"

	USER Q R LC RP M MP CC D
	fleet-admin ✘ ✔ ✘ ✔ ✔ ✔ ✘ ✔
	fleet-collaborator ✘ ✔ ✘ ✔ ✔ ✔ ✘ ✔
	fleet-viewer ✘ ✔ ✘ ✔ ✘ ✘ ✘ ✘
	silo1-admin ✘ ✔ ✔ ✔ ✔ ✔ ✔ ✔
	silo1-collaborator ✘ ✔ ✔ ✔ ✘ ✘ ✔ ✘
	silo1-viewer ✘ ✔ ✔ ✔ ✘ ✘ ✘ ✘
	silo1-proj1-admin ✘ ✔ ✘ ✔ ✘ ✘ ✘ ✘
	silo1-proj1-collaborator ✘ ✔ ✘ ✔ ✘ ✘ ✘ ✘
	silo1-proj1-viewer ✘ ✔ ✘ ✔ ✘ ✘ ✘ ✘
	unauthenticated ! ! ! ! ! ! ! !

	/// Synthetic resource describing the list of Identity Providers associated with
	/// a Silo
	#[derive(Clone, Debug, Eq, PartialEq)]
	pub struct SiloIdentityProviderList(Silo);

	impl SiloIdentityProviderList {
	pub fn new(silo: Silo) -> SiloIdentityProviderList {
	SiloIdentityProviderList(silo)
	}

	pub fn silo(&self) -> &Silo {
	&self.0
	}
	}

	impl oso::PolarClass for SiloIdentityProviderList {
	fn get_polar_class_builder() -> oso::ClassBuilder<Self> {
	oso::Class::builder()
	.with_equality_check()
	.add_attribute_getter("silo", \|list: &SiloIdentityProviderList\| {
	list.0.clone()
	})
	}
	}


		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

Are you sure you want to change the base?

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

Conversation

jmpesp commented Oct 9, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

david-crespo Oct 16, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

david-crespo commented Oct 16, 2025

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

david-crespo Oct 16, 2025 •

edited

Loading