deltaspike-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gerhard Petracek <gerhard.petra...@gmail.com>
Subject Re: [DISCUSS] DELTASPIKE-77 Identity API
Date Wed, 15 Feb 2012 10:56:05 GMT
hi shane,

@#tryLogin:
ok - since we can provide a simplified handling for important use-cases ->
i'm ok with it.

@#login:
matt answered it already (it even works with jsf 1.2)

@#hasRole (overloaded)
agreed - i also don't like that one. if we keep  #tryLogin, we should use
the same naming convention for both.
#tryHasRole might not be the best name, however, we could think about an
alternative prefix or we just keep it for such methods in general (for all
modules - we already have such methods e.g. in ClassUtils).

@#addXyz by example

current usage:

@Inject
private Identity identity;
//...
identity.removeRole(...);
identity.logout();

suggested usage to keep the api simpler for 90+ % of all use-cases:
@Inject
private Identity identity;
//...
identity.logout();

and as soon as users need the "write-methods" which aren't intended to be
used frequently, they just change it to:

@Inject
private EditableIdentity identity;
//...
identity.removeRole(...);
identity.logout();

that works because EditableIdentity extends Identity and there is just one
producer internally (so they could even cast the injected instance - if
they prefer it).

regards,
gerhard



2012/2/14 Shane Bryzak <sbryzak@redhat.com>

> On 13/02/12 19:55, Gerhard Petracek wrote:
>
>> hi shane,
>>
>> i'm sure there are good reasons for most/all details. since i don't know
>> them, i just list the topics which come to my mind after reading the
>> provided information.
>>
>> #tryLogin
>> i don't see the need for it compared to #login, because it can be done by
>> users easily (if needed).
>> at least at the beginning i would keep the api as minimal as possible. we
>> can add further parts based on concrete and important use-cases.
>>
>
> There are actually two quite important use cases for this - applications
> that implement some kind of "Remember Me" function must be able to
> authenticate without producing the usual events/messages generated when
> performing a conscious authentication.  The same goes for stateless
> applications that must authenticate on every request.  We can probably
> remove the quietLogin() method and make it an implementation detail.
>
>
>> #login
>> uses string as return type instead of an enum.
>> in>general<  : we should define a general rule for the usage of exceptions
>> which should be used by most deltaspike apis.
>>
>
> This method returns a String to make it easy to write navigation rules for
> JSF.  The three possible values are "success", "failure" or "exception",
> which allows the navigation rules to redirect the user to an appropriate
> page based on the result.  I'm happy to hear alternatives here if someone
> has other suggestions.
>
>
>> #quietLogin
>>
>>> is method is intended to be used primarily as an internal API call
>>>
>> ... then it shouldn't be part of the api. (that's related to a 2nd topic.
>> see api vs. spi)
>>
>
> See comment above.
>
>
>> #hasRole
>>
>>> Checks if the authenticated user is a member of the specified role.
>>>
>> #checkRole
>>
>>> Checks that the current authenticated user is a member of the specified
>>>
>> role.
>>
>> #hasRole and #checkRole look very similar - if the exception is the only
>> difference: see my comment about #tryLogin. at least we should think about
>> unified names.
>>
>
> We can probably merge these into a single method / overloaded methods:
>
> hasRole(String role, String group) // default doesn't throw exception
> hasRole(String role, String group, boolean throwException) // throws
> exception is throwException == true
>
> I personally think this looks ugly though.  Another alternative is to just
> remove the checkRole() method altogether.  Since most authorization now
> should be performed using the typesafe security bindings, it can simply be
> left to the authorizer method to implement the desired behaviour.  Same
> thing goes for checkGroup().
>
>
>> #addXyz
>> that's a general topic we have to discuss. in myfaces codi we have all
>> "write-methods" which aren't intended to be used frequently in the spi to
>> keep the api simple and small ([1] and [2] illustrate it in case of the
>> initial refactoring of @Secured for deltaspike - the naming convention is
>> always Editable[ApiName]).
>>
>
> I agree that it would be nice to separate these methods into an SPI class.
>  The reason they're in the API is so that developers can simply inject a
> single class (Identity) into their custom authentication class and have all
> the things they need there to perform authentication.  I'll try to think of
> a more suitable alternative that doesn't put too much additional burden on
> the developer.
>
>
>> @type "string" in the api:
>> i know - it's easy, generic and sometimes just needed to use strings.
>> however, at least we should re-visit them and just use them if there is no
>> useful alternative.
>>
>> @rudy:
>> i agree with you.
>>
>> for v0.1 we always started to discuss the basic use-cases and requirements
>> and afterwards the concrete api.
>>
>>> imo<  that leads to a better api and we should try to keep this approach
>>>
>> (for sure the content of v0.1 was easier).
>>
>> regards,
>> gerhard
>>
>> [1] http://s.apache.org/4sL
>> [2] http://s.apache.org/j2E
>>
>>
>>
>> 2012/2/13 Rudy De Busscher<rdebusscher@gmail.com**>
>>
>>  Hi,
>>>
>>> I think it is also important that you can work with permissions.  It is
>>> much more fine grained then roles but more flexible. It becomes much
>>> easier
>>> to change what a group of people can do, even without changing the code.
>>> I
>>> never use roles in an application, only permissions.
>>>
>>> I'm not saying we need to do the implementation of it by default in
>>> deltaspike but should have at least something like
>>> boolean hasPermission(String permissionName)
>>> in the identity API.
>>>
>>>
>>> my idea about security.
>>>
>>>
>>> regards
>>> Rudy
>>>
>>> On 12h February 2012 23:33, Shane Bryzak<sbryzak@redhat.com>  wrote:
>>>
>>>  I've created an umbrella issue to track the work we do on the security
>>>> module [1], and the first task is to review and discuss the Identity
>>>> API.
>>>>  This API forms the core of the security module, and all other features
>>>> build on top of it to implement a complete security framework.
>>>>
>>>> The Identity interface represents an individual, current user of an
>>>> application and its implementation must be session-scoped to provide
>>>> security services for the entirety of a user's session.  I'm going to
>>>>
>>> paste
>>>
>>>> the (slightly modified) code from Seam as it's mostly well documented,
>>>>
>>> and
>>>
>>>> so we have a baseline from which to further discuss:
>>>>
>>>> public interface Identity {
>>>> public static final String RESPONSE_LOGIN_SUCCESS = "success";
>>>> public static final String RESPONSE_LOGIN_FAILED = "failed";
>>>> public static final String RESPONSE_LOGIN_EXCEPTION = "exception";
>>>> /**
>>>> * Simple check that returns true if the user is logged in, without
>>>> attempting to authenticate
>>>> *
>>>> * @return true if the user is logged in
>>>> */
>>>> @Secures
>>>> @LoggedIn
>>>> boolean isLoggedIn();
>>>> /**
>>>> * Returns true if the currently authenticated user has provided their
>>>> correct credentials
>>>> * within the verification window configured by the application.
>>>> *
>>>> * @return
>>>> */
>>>> boolean isVerified();
>>>> /**
>>>> * Will attempt to authenticate quietly if the user's credentials are set
>>>> and they haven't
>>>> * authenticated already. A quiet authentication doesn't throw any
>>>> exceptions if authentication
>>>> * fails.
>>>> *
>>>> * @return true if the user is logged in, false otherwise
>>>> */
>>>> boolean tryLogin();
>>>> /**
>>>> * Returns the currently authenticated user
>>>> *
>>>> * @return
>>>> */
>>>> User getUser();
>>>>
>>>> /**
>>>> * Attempts to authenticate the user. This method raises the following
>>>> events in response
>>>> * to whether authentication is successful or not. The following events
>>>>
>>> may
>>>
>>>> be raised
>>>> * during the call to login():
>>>> *<p/>
>>>> * org.apache.deltaspike.****security.events.LoggedInEvent - raised when
>>>> authentication is successful
>>>> * org.apache.deltaspike.****security.events.****LoginFailedEvent -
>>>> raised
>>>> when authentication fails
>>>> * org.apache.deltaspike.****security.events.****AlreadyLoggedInEvent -
>>>> raised
>>>> if the user is already authenticated
>>>> *
>>>> * @return String returns RESPONSE_LOGIN_SUCCESS if user is
>>>> authenticated,
>>>> * RESPONSE_LOGIN_FAILED if authentication failed, or
>>>> * RESPONSE_LOGIN_EXCEPTION if an exception occurred during
>>>>
>>> authentication.
>>>
>>>> These response
>>>> * codes may be used to control user navigation. For deferred
>>>> authentication methods, such as Open ID
>>>> * the login() method will return an immediate result of
>>>> RESPONSE_LOGIN_FAILED (and subsequently fire
>>>> * a LoginFailedEvent) however in these conditions it is the
>>>>
>>> responsibility
>>>
>>>> of the Authenticator
>>>> * implementation to take over the authentication process, for example by
>>>> redirecting the user to
>>>> * another authentication service.
>>>> *
>>>> */
>>>> String login();
>>>> /**
>>>> * Attempts a quiet login, suppressing any login exceptions and not
>>>>
>>> creating
>>>
>>>> * any faces messages. This method is intended to be used primarily as an
>>>> * internal API call, however has been made public for convenience.
>>>> */
>>>> void quietLogin();
>>>> /**
>>>> * Logs out the currently authenticated user
>>>> */
>>>> void logout();
>>>> /**
>>>> * Checks if the authenticated user is a member of the specified role.
>>>> *
>>>> * @param role String The name of the role to check * @param group String
>>>> the name of the group in which the role exists
>>>> * @return boolean True if the user is a member of the specified role
>>>> */
>>>> boolean hasRole(String role, String group);
>>>> /**
>>>> * Adds a role to the authenticated user. If the user is not logged in,
>>>> * the role will be added to a list of roles that will be granted to the
>>>> * user upon successful authentication, but only during the
>>>> authentication
>>>> * process.
>>>> *
>>>> * @param role The name of the role to add * @param group The name of the
>>>> group in which to create the role
>>>> */
>>>> boolean addRole(String role, String group);
>>>> /**
>>>> * Checks if the authenticated user is a member of the specified group
>>>> *
>>>> * @param name The name of the group
>>>> * @return true if the user is a member of the group
>>>> */
>>>> boolean inGroup(String name);
>>>> /**
>>>> * Adds the user to the specified group. See hasRole() for semantics in
>>>> * relationship to the authenticated status of the user.
>>>> *
>>>> * @param name The name of the group
>>>> * @return true if the group was successfully added
>>>> */
>>>> boolean addGroup(String name);
>>>> /**
>>>> * Removes the currently authenticated user from the specified group
>>>> *
>>>> * @param name The name of the group
>>>> */
>>>> void removeGroup(String name);
>>>> /**
>>>> * Removes a role from the authenticated user
>>>> *
>>>> * @param role The name of the role to remove
>>>> */
>>>> void removeRole(String role, String group);
>>>> /**
>>>> * Checks that the current authenticated user is a member of
>>>> * the specified role.
>>>> *
>>>> * @param role String The name of the role to check
>>>> * @throws AuthorizationException if the authenticated user is not a
>>>>
>>> member
>>>
>>>> of the role
>>>> */
>>>> void checkRole(String role, String group);
>>>> /**
>>>> * @param group
>>>> * @param groupType
>>>> */
>>>> void checkGroup(String group);
>>>>    /**
>>>> * Returns an immutable set containing all the current user's granted
>>>>
>>> roles
>>>
>>>> *
>>>> * @return
>>>> */
>>>> Set<Role>  getRoles();
>>>> /**
>>>> * Returns an immutable set containing all the current user's group
>>>> memberships
>>>> *
>>>> * @return
>>>> */
>>>> Set<Group>  getGroups();
>>>> }
>>>>
>>>>
>>>> Some particular points to review:
>>>>
>>>> 1. Should we attempt to use the security classes provided by Java SE,
>>>>
>>> such
>>>
>>>> as Principal, Subject, etc or use our own User API - this will affect
>>>>
>>> what
>>>
>>>> is returned by the getUser() method above.  Keep in note that we will
>>>>
>>> have
>>>
>>>> at least a simple User/Role/Group API as part of Identity Management.
>>>>  In
>>>> Seam 2 we originally used the built-in Java classes (which made more
>>>>
>>> sense
>>>
>>>> because the authentication process was based on JAAS), however in Seam 3
>>>> (where we removed JAAS because it doesn't support asynchronous
>>>> authentication as required by OpenID etc) we based the security module
>>>> on
>>>> the PicketLink User API.  IMHO, this is not a critical choice either way
>>>>
>>> -
>>>
>>>> the Java security classes have the advantage of being familiar to many
>>>> users, while on the flipside if we provide our own User API we have the
>>>> flexibility of being able to extend it in the future.  So both options
>>>>
>>> have
>>>
>>>> their own advantages.
>>>>
>>>> 2. The addRole() and addGroup() methods are intended to be only used
>>>> during the authentication process to grant particular user memberships
>>>>
>>> for
>>>
>>>> the duration of their session only.  A few users have found this a
>>>> little
>>>> confusing, as they were using identity management, and expected these
>>>> methods to grant a permanent membership for the user.  One solution may
>>>>
>>> be
>>>
>>>> to simply rename these methods to addSessionRole() and addSessionGroup()
>>>>
>>> -
>>>
>>>> thoughts?
>>>>
>>>> 3. We're touching a little bit on the authorization API here also with
>>>>
>>> the
>>>
>>>> hasRole() / inGroup() methods.  I'll provide a quick description of
>>>> these
>>>> core security API concepts here:
>>>>
>>>> * User - represents an individual user of an application.  Can either be
>>>> human or non-human, and can represent either a user managed locally
>>>> (i.e.
>>>> through the IDM API) or an externally authenticated User, such as one
>>>>
>>> that
>>>
>>>> has logged in with OpenID.
>>>> * Group - a collection of users and other groups.  The intent is that
>>>> privileges can be either assigned to individual users, groups or roles.
>>>>  Groups have a hierarchical structure and can be a member of zero or
>>>> more
>>>> other groups.
>>>> * Role - represent a particular real life role of a user.  Roles are
>>>> defined as a three-way relationship between user, group and role type.
>>>>
>>>  For
>>>
>>>> example, user "Bob" might be an "accounts clerk" (the role type) at
>>>> "head
>>>> office" (the group).  It is also possible for a user to have a role in a
>>>> group, without being a member of that group.
>>>>
>>>>
>>>>
>>>> [1] https://issues.apache.org/****jira/browse/DELTASPIKE-76<https://issues.apache.org/**jira/browse/DELTASPIKE-76>
>>>> <
>>>>
>>> https://issues.apache.org/**jira/browse/DELTASPIKE-76<https://issues.apache.org/jira/browse/DELTASPIKE-76>
>>> >
>>>
>>>> [2] https://github.com/seam/****security/blob/develop/api/src/****<https://github.com/seam/**security/blob/develop/api/src/**>
>>>> main/java/org/jboss/seam/****security/Identity.java<
>>>>
>>> https://github.com/seam/**security/blob/develop/api/src/**
>>> main/java/org/jboss/seam/**security/Identity.java<https://github.com/seam/security/blob/develop/api/src/main/java/org/jboss/seam/security/Identity.java>
>>>
>>>>
>>>>
>>>
>>> --
>>> Rudy De Busscher
>>>
>>>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message