I've read through the gist and my comments are inline:
Developer Side Notes
Definitions
What is the difference between getKey() and getId() methods. Can we have getId() on the IdentityType ?
I think we can associate the getKey with the username, for example. The getId can be used to let stores identify the user internally, like a generated identifier or something.
Another option is have a getName method on User type and remove the getKey. That way we have User.getId and User.getName. Remember that other IdentityTypes like Role and Group have a getName method.
The getKey() method returns a "globally" unique identifier for that identity object. E.g. for a group called "admins" the key would be "GROUP://admins", for a user called jsmith the key would be "USER://jsmith". We need this distinction because permissions can be stored against users, groups, or roles using their key and we need a reliable way to map this value back to the actual identity object. The getId() method is specific to certain identity types, such as User (in which case the id is their user ID, i.e. "jsmith") or Group (where the id is the full hierarchy of the group, e.g. "/branches/headoffice/managers").
API Design
Method IdentityManager.grantRole(role, identityType, group) can be split in:
- IdentityManager.grantRole(role, user)
- IdentityManager.grantRole(role, group)
- IdentityManager.addMember(group, identityType)
Same thing for revokeRole(role, identityType, group), hasRole(role, identityType, group)
The role management methods could probably do with some improvement. For one thing we don't have explicit support for application roles yet. I would suggest something like the following methods:
IdentityManager.grantRole(IdentityType member, Group parent, String roleName)
IdentityManager.grantApplicationRole(IdentityType member, String roleName)
Customization of ldap attributes and db stuff based on preexisting DBs and LDAP stores. For databases there is some working done in previous versions of PicketLink IDM.
Serialization of User, Role, Group and Membership types. I think is important to make those classes work in a clustered environment.
+1, we should make these interfaces Serializable
The IdentityManager provides two methods for creating groups providing the parent: createGroup(String, Group) and createGroup(String, String). Maybe we can have only createGroup(String, Group) considering that the parent must be always created.
+1, good idea
Query API Design
- Common interface and base class for UserQuery, RoleQuery, GroupQuery and MembershipQuery interfaces/implementations.
+1, all the common stuff should go in a base interface
- Do we need the *Query.executeQuery(query, range) method ? We already have the *Query.executeQuery().
I don't think we need this, the range can be set explicitly on the Query object.
- We can also have a *Query.executeQuery(range) method to configure how the query is executed. Instead of always force the range argument.
- The UserQuery interface defines a getName method, but there is no such method/property in the User interface. Should we map the UserQuery.getName to User.getKey ? This item is related with item #1 from the Definitions section.
This should probably be getId() instead.
- Add support to query users by creation and expiration date. There are not methods in the UserQuery to search using these attributes.
+1, this is a good idea
Messages and Logging
- Better exception hierarchy and handling
+1, we should also define a list of error codes, I'll ask Pete for some advice on this.
- JBoss Logging for messages/exceptions and log messages
My concern here is how we integrate the logging in an SE module with CDI. It would be nice to provide some kind of i18n support, maybe Jason would have some suggestions as to how we best achieve this.
- More logging code (warn, info, error and debug levels)
+1, comprehensive logging is always good
Configuration
- Review the builder code ? Use xml or something else ?
Documentation
- Start to document what we have so far
Feature Proposal
- Password Management API. Support different credentials and management features (reset, strength, etc)
- IDM example appplication
- REST endpoints for the IdentityManager. As Anil suggested.
- Event Handling. Which events should be supported (user account created/removed/updated/expired, membership created/removed/update, etc) ?
On 09/10/12 02:23, Anil Saldhana wrote:
Hi all, I am wondering if we can hold a discussion on the IDM API so we lock it down in the next couple of weeks. Recently, Pedro created the following gist page. https://gist.github.com/3801805. Regards, Anil _______________________________________________ security-dev mailing list security-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/security-dev
_______________________________________________
security-dev mailing list
security-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/security-dev