Skip to content

Add Static Factories to Saml2X509Credential #8822

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
Closed
wants to merge 1 commit into from
Closed

Add Static Factories to Saml2X509Credential #8822

wants to merge 1 commit into from

Conversation

ThomasVitale
Copy link
Contributor

  • Add static factories to Saml2X509Credential for verification, encryption,
    signing, and decryption.
  • Add unit tests for new static factories in Saml2X509Credential.

Fixes gh-8789

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 9, 2020
@ThomasVitale ThomasVitale mentioned this pull request Jul 9, 2020
Closed
@jzheaux
Copy link
Contributor

jzheaux commented Jul 10, 2020

@ThomasVitale, thanks for the PR!

Based on the build output, it appears some tests may have failed. Are you able to take a look at those?

@jzheaux jzheaux self-assigned this Jul 10, 2020
@jzheaux jzheaux added in: saml2 An issue in SAML2 modules type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Jul 10, 2020
@ThomasVitale
Copy link
Contributor Author

@jzheaux Yeah, something failed but it didn't seem linked to the new code in this PR. After pulling the latest changes from the master branch and running the build again, now there's no failure.

jzheaux
jzheaux requested changes Jul 14, 2020
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.

Thanks for the PR, @ThomasVitale! I've left some inline feedback about the JavaDoc.

@ThomasVitale
Copy link
Contributor Author

The build is failing with different errors not related to my changes, including an unavailable "commons-codec" dependency. How can I fix that?

jzheaux
jzheaux requested changes Jul 16, 2020
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.

Thanks, @ThomasVitale! I've left one more piece of feedback.

Regarding the build, I'm seeing the same thing locally about commons-codec. I'm not sure what's happening yet, but I agree it's unrelated to your changes.

@ThomasVitale
Add Static Factories to Saml2X509Credential
329261b 
- Add static factories to Saml2X509Credential for verification, encryption,
signing, and decryption.
- Add unit tests for new static factories in Saml2X509Credential.

Fixes gh-8789
@jzheaux
Copy link
Contributor

jzheaux commented Jul 20, 2020

Thanks, @ThomasVitale! This is now merged into master via 3978cc5 and 97ccbe5.

Note that credentials.Saml2X509Credential was deprecated in favor of core.Saml2X509Credential recently so I added a polish commit to move the methods to the new class in core`.

@jzheaux jzheaux closed this Jul 20, 2020
@jzheaux jzheaux added this to the 5.4.0-RC1 milestone Aug 5, 2020
@jzheaux jzheaux added the status: duplicate A duplicate of another issue label Aug 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jzheaux jzheaux jzheaux approved these 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.4.0-RC1
Development

Successfully merging this pull request may close these issues.

Add Static Factories to Saml2X509Credential
3 participants
@ThomasVitale @jzheaux @spring-projects-issues