Hi Stian,
Your thoughts make a lot of sense to me. Comments inline.
----- Original Message -----
From: "Stian Thorgersen" <stian(a)redhat.com>
To: security-dev(a)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@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(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/security-dev