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.

Sign up for GitHub

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

Jump to bottom

make SP NameIDPolicy configurable in RelyingPartyRegistration #9227

Closed
wants to merge 2 commits into from
Closed

make SP NameIDPolicy configurable in RelyingPartyRegistration #9227

wants to merge 2 commits into from

Conversation

amergey
Copy link
Contributor

@amergey amergey commented Dec 1, 2020

closes gh-9115

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Dec 1, 2020
@jzheaux
Copy link
Contributor

jzheaux commented Dec 1, 2020

Thanks for the PR, @amergey! This PR seems broader than the scope of #9115, so I think we first should take a step back and make sure that we're on the same page about what support should go into the framework and where.

To start that, will you please add some detail to #9115 about your use case?

@jzheaux jzheaux self-assigned this Dec 1, 2020
@amergey
Copy link
Contributor Author

amergey commented Dec 9, 2020 •
edited
Loading

I have updated #9115, I am not the reporter so, I added a comment, I cannot edit the issue.

jzheaux
jzheaux requested changes Nov 5, 2021
View reviewed changes
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.

@amergey, thanks for your patience on this review. I've left some feedback inline.

Would you please update the copyright year on any edited files and also squash your commits?

We can schedule this for 5.7.0-M1.

...g/springframework/security/saml2/provider/service/registration/RelyingPartyRegistration.java Outdated
@@ -81,6 +81,8 @@

private final Saml2MessageBinding assertionConsumerServiceBinding;

private final String nameIDFormat;
Copy link
Contributor

Choose a reason for hiding this comment

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

For consistency, will you please lowercase acronyms like so: nameIdFormat. Please do the same for the associated getter and setter.

...g/springframework/security/saml2/provider/service/registration/RelyingPartyRegistration.java Outdated
/**
* Get the NameID format.
* @return the NameID format
* @since 5.5
Copy link
Contributor

Choose a reason for hiding this comment

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

Please update @since flags to 5.7

@jzheaux jzheaux added in: saml2 An issue in SAML2 modules status: duplicate A duplicate of another issue type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 5, 2021
@jzheaux jzheaux added this to the 5.7.0-M1 milestone Nov 5, 2021
@amergey
Copy link
Contributor Author

amergey commented Nov 22, 2021

I tried to made requested changes, but this PR is quite old already and I have trouble to rebase it to main, it would have been better to make these suggestions last year ....

@jzheaux jzheaux modified the milestones: 5.7.0-M1, 6.0.0-M1 Dec 1, 2021
@jzheaux
Copy link
Contributor

jzheaux commented Dec 1, 2021

it would have been better to make these suggestions last year ....

I'm sorry it took so long to get back to this. I corrected the rebase and this is now merged via a17dfb8456cf3a517d44cfcccdaa7a1d2ae6121f for 6.0 and dbe4d704f844d7d3a4b7048e004c07e1496033d2 for 5.7.

@jzheaux jzheaux closed this Dec 1, 2021
@jzheaux jzheaux modified the milestones: 6.0.0-M1, 5.7.0-M1 Oct 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jzheaux jzheaux jzheaux requested changes

Assignees

@jzheaux jzheaux

Labels
in: saml2 An issue in SAML2 modules status: duplicate A duplicate of another issue type: enhancement A general enhancement
Projects
None yet
Milestone
5.7.0-M1
Development

Successfully merging this pull request may close these issues.

Add NameIdFormat support to RelyingPartyRegistration
3 participants
@amergey @jzheaux @spring-projects-issues