[security-dev] IdentityManager API review

[Shane Bryzak]

On 11/12/2012 09:13 PM, Boleslaw Dawidowicz wrote:
> On Nov 12, 2012, at 4:01 AM, Shane Bryzak <sbryzak at redhat.com> wrote:
>
>> On 11/08/2012 11:32 PM, Bolesław Dawidowicz wrote:
>>> On 11/07/2012 07:08 PM, Jason Porter wrote:
>>>> Serializable works for me. Do we do have a generic Attribute<? extends
>>>> Serializable> or just forgo the generics?
>>> Serializable could be fine. To be honest I'm fairly biased in this
>>> topic. Mainly becuase for IDM 1.x I lost quite significant amount of
>>> time to implement and properly test binary attributes support for like 7
>>> different databases. It was never really used. I would start small in
>>> this area. It is always possible to extend this part of the api later
>>> however we may find out that there is no need for this.
>>>
>>> We should think what are our usecases for attributes. Maybe Bill should
>>> jump in and express his usecases and ideas. Most security tokens can be
>>> serialized into string I guess.
>>>
>>> I also agree that we should spend some time to check compatibility with
>>> JSR351 API here. It will be so flexible that we rather won't be able to
>>> implement it fully however it would be good to reduce limitations for
>>> JSR351IdentityStore.
>>>
>>> Another obvious usecase is storing avatar or other binary content. My
>>> take is that application should rather implement separate binary store
>>> and just keep basic reference as user attribute. I would simply veto any
>>> binary attribute support idea until we have clear agreement that it is
>>> really necessary.
>>>
>>> Are we addressing any attribute metadata so far? This is probably more
>>> identity store concern however part of attributes may be marked
>>> readonly. Also in LDAP scenario multi value support is attribute
>>> specific and this info may be worth exposing in the API directly.
>>>
>>> Another one that is quite huge is to support unique attributes. Common
>>> requirement is to have unique email attribute value. It is also common
>>> to search user by unique attribute - either email or token. This needs
>>> to be easy and efficient on the implementation side.
>>>
>>>
>>>
>> Ok, so let's just keep it simple for now and support String valued
>> attributes. If we ever decide to support more complex types, backwards
>> compatibility shouldn't be an issue as String is Serializable anyway.
> What do you think about adding some attributes metadata?
>

Sure, we can do that - which specific metadata were you thinking of in 
relation to attributes?  I've thought about Jason's suggestion to create 
a generic Attribute value, and I think it's a good idea.  We can leave 
it up to the IdentityStore implementation whether it supports certain 
types of attributes - this way we can document the supported types and 
throw an UnsupportedOperationException (or something like that) if an 
unsupported attribute value is passed. We also leave it up to the 
developer if they wish to extend the IdentityStore SPI to provide 
support for attribute types that we don't support out of the box.  
Here's what I'm proposing for the Attribute class:

public class Attribute<T extends Serializable> {
     private String name;
     private T value;

     public Attribute(String name, T value) {
         this.name = name;
         this.value = value;
     }

     public String getName() {
         return name;
     }

     public T getValue() {
         return value;
     }
}


This would mean the following changes to IdentityManager (including the 
corresponding changes to IdentityStore):

Replace:

     void setAttribute(IdentityType identityType, String attributeName, 
String attributeValue);
     String getAttribute(IdentityType identityType, String attributeName);

With:

     void setAttribute(IdentityType identityType, Attribute attribute);
     <T extends Serializable> Attribute<T> getAttribute(IdentityType 
identityType, String attributeName);

And the following changes to IdentityType:

Replace:

     void setAttribute(String name, String value);
     void setAttribute(String name, String[] values);
     void removeAttribute(String name);
     String getAttribute(String name);
     String[] getAttributeValues(String name);
     Map<String, String[]> getAttributes();

With:

     void setAttribute(String name, Attribute value);
     void removeAttribute(String attributeName);
     <T extends Serializable> Attribute<T> getAttribute(String 
attributeName);