Skip to content

Add Static Factories to Saml2X509Credential #8789

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

Closed
jzheaux opened this issue Jul 2, 2020 · 3 comments
Closed

Add Static Factories to Saml2X509Credential #8789

jzheaux opened this issue Jul 2, 2020 · 3 comments
Assignees
Labels
in: saml2 An issue in SAML2 modules type: enhancement A general enhancement
Milestone

Comments

@jzheaux
Copy link
Contributor

jzheaux commented Jul 2, 2020

Saml2X509Credential constructors are somewhat complicated to use.

For example, there are possible constructs that don't make sense:

new Saml2X509Credential(privateKey, x509Cerificate, Saml2X509CredentialType.ENCRYPTION)

The above is awkward in the context of SAML 2.0 since the common use case for encryption is to encrypt something using the other party's public certificate, not their private key as well.

Also, some constructs are possible at compile-time, but then throw an exception at runtime:

new Saml2X509Credential(x509Cerificate, Saml2X509CredentialType.DECRYPTION)

When run, the above will error because no private key was supplied.

Both of these scenarios can be addressed by adding some simple static factories to Saml2X509Credential like:

public static Saml2X509Credential encryption(X509Certificiate certificate) { ... }

public static Saml2X509Credential decryption(PrivateKey key, X509Certificate certificate) { ... }

The nice thing about these as well is that they remove the application's need to work with the Saml2X509CredentialType class, further simplifying their application.

@jzheaux jzheaux added type: enhancement A general enhancement status: ideal-for-contribution An issue that we actively are looking for someone to help us with in: saml2 An issue in SAML2 modules labels Jul 2, 2020
@jzheaux jzheaux added this to the 5.4.0-RC1 milestone Jul 2, 2020
@ThomasVitale
Copy link
Contributor

Hi @jzheaux, can I help with this task?

@jzheaux
Copy link
Contributor Author

jzheaux commented Jul 7, 2020

Nice, @ThomasVitale! The issue is yours.

@jzheaux jzheaux removed the status: ideal-for-contribution An issue that we actively are looking for someone to help us with label Jul 8, 2020
@ThomasVitale
Copy link
Contributor

@jzheaux I opened a PR here: #8822 I added static factories for the 4 main operations plus unit tests. How does it look?

jzheaux added a commit that referenced this issue Jul 20, 2020
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 a pull request may close this issue.

2 participants