Add HSM Support for Decrypting Assertions

[ryan13mt]

To solve issue: #9044

[jzheaux]

Thanks, @ryan13mt, for another PR! I've left some feedback inline.

[jzheaux]

Thanks for the updates, @ryan13mt! I like where this is headed; I've left some feedback inline.

Also, just a reminder to add tests for the setters. I believe it would suffice to use mock Consumer instances.

[jzheaux]

This isn't thread-safe. Because the decrypter instance is changed with each authentication, two concurrent authentication attempts will clobber each other's value here.

Instead, the default values below will need to construct a Decrypter using the details from Saml2AuthenticationToken. For assertionDecrypter, I think this will look something like:

Decrypter decrypter = this.decrypterConverter.convert(responseToken.getToken());
for (EncryptedAssertion encryptedAssertion : responseToken.getResponse().getEncryptedAssertions()) {
// ...

with something similar being done in principalDecrypter.

[jzheaux]

To simplify the code, I recommend removing these getters and having the code refer directly to the fields.

[ryan13mt]

@jzheaux locally this is failing as well like the other pull request. Not really sure on what the reason is.

Execution failed for task ':spring-security-samples-boot-oauth2resourceserver-webflux:integrationTest'
Execution failed for task ':spring-security-samples-boot-oauth2webclient-webflux:test'

[jzheaux]

Thanks for the updates, @ryan13mt! In preparation for merging, will you please squash your commits? Also, if you like, please consider adding yourself to the list of authors.

[ryan13mt]

Thanks @jzheaux hope i squashed my commits the right way. Is there anything else needed to close this PR please?

Also i noticed this is not linked to the issue yet.

[ryan13mt]

Hi again @jzheaux is there any other input needed from my end cause i am going to be reassigned into a new project and would have very limited time to contribute further to this and the other pull request #9056

Thanks and sorry for the spam 🤣

[jzheaux]

Thanks for your contribution, @ryan13mt! This is now merged into master via 535ae3e as well as a polish commit of d0581c9 to adjust the method names.

In a few minutes, it should be available in 5.5.0-SNAPSHOT in https://repo.spring.io/libs-snapshot. Please try it out to make sure it meets your needs.

[ryan13mt]

Hello @jzheaux thanks for closing this issue and merging to master.

I am finding a problem with the polishing commit. You added a check in d0581c9#diff-e1434c9a229d033aa3fe922ab7b9637a54ba5a2521711e3b362e70bc7f787833R511 where you are checking if a response is signed and if true, doing the elements decryption. Is there a reason this check was added?

I believe this is not correct since there are use cases where the idp will sign and encrypt the assertion but will leave the response unsigned as is in our case. Thus having an unsigned SAML Response with an encrypted signed Assertion should still decrypt the assertion and then checking that it has been signed before throwing the error "Either the response or one of the assertions is unsigned. Please either sign the response or all of the assertions.". or in my case it's throwing "No assertions found in response." since the encrypted assertions have not being decrypted yet.

With this new check, the library is now requiring that a response must be signed if it contains an encrypted assertion.

Thanks