[security-dev] IdentityManager review - queries

[Jason Porter]

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.

On Mon, Oct 29, 2012 at 7:10 AM, Pedro Igor Silva <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>
> To: 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
> 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
>



-- 
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, pgp.mit.edu
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/security-dev/attachments/20121029/8309854a/attachment.html