Skip to content

Add the SAML remote logout endpoint to the metadata#5709

Merged
orenyk merged 1 commit intomainfrom
oyk-saml-metadata-logout
May 6, 2022
Merged

Add the SAML remote logout endpoint to the metadata#5709
orenyk merged 1 commit intomainfrom
oyk-saml-metadata-logout

Conversation

@orenyk
Copy link
Contributor

@orenyk orenyk commented Dec 15, 2021

Why: This needs to be configured as a separate item after upgrading
the saml_idp gem.

This commit also restricts remote logout requests to the POST HTTP
method since that is the only binding we're supporting for that
functionality (not HTTP-Redirect)

@orenyk orenyk requested a review from jmhooper December 15, 2021 02:36
@orenyk orenyk force-pushed the oyk-saml-metadata-logout branch 2 times, most recently from 2577b6e to 90c5fda Compare December 15, 2021 03:29
@orenyk
Copy link
Contributor Author

orenyk commented Dec 15, 2021

Blocked on #5710

Gemfile Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO: replace once 18F/saml_idp#51 is merged and the tag is pushed

config/routes.rb Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the other actions since we really only support the POST binding and not the Redirect binding.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This actually wasn't necessary to keep the tests passing so I also added request specs to ensure that we're only permitting POST requests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed this file, the first describe block is old.

Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM

@orenyk orenyk force-pushed the oyk-saml-metadata-logout branch 2 times, most recently from e6bb64e to ef6b8e9 Compare December 16, 2021 03:34
@orenyk
Copy link
Contributor Author

orenyk commented Dec 16, 2021

Going to hold on merging this in until January, we'll want to have partners test it in int for a couple of weeks to make sure it doesn't cause any problems.

@orenyk orenyk force-pushed the oyk-saml-metadata-logout branch 2 times, most recently from 58d7104 to 2e09a8a Compare December 17, 2021 01:21
@zachmargolis
Copy link
Contributor

Is this change still being tested? PR has been open over 2 months

@orenyk
Copy link
Contributor Author

orenyk commented Mar 3, 2022

@zachmargolis whoops no, it just fell off my radar. We need some time to draft an email to partners letting them know and then I think I'd want to give them ~1 week in int to make sure nothing breaks.

@aduth
Copy link
Contributor

aduth commented May 4, 2022

Can we close this and #5652 ? We can always bring it back if we need, but I don't feel like we should leave pull requests opened for months.

@orenyk
Copy link
Contributor Author

orenyk commented May 5, 2022

@aduth thanks for flagging - this should be merge-able, I just have had zero time to get back to it and draft the partner notification. I'll work with @jmhooper on getting this and #5652 out of the door.

@orenyk orenyk force-pushed the oyk-saml-metadata-logout branch 4 times, most recently from a1d2085 to 724812d Compare May 5, 2022 04:30
**Why:** This needs to be configured as a separate item after upgrading
the saml_idp gem.

This commit also restricts remote logout requests to the POST HTTP
method since that is the only binding we're supporting for that
functionality (not HTTP-Redirect)

changelog: Improvements, Authentication, Add SAML remote logout endpoint to metadata
@orenyk orenyk force-pushed the oyk-saml-metadata-logout branch from 724812d to ab29f7b Compare May 6, 2022 17:06
@orenyk orenyk merged commit 9ece88c into main May 6, 2022
@orenyk orenyk deleted the oyk-saml-metadata-logout branch May 6, 2022 18:37
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