[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Aerogear-dev] [DISCUSS] [DELTASPIKE-76/127] security module - login/logout

On Mar 26, 2012, at 12:36 PM, Jay Balunas wrote:

On Mar 26, 2012, at 10:09 AM, Bruno Oliveira wrote:

Hi, I have been following DS discussions/JIRA/repository and I hope to contribute with some thoughts, we have a lot to contribute.

The good news is that they are moving forward with integration tests. I was wondering about introduce this topic at rwp mailing list with aerogear-security.


+1, I think the more we are sharing and discussing at this point the better.  

What about their approach - what are you liking, not liking?  How do you think it will tie in with what you already wrote up?

They have some ideas to use apache shiro (https://issues.apache.org/jira/browse/DELTASPIKE-62), but if it become true a lot of work to do with CDI will come (http://shiro.apache.org/integration.html). Looking into the sources, they don't have Shiro implemented, some basic features are still missing (https://github.com/abstractj/incubator-deltaspike/blob/master/deltaspike/modules/security/impl/src/main/java/org/apache/deltaspike/security/impl/IdentityImpl.java#L246) and must be discussed.

The closest thing of abstraction that we're looking for with DS is a SecurityInterceptor (https://github.com/abstractj/incubator-deltaspike/blob/master/deltaspike/modules/security/impl/src/main/java/org/apache/deltaspike/security/impl/SecurityInterceptor.java#L34), but we still must make use of secure annotations. 

I'll study how it fits with what I already wrote and throw the question to DS mailing list and rwp.

On Mar 26, 2012, at 8:54 AM, Jay Balunas wrote:

Hi Bruno, 

You may want to at least "lurk" in the deltaspike-dev mailing list, as I think being part of these discussions will be important for defining our security solution.



Begin forwarded message:

From: Shane Bryzak <sbryzak redhat com>
Subject: Re: [DISCUSS] [DELTASPIKE-76/127] security module - login/logout
Date: March 24, 2012 5:48:26 PM EDT
Cc: Gerhard Petracek <gerhard petracek gmail com>

A few points:

1) Identity and DefaultIdentity should not be in the authentication package.

2) DefaultLoginCredential should be in the credential package, not authentication.

3) The following code has been modified in the login() method of the Identity implementation.  This code is important, it ensures that the authentication events are still fired and the login() method returns a success in the situation where the user performs an explicit login but a silent login occurs during the same request.

           if (isLoggedIn())
               // If authentication has already occurred during this request via a silent login,
               // and login() is explicitly called then we still want to raise the LOGIN_SUCCESSFUL event,
               // and then return.
               if (requestSecurityState.get().isSilentLogin())
                   beanManager.fireEvent(new LoggedInEvent(user));
                   return AuthenticationResult.success;

               beanManager.fireEvent(new AlreadyLoggedInEvent());
               return AuthenticationResult.success;

4) I'm not so sure this is a good idea:

//force a logout if a different user-id is provided
if (isAuthenticationRequestWithDifferentUserId())

There's many reasons I'm -1 on this, here's a few of them:

a) In most typical applications the login form won't even be visible to the user after they have logged in already.

b) It's important to keep a clean separation between operations performed within different authentication contexts (I don't mean CDI context here, I mean the context of being logged in as a certain user).  For example, things like auditing can be potentially affected by this, where an application is logging what happens during each request under each user's user ID.

c) The test for determining if the user logging in is a different user is problematic - there's no guarantee that the username/password they provide is going to be equal to the current User.userID value, and it doesn't take into account authentication by means other than a username/password.

d) Automatically throwing away the user's session as a side effect of this functionality (by calling logout()) could potentially be dangerous, as there may be important state that the user can lose.  I'm of the strong opinion that logging out should always be an explicit operation performed intentionally by the user.

In general, I think if a user that is already authenticated tries to log in with a different username it should throw an exception.

5) The quietLogin() method is missing.  This method is a non-functional requirement of the "Remember Me" use case.

On 23/03/12 22:46, Gerhard Petracek wrote:
hi @ all,

as mentioned in [1] we switched to a step by step discussion for the
security module.
the first step of part 1 (see [2]) is a>simple<  credential based
login(/logout) use-case.

some of us reviewed and improved the current draft [3] already.
you can see the result at [3] (and not in our repository). [3] also
contains a link to the refactored api (+ new tests).
this version includes what we need for the>simple<  login/logout scenario
mentioned by part 1.
that means the api and spi you can see at [3] is just a first step and will
be changed based on further scenarios (of the other parts).
(e.g. right now "User" is a class and will be refactored to an interface as
soon as we need to change it.)

if there are no basic objections, i'll push those changes to our repository
on sunday (and i'll add further tests afterwards).
furthermore, everything in [3] which is marked as "agreed" will be added to
our temporary documentation and will be part of v0.2-incubating.
(as mentioned before: even those parts might be changed later on, but not
before the release of v0.2-incubating which should be available soon.)


[1] http://s.apache.org/uc6
[2] http://s.apache.org/HC1
[3] http://s.apache.org/6OE

aerogear-dev mailing list
aerogear-dev redhat com

aerogear-dev mailing list
aerogear-dev redhat com

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]