bugSamizdat - Bugs: bug #20678, member_controller: user...

 
 

bug #20678: member_controller: user successfully logs in but thinks that s/he has failed (loginpatch-0.2 vs 070818)

Submitter:  Boud Roukema <boud>
Submitted:  Fri 03 Aug 2007 10:21:39 PM UTC
   
 
Category:  None Severity:  3 - Normal
Status:  Fixed Privacy:  Public
Assigned to:  None Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Tue 24 Jun 2008 12:03:31 AM UTC, comment #6: 

For people not yet updated from 070818, an incremental patch to 070820_loginpatch-0.2 is provided here. It should be supplemented in the obvious way for any other types of URLs which the user should not be referred back to. Alternatively, we could have a whitelist and assume that any other URLs should not be referred back to. Or else there should be a method to decide on what sort of URL is which.

Anyway, the following should at least avoid frightening new logged-in users:


--- old/controllers/member_controller.rb~        2007-08-25 02:20:24.000000000 +0200
+++ controllers/member_controller.rb        2008-06-24 01:49:28.000000000 +0200
@@ -183,6 +183,9 @@
     referer = @request.referer
     prelogin_referer = (referer =~ %r{member/login\Z}) ?
           prelogin_referer : referer
+
+    prelogin_referer = nil if prelogin_referer =~ %r{member/confirm}
+

     if login and password
       if cookie = Session.start(login, password)


Boud Roukema <boud>
Group Member
Thu 21 Feb 2008 06:25:37 PM UTC, comment #5: 

Fixed in 2008-01-20 snapshot: redirect_when_done cookie makes sure user is always redirected to the right place.

Dmitry Borodaenko <angdraug>
Group administrator
Tue 21 Aug 2007 12:41:08 AM UTC, comment #4: 

Here's a proposed solution which goes back to the referer
even if the user makes an error in logging in. The key idea
is to avoid using the "referer" parameter which i guess is
handled by apache (since i couldn't find it in any ruby
code).

A file 070820_loginpatch-0.2 i attached as a patch against
0.6.0.070818-1.




(file #13748)

Boud Roukema <boud>
Group Member
Mon 20 Aug 2007 11:49:52 PM UTC, comment #3: 

On the other hand, i don't think we should throw out the baby
with the bathwater.

The 070618 version works fine when someone logs in without
making any error and logging out is also fine. Even if some
more complex solution might be possible in the future, i think
that the case of someone successfully logging in or out (logging
out is hard to do unsuccessfully... ;) is a common enough case
that we should make things comfortable for the user by returning
to the referer. We know the solution, i don't see why we should
drop it, especially as it was requested by users.

i think it's reasonable that at the same time, the particular
case of a user unsuccessfully logging in and then finally
logging in successfully can be treated by redirecting him/her
to the frontpage.

So i'm posting a patch to 0.6.0.070818 which reverts to 070618
for member_controller + puts in my patch suggested earlier,
except that the redirection is to nil, i.e. to the frontpage.

i've changed that status to 'in progress', since at the moment
i don't think we (yet) have consensus. A fix which destroys
a simple and nice (even if limited) functionality is IMHO
not (yet) a fix.



(file #13745)

Boud Roukema <boud>
Group Member
Mon 20 Aug 2007 11:09:40 PM UTC, comment #2: 

seems to work

Boud Roukema <boud>
Group Member
Sat 18 Aug 2007 06:06:01 PM UTC, comment #1: 

Fixed in snapshot 2007-08-18.

It looks like the the whole idea of redirecting back to where login is initiated from is too complex to be implemented without bugs or kludges; since it's not tremendously useful, I removed it for now, this will make this bug go away.

Dmitry Borodaenko <angdraug>
Group administrator
Fri 03 Aug 2007 10:21:39 PM UTC, original submission:  

hi samizdat-devel

PROBLEM: Sometimes a user when logging in may make some errors, e.g.
login or password may be missing, and so the "referer" URI becomes the
URI of member/login itself. In this situation, when the user clicks on
the submit button and successfully logs in, a redirect occurs to
member/login itself, so the part of the screen closest to the region
where s/he clicked appears to be totally unchanged. If the user looks
to the top-right, s/he will notice that his/her "full name" is now
shown, and will realise that s/he has successfully logged in.
However, the submit button is at the bottom left and it is easy for the
user to miss this changed information. Moreover, the main content of
the page invites the user to log in, despite the fact (unclear to the
user) that s/he has successfully logged in.

This leads to a user being frustrated and thinking that s/he has not
successfully logged in.

Not only is this in itself a Bad Thing, but a frustrated user might
unnecessarily create a new login, or try his/her passwords normally
used on other sites, or switch from https to http since it's "better
to try something than just give up" - plenty of things to weaken
security.

This problem is asymmetric with respect to the member/logout method -
because the logout button leads to changes in the content of the page
at a point very close to where the user has clicked, so the change is
obvious in that case.

PROPOSED SOLUTION: If the referer is member/login itself, then
redirect to some other page which should signal to the user that s/he
has successfully logged in. This could be a new page:  @title = _('Login
succeeded')... , or, as i suggest here, a redirect to the member page.


cheers
boud


--- /tmp/tmp_snapshot/samizdat/lib/samizdat/controllers/member_controller.rb 2007-05-05 14:56:27.000000000 +0200
+++ /usr/lib/ruby/1.8/samizdat/controllers/member_controller.rb 2007-08-03 23:54:14.579828264 +0200
@@ -183,7 +191,8 @@
     if login and password
       if cookie = Session.start(login, password)
         @request.set_cookie('session', cookie, config['timeout']['last'])
-        @request.redirect(referer)
+        referer =~ %r{member/login\Z} ?
+           @request.redirect('member') : @request.redirect(referer)
       else
         @title = _('Login Failed')
         @content_for_layout = box(@title,


Boud Roukema <boud>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #13748:  070820_loginpatch-0.2 added by boud (1KiB - application/octet-stream)
file #13745:  070820_loginpatch-0.1 added by boud (1KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by angdraug (Posted a comment)
  • -email is unavailable- added by boud (Submitted the item)
  •  

    Follow 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2008-02-21 angdraug StatusIn Progress Fixed
        Open/ClosedOpen Closed
    2007-08-21 boud Attached File- Added 070820_loginpatch-0.2, #13748
        Summarymember_controller: user successfully logs in but thinks that s/he has failed (patch vs 070818) member_controller: user successfully logs in but thinks that s/he has failed (loginpatch-0.2 vs 070818)
    2007-08-20 boud Attached File- Added 070820_loginpatch-0.1, #13745
        StatusFixed In Progress
        Summarymember_controller: user successfully logs in but thinks that s/he has failed member_controller: user successfully logs in but thinks that s/he has failed (patch vs 070818)
    2007-08-18 angdraug StatusWorks For Me Fixed

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code