[security-dev] IdentityManager review - queries
Shane Bryzak
sbryzak at redhat.com
Mon Oct 29 23:31:45 EDT 2012
On 10/30/2012 12:32 PM, Jason Porter wrote:
> On Oct 29, 2012, at 17:20, Shane Bryzak <sbryzak at redhat.com
> <mailto:sbryzak at redhat.com>> wrote:
>
>> On 10/30/2012 01:20 AM, Jason Porter wrote:
>>> I very much like the DSL you have going there. Removing the Range
>>> class is also a good idea, imo. However, I'm at -1 for the use of
>>> enums. You can't extend them, and you can't anticipate everything a
>>> user would add to a class. Sure we could do it in the simple idm
>>> idea where we provide the structure, but as soon as they advance
>>> past that then they're forced to use a different API.
>>
>> I agree, I wasn't a fan of using an enum either but at the time I
>> hashed this out I couldn't think of a better alternative. How about
>> instead of enums we introduce a marker interface like this:
>>
>> public interface QueryParameter {
>>
>> }
>>
>> Then we can simply define supported parameters directly on the core
>> identity model interfaces, as Pedro has suggested:
>>
>> public interface User extends IdentityType, Serializable {
>> public static final QueryParameter FIRST_NAME = new
>> QueryParameter() {};
>> public static final QueryParameter LAST_NAME = new
>> QueryParameter() {};
>> ...
>> }
>>
>> Then the method signature for IdentityQuery.setParameter would look
>> something like this:
>>
>> IdentityQuery<T> setParameter(QueryParameter param, Object value);
>>
>> This then makes the parameter set extensible (anyone can create their
>> own) and we can create an SPI (or just make the IdentityStore
>> implementations extensible themselves) so that developers can provide
>> support for their own custom query parameters (not to mention it
>> makes it more future proof if we want to add more parameter types later).
>
> Any reason not to do a generic interface or abstract class?
> QueryParam<TYPE> and have a void setValue(TYPE val), a <TYPE>
> getValue() and a String getName() on that interface/class? That would
> cut down on an argument.
I'm not quite getting this - could you give an example of what the API
would look like?
>
> Though the final empty interfaces from the classes does read nicely.
>
>>>
>>> On Mon, Oct 29, 2012 at 7:10 AM, Pedro Igor Silva <psilva at redhat.com
>>> <mailto:psilva at redhat.com>> wrote:
>>>
>>> I liked your proposal, it is simple and clear.
>>>
>>> Considering what you did I thought in another solution. At first
>>> glance, it seems more odd. But I think it can be more intuitive
>>> for users. For example, instead of having the Param.memberOf,
>>> users may use query.role(Role) or query.group(Group) directly.
>>>
>>> IdentityManager identityManager = // get the identity
>>> manager;
>>>
>>> Query<User> query = identityManager.<User>createQuery();
>>>
>>> query
>>> .limit(100)
>>> .offset(1)
>>> .where()
>>> .id("1")
>>>
>>> .property(User.FIRST_NAME, OPERATOR.equals, "John")
>>> .property(User.EXPIRATION_DATE,
>>> OPERATOR.lessThan, new Date())
>>>
>>> .attribute("ssn", OPERATOR.equals, "SSN-123")
>>>
>>> .role(new SimpleRole("Payroll Officer"))
>>>
>>> .group(new SimpleGroup("Superuser", "Superuser",
>>> null));
>>>
>>> List<User> list = query.list();
>>>
>>> for (User user : list) {
>>> // handle users
>>> }
>>>
>>> I think we can avoid having a Range class/interface by using
>>> some methods on the Query interface (eg. limit and offset
>>> above). Th Query interface can be used only to configure how the
>>> query should be executed, global configuration, etc.
>>>
>>> The Where interface can be useful to provide some specific
>>> validation depending of the IdentityType and how the
>>> restrictions are configured. For example, users do not have a
>>> parent relationship.
>>>
>>> The property names (firstName, email, etc) can be a Enum for
>>> each IdentityType type (user,group and role). Each IdentityType
>>> type should specify which properties are searchable + the common
>>> properties defined by the IdentityType interface.
>>>
>>> Not sure if we need the query.reset() method ... Why not just
>>> discard the query instance and build another one ?
>>>
>>> Regards.
>>> Pedro Igor
>>>
>>> ----- Original Message -----
>>> From: "Shane Bryzak" <sbryzak at redhat.com
>>> <mailto:sbryzak at redhat.com>>
>>> To: security-dev at lists.jboss.org
>>> <mailto:security-dev at lists.jboss.org>
>>> Sent: Monday, October 29, 2012 6:38:14 AM
>>> Subject: [security-dev] IdentityManager review - queries
>>>
>>> I've started reviewing the IdentityManager interface to see
>>> where we can
>>> improve the API. The first area I'd like to visit is the Query
>>> API, of
>>> which I've come to the conclusion that we need to do some serious
>>> redesign - the current API is non-intuitive, too verbose and not
>>> future
>>> proof.
>>>
>>> What I'd like to do is throw it all out and start again,
>>> replacing it
>>> with a new cleaner API that looks something like this:
>>>
>>> public interface IdentityManager {
>>> // <snip other methods>
>>>
>>> <T extends IdentityType> IdentityQuery<T> createQuery();
>>> }
>>>
>>> public interface IdentityQuery<T extends IdentityType> {
>>> public enum Param {id, key, created, expired, enabled,
>>> firstName,
>>> lastName, email, name, parent, memberOf};
>>>
>>> public enum Operator { equals, notEquals, greaterThan,
>>> lessThan };
>>>
>>> IdentityQuery<T> reset();
>>>
>>> IdentityQuery<T> setParameter(Param param, Object value);
>>>
>>> IdentityQuery<T> setParameter(Param param, Operator operator,
>>> Object value);
>>>
>>> IdentityQuery<T> setAttributeParameter(String
>>> attributeName, Object
>>> value);
>>>
>>> IdentityQuery<T> setAttributeParameter(String attributeName,
>>> Operator operator, Object value);
>>>
>>> IdentityQuery<T> setRange(Range range);
>>>
>>> List<T> getResultList();
>>> }
>>>
>>> This unified API basically replaces the 4 separate existing
>>> interfaces
>>> we currently have; UserQuery, RoleQuery, GroupQuery and
>>> MembershipQuery. I've put together a few usage scenarios to
>>> show how it
>>> might work:
>>>
>>> 1) Find users with first name 'John':
>>>
>>> List<User> users = identityManager.<User>createQuery()
>>> .setParameter(Param.firstName, "John")
>>> .getResultList();
>>>
>>> 2) Find all expired users:
>>>
>>> List<User> users = identityManager.<User>createQuery()
>>> .setParameter(Param.expired, Operator.lessThan, new Date())
>>> .getResultList();
>>>
>>> 3) Find all users that are a member of the "Superuser" group
>>>
>>> List<User> users = identityManager.<User>createQuery()
>>> .setParameter(Param.memberOf,
>>> identityManager.getGroup("Superuser"))
>>> .getResultList();
>>>
>>> 4) Find all sub-groups of the "Employees" group:
>>>
>>> List<Group> groups = identityManager.<Group>createQuery()
>>> .setParameter(Param.memberOf,
>>> identityManager.getGroup("Employees"))
>>> .getResultList();
>>>
>>> 5) Find all disabled roles:
>>>
>>> List<Role> roles = identityManager.<Role>createQuery()
>>> .setParameter(Param.enabled, false)
>>> .getResultList();
>>>
>>> 6) Find all Users, Groups and Roles that have been granted the
>>> "Payroll
>>> Officer" role in the "Human Resources" group:
>>>
>>> List<IdentityType> identities =
>>> identityManager.<IdentityType>createQuery()
>>> .setParameter(Param.memberOf, identityManager.getGroup("Human
>>> Resources"))
>>> .setParameter(Param.memberOf, identityManager.getRole("Payroll
>>> Officer"))
>>> .getResultList();
>>>
>>> 7) Find all Users that have an attribute named "Citizenship" with a
>>> value of "Greenland":
>>>
>>> List<User> users = identityManager.<User>createQuery()
>>> .setAttributeParameter("Citizenship", "Greenland")
>>> .getResultList();
>>>
>>> I'm *pretty* certain that this API is at least as capable as what we
>>> currently have, if not more so, and IMHO provides a far simpler
>>> and more
>>> versatile design (being able to select different IdentityTypes in a
>>> single query I think is a big plus). I'd love to hear any
>>> feedback on
>>> whether you like it, hate it or can think of any improvements to the
>>> design to make it better for our developers. Also, please think
>>> especially about additional usage scenarios and whether or not
>>> there are
>>> any particular use cases which might be problematic for this API.
>>>
>>>
>>> Thanks!
>>> Shane
>>> _______________________________________________
>>> security-dev mailing list
>>> security-dev at lists.jboss.org <mailto:security-dev at lists.jboss.org>
>>> https://lists.jboss.org/mailman/listinfo/security-dev
>>> _______________________________________________
>>> security-dev mailing list
>>> security-dev at lists.jboss.org <mailto:security-dev at lists.jboss.org>
>>> https://lists.jboss.org/mailman/listinfo/security-dev
>>>
>>>
>>>
>>>
>>> --
>>> Jason Porter
>>> http://lightguard-jp.blogspot.com
>>> http://twitter.com/lightguardjp
>>>
>>> Software Engineer
>>> Open Source Advocate
>>>
>>> PGP key id: 926CCFF5
>>> PGP key available at: keyserver.net <http://keyserver.net>,
>>> pgp.mit.edu <http://pgp.mit.edu>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/security-dev/attachments/20121030/97f5408e/attachment.html
More information about the security-dev
mailing list