Decorators and alternatives not working on IdentityStores

[glassfishrobot]

I've created the following decorator to do some operations with the returned groups:

@Priority(2000)
@Decorator
public class IdentityStoreDecorator implements IdentityStore {

    @Inject
    @Delegate
    @Any
    private IdentityStore decorated;

    @Override
    public CredentialValidationResult validate(Credential credential) {
        return decorated.validate(credential);
    }

    @Override
    public Set<String> getCallerGroups(CredentialValidationResult validationResult) {
        return decorated.getCallerGroups(validationResult);
    }

}

But the CdiUtils#getBeanDefinitions() method which is internally called to get the enabled IdentityStores returns the original object (org.glassfish.soteria.cdi.CdiProducer interface javax.security.enterprise.identitystore.DatabaseIdentityStoreDefinition).

I guess the problem is on the org.glassfish.soteria.cdi.CdiProducer which is creating instances without taking decorators into account. The same applies to @Alternative's.

[glassfishrobot]

• Issue Imported From: https://github.com/javaee/security-soteria/issues/106
• Original Issue Raised By:@ggam
• Original Issue Assigned To: Unassigned

[glassfishrobot]

@arjantijms Commented I'm not 100% sure if it's the creating, since both Decorators and Alternatives are supposed to work on the Bean<T> that's added to the CDI runtime, not on the instance that's returned from the create() method of said Bean<T>.

I wonder, could this not be a bug with the CDI implementation, that Decorators in general don't work on a programmatically added Bean<T>?

Which CDI implementation did you try it on?

[glassfishrobot]

@arjantijms Commented Btw, just thinking but because the code asks for all bean definitions, is a decorator or alternative even supposed to work?

At least the alternative for the handler does work, and is tested for here: https://github.com/javaee-security-spec/soteria/blob/master/test/app-custom-identity-store-handler/src/main/java/org/glassfish/soteria/test/CustomIdentityStoreHandler.java#L67

[glassfishrobot]

@ggam Commented I was using WildFly patched to used latest Weld for CDI 1.2 (2.4.4). I can't provide much more info right now but I'll try to create a reproducer so we all can verify it on e.g. TomEE which uses OpenWebBeans.

[glassfishrobot]

@arjantijms Commented Ok, thx!

If I try an alternative for a build-in mechanism (like, e.g. Basic), things do work:

@Alternative
@Priority(1000)
public class AlternativeMechanism implements HttpAuthenticationMechanism {

    @Override
    public AuthenticationStatus validateRequest(HttpServletRequest request, HttpServletResponse response, HttpMessageContext httpMessageContext) throws AuthenticationException {
        System.out.println("alternative");
        return AuthenticationStatus.SUCCESS;
    }
    
}

But on Weld at least the decorator indeed doesn't. I've to investigate, but I vaguely remember trying it with adding the most basic Bean<T> programmatically before and there it didn't work either.

[glassfishrobot]

@ggam Commented Here's a sample reproducer WAR: example.zip

The alternative works for HttpAuthenticationMechanism as you said, but is ignored for the IdentityStore. Seems like the beanReference = beanManager.getReference(bean, type, beanManager.createCreationalContext(bean)); is not behaving the same way as CDI.select() which is used on the authentication mechanism case. Any reason not to use that method anyway?

[glassfishrobot]

@arjantijms Commented It may not be beanManager.createCreationalContext(bean)) vs CDI.select(), but just the fact that multiple identity stores are being requested.

If the HttpAuthenticationMechanism is part of the .war, then if I remember correctly the decorator for it does work.

CDI.select() as well as the bean manager one obtains from JNDI are not exactly clear about the classes they see (which bean archives are visible to it). But in practice it seems the JNDI one always contains both container classes and application archive classes, while for CDI.select() it depends.

Still have to get to the bottom of this though.

[glassfishrobot]

@ggam Commented So do you have enough information to investigate it? Or do you want me to do something more?

Just to explain my usecase: I now have a (WildFly) security domain with a custom group to role mapping strategy, where the "admin" group maps to both "admin" and "category_manager" roles.

Since the Security Spec has unfortunately not touched the mapping, and I can't use WildFly provided mapping modules with Soteria's SAM, I thought about manually doing the mapping myself. The simplest approach I found was this @Decorator/@Alternative idea.

Another solution I'm now thinking about would be to create two @DatabaseIdentityStoreDefinition's, one for validation and another one for groups, and then a third (custom) identity store which adds the needed roles. That could do the trick I guess and would be a more "spec complaint" solution.

[glassfishrobot]

@ggam Commented Also, regarding the @Inject vs JNDI bean manager, I believe Antoine is thinking about a CDI 2.1 MR to be started relatively soon. This could be an item for such a MR.

[glassfishrobot]

@arjantijms Commented

Also, regarding the @Inject vs JNDI bean manager, I believe Antoine is thinking about a CDI 2.1 MR to be started relatively soon. This could be an item for such a MR.

I already brought this to the attention of the CDI EG a while back, so might be a good item indeed.

See:

• http://lists.jboss.org/pipermail/cdi-dev/2016-April/008181.html
• http://lists.jboss.org/pipermail/cdi-dev/2016-May/008297.html

Just to explain my usecase: I now have a (WildFly) security domain with a custom group to role mapping strategy, where the "admin" group maps to both "admin" and "category_manager" roles.

That's indeed a valid use case for this, but wrt to the alternatives, don't forget that if you define 1 alternative it overshadows all others. See e.g. this https://struberg.wordpress.com/2012/05/11/the-tricky-cdi-serviceprovider-pattern

Decorating at least the handler should really work. Decorating beans when there are multiple candidates, I'm not sure how that should work. I mean, how do you know which delegate the injected delegate in the decorator refers to if there are multiple candidates?

Since the Security Spec has unfortunately not touched the mapping, and I can't use WildFly provided mapping modules with Soteria's SAM,

Technically it should work, as the AS should not distinguish where the groups come from (SAM, HAM or proprietary authentication mechanism). Or do you mean WildFly's one is not convenient to use?

[glassfishrobot]

@ggam Commented

Technically it should work, as the AS should not distinguish where the groups come from (SAM, HAM or proprietary authentication mechanism). Or do you mean WildFly's one is not convenient to use?

In WildFly you can create a login-stack where a SAM delegates to some provided JAAS login module, but I believe the SAM needs to extend some propietary "delegator" class. At least I haven't found a way to make it work on Soteria.

[glassfishrobot]

@ggam Commented I'm not so sure the problem comes from BeanManager class visibility. I've now decorated the IdentityStoreHandler and injected it on a test Servlet via @Inject, but I'm still getting the original instance.

But Weld Probe tells me the decorator has been associated:

[glassfishrobot]

@arjantijms Commented

n WildFly you can create a login-stack where a SAM delegates to some provided JAAS login module, but I believe the SAM needs to extend some propietary "delegator" class. At least I haven't found a way to make it work on Soteria.

Delegating to a JAAS login module is a different case from group to role mapping, but that too was in scope for this JSR.

One of the proposals was to let the container delegate to whatever proprietary store has been configured at the container level (possibly, but not necessarily a jaas login module) if no identity store has been configured.

I've to check if that made it into the spec.

[glassfishrobot]

@ggam Commented @wmhopkins this is a limitation/bug on Weld. We should definitely reach the Weld team and check on other CDI implementations (i.e. Apache OpenWebBeans), but there's little we can do about it for now. So I think you can safely remove the "RI Code Complete" label since this is something that will presumably take time to fix.

[glassfishrobot]

@wmhopkins Commented @ggam, @arjantijms : Once CDI/Weld is fixed, would we need to make any changes in our CDI extension or anything else?

If not, then this is purely a Weld issue -- nothing for us to do -- and can just be closed.

[glassfishrobot]

@ggam Commented I don't think any changes will be needed from our side. I know Arjan dealt with issues like this in the past so he will probably know better.

If you don't mind, I'd leave this issue open to remind us that we have to:

• Report this to Weld
• Check if it works on TomEE/OpenWebBeans

Maybe you can create a new "future" label so we can easily filter out?

[glassfishrobot]

@wmhopkins Commented @ggam -- We can leave this open for a bit. Created the "Futures" milestone. (And discovered you can use "-" to filter things out, even though it's not documented on the GitHub filter syntax page.)

[glassfishrobot]

@arjantijms Commented The negation operator is the "-" symbol I think. You need to put it before the entire label expression.

On Tue, Aug 8, 2017 at 8:57 PM, Will Hopkins [email protected] wrote:

We can leave this open for a bit. I don't think the github filter syntax supports filtering things out, though -- I haven't found a negation operator (or logical expressions like and/or). You can filter it out by selecting the RI Code Complete milestone.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/javaee/security-soteria/issues/106#issuecomment-321064335, or mute the thread https://github.com/notifications/unsubscribe-auth/AC5XTtAffMGWMb8-YilY4DnYMyj6KVX5ks5sWL2egaJpZM4OX3zZ .

[glassfishrobot]

@wmhopkins Commented @arjantijms -- Yup, figured that out and updated my comment. It doesn't seem to be documented in the GitHub syntax reference, but indeed it works.

[arjantijms]

I created a new, more specialised issue for this at: eclipse-ee4j/soteria#225

I'm working on a fix now.

[arjantijms]

@ggam I've created a CDI spec issue for this here: https://github.com/eclipse-ee4j/cdi/issues/459