Project

General

Profile

Bug #7253

unable to modify user in UI as it incorrectly states "Administrator cannot be removed from the last admin account"

Added by Thomas McKay over 4 years ago. Updated 11 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Authentication
Target version:
Difficulty:
Triaged:
Bugzilla link:
Team Backlog:
Fixed in Releases:
Found in Releases:

Description

The bit of code in app/model/user.rb ensure_last_admin_remains_admin()

!new_record? && admin_changed? && !admin && User.unscoped.only_admin.except_hidden.size <= 1

evaluates (incorrectly) to true.

User.unscoped.only_admin.except_hidden.size = 1

Why is more than one admin required?

Associated revisions

Revision b8391adc (diff)
Added by Dominic Cleal over 4 years ago

fixes #7253 - change nil admin field on users to false, matches usergroups

When the admin field was nil, admin_changed? in user model validations can
evaluate to true if the field changed from nil to false.

Revision 12ec0b1d (diff)
Added by Dominic Cleal over 4 years ago

fixes #7253 - change nil admin field on users to false, matches usergroups

When the admin field was nil, admin_changed? in user model validations can
evaluate to true if the field changed from nil to false.

(cherry picked from commit b8391adc2f720734551c57af96a2a50b9699fb58)

History

#1 Updated by Thomas McKay over 4 years ago

  • Bugzilla link set to 1133679

#2 Updated by Dominic Cleal over 4 years ago

  • Status changed from New to Feedback

Under what conditions is this a problem? Any user? Any non-admin user? When only one admin user is present? Please provide reproducer steps.

At least one admin is required to retain administrative level access, else recovery methods are required to regain access.

#3 Updated by Thomas McKay over 4 years ago

  • Status changed from Feedback to New

I have one admin and when I edit users in the UI, it errors due to that line preventing any modification of users.

#4 Updated by Dominic Cleal over 4 years ago

  • Target version set to 1.7.4
  • Legacy Backlogs Release (now unused) set to 22

Reproducer:

1. create a non-admin user via the API (or another means, not the UI)
2. ensure the admin field is nil, not false:

2.0.0-p353 :001 > User.find_by_login("wraptest5").admin?
 => false 
2.0.0-p353 :002 > User.find_by_login("wraptest5").admin
 => nil 

3. as an admin, edit the non-admin user

This means "admin_changed?" results in true as it moves from nil to false, rather than false to true as the expression was intended to catch.

[1] pry(#<User>)> changes
=> {"admin"=>[nil, false], ...

#5 Updated by Dominic Cleal over 4 years ago

  • Status changed from New to Assigned
  • Assignee set to Dominic Cleal
  • Legacy Backlogs Release (now unused) changed from 22 to 10

#6 Updated by The Foreman Bot over 4 years ago

  • Status changed from Assigned to Ready For Testing
  • Pull request https://github.com/theforeman/foreman/pull/1749 added
  • Pull request deleted ()

#7 Updated by Dominic Cleal over 4 years ago

  • Status changed from Ready For Testing to Closed
  • % Done changed from 0 to 100

Also available in: Atom PDF