Allow applications to customize how roles are decoded from raw identity #3124
Conversation
pedroigor
changed the title
|
@pedroigor could you run the formatter and amend your commit, please? Thanks! |
|
|
||
| @Override | ||
| public Roles decodeRoles(AuthorizationIdentity authorizationIdentity) { | ||
| RoleDecoder delegate = getDelegate(); |
There was a problem hiding this comment.
Could we do that in the constructor instead? It doesn't sound like a good idea to do that every time.
There was a problem hiding this comment.
Yeah, but if we use the constructors we are assuming that app-defined instances are application-scoped and I'm not sure if that is always the case.
Or are you thinking about something else that I may be missing?
There was a problem hiding this comment.
If you think that we should worry about performance now, maybe we could check whether or not the first produced instance is application-scoped and then cache it?
Or maybe all this logic to use application provided instances into instances created during the build steps could be managed by Quarkus somehow? For instance, one drawback with the approach I'm using is that we may want to allow applications to provide instances for RoleMapper or PermissionMapper. For each of them we would need a similar instance as DefaultRoleDecoder just to use Instance to obtain the delegate.
| if (delegate == null) { | ||
| Attributes.Entry groups = authorizationIdentity.getAttributes().get(DEFAULT_ATTRIBUTE_NAME); | ||
| Set<String> roles = new HashSet<>(groups); | ||
| return new Roles() { |
There was a problem hiding this comment.
I would make it a proper static inner class.
There was a problem hiding this comment.
I realized that Roles provides a fromSet method that serves to the same purpose. Thanks for catching this.
pedroigor
force-pushed
the
issue-3123
branch
2 times, most recently
from
e8a1767
to
c8bd01f
Compare
…e decoded from raw identity
|
I force pushed a rebase due to conflicts with the recent s/template/recorder/ changes. Let's wait to see what CI has to say. |
gsmet
added
the
triage/waiting-for-ci
gsmet
changed the title
Fixes #3123