[drupal-devel] [bug] Wrong Login Error

[deekayen]

Issue status update for http://drupal.org/node/3962

 Project:      Drupal
 Version:      cvs
 Component:    user system
 Category:     bug reports
 Priority:     minor
 Assigned to:  deekayen
 Reported by:  dziemecki
 Updated by:   deekayen
 Status:       patch
 Attachment:   http://drupal.org/files/issues/blockedusermessage_0.diff (3.14 KB)

This patch uses "deactivated" instead of "blocked", but is otherwise not
different from the previous patch.




deekayen



Previous comments:
------------------------------------------------------------------------

November 5, 2003 - 21:01 : dziemecki

When a new user logs on after they're ID and password were sent to them,
but before their account was "unblocked", they get the follwoing error:


"Sorry. Unrecognized username or password. Have you forgotten your
password?"


That may or may not count as a bug, but it not entirely accurate, and
might be a little confusing.




------------------------------------------------------------------------

June 14, 2005 - 03:19 : deekayen

I don't mind assigning this one to myself, but I'm not familiar enough
with the function of the access table to complete the patch. Should
blocked users be entered into the access table?


Right now, user.module has a drupal_deny check with the correct error
message, but it doesn't get displayed because the only place a person
gets blocked is in the users table. To be more normalized, I'd just
leave the block in the users table, but does someone intend to move the
user block to the access table?




------------------------------------------------------------------------

June 15, 2005 - 21:37 : deekayen

Attachment: http://drupal.org/files/issues/blockedloginmessage.patch (1.26 KB)

Right now when users are blocked, they're not added to the access table,
so this patch just checks the users table for blocked logins. The check
in user.module's user_login() should work now and give the correct
error message that was there all along.




------------------------------------------------------------------------

June 15, 2005 - 21:45 : deekayen

Attachment: http://drupal.org/files/issues/removeactivereset.patch (882 bytes)

This patch goes with the previous one. Without it, when you edit a
blocked user to make them active again, they remain blocked and the
edit screen shows an error message about the user being blocked. I
don't really know if that's the intended behavior to have blocks be
permanent, but permanent blocks don't seem right to me.




------------------------------------------------------------------------

June 16, 2005 - 09:04 : moshe weitzman

I'm not so sure about this one. We should continue to distinguish
between a 'user block' and a 'host ban'.




------------------------------------------------------------------------

June 16, 2005 - 09:12 : killes at www.drop.org

I agree with Moshe.


I think we should test whether a user hasn't logged in before and then
give a different message then when he got blocked later.




------------------------------------------------------------------------

June 17, 2005 - 00:37 : deekayen

Attachment: http://drupal.org/files/issues/blockeduser.diff (1.29 KB)

Just your comments made me see where I went wrong with the previous
patches. I'd never used the access control rules before, so I thought
blocked user == access controlled username.


As far as I could tell, the only place to where a blocked username
check was needed is at login. This patch adds a user_blocked() to just
user.module and has a separate blocked error message from the access
check.




------------------------------------------------------------------------

June 17, 2005 - 00:46 : deekayen

Attachment: http://drupal.org/files/issues/blockeduser_0.diff (1.29 KB)

I'm not used to the Drupal bracket formatting, yet. This is a spacing
revision.




------------------------------------------------------------------------

June 19, 2005 - 02:28 : Dries

I think this patch makes sense, however the difference between 'the name
X has been denied access' and 'the name X has been blocked' is too
subtle for the average Drupal user/administrator to grasp. I'd like to
have us think about rewording the error messages so they are somewhat
more descriptive to administrators without exposing internal/technical
detail to the user.  How about writing 'the name X is a reserved user
name' for the former, and 'the user with name X is no longer allowed
access' for the latter?


(One could argue we need the drupal_deny() check at login time.)




------------------------------------------------------------------------

June 19, 2005 - 14:48 : deekayen

Attachment: http://drupal.org/files/issues/blockedusermessage.diff (1.5 KB)

What about "The username %s has been blocked." and "The name %s is a
reserved username."? I think it's important that the user
administration "blocked" status matches the error message.


Otherwise perhaps the user administration "blocked" status should be
changed to "Deactivated" or "Denied" with "The username %s has been
deactivated." as the error message.


The attached patch is revised to have the first two messages.