Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[2.10.0] - Update SAML Pages with SLO Feature #1335

Merged
merged 6 commits into from
Nov 6, 2024

Conversation

sunilarjun
Copy link
Contributor

@sunilarjun sunilarjun commented Jun 11, 2024

Added information regarding new feature SAML SLO configuration through Rancher UI in SAML pages. Rancher issue link for reference: rancher/dashboard#10941

Fixes docs issue #1331

…in Okta SAML page. Updating to other SAML pages currently after UX PR was finalized.

Signed-off-by: Sunil Singh <[email protected]>
@sunilarjun sunilarjun added this to the v2.9-Next1 milestone Jun 11, 2024
@martyav martyav added the MERGE ON RELEASE Don't merge until the moment the next release publishes label Jun 24, 2024
@sunilarjun sunilarjun modified the milestones: v2.9-Next1, v2.10-Next1 Jul 11, 2024
@sunilarjun sunilarjun changed the title [2.9.0] - Update SAML Pages with SLO Feature [2.10.0] - Update SAML Pages with SLO Feature Jul 12, 2024
@martyav
Copy link
Contributor

martyav commented Jul 26, 2024

Since we have so many "Merge on release" PRs for 2.9.0 and 2.8.6, I'm going to remove the label and add a "DO NOT MERGE" so that there's less confusion on the next release day

@martyav martyav added DO NOT MERGE and removed MERGE ON RELEASE Don't merge until the moment the next release publishes labels Jul 26, 2024
@sunilarjun sunilarjun marked this pull request as ready for review November 5, 2024 16:06
@sunilarjun
Copy link
Contributor Author

sunilarjun commented Nov 5, 2024

Hello @aalves08! I am not able to tag you as a reviewer but would you be able to look over the content addition and see if there is anything missing or that needs to be adjusted for the configuration steps, thank you!

Copy link
Contributor

@btat btat left a comment

Choose a reason for hiding this comment

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

If this is applicable to all SAML providers, Microsoft AD FS and Shibboleth are missing.

Not a blocker, but since this block is repeated multiple times per version it may be worth putting it in a partial/shared-files (similar to #1142) to keep things DRY.

@sunilarjun
Copy link
Contributor Author

@btat Added the content to Microsoft AD FS/Shibboleth/translation pages, as well as added _configure-slo.md file to shared files to go the partial route, thanks for reviewing!

@btat
Copy link
Contributor

btat commented Nov 5, 2024

@sunilarjun can you update the base branch to v2.10.0 so that we don't publish the changes to /docs right away when this gets merged. Everything else LGTM. Will submit approval after the base change.

@sunilarjun sunilarjun changed the base branch from main to v2.10.0 November 5, 2024 23:03
@sunilarjun
Copy link
Contributor Author

@btat Just updated base branch, thanks for catching!

@sunilarjun sunilarjun changed the base branch from v2.10.0 to main November 6, 2024 00:22
@sunilarjun sunilarjun changed the base branch from main to v2.10.0 November 6, 2024 00:22
@btat btat merged commit da8d7eb into rancher:v2.10.0 Nov 6, 2024
1 check passed
@sunilarjun sunilarjun mentioned this pull request Nov 6, 2024
@aalves08
Copy link
Member

aalves08 commented Nov 6, 2024

@sunilarjun, sorry for the late reply, but I've been held up with other topics

The _configure-slo looks fine. I would just add that the UI part about the Log Out behavior will only appear if that particular SAML provider allows for SAML SLO. Some may not provide that option, but I've only tested it with Okta so take that with a grain of salt (meaning that the backend will send a flag logoutAllSupported, which is the trigger for the UI to show the Log Out behavior UI part).

If the Log Out behavior UI is not available for a SAML provider, it means that the backend hasn't deemed it supported.

Probably worth mentioning because it may reduce number of support issues being filed 🙏

@sunilarjun sunilarjun mentioned this pull request Nov 7, 2024
@sunilarjun
Copy link
Contributor Author

Hello @aalves08 thanks for the response! I added in a note in this docs PR here that hopefully clarifies, let me know if any other edits are needed. Thank you!

@aalves08
Copy link
Member

aalves08 commented Nov 7, 2024

That's perfect @sunilarjun 🙏 Thank you 🙇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants