Skip to content

Support for SAML as a Silo IdP, part 2#1210

Merged
jmpesp merged 20 commits into
oxidecomputer:mainfrom
jmpesp:saml_login
Jun 28, 2022
Merged

Support for SAML as a Silo IdP, part 2#1210
jmpesp merged 20 commits into
oxidecomputer:mainfrom
jmpesp:saml_login

Conversation

@jmpesp

@jmpesp jmpesp commented Jun 15, 2022

Copy link
Copy Markdown
Contributor

This PR adds the second half of the SAML login: accepting the IdP's SAML
response, creating (or looking up) a silo user, and establishing a
session token for them.

A silo identity provider will send authenticated users to Nexus along
with some form of credentials, or Nexus will query an identity provider
in order to determine if user-supplied credentials are correct. In the
SAML case, the SAML IdP will POST credentials to Nexus.

Silos now have a "user provision type" setting: if set to "fixed", users
must be defined before the SAML IdP POSTs the SAMLResponse. If set to
"jit", users will be created if they do not exist.

A Silo's identity provider consumes an IdP response and emits an
"authenticated subject". This authenticated subject is in terms of the
IdP's realm, and contains an external ID along with groups that the
subject is part of. Nexus will create or lookup a silo user based on
that external id. Silo users are mapped to external subjects using this
external id, and as a result, silo users now have an "external_id"
field. This is a one to one mapping and assumes that the external id
will not change.

Future work:

  • add silo groups
  • get cached information from a user's session when logging in
  • SAML logout flow (requires samael crate PR)

This PR adds the second half of the SAML login: accepting the IdP's SAML
response, creating (or looking up) a silo user, and establishing a
session token for them.

A silo identity provider will send authenticated users to Nexus along
with some form of credentials, or Nexus will query an identity provider
in order to determine if user-supplied credentials are correct. In the
SAML case, the SAML IdP will POST credentials to Nexus.

Silos now have a "user provision type" setting: if set to "fixed", users
must be defined before the SAML IdP POSTs the SAMLResponse. If set to
"jit", users will be created if they do not exist.

A Silo's identity provider consumes an IdP response and emits an
"authenticated subject". This authenticated subject is in terms of the
IdP's realm, and contains an external ID along with groups that the
subject is part of. Nexus will create or lookup a silo user based on
that external id. Silo users are mapped to external subjects using this
external id, and as a result, silo users now have an "external_id"
field. This is a one to one mapping and assumes that the external id
will not change.

Future work:
- add silo groups
- get cached information from a user's session when logging in
- SAML logout flow (requires samael crate PR)
@davepacheco davepacheco mentioned this pull request Jun 15, 2022
69 tasks
Comment thread common/src/sql/dbinit.sql Outdated
time_deleted TIMESTAMPTZ,

silo_id UUID NOT NULL,
external_id TEXT

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is their username with the IdP, right? like their company email address?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it could be that, it could also be some pseudo-random id - see section 8.3 of http://docs.oasis-open.org/security/saml/v2.0/saml-core-2.0-os.pdf. we can't rely on this being anything legible like an email or username.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. It's whatever they choose. It is not a display name.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although, we could say that if the type is emailAddress then it could be used as a display name.

@david-crespo david-crespo Jun 21, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't we be storing other metadata about the user that will let us reliably get display names? I think we need that, unless we're able to hit IdP endpoints for user data instead (which I think would have to be proxied through our API instead of hit directly from the client).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we JIT users like this, there's no other information other than what's in the name format. We also can't query a SAML IdP because that's not how SAML works - the IdP only sends us SAMLResponses. If we supported SCIM then users could be provisioned but we don't yet.

Comment thread common/src/sql/dbinit.sql Outdated
Comment thread nexus/src/db/datastore.rs
Comment thread nexus/src/db/datastore.rs
Comment thread nexus/src/db/datastore.rs Outdated
Comment thread nexus/src/db/datastore.rs
Comment thread nexus/src/external_api/console_api.rs
Comment thread nexus/src/external_api/console_api.rs Outdated
Comment thread nexus/src/external_api/console_api.rs Outdated
Comment thread nexus/src/external_api/console_api.rs Outdated
Comment thread nexus/tests/integration_tests/saml.rs Outdated
Comment thread common/src/sql/dbinit.sql
.map_err(|e| {
HttpError::for_internal_error(format!(
"{}",
"referer header to_str failed! {}",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super nitty but I imagine this should be a 400 for providing an unsupported header?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that makes sense. I originally thought that the user wouldn't control the referer header, but of course they do, because they navigate to a URL in the first place. Changed in d66038f

Comment thread nexus/tests/integration_tests/data/README.md
default send to / instead of organizations

@davepacheco davepacheco left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(meant to approve this earlier, sorry)

@jmpesp jmpesp merged commit 535e765 into oxidecomputer:main Jun 28, 2022
@jmpesp jmpesp deleted the saml_login branch June 28, 2022 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants