[security-dev] Some thoughts on PL Subsystem

Thu Apr 11 10:49:35 EDT 2013

So, if you guys agree we can start working on the following improvements:

    1) Rename the attribute jndi-url to jndi-name;
    2) Publish in JNDI an IdentityManager for each realm. That would look like this:

       picketlink/MyIdentityManagerFactory
       picketlink/MyIdentityManagerFactory/default (for the default realm)          
       picketlink/MyIdentityManagerFactory/SomeRealm
       picketlink/MyIdentityManagerFactory/AnotherRealm

    3) Add the default attribute for the identity-management element and handle it properly

    4) Supports a @Realm annotation in order to allow the injection of IdentityManager that maps to a specific realm

    5) Support custom entities using a attribute to specify a module from where the @IDMEntity classes are + persistence.xml;

What do you think ?

----- Original Message -----
From: "Pete Muir" <pmuir at redhat.com>
To: "Stian Thorgersen" <stian at redhat.com>
Cc: "Pedro Igor Silva" <psilva at redhat.com>, security-dev at lists.jboss.org
Sent: Thursday, April 11, 2013 11:16:14 AM
Subject: Re: [security-dev] Some thoughts on PL Subsystem

On 11 Apr 2013, at 14:35, Stian Thorgersen <stian at redhat.com> wrote:

> For custom entity classes I have two use cases in mind that we need should test/support:
> 
> * Layered product that needs to use custom entity classes for sub-systems - in this case there's no JavaEE deployments and the entity classes needs to be within a module. It's also fairly cumbersome to create an EntityManagerFactory from a subsystem so I don't think that should be required
> 
> * Two applications sharing the same custom entity classes - for example there's a main web app that contains the custom entity classes and the persistence.xml, then there's a utility war that contains one single @Startup @Singleton that is used to create some initial users - the utility war would load a lot quicker than the main web app, so the EMF may not be registered in JNDI in time
> 
> 
> ----- Original Message -----
>> From: "Pedro Igor Silva" <psilva at redhat.com>
>> To: "Stian Thorgersen" <stian at redhat.com>
>> Cc: security-dev at lists.jboss.org
>> Sent: Thursday, 11 April, 2013 2:04:17 PM
>> Subject: Re: [security-dev] Some thoughts on PL Subsystem
>> 
>> Hi Stian,
>> 
>>    Your thoughts make a lot of sense to me. Comments inline.
>> 
>> ----- Original Message -----
>>> From: "Stian Thorgersen" <stian at redhat.com>
>>> To: security-dev at lists.jboss.org
>>> Sent: Thursday, April 11, 2013 9:37:59 AM
>>> Subject: [security-dev] Some thoughts on PL Subsystem
>>> 
>>> I've had a look at https://community.jboss.org/wiki/PicketLink3Subsystem
>>> and
>>> also had a bit of a play with it. It's starting to look really good. I've
>>> just got a few suggestions:
>>> 
>>> 
>>> Suppress logging
>>> ----------------
>>> At the moment there's a lot of logging at info level produced by the
>>> subsystem, this is mostly Hibernate. It would be great if we could somehow
>>> manage to suppress this logging output, might be problematic though as
>>> Hibernate logs this stuff at INFO level when it really should be DEBUG.
>>> There's also a few WARN's we might want to look into fixing.
>> 
>> Review the logging and messages is one of the things in our TODO list.
>> 
>>> 
>>> 
>>> JNDI names in standalone.xml
>>> ----------------------------
>>> I think it makes sense to use the same format for JNDI names as the
>>> datasource element, since folks will already be used to that. So I suggest
>>> we change it slightly to look like this:
>>> 
>>> <jpa-store data-source=”java:jboss/datasources/ExampleDS" ...>
>>> <identity-management jndi-name="java:picketlink/ExampleIDM" ...>
>>> 
>>> * Full jndi name (including java:) and use jndi-name instead of jndi-url
>> 
>> +1 for that. Not sure from where I got the jndi-url if the jndi-name is like
>> a pattern used by other subsystems :)
>> 
>>> 
>>> 
>>> Manifest.mf
>>> -----------
>>> We need to make sure it works when including org.picketlink,
>>> org.picketlink.idm, etc in manifest.mf as well as
>>> jboss-deployment-structure.xml. The documentation should also reflect this.
>>> One thing I also thought of is that for the future it may be nice to have
>>> something that detects PicketLink usage in a deployment and automatically
>>> adds dependencies as required. For example if deployment uses
>>> @IdentityManager, @Identity, etc. annotations.
>>> 
>> 
>> +1. I like the idea, ans also mark them as IDM or Core deployments and handle
>> them properly.
>> 
>>> 
>>> JNDI
>>> ----
>>> @Resource doesn't require CDI, so it should be possible to do the following
>>> without CDI (and without org.picketlink.core):
>>> 
>>> @Resource(lookup = "java:/picketlink/DevIdentityManager")
>>> private IdentityManagerFactory identityManagerFactory;
>>> 
>>> I was wondering if we wanted to have the IdentityManager available in JNDI
>>> as
>>> well?
>> 
>> The problem in publishing the IdentityManager in JNDI is related with realms.
>> If the IDM config has multiple realms which one should we put ? The default
>> ?
>> 
>> Give to users the IdentityManagerFactory instead, allow them to use their
>> configurations as they want.
>> 
>> One thing that I thought about that is if is a good idea to publish all
>> IdentityManager instances for each configured realm. So, if the IDM config
>> defines multiple realms, we publish a IdentityManager instance for each of
>> them. But as we discussed this may become messy.

I think this is the right approach.

>> 
>> What do you think ?
>> 
>>> 
>>> 
>>> CDI
>>> ---
>>> I was thinking about a nice way to do the CDI support of injecting a
>>> 'default' IdentityManager. I propose adding the attribute 'default' to the
>>> 'identity-management' element (<identity-management default="true" ...>).
>>> We
>>> should throw a warning if a user has specified multiple, then we just pick
>>> one (first one?).
>> 
>> I think we had some discussion about that. I'm +1 for the default attribute.
>> 
>> Ideally, we should throw an exception if multiple configurations are provided
>> with the default attribute, IMO.

Agreed, this should be an error.

>> 
>>> 
>>> This does mean that if a 'identity-management' has the 'default' attribute
>>> set on it all deployments will by default have that IdentityManager
>>> injected
>>> into it. We also need a way for users to override this on a per-deployment
>>> basis. Can we easily detect if a deployment contains configuration for a
>>> IdentityManager itself?
>> 
>> The IMF can be obtained today in the following ways:
>> 
>>    1) From JNDI (@Resource, InitialContext, etc)
>>    2) Providing a @Producer that produces a IdentityConfiguration. In this
>>    case the deployment provides its own configuration, instead of using the
>>    subsystem config.
>>    3) When using the Core services, the deployment must specify a
>>    web.xml#resource-ref. Otherwise the deployment must provides its own
>>    configuration (normal usage of PicketLink Core)
>> 
>> Considering 2), if no IdentityConfiguration is produced, we can automatically
>> choose the default.
>> 
>> Considering 3), if no web.xml at resource-ref is defined, we can automatically
>> choose the default.
>> 
>>> 
>>> Further we need to have a way for a user to specify which IdentityManager
>>> to
>>> inject. I think this should be done based on the 'alias' attribute and not
>>> the 'jndi-name', as we should leave jndi completely out of the picture for
>>> CDI (resource-ref in web.xml/ejb.xml should be used for JNDI lookup,
>>> InitialContext#lookup and @Resource, I find it confusing to use this for
>>> CDI). I propose that we use the ServiceRegistry to retrieve the
>>> IdentityManagerFactory service based on the alias specified by a @Alias
>>> qualifer:
>> 
>> If you look at the Infinispan subsystem, this is the way it works. Using the
>> @Resource annotation to inject cachecontainers, etc.
>> 
>> I like that because it is very simple, and requires very little from our and
>> users side.

This is also the approach the spec defines to access server resources.

>> 
>> We have a test case that shows how to use CDI qualifiers. It is quite simple.
>> 
>> But at the same time, I agree that use the name is more beautiful than the
>> jndi-name :).
>> 
>> We can try that, if you want.

We shouldn't do this, it encourages the CDI anti-pattern of using string based qualifiers.

>> 
>>> 
>>> @Inject
>>> @Alias(“development”)
>>> private IdentityManager identityManager;
>>> 
>>> Obviously users should also be able to add their own qualifiers, I think
>>> this
>>> should work:
>>> 
>>> @Inject @Alias(“development”)
>>> @Produces @Development
>>> private IdentityManager identityManager;

This won't work, CDI will give you a definition error. You need to use @Resource to access server resources, or what Pedro suggests below.

>>> 
>>> One alternative to the above is to change 'alias' to 'name' then we could
>>> use
>>> the standard @Named annotation instead of @Alias.
>> 
>> We are not injecting the IdentityManager anymore, but the
>> IdentityManagerFactory. The @Alias makes sense to get a IdentityManager
>> instance for a configured realm. Maybe we should consider @Realm, instead.
>> 
>>> 
>>> 
>>> Custom Entity Classes
>>> ---------------------
>>> Personally I don't like the idea of custom entity classes (and
>>> persistence.xml) being deployed as JavaEE deployments (i.e.
>>> standalone/deployments). This is also problematic for sub-systems that
>>> wants
>>> to use the IDM if they need to use custom entity classes (there's a good
>>> chance we'll need this for EventJuggler). I also think this will be
>>> problematic if multiple deployments uses the same IdentityManager.
>>> 
>>> One idea I had was that we could create a module that contains the custom
>>> Entity classes, then specify that on the 'jpa-store' element:
>>> 
>>> <jpa-store data-source=”java:jboss/datasources/ExampleDS"
>>> custom-entity-module='org.company.acme.pl' />

This should work IMO.

>>> 
>>> The module 'org.company.acme.pl' would contain a single jar with the Entity
>>> classes. When 'custom-entity-module' is used we include that module instead
>>> of 'org.picketlink.idm.schema' module when creating the EMF + we should be
>>> able to detect the correct classes using the @IDMEntity.
>> 
>> The JPA store lets you use the EMF in two ways:
>> 
>>   1) Using a embedded persistence unit. In this case you need only yo
>>   provide the datasource. The built-in schema (pl-idm-schema) will be used.
>>   2) Using your own persistence unit. In this case you need to expose your
>>   EMF via JNDI.
>> 
>> Regarding 2), you are not forced to deploy your persistence.xml as a
>> separated deployment. You can also use the persistence unit deployed with
>> your application.
>> 
>> I'm going to create some tests so check a possible classloader issue when
>> using custom entity classes.

>> 
>>> 
>>> 
>>> _______________________________________________
>>> security-dev mailing list
>>> security-dev at lists.jboss.org
>>> https://lists.jboss.org/mailman/listinfo/security-dev
>> 
>> _______________________________________________
>> security-dev mailing list
>> security-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/security-dev
> 
> _______________________________________________
> security-dev mailing list
> security-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/security-dev