Skip to content

Conversation

@ghaege
Copy link

@ghaege ghaege commented Jan 25, 2023

Add RelayState Customizer to SAML Logout

Closes gh-12538

Copy link
Contributor

@jzheaux jzheaux left a comment

Choose a reason for hiding this comment

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

Thanks, @ghaege! I appreciate the contribution, and sorry that it took me a bit to get back to you. I've left some feedback inline. When you submit your update, please squash your commits into a single one.

In addition to that, please have that single commit include the bug that it being closed, like so:

Your Commit Title

Closes gh-12538

Thanks!

@jzheaux jzheaux self-assigned this Feb 7, 2023
@jzheaux jzheaux added type: enhancement A general enhancement in: saml2 An issue in SAML2 modules and removed status: waiting-for-triage An issue we've not yet triaged labels Feb 7, 2023
@jzheaux jzheaux added this to the 6.1.0-M2 milestone Feb 7, 2023
@ghaege
Copy link
Author

ghaege commented Feb 9, 2023

Thanks, @jzheaux for your feedback.
But since your recommendations are mainly by "polishing-nature and spring internal rules" and I don't have time, I would like to ask you to do the changes.

Thanx for your understanding.

@jzheaux
Copy link
Contributor

jzheaux commented Feb 15, 2023

Thanks, @ghaege. No problem, we'll make sure this gets in.

To keep each commit atomic, I am going to need to split the unrelated cleanups into a separate commit. This will remove your authorship, and I want to make sure you get credit. So, what I propose is that I will instead add the relay state feature myself and add you as a co-author in the commit message. Does that sound okay to you? I'll go ahead and proceed this way if I don't hear from you in the next few days.

If you want to resubmit the cleanups once you have time again, then you can always rebase your gh-12538 branch with main and resubmit the PR.

@ghaege
Copy link
Author

ghaege commented Feb 16, 2023

Perfect - do it like you proposed.

@jzheaux jzheaux merged commit c1c2837 into spring-projects:main Feb 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

in: saml2 An issue in SAML2 modules type: enhancement A general enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Custom RelayState with OpenSamlLogoutRequestResolver

3 participants