[security-dev] IdentityManager review - queries
Shane Bryzak
sbryzak at redhat.com
Mon Oct 29 19:20:36 EDT 2012
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).
>
> 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/f385e29e/attachment.html
More information about the security-dev
mailing list