Merge pull request #1875 from adrianschroeter/simplify_user_state

[api] simplify user state code

src/api/app/controllers/application_controller.rb

@@ -119,8 +119,7 @@ def extract_proxy_user
         @http_user = User.new(
             login: proxy_user,
             state: User.default_user_state,
-            password: fakepw,
-            password_confirmation: fakepw)
+            password: fakepw)
       end
 
       # update user data from login proxy headers
@@ -206,14 +205,14 @@ def check_extracted_user
       raise AuthenticationRequiredError.new "Unknown user '#{@login}' or invalid password"
     end
 
-    if @http_user.state == User::STATES['ichainrequest'] or @http_user.state == User::STATES['unconfirmed']
+    if @http_user.state == 'unconfirmed'
       raise UnconfirmedUserError.new "User is registered but not yet approved. " +
                                          "Your account is a registered account, but it is not yet approved for the OBS by admin."
     end
 
     User.current = @http_user
 
-    if @http_user.state == User::STATES['confirmed']
+    if @http_user.state == 'confirmed'
       logger.debug "USER found: #{@http_user.login}"
       @user_permissions = Suse::Permission.new(@http_user)
       return true

src/api/app/controllers/person_controller.rb

@@ -80,8 +80,8 @@ def put_userinfo
       end
     else
       if User.current.is_admin?
-        user = User.create(:login => login, :password => "notset", :password_confirmation => "notset", :email => "TEMP")
-        user.state = User::STATES["locked"]
+        user = User.create(:login => login, :password => "notset", :email => "TEMP")
+        user.state = "locked"
       else
         logger.debug "Tried to create non-existing user without admin rights"
         @errorcode = 404
@@ -96,7 +96,7 @@ def put_userinfo
     user.realname = xml.value('realname') || ''
     if User.current.is_admin?
       # only admin is allowed to change these, ignore for others
-      user.state = User::STATES[xml.value('state')]
+      user.state = xml.value('state')
       update_globalroles(user, xml)
     end
     update_watchlist(user, xml)

src/api/app/controllers/webui/user_controller.rb

@@ -103,7 +103,7 @@ def save
     @displayed_user.realname = params[:realname]
     @displayed_user.email = params[:email]
     if User.current.is_admin?
-      @displayed_user.state = User::STATES[params[:state]] if params[:state]
+      @displayed_user.state = params[:state] if params[:state]
       @displayed_user.update_globalroles([params[:globalrole]]) if params[:globalrole]
     end
 
@@ -123,19 +123,19 @@ def edit
   end
 
   def delete
-    @displayed_user.state = User::STATES['deleted']
+    @displayed_user.state = 'deleted'
     @displayed_user.save
     redirect_back_or_to :action => 'show', user: @displayed_user
   end
 
   def confirm
-    @displayed_user.state = User::STATES['confirmed']
+    @displayed_user.state = 'confirmed'
     @displayed_user.save
     redirect_back_or_to :action => 'show', user: @displayed_user
   end
 
   def lock
-    @displayed_user.state = User::STATES['locked']
+    @displayed_user.state = 'locked'
     @displayed_user.save
     redirect_back_or_to :action => 'show', user: @displayed_user
   end

src/api/app/controllers/webui/webui_controller.rb

@@ -220,8 +220,7 @@ def check_user
                      email: request.env['HTTP_X_EMAIL'],
                      state: User.default_user_state,
                      realname: "#{request.env['HTTP_X_FIRSTNAME']} #{request.env['HTTP_X_LASTNAME']}".strip,
-                     password: fakepw,
-                     password_confirmation: fakepw)
+                     password: fakepw)
       end
 
       User.current = User.find_by_login(user_login)

src/api/app/models/owner.rb

@@ -153,18 +153,19 @@ def self.find_containers_without_definition(rootproject, devel = true, filter =
     end
 
     # find all groups which have an active user
-    maintained_groups = Group.joins(:groups_users).joins(:users).where("users.state = ?", User::STATES['confirmed']).to_a
+    maintained_groups = Group.joins(:groups_users).joins(:users).where("users.state = 'confirmed'").to_a
 
     # fast find packages with defintions
     # relationship in package object by user
-    defined_packages = Package.where(project_id: projects).joins(:relationships => :user).where(["relationships.role_id IN (?) AND users.state = ?",
-                                                                                                 roles, User::STATES['confirmed']]).pluck(:name)
+    defined_packages = Package.where(project_id: projects).joins(:relationships => :user).\
+                               where(["relationships.role_id IN (?) AND users.state = 'confirmed'",
+                                      roles]).pluck(:name)
     # relationship in package object by group
     defined_packages += Package.where(project_id: projects).joins(:relationships).where(["relationships.role_id IN (?) AND group_id IN (?)",
                                                                                          roles, maintained_groups]).pluck(:name)
     # relationship in project object by user
-    Project.joins(:relationships => :user).where("projects.id in (?) AND role_id in (?) AND users.state = ?",
-                                                 projects, roles, User::STATES['confirmed']).each do |prj|
+    Project.joins(:relationships => :user).where("projects.id in (?) AND role_id in (?) AND users.state = 'confirmed'",
+                                                 projects, roles).each do |prj|
       defined_packages += prj.packages.pluck(:name)
     end
     # relationship in project object by group
@@ -336,14 +337,14 @@ def self._extract_from_container(m, r, sql, objfilter)
     groupsql = sql << " AND group_id = " << objfilter.id.to_s if objfilter.class == Group
 
     r.users.where(usersql).each do |p|
-      next unless p.user.state == User::STATES['confirmed']
+      next unless p.user.state == "confirmed"
       m.users ||= {}
       m.users[p.role.title] ||= []
       m.users[p.role.title] << p.user.login
     end unless objfilter.class == Group
 
     r.groups.where(groupsql).each do |p|
-      next unless p.group.users.where(state: User::STATES['confirmed']).length > 0
+      next unless p.group.users.where(state: "confirmed").length > 0
       m.groups ||= {}
       m.groups[p.role.title] ||= []
       m.groups[p.role.title] << p.group.title

src/api/app/models/unregistered_user.rb

@@ -41,28 +41,19 @@ def self.can_register?
     raise ErrRegisterSave, 'Sorry, sign up is disabled'
   end
 
-  def self.get_state
-    state = User::STATES.key(User.default_state)
-    state = 'unconfirmed' if ::Configuration.registration == 'confirmation'
-    state = 'confirmed' if ::Configuration.registration == 'allow'
-    logger.debug "User state is: #{state}"
-    return state
-  end
-
   def self.register(opts)
     can_register?
 
     opts[:note] = nil unless User.current and User.current.is_admin?
-    state = get_state
+    state = (::Configuration.registration == 'allow') ? "confirmed" : "unconfirmed"
 
     newuser = User.create(
         :login => opts[:login],
         :password => opts[:password],
-        :password_confirmation => opts[:password],
         :email => opts[:email] )
 
     newuser.realname = opts[:realname] || ""
-    newuser.state = User::STATES[state]
+    newuser.state = state
     newuser.adminnote = opts[:note]
     logger.debug("Saving new user #{newuser.login}")
     newuser.save
@@ -72,7 +63,7 @@ def self.register(opts)
       raise ErrRegisterSave.new "Could not save the registration, details: #{details}"
     end
 
-    if newuser.state == User::STATES["unconfirmed"]
+    if newuser.state == "unconfirmed"
       raise ErrRegisterSave.new "Thank you for signing up! An admin has to confirm your account now. Please be patient."
     end
   end

src/api/app/models/user.rb

@@ -22,15 +22,6 @@ def groups(user)
 class User < ActiveRecord::Base
   include CanRenderModel
 
-  STATES = {
-    'unconfirmed'        => 1,
-    'confirmed'          => 2,
-    'locked'             => 3,
-    'deleted'            => 4,
-    'ichainrequest'      => 5,
-    'retrieved_password' => 6
-  }
-
   has_many :taggings, :dependent => :destroy
   has_many :tags, :through => :taggings
 
@@ -60,9 +51,6 @@ class User < ActiveRecord::Base
   # required.
   attr_accessor :new_password
 
-  # Generate accessors for the password confirmation property.
-  attr_accessor :password_confirmation
-
   validates_presence_of :login, :email, :password, :password_hash_type, :state,
                         :message => 'must be given'
 
@@ -134,7 +122,7 @@ def create_home_project
   # (@new_password) and the login failure count to
   # unconfirmed/false/0 when it has not been set yet.
   before_validation(:on => :create) do
-    self.state = STATES['unconfirmed'] if self.state.nil?
+    self.state ||= 'unconfirmed'
     self.password_hash_type = 'md5' if self.password_hash_type.to_s == ''
 
     @new_password = false if @new_password.nil?
@@ -156,10 +144,6 @@ def create_home_project
       # write encrypted password to object property
       write_attribute(:password, hash_string(password))
 
-      # mark password as "not new" any more
-      @new_password = false
-      self.password_confirmation = nil
-
       # mark the hash type as "not new" any more
       @new_hash_type = false
     else
@@ -174,8 +158,7 @@ def before_create
 
   # the default state of a user based on the api configuration
   def self.default_user_state
-    return STATES['unconfirmed'] if ::Configuration.registration == "confirmation"
-    STATES['confirmed']
+    ::Configuration.registration == 'confirmation' ? 'unconfirmed' : 'confirmed'
   end
 
   # This method returns an array with the names of all available
@@ -209,15 +192,10 @@ def self.update_notifications(params, user = nil)
     end
   end
 
-  # Returns the default state of new User objects.
-  def self.default_state
-    STATES['unconfirmed']
-  end
-
   # Returns true when users with the given state may log in. False otherwise.
   # The given parameter must be an integer.
   def self.state_allows_login?(state)
-    [ STATES['confirmed'], STATES['retrieved_password'] ].include?(state)
+    'confirmed' == state
   end
 
   # This static method tries to find a user with the given login and password
@@ -268,7 +246,6 @@ def self.find_with_credentials(login, password)
       password = SecureRandom.base64
       user = User.create( :login => login,
                           :password => password,
-                          :password_confirmation => password,
                           :email => ldap_info[0],
                           :last_logged_in_at => Time.now)
       unless user.errors.empty?
@@ -321,7 +298,7 @@ def self.authenticate(user_login, password)
     user = User.find_with_credentials(user_login, password)
 
     # User account is not confirmed yet
-    return if user.try(:state) == STATES['unconfirmed']
+    return if user.try(:state) == 'unconfirmed'
 
     Rails.logger.debug "Authentificated user '#{user.try(:login)}'"
 
@@ -338,15 +315,14 @@ def self.get_default_admin
   def self.find_nobody!
     Thread.current[:nobody_user] ||= User.create_with(email: "nobody@localhost",
                                                       realname: "Anonymous User",
-                                                      state: STATES['locked'],
-                                                      password: "123456",
-                                                      password_confirmation: "123456").find_or_create_by(login: nobody_login)
+                                                      state: 'locked',
+                                                      password: "123456").find_or_create_by(login: nobody_login)
     Thread.current[:nobody_user]
   end
 
   def self.find_by_login!(login)
     user = find_by_login(login)
-    if user.nil? or user.state == STATES['deleted']
+    if user.nil? || user.state == 'deleted'
       raise NotFoundError.new("Couldn't find User with login = #{login}")
     end
     return user
@@ -369,20 +345,14 @@ def self.realname_for_login(login)
 
   public
 
-  # Overriding this method to do some more validation: Password equals
-  # password_confirmation, state an password hash type being in the range
-  # of allowed values.
+  # Overriding this method to do some more validation:
+  # state an password hash type being in the range of allowed values.
   def validate
     # validate state and password has type to be in the valid range of values
     errors.add(:password_hash_type, 'must be in the list of hash types.') unless User.password_hash_types.include? password_hash_type
     # check that the state transition is valid
     errors.add(:state, 'must be a valid new state from the current state.') unless state_transition_allowed?(@old_state, state)
 
-    # validate the password
-    if @new_password and not password.nil?
-      errors.add(:password, 'must match the confirmation.') unless password_confirmation == password
-    end
-
     # check that the password hash type has not been set if no new password
     # has been provided
     if @new_hash_type and (!@new_password or password.nil?)
@@ -413,9 +383,8 @@ def new_password?
   end
 
   # Method to update the password and confirmation at the same time. Call
-  # this method when you update the password from code and don't need
-  # password_confirmation - which should really only be used when data
-  # comes from forms.
+  # this method when you update the password from code  - which should really
+  # only be used when data comes from forms.
   #
   # A ussage example:
   #
@@ -425,7 +394,6 @@ def new_password?
   #
   def update_password(pass)
     self.password_crypted = hash_string(pass).crypt('os')
-    self.password_confirmation = hash_string(pass)
     self.password = hash_string(pass)
   end
 
@@ -459,9 +427,9 @@ def create_user_registration
   def confirm_registration(token)
     return false if self.user_registration.nil?
     return false if user_registration.token != token
-    return false unless state_transition_allowed?(state, STATES['confirmed'])
+    return false unless state_transition_allowed?(state, 'confirmed')
 
-    self.state = STATES['confirmed']
+    self.state = 'confirmed'
     self.save!
     user_registration.destroy
 
@@ -489,32 +457,26 @@ def did_log_in
   end
 
   # Returns true if the the state transition from "from" state to "to" state
-  # is valid. Returns false otherwise. +new_state+ must be the integer value
-  # of the state as returned by +User::STATES['state_name']+.
+  # is valid. Returns false otherwise.
   #
   # Note that currently no permission checking is included here; It does not
   # matter what permissions the currently logged in user has, only that the
   # state transition is legal in principle.
   def state_transition_allowed?(from, to)
     from = from.to_i
     to = to.to_i
-    desired_state = STATES.key(to)
 
     return true if from == to # allow keeping state
 
     case from
-    when STATES['unconfirmed']
+    when 'unconfirmed'
       true
-    when STATES['confirmed']
-      ["retrieved_password", "locked", "deleted", "ichainrequest"].include?(desired_state)
-    when STATES['locked']
-      ["confirmed", "deleted"].include?(desired_state)
-    when STATES['deleted']
-      desired_state == "confirmed"
-    when STATES['ichainrequest']
-      ["locked", "confirmed", "deleted"].include?(desired_state)
-    when 0
-      desired_state.present?
+    when 'confirmed'
+      ["locked", "deleted"].include?(to)
+    when 'locked'
+      ["confirmed", "deleted"].include?(to)
+    when 'deleted'
+      to == "confirmed"
     else
       false
     end
@@ -529,10 +491,6 @@ def render_axml( watchlist = false )
     render_xml(watchlist: watchlist)
   end
 
-  def state_name
-    STATES.invert[state]
-  end
-
   def home_project_name
     "home:#{self.login}"
   end
@@ -578,7 +536,7 @@ def is_nobody?
   end
 
   def is_active?
-    self.state == STATES['confirmed']
+    self.state == 'confirmed'
   end
 
   # used to avoid