[security-dev] Some thoughts on PL Subsystem
Shane Bryzak
sbryzak at redhat.com
Thu Apr 11 18:59:21 EDT 2013
There's a not insignificant cost in creating an IdentityManager
instance. If we can't have a new instance injected each time with
@Resource, then it's better to not support the feature at all rather
than resorting to a hack to make it work.
On 12/04/13 08:52, Pedro Igor Silva wrote:
> We're already making the IMF available. The idea is support both the IMF and IMs.
>
> The IdentityManager idea came up after some discussions earlier. I have proposed that but I at that time I was not sure if we should do it or not. As Pete agreed with this, I did it.
>
> If you think this is not ideal, we can review that and let only the IMF exposed via JNDI. As it was before.
>
> Regards.
> Pedro Igor
>
> ----- Original Message -----
> From: "Shane Bryzak" <sbryzak at redhat.com>
> To: "Pedro Igor Silva" <psilva at redhat.com>
> Cc: security-dev at lists.jboss.org
> Sent: Thursday, April 11, 2013 7:29:37 PM
> Subject: Re: [security-dev] Some thoughts on PL Subsystem
>
> This is not ideal at all. A better option would just be to make the
> IdentityManagerFactory available instead, and the application can then
> create its own IdentityManager instances.
>
> On 12/04/13 08:05, Pedro Igor Silva wrote:
>> To overcome this I'm thinking to use a wrapper class that creates a fresh IdentityManager instance for each method invocation.
>>
>> public class IdentityManagerProxy implements IdentityManager {
>>
>> ...
>>
>> public void add(IdentityType identityType) throws IdentityManagementException {
>> createIdentityManager().add(identityType);
>> }
>>
>> private IdentityManager createIdentityManager() {
>> return this.identityManagerFactory.createIdentityManager(new Realm(this.realm));
>> }
>>
>> ...
>> }
>>
>> I'm still testing, but it seems we can use that.
>>
>> Thanks.
>> Pedro Igor
>>
>> ----- Original Message -----
>> From: "Shane Bryzak" <sbryzak at redhat.com>
>> To: security-dev at lists.jboss.org
>> Sent: Thursday, April 11, 2013 6:26:55 PM
>> Subject: Re: [security-dev] Some thoughts on PL Subsystem
>>
>> On 12/04/13 01:30, Pedro Igor Silva wrote:
>>> So, the updated list would be:
>>>
>>> 1) Rename the attribute jndi-url to jndi-name;
>>> 2) Publish in JNDI an IdentityManager for each realm.
>> Keep in mind that each IdentityManager instance has its own
>> SecurityContext, which is designed to be request-scoped. If we don't
>> have the capacity to support request-scoped instances in JNDI, then they
>> should be stateless (i.e. a new instance created every time).
>>
>>> 3) Support custom entities using a attribute to specify a module from
>>> where the @IDMEntity classes are + persistence.xml;
>>>
>>> ----- 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 12:22:54 PM
>>> Subject: Re: [security-dev] Some thoughts on PL Subsystem
>>>
>>> If you come up with one, let me know - this is something no one has solved in any situation ;-)
>>>
>>> On 11 Apr 2013, at 16:11, Stian Thorgersen <stian at redhat.com> wrote:
>>>
>>>> I think for now we should drop the default attribute + @Realm, and only support @Resource (i.e. user has to create @Produce @Resource to be able to inject IdentityManager for sub-system). If we can think of a nice way to inject @IdentityManager allowing user to specify correct identity-management and realm that would be great, but I don't think we have an approach to this at the moment.
>>>>
>>>> ----- Original Message -----
>>>>> From: "Pedro Igor Silva" <psilva at redhat.com>
>>>>> To: "Pete Muir" <pmuir at redhat.com>
>>>>> Cc: "Stian Thorgersen" <stian at redhat.com>, security-dev at lists.jboss.org
>>>>> Sent: Thursday, 11 April, 2013 3:49:35 PM
>>>>> Subject: Re: [security-dev] Some thoughts on PL Subsystem
>>>>>
>>>>> 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
>>>>> _______________________________________________
>>>>> 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
More information about the security-dev
mailing list