[security-dev] Some thoughts on PL Subsystem
stian at redhat.com
Thu Apr 11 08:37:59 EDT 2013
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:
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.
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
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.
@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?
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?).
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?
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:
private IdentityManager identityManager;
Obviously users should also be able to add their own qualifiers, I think this should work:
private IdentityManager identityManager;
One alternative to the above is to change 'alias' to 'name' then we could use the standard @Named annotation instead of @Alias.
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' />
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.
More information about the security-dev