Open Bug 685254 Opened 13 years ago Updated 5 years ago

Allow manually unlocking a user account after user lockout

Categories

(Bugzilla :: Administration, task)

3.6.3
task
Not set
normal

Tracking

()

People

(Reporter: Clinton.Bush, Assigned: alim94, Mentored)

References

Details

Attachments

(1 file, 1 obsolete file)

User Agent: Mozilla/4.0 (compatible; MSIE 8.0; Windows NT 6.1; Trident/4.0; SLCC2; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729; InfoPath.2; .NET4.0C; .NET4.0E) Steps to reproduce: Attempted to log into an account more than 5 times and my account was locked out from my PC. Actual results: We are using a private network that has no SMTP server so I could not used the e-mail from Bugzilla to unlock my account. The administrator cannot unlock my account for my IP address because there is no feature in the Administration page for this. Expected results: There should be a feature in the Administration page of bugzilla to unlock blocked IP addresses.
SMTP being down plays no role as the email doesn't contain any link to unlock your account. The account is automatically unlocked after 30 minutes.
Assignee: general → administration
Severity: normal → enhancement
Component: Bugzilla-General → Administration
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Cannot Unlock IP Address From Administration Page → Allow manually unlocking a user account after user lockout
I agree that we should allow this. :-)
Priority: -- → P2
How should this work? A checkbox on the edit user page to show the locked out status? And then the admin unchecks the box to remove the lock out?
Has this made its way into any current releases?
(In reply to Dave from comment #5) > Has this made its way into any current releases? Unfortunately none that I am aware of. This would be a good candidate for a contributors to use as a first bug as technically it would not be a lot of work.
Priority: P2 → --
Whiteboard: [good first bug][lang=perl][mentor=dkl]
Target Milestone: --- → Bugzilla 5.0
Mentor: dkl
Whiteboard: [good first bug][lang=perl][mentor=dkl] → [good first bug][lang=perl]
I'm new and I would like to help, can I take this bug?
(In reply to helene.tindon from comment #7) > I'm new and I would like to help, can I take this bug? Feel free to work on this and ask me if you have any questions. dkl
Assignee: administration → helene.tindon
Status: NEW → ASSIGNED
Target Milestone: Bugzilla 5.0 → ---
resetting the assignedto field due to inactivity. :dkl, please confirm this bug is still needed and that it makes a good first bug.
Assignee: helene.tindon → administration
Status: ASSIGNED → NEW
Flags: needinfo?(dkl)
(In reply to Joel Maher (:jmaher) from comment #9) > resetting the assignedto field due to inactivity. :dkl, please confirm this > bug is still needed and that it makes a good first bug. This is still needed and still would make a good first bug. dkl
Flags: needinfo?(dkl)
We might get some takers for this bug if someone could show where in the codebase a contributor would need to look to get started.
Yeah good idea. Here are the files that would need to be updated: http://git.mozilla.org/?p=bugzilla/bugzilla.git;a=blob;f=Bugzilla/User.pm [github] lines 2308-2351 Add a method to clear the current lockout row for the user. Either check for membership in the 'editusers' group in the method or in editusers.cgi before calling the object method. Maybe call the method 'clear_lockout' or similar. http://git.mozilla.org/?p=bugzilla/bugzilla.git;a=blob;f=editusers.cgi [github] lines 246-296 Add code in the 'update' action to check for a form value passed in from the web UI that says to clear the lockout for the specific user by calling the new $otherUser->clear_lockout method in User.pm. http://git.mozilla.org/?p=bugzilla/bugzilla.git;a=blob;f=template/en/default/admin/users/edit.html.tmpl [github] between lines 114-115 Only if the user being edited is locked out, add form input, such as checkbox, that when checked will clear the user's lockout from the DB using the proper method in editusers.cgi. Let me know if I can give any additional help. dkl
I would like to start working on this.
(In reply to andpirvulescu from comment #13) > I would like to start working on this. Awesome. I will assign you to this bug and when you have a patch ready, you can put it up for review. I can be the reviewer if noone else has the time. Here is some information to get you started. We are working on improving new developer documentation as well so let us know if you have any feedback about that as well. https://wiki.mozilla.org/Bugzilla:Developers Thanks dkl
Assignee: administration → andpirvulescu
No longer blocks: 1126453
Any progress on this?
I would like to work on this. It is long since this was last updated. Can u assign it to me ?
Flags: needinfo?(dkl)
Unassigning from andpirvulescu due to inactivity and assigning to Mukhtar.
Assignee: andpirvulescu → alim94
Flags: needinfo?(dkl)
(In reply to Mukhtar Ali from comment #16) > I would like to work on this. It is long since this was last updated. Can u > assign it to me ? Finding where to make changes can be hard, so I'll offer these guide posts: you'll want to look at edituser.cgi and template/en/default/admin/users/edit.html.tmpl. There is a Bugzilla::User->clear_login_failures() method that illustrates how lockouts can be cleared (although that particular method will not work for clearing another user's lockout). Hope this helps :-)
Attached patch patch.diff (obsolete) (deleted) — Splinter Review
I was able to get the desired functionality with these minimal changes.
Attachment #8695475 - Flags: review?
Attachment #8695475 - Flags: feedback?
Comment on attachment 8695475 [details] [diff] [review] patch.diff Review of attachment 8695475 [details] [diff] [review]: ----------------------------------------------------------------- This won't work in practice. clear_login_failures() only deletes entries for the *current* user's IP. https://github.com/bugzilla/bugzilla/blob/master/Bugzilla/User.pm#L2340 Probably best to add a new method: clear_all_login_failures() that does not use the remote_ip()
Attachment #8695475 - Flags: review?
Attachment #8695475 - Flags: review-
Attachment #8695475 - Flags: feedback?
(In reply to Dylan William Hardison [:dylan] from comment #20) > Comment on attachment 8695475 [details] [diff] [review] > patch.diff > > Review of attachment 8695475 [details] [diff] [review]: > ----------------------------------------------------------------- > > This won't work in practice. clear_login_failures() only deletes entries for > the *current* user's IP. > https://github.com/bugzilla/bugzilla/blob/master/Bugzilla/User.pm#L2340 > > Probably best to add a new method: clear_all_login_failures() that does not > use the remote_ip() I think this wiil be good enough : sub clear_all_login_failures { my $self = shift; Bugzilla->dbh->do( 'DELETE FROM login_failure WHERE user_id = ?', undef, $self->id); }
Attached patch patch3.diff (deleted) — Splinter Review
Changes made as per review. Let me know issues if any.
Attachment #8695764 - Flags: review?
Attachment #8695764 - Flags: feedback?
Need to close this. Can u have a look quickly and let me know if any thing more needed from my side.
Flags: needinfo?(dkl)
Flags: needinfo?(dkl)
Attachment #8695764 - Flags: review?(dkl)
Attachment #8695764 - Flags: review?
Attachment #8695764 - Flags: feedback?
Attachment #8695475 - Attachment is obsolete: true
Comment on attachment 8695764 [details] [diff] [review] patch3.diff Review of attachment 8695764 [details] [diff] [review]: ----------------------------------------------------------------- t/004template.t ...... ok t/005whitespace.t .... 1/1575 # Failed test 'editusers.cgi contains tabs --WARNING' # at t/005whitespace.t line 37. # Looks like you failed 1 test of 1575. t/005whitespace.t .... Dubious, test returned 1 (wstat 256, 0x100) Need to configure your editor to use 4 hard spaces instead of a tab character. t/011pod.t ........... 225/380 # Failed test 'Bugzilla/User.pm POD coverage is 97%. Undocumented methods: account_is_all_locked_out, clear_all_login_failures, account_all_login_failures' # at t/011pod.t line 109. The new methods being added in User.pm will need related POD entries added to the bottom of the file. ::: Bugzilla/User.pm @@ +2330,5 @@ > + > +sub account_all_login_failures { > + my $self = shift; > + my $dbh = Bugzilla->dbh; > + my $time = $dbh->sql_date_math('LOCALTIMESTAMP(0)', '-', remove extra whitespace at end. @@ +2336,5 @@ > + $self->{account_login_failures} ||= Bugzilla->dbh->selectall_arrayref( > + "SELECT login_time, user_id FROM login_failure > + WHERE user_id = ? AND login_time > $time > + ORDER BY login_time", {Slice => {}}, $self->id); > + return $self->{account_login_failures}; rename to account_all_login_failures @@ +2360,4 @@ > delete $self->{account_ip_login_failures}; > } > > +sub clear_all_login_failures { We do not need another method to accomplish this. We can just improve the current clear_login_failures() method to either clear only for specific IP or all failures. Example: sub clear_login_failures { my ($self, $clear_all) = shift; my $query = "DELETE FROM login_failure WHERE user_id = ?"; my @values = ($self->id); unless ($clear_all) { my $ip_addr = remote_ip(); trick_taint($ip_addr); $query += " AND ip_addr = ?"; push(@values, $ip_addr); } Bugzilla->dbh->do($query, undef, @values); $clear_all ? delete $self->{account_ip_login_failures}; } ::: editusers.cgi @@ +264,4 @@ > my @group_ids = grep { s/group_// } keys %{ Bugzilla->cgi->Vars }; > $otherUser->set_groups({ set => \@group_ids }); > > + # Unlock user account remove ending whitespace @@ +264,5 @@ > my @group_ids = grep { s/group_// } keys %{ Bugzilla->cgi->Vars }; > $otherUser->set_groups({ set => \@group_ids }); > > + # Unlock user account > + if ( $cgi->param('unlock_user') ){ style: remove extra space before ) and after (. ::: template/en/default/admin/users/userdata.html.tmpl @@ +88,5 @@ > [% END %] > + > + > + [% IF otheruser.account_is_all_locked_out %] > + <tr> style: indent 2 additional spaces inside the [% IF %] block @@ +95,5 @@ > + <input type="checkbox" name="unlock_user" id="unlock_user" checked="checked"> > + </td> > + </tr> > + [% END %] > + whitespace
Attachment #8695764 - Flags: review?(dkl) → review-
is this bug still open, if yes,i would like to work on it, and please i need someone's help, i am new here in bugzilla, good at javascript and searching for some good first bug, and also for a mentor please someone help me to grow, i really want to do something for open source, any help would be highly appreciated :)

Removing good-first-bug keyword because team does not have bandwidth to mentor at the moment.

Keywords: good-first-bug
Whiteboard: [good first bug][lang=perl]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: