Matt Raible wrote:
On 4/19/07, Allen Gilliland <[EMAIL PROTECTED]> wrote:


Matt Raible wrote:
> The old code probably did some checking in the edit() method to see if
> the user logged in with Remember Me. If so, it set that value, which
> the JSP checked and hid the fields for.

I looked at that, but all that I can see is a conditional in the edit()
method which adds an action message if it's a cookie login, and in the
jsp it doesn't show the password fields if it's a cookie login.  I
couldn't find anywhere in the code where that cookie login state is
actually set, the only thing i can think of is if it happens inside of
the Acegi code.


That's weird - here's what I use in AppFuse in the UserAction.edit() method:

   // if user logged in with remember me, display a warning that they
can't change passwords
   log.debug("checking for remember me login...");

   AuthenticationTrustResolver resolver = new
AuthenticationTrustResolverImpl();
   SecurityContext ctx = SecurityContextHolder.getContext();

   if (ctx != null) {
       Authentication auth = ctx.getAuthentication();

       if (resolver.isRememberMe(auth)) {
           getSession().setAttribute("cookieLogin", "true");
           saveMessage(getText("userProfile.cookieLogin"));
       }
   }

the sruts1 yourProfile action is still untouched and there is no code like that in there. there is none of that Acegi stuff about checking if the auth came from the remember me service, but it does look for the attribute "cookieLogin" in the session and set an action message if it exists.

in any case, i don't think that matters anyways. the real question is, do we think there is a need to enforce greater security on the profile edit page to prevent unwanted folks from modifying a profile? if we do then the most appropriate way to do that is to always ask for the users existing password before allowing a profile change.

my personal opinion is that this would be wasted effort. if someone is somehow granted a session via the remember me service who is not the actual owner of that account then the security has already been breached, whether they can change the account password or not doesn't really matter. currently the code would let that person ... change the account email address, post entries on the weblog, delete anything from the weblog, delete the entire weblog, invite other accounts to the weblog, and on and on. the fact is that our security model is such that if someone is given a session we have to expect that session is properly authenticated. we have no valid use cases where someone should be given a partially authenticated session.

-- Allen



>
> A better solution for the JSP is to remove the fields completely, then
> use a Preparable interface on UserAction (or whatever its called) to
> refetch the user on save. That way, all the values from the database
> would be retained, and any ones specified in request parameters (from
> the form) would be overwritten.

Well, we do lookup the user when a profile save happens and we only set
the properties which are allowed to be changed.  I don't think it's good
usability to remove the password fields though.  If someone is logged in
via a remember me cookie then they should be in equal standing with any
other logged in user.

I'm fine with that, but when I first started digging into the Remember
Me stuff (years ago) a lot of folks recommended that it not be given
the same status as a user who logs in with a keyboard.

Matt


-- Allen


>
> Matt
>
> On 4/19/07, Allen Gilliland <[EMAIL PROTECTED]> wrote:
>> I just wanted to send this out to make sure I am not wrong about this.
>> In the new struts2 profile update form I have removed a small amount of >> code which tried to prevent users who were logged in via a 'cookieLogin'
>> from changing their passwords.  I don't really know the history behind
>> this code and don't even know if it was working anymore, but I am
>> removing it because ...
>>
>> 1. It *looks* like it doesn't even take effect anymore because I can't
>> find anywhere in the code which sets the "cookieLogin" attribute on a
>> session to trigger the behavior.
>>
>> 2. The old code wasn't actually preventing this scenario, it was only
>> discouraging it.  In the old action & jsp it would simply not display
>> the password form fields when the user had logged in via cookie, but
>> technically if you submitted the form with the right fields the action
>> would accept them and modify the password.
>>
>> 3. If this is still a valid security constraint that we want to imply
>> then I think we can do it in a better way, i.e. to ask for the users
>> existing password on the profile update form at all times. Personally I
>> think this would be overkill, but it's not necessarily unreasonable.
>>
>> So, if anyone who knows more about this old "cookieLogin" check thinks
>> it still needs to be in the code, please say so.
>>
>> -- Allen
>>
>
>



Reply via email to