ELY-2921: Add principal tranformer support to TokenSecurityRealm#2296
ELY-2921: Add principal tranformer support to TokenSecurityRealm#2296darranl merged 1 commit intowildfly-security:2.xfrom
Conversation
| this.claimToPrincipal = configuration.claimToPrincipal; | ||
| } | ||
|
|
||
| if (configuration.principalTransformer == null) { |
There was a problem hiding this comment.
Is the "if" stmt needed. I appears that this.principalTransormer is always assigned to what ever the value of configration.principalTransformer is.
There was a problem hiding this comment.
you're right! I've removed the if statement.
|
|
||
| TokenRealmIdentity(Evidence evidence, Function<Principal, Principal> principalTransformer) { | ||
| this(evidence); | ||
| if (principalTransformer != null) { |
There was a problem hiding this comment.
As above is the "if" stmt really needed here?
There was a problem hiding this comment.
Turns out that this constructor was not needed at all, so I've removed it.
|
Thanks, @rsearls for the review! i've made the required changes and also added a test. Would you mind having another look? |
|
Hi @theashiot , thanks for working on this! Just checking, this issue is a part of a feature request https://issues.redhat.com/browse/WFCORE-7301 ? SHould we have proposal PR to the wildfly-proposals for this? |
|
FYI on this one I think we will be able to merge to Elytron once we are happy it satisifes the requiurements of the feature, I believe this is not a part of public API and it will not be available until end users until the subsystem changes are mered so I think we can merge early. |
|
FYI had a catch up with @theashiot who will add a couple more tests before we merge. |
|
I added a test to check what happens when a null is passed instead of a principal-transformer: https://github.com/wildfly-security/wildfly-elytron/pull/2296/files#diff-751480ea2b6fff70f36f431bf3f6170c6bdb75a2c5e411f71ab66e3a1f0efeb5R135 best, ashwin |
4 participants
https://issues.redhat.com/browse/ELY-2921