Skip to content

Conversation

albertodonato
Copy link
Contributor

@albertodonato albertodonato commented Jul 16, 2025

ENG-4075

what

support selecting an identity provider in auto-configure when the server
reports multiple.
If multiple IDPs are detected, the --idp option is required to pick one.

why

Allow logging in when multiple IDPs are configured in platform.

testing

tested against my sandbox with the corresponding platform change

docs

n/a

@albertodonato albertodonato requested a review from a team as a code owner July 16, 2025 06:41
@albertodonato albertodonato force-pushed the ack/multiple-idp-support branch 2 times, most recently from f972775 to 23f1163 Compare July 16, 2025 07:09
tvansteenburgh
tvansteenburgh previously approved these changes Jul 16, 2025
Copy link
Contributor

@tvansteenburgh tvansteenburgh left a comment

Choose a reason for hiding this comment

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

Thanks @albertodonato. Good with it as-is, but a test would be nice.

@@ -134,13 +134,17 @@ def configure(
required=True,
help="A Stacklet console URL or deployment prefix",
)
@click.option(
"--idp",
help="Name of the Identity Provider to use for authentication, in case more than one are used",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
help="Name of the Identity Provider to use for authentication, in case more than one are used",
help="Name of the Identity Provider to use for authentication, if more than one is available",

if len(saml_config) == 1:
idp_id, _ = saml_config.popitem()
else:
name_to_id = {idp_id: name for name, idp_id in saml_config.items()}
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems backwards to me, which makes it hard to understand when reading the code. Shouldn't it be:

name_to_id = {name: idp_id for idp_id, name in saml_config.items()}

[ENG-4075](https://stacklet.atlassian.net/browse/ENG-4075)

### what

support selecting an identity provider in `auto-configure` when the server
reports multiple.
If multiple IDPs are detected, the `--idp` option is required to pick one.

### why

Allow logging in when multiple IDPs are configured in platform.

### testing

tested against my sandbox with the corresponding platform change

### docs

n/a
@albertodonato albertodonato merged commit 930a268 into main Jul 16, 2025
7 checks passed
@albertodonato albertodonato deleted the ack/multiple-idp-support branch July 16, 2025 13:49
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.

2 participants