[security-dev] Some thoughts on PL Subsystem

Thu Apr 11 09:04:17 EDT 2013

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.

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.

> 
> 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.

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.

> 
> @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;
> 
> 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' />
> 
> 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