Support for SAML as a Silo IdP, part 1 #1139

jmpesp · 2022-05-30T21:20:26Z

Add the db schemas, models, and some endpoints to support configuring a
SAML IdP for a Silo. Enough functionality is here to support the first
step of SP-initiated SAML login flow: concretely, created a signed SAML
request, and sending that to the IdP. More work is required to support
receiving the SAML IdP's response, and actually creating and logging in
the user.

Two tables were added here: one that relates a silo to a list of typed
identity providers, and one for SAML configuration. The order of columns
in the silo table was corrected to match the DB model's field order.

Support for serializing and deserializing SAML XML is provided by the
samael crate, but for now use Cargo patch to get a specific branch from
an oxidecomputer fork. A PR was made upstream so follow up will be
required after that is merged.

Add the db schemas, models, and some endpoints to support configuring a SAML IdP for a Silo. Enough functionality is here to support the first step of SP-initiated SAML login flow: concretely, created a signed SAML request, and sending that to the IdP. More work is required to support receiving the SAML IdP's response, and actually creating and logging in the user. Two tables were added here: one that relates a silo to a list of typed identity providers, and one for SAML configuration. The order of columns in the silo table was corrected to match the DB model's field order. Support for serializing and deserializing SAML XML is provided by the samael crate, but for now use Cargo patch to get a specific branch from an oxidecomputer fork. A PR was made upstream so follow up will be required after that is merged.

accept a SAML IDP descriptor document as a base64 encoded string, in addition to fetching it from a URL. don't store the url in the database anymore.

david-crespo · 2022-06-07T20:16:48Z

nexus/src/external_api/views.rs

+    }
+}
+
+/// Client view of an ['IdentityProvider']


Don't know if it matters, but all the other [] things in doc comments have backticks:

Suggested change

/// Client view of an ['IdentityProvider']

/// Client view of an [`IdentityProvider`]

ah, fixed in 9ac1572

david-crespo · 2022-06-07T20:18:20Z

openapi/nexus.json

+              "type": {
+                "type": "string",
+                "enum": [
+                  "base64_encoded_x_m_l"


Base64EncodedXML should be Base64EncodedXml to prevent this snake case silliness

fixed in 32a46ae

david-crespo · 2022-06-07T20:28:31Z

nexus/src/external_api/console_api.rs

+   path = "/login/{silo_name}/{provider_name}",
+   tags = ["login"],
+}]
+pub async fn ask_user_to_login_to_provider(


tbh I find the level of detail in this name more confusing than helpful. This is supposed to fully replace my fake /login right? I think login would be a fine name for this. Maybe login_page_or_redirect to reflect the behavior, though the doc comment covers that. It wouldn't matter at all, but this name ends up in the generated clients as the name of the method and its params type, and it threw me off a bit:

/** * Ask the user to login to their identity provider */ askUserToLoginToProvider: ( { providerName, siloName }: AskUserToLoginToProviderParams, params: RequestParams = {} ) => this.request<void>({ path: `/login/${siloName}/${providerName}`, method: 'GET', ...params, }),

In my experience it's common to see a GET /login(/whatever) and POST /login in an API and they do exactly what we're doing here.

renamed in 20fdc11

david-crespo · 2022-06-07T20:30:39Z

nexus/src/external_api/console_api.rs

+   path = "/login/{silo_name}/{provider_name}",
+   tags = ["login"],
+}]
+pub async fn consume_credentials_and_authn_user(


Same deal here, I would lean toward calling this a variation on login.

renamed in 20fdc11

david-crespo · 2022-06-07T21:15:03Z

common/src/api/external/mod.rs

+    #[serde(skip_serializing)]
+    pub private_key: Option<String>,
+}
+


This is unused — the one in nexus/views is used instead. I deleted it and Nexus still compiles.

good catch, removed in 147a488

david-crespo · 2022-06-07T21:44:16Z

nexus/src/db/model/identity_provider.rs

+            private_key: self.private_key,
+        }
+    }
+}


This disappears along with external::SamlIdentityProvider

nice! 147a488

david-crespo · 2022-06-07T21:46:52Z

nexus/tests/integration_tests/silos.rs

+use nexus_test_utils::http_testing::{AuthnMode, NexusRequest, RequestBuilder};
+use omicron_common::api::external::{
+    IdentityMetadataCreateParams, Name, SamlIdentityProvider,
+};
+use omicron_nexus::authn::silos::IdentityProviderType;
+use omicron_nexus::external_api::params;
+use omicron_nexus::external_api::views::{
+    self, IdentityProvider, Organization, Silo,
+};


need to import SamlIdentityProvider from views

+1, 147a488

david-crespo

I tried to review mostly from the standpoint of an API consumer, mostly looking at the changes in the TS client when it's built off this branch. Looks good, just a couple of small comments mostly about names.

nexus/src/external_api/views.rs

…n_user

david-crespo · 2022-06-08T19:53:12Z

Changes look good!

Crucible changes: Remove unused fields in IOop (#1149) New downstairs clone subcommand. (#1129) Simplify the do_work_task loop (#1150) Move `Guest` stuff into a module (#1125) Bump nix to 0.27.1 and use new safer Fd APIs (#1110) Move `FramedWrite` work to a separate task (#1145) Use fewer borrows in ExtentInner API (#1147) Update Rust crate reedline to 0.28.0 (#1141) Update Rust crate tokio to 1.36 (#1143) Update Rust crate slog-bunyan to 2.5.0 (#1139) Update Rust crate rayon to 1.8.1 (#1138) Update Rust crate itertools to 0.12.1 (#1137) Update Rust crate byte-unit to 5.1.4 (#1136) Update Rust crate base64 to 0.21.7 (#1135) Update Rust crate async-trait to 0.1.77 (#1134) Discard deferred msgs (#1131) Minor Downstairs cleanup (#1127) Update test_fail_live_repair to support pstop (#1128) Ignore client messages after stopping the IO task (#1126) Move client IO task into a struct (#1124) Bump Rust to 1.75 and fix new Clippy lints (#1123) Propolis changes: PHD: convert to async (#633) PHD: assume specialized Windows images (#636) propolis-standalone-config needn't be a crate standalone: Use tar for snapshot/restore phd: use latest "lab-2.0-opte" target, not a specific version (#637) PHD: add tests for migration of running processes (#623) PHD: fix `cargo xtask phd` tidy not doing anything (#630) PHD: add documentation for `cargo xtask phd` (#629) standalone: improve virtual device creation errors (#632) phd: add Windows Server 2019 guest adapter (#627) PHD: add `cargo xtask phd` to make using PHD nicer (#619)

Crucible changes: Remove unused fields in IOop (#1149) New downstairs clone subcommand. (#1129) Simplify the do_work_task loop (#1150) Move `Guest` stuff into a module (#1125) Bump nix to 0.27.1 and use new safer Fd APIs (#1110) Move `FramedWrite` work to a separate task (#1145) Use fewer borrows in ExtentInner API (#1147) Update Rust crate reedline to 0.28.0 (#1141) Update Rust crate tokio to 1.36 (#1143) Update Rust crate slog-bunyan to 2.5.0 (#1139) Update Rust crate rayon to 1.8.1 (#1138) Update Rust crate itertools to 0.12.1 (#1137) Update Rust crate byte-unit to 5.1.4 (#1136) Update Rust crate base64 to 0.21.7 (#1135) Update Rust crate async-trait to 0.1.77 (#1134) Discard deferred msgs (#1131) Minor Downstairs cleanup (#1127) Update test_fail_live_repair to support pstop (#1128) Ignore client messages after stopping the IO task (#1126) Move client IO task into a struct (#1124) Bump Rust to 1.75 and fix new Clippy lints (#1123) Propolis changes: PHD: convert to async (#633) PHD: assume specialized Windows images (#636) propolis-standalone-config needn't be a crate standalone: Use tar for snapshot/restore phd: use latest "lab-2.0-opte" target, not a specific version (#637) PHD: add tests for migration of running processes (#623) PHD: fix `cargo xtask phd` tidy not doing anything (#630) PHD: add documentation for `cargo xtask phd` (#629) standalone: improve virtual device creation errors (#632) phd: add Windows Server 2019 guest adapter (#627) PHD: add `cargo xtask phd` to make using PHD nicer (#619) Co-authored-by: Alan Hanson <[email protected]>

jmpesp added 2 commits May 30, 2022 16:52

install pre-reqs in more jobs

5258e0a

jmpesp requested review from davepacheco and david-crespo May 31, 2022 15:06

jmpesp added 2 commits June 3, 2022 17:00

accept SAML IDP descriptor doc as base64 string

27b743a

accept a SAML IDP descriptor document as a base64 encoded string, in addition to fetching it from a URL. don't store the url in the database anymore.

Merge remote-tracking branch 'upstream/main' into silo_authn_providers

fca4e76

david-crespo reviewed Jun 7, 2022

View reviewed changes

david-crespo approved these changes Jun 7, 2022

View reviewed changes

plotnick reviewed Jun 7, 2022

View reviewed changes

nexus/src/external_api/views.rs Outdated Show resolved Hide resolved

jmpesp added 6 commits June 8, 2022 13:52

backticks instead of quotes

9ac1572

avoid snake case silliness

32a46ae

rename ask_user_to_login_to_provider and consume_credentials_and_auth…

20fdc11

…n_user

remove unused external::SamlIdentityProvider

147a488

DerEncodedKeyPair -> just public cert

72a0272

fmt

dbdf1a9

jmpesp added 2 commits June 8, 2022 16:02

Merge remote-tracking branch 'upstream/main' into silo_authn_providers

d02f5d3

Cargo.lock update after merge

028f40a

jmpesp merged commit 5c76e0e into oxidecomputer:main Jun 8, 2022

jmpesp deleted the silo_authn_providers branch June 8, 2022 21:15

david-crespo mentioned this pull request Jun 9, 2022

[console] Rename LoginParams to SpoofLoginBody for client gen reasons #1183

Merged

davepacheco mentioned this pull request Jun 14, 2022

tracking issue for MVP IAM work #849

Closed

69 tasks

leftwo mentioned this pull request Feb 9, 2024

Update Crucible and Propolis #5039

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for SAML as a Silo IdP, part 1 #1139

Support for SAML as a Silo IdP, part 1 #1139

jmpesp commented May 30, 2022

david-crespo Jun 7, 2022

jmpesp Jun 8, 2022

david-crespo Jun 7, 2022

jmpesp Jun 8, 2022

david-crespo Jun 7, 2022 •

edited

Loading

jmpesp Jun 8, 2022

david-crespo Jun 7, 2022

jmpesp Jun 8, 2022

david-crespo Jun 7, 2022

jmpesp Jun 8, 2022

david-crespo Jun 7, 2022

jmpesp Jun 8, 2022

david-crespo Jun 7, 2022

jmpesp Jun 8, 2022

david-crespo left a comment

david-crespo commented Jun 8, 2022

	/// Client view of an ['IdentityProvider']
	/// Client view of an [`IdentityProvider`]

Support for SAML as a Silo IdP, part 1 #1139

Support for SAML as a Silo IdP, part 1 #1139

Conversation

jmpesp commented May 30, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

david-crespo Jun 7, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

david-crespo left a comment

Choose a reason for hiding this comment

david-crespo commented Jun 8, 2022

david-crespo Jun 7, 2022 •

edited

Loading