Merge pull request #4721 from bgeuken/ldap_backports_2.9

Backport LDAP local user feature to 2.9 branch

src/api/app/controllers/person_controller.rb

@@ -51,7 +51,7 @@ def post_userinfo
     if params[:cmd] == 'change_password'
       login ||= User.current.login
       password = request.raw_post.to_s.chomp
-      if (login != User.current.login && !User.current.is_admin?) || !::Configuration.passwords_changable?
+      if (login != User.current.login && !User.current.is_admin?) || !::Configuration.passwords_changable?(User.current)
         render_error status: 403, errorcode: 'change_password_no_permission',
                      message: "No permission to change password for user #{login}"
         return
@@ -87,7 +87,7 @@ def put_userinfo
     login = params[:login]
     user = User.find_by_login(login) if login
 
-    unless ::Configuration.accounts_editable?
+    unless ::Configuration.accounts_editable?(user)
       render_error(status: 403, errorcode: 'change_userinfo_no_permission',
                    message: "no permission to change userinfo for user #{user.login}")
       return
@@ -117,6 +117,7 @@ def put_userinfo
       # only admin is allowed to change these, ignore for others
       user.state = xml.value('state')
       update_globalroles(user, xml)
+      user.update(ignore_auth_services: xml.value('ignore_auth_services').to_s == 'true')
 
       if xml['owner']
         user.state = :subaccount
@@ -241,6 +242,7 @@ def change_password(login, password)
       @errorcode = 401
       @summary = 'No user logged in, permission to changing password denied'
       render template: 'error', status: 401
+      return
     end
 
     if login.blank? || password.blank?
@@ -251,7 +253,7 @@ def change_password(login, password)
     user = User.get_by_login(login)
 
     # change password to LDAP if LDAP is enabled
-    if CONFIG['ldap_mode'] == :on
+    unless ::Configuration.passwords_changable?(User.current)
       render_error status: 404, errorcode: 'change_passwd_failure',
                                 message: 'LDAP passwords can not be changed in OBS. Please refer to your LDAP server to change it.'
       return

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

@@ -1,13 +1,14 @@
 class Webui::UserController < Webui::WebuiController
-  before_action :check_display_user, only: [:show, :edit, :list_my, :delete, :confirm, :admin, :lock]
+  before_action :check_display_user, only: [:show, :edit, :list_my, :delete, :confirm, :admin, :lock, :make_user_local]
   before_action :require_login, only: [:edit, :save, :index]
-  before_action :require_admin, only: [:edit, :delete, :lock, :confirm, :admin, :index]
+  before_action :require_admin, only: [:edit, :delete, :lock, :confirm, :admin, :make_user_local, :index]
   before_action :kerberos_auth, only: [:login]
 
   skip_before_action :check_anonymous, only: [:do_login]
 
   def index
-    @users = User.all_without_nobody.includes(:owner).select(:id, :login, :email, :state, :realname, :owner_id, :updated_at)
+    @users = User.all_without_nobody.includes(:owner).
+             select(:id, :login, :email, :state, :realname, :owner_id, :updated_at, :ignore_auth_services)
   end
 
   def logout
@@ -73,19 +74,19 @@ def save
     @displayed_user = User.find_by_login(params[:user][:login])
 
     unless User.current.is_admin?
-      if User.current != @displayed_user || !@configuration.accounts_editable?
+      if User.current != @displayed_user || !@configuration.accounts_editable?(@displayed_user)
         flash[:error] = "Can't edit #{@displayed_user.login}"
         redirect_back(fallback_location: root_path)
         return
       end
     end
 
-    if @configuration.accounts_editable?
+    if @configuration.accounts_editable?(@displayed_user)
       @displayed_user.assign_attributes(params[:user].slice(:realname, :email).permit!)
     end
 
     if User.current.is_admin?
-      @displayed_user.assign_attributes(params[:user].slice(:state).permit!)
+      @displayed_user.assign_attributes(params[:user].slice(:state, :ignore_auth_services).permit!)
       @displayed_user.update_globalroles(Role.global.where(id: params[:user][:role_ids])) unless params[:user][:role_ids].nil?
     end
 
@@ -113,6 +114,16 @@ def confirm
     redirect_back(fallback_location: { action: 'show', user: @displayed_user })
   end
 
+  def make_user_local
+    @displayed_user.update(ignore_auth_services: true)
+    if @displayed_user.save
+      flash[:notice] = "Updated user '#{@displayed_user.login}'."
+    else
+      flash[:error] = "Updating user '#{@displayed_user.login}' failed: #{@displayed_user.errors.full_messages.to_sentence}"
+    end
+    redirect_back(fallback_location: user_show_path(@displayed_user))
+  end
+
   def lock
     @displayed_user.state = 'locked'
     @displayed_user.save
@@ -183,14 +194,14 @@ def password_dialog
   end
 
   def change_password
-    unless @configuration.passwords_changable?
+    user = User.current
+
+    unless @configuration.passwords_changable?(user)
       flash[:error] = "You're not authorized to change your password."
       redirect_back fallback_location: root_path
       return
     end
 
-    user = User.current
-
     if user.authenticate(params[:password])
       user.password = params[:new_password]
       user.password_confirmation = params[:repeat_password]

src/api/app/models/configuration.rb

@@ -82,13 +82,17 @@ def amqp_namespace
     CONFIG['amqp_namespace'] || 'opensuse.obs'
   end
 
-  def passwords_changable?
-    change_password && CONFIG['proxy_auth_mode'] != :on && CONFIG['ldap_mode'] != :on
+  def passwords_changable?(user = nil)
+    change_password && CONFIG['proxy_auth_mode'] != :on && (user.try(:ignore_auth_services?) || CONFIG['ldap_mode'] != :on)
   end
 
-  def accounts_editable?
-    (CONFIG['proxy_auth_mode'] != :on || (CONFIG['proxy_auth_mode'] == :on && CONFIG['proxy_auth_account_page'].present?)) && \
-      CONFIG['ldap_mode'] != :on
+  def accounts_editable?(user = nil)
+    (
+      CONFIG['proxy_auth_mode'] != :on ||
+        CONFIG['proxy_auth_mode'] == :on && CONFIG['proxy_auth_account_page'].present?
+    ) && (
+      user.try(:ignore_auth_services?) || CONFIG['ldap_mode'] != :on
+    )
   end
 
   def update_from_options_yml

src/api/app/models/unregistered_user.rb

@@ -8,7 +8,7 @@ class ErrRegisterSave < APIException
   # Returns true if a user can register
   def self.can_register?
     # No registering if LDAP is on
-    if CONFIG['ldap_mode'] == :on
+    if CONFIG['ldap_mode'] == :on && !User.current.is_admin?
       logger.debug 'Someone tried to register with "ldap_mode" turned on'
       raise ErrRegisterSave, 'Sorry, new users can only sign up via LDAP'
     end
@@ -54,7 +54,8 @@ def self.register(opts)
       password_confirmation: opts[:password_confirmation],
       email:                 opts[:email],
       state:                 state,
-      adminnote:             opts[:note]
+      adminnote:             opts[:note],
+      ignore_auth_services:  Configuration.ldap_enabled?
     )
 
     unless newuser.save

src/api/app/models/user.rb

@@ -147,25 +147,15 @@ def self.find_with_credentials(login, password)
     end
 
     user = find_by_login(login)
-
-    if user.try(:authenticate, password)
-      user.mark_login!
-      return user
-    end
-
-    # Otherwise increase the login count - if the user could be found - and return nil
-    if user
-      user.login_failure_count = user.login_failure_count + 1
-      user.save!
-    end
-
-    return
+    user.try(:authenticate_via_password, password)
   end
 
   def self.find_with_credentials_via_ldap(login, password)
     user = find_by_login(login)
     ldap_info = nil
 
+    return user.authenticate_via_password(password) if user.try(:ignore_auth_services?)
+
     if CONFIG['ldap_mode'] == :on
       begin
         require 'ldap'
@@ -252,6 +242,16 @@ def self.realname_for_login(login)
     ''
   end
 
+  def authenticate_via_password(password)
+    if authenticate(password)
+      mark_login!
+      self
+    else
+      update_attributes!(login_failure_count: login_failure_count + 1)
+      nil
+    end
+  end
+
   # Overriding this method to do some more validation:
   # state an password hash type being in the range of allowed values.
   # FIXME: This is currently not used. It's not used by rails validations.

src/api/app/views/models/_user.xml.builder

@@ -13,6 +13,8 @@ xml.person do
     xml.globalrole(role.title)
   end
 
+  xml.ignore_auth_services(my_model.ignore_auth_services) if my_model.is_admin?
+
   # Show the watchlist only to the user for privacy reasons
   if watchlist
     xml.watchlist do

src/api/app/views/webui/user/_dropdown_menu.html.haml

@@ -21,6 +21,11 @@
       = sprite_tag('cog_add')
       = link_to('Make user admin', {controller: "user", action: "admin", user: user.login }, |
               method: :post, data: { confirm: "Are you sure?" }) |
+    - if Configuration.ldap_enabled? && !user.ignore_auth_services
+      %li
+        = sprite_tag('cog_add')
+        = link_to('Make this a local user', { controller: "user", action: "make_user_local", user: user.login }, |
+                  method: :post, data: { confirm: "Are you sure?" }) |
     %li
       = sprite_tag('email')
       = mail_to(user.email, 'Contact user')

src/api/app/views/webui/user/edit.html.haml

@@ -12,14 +12,18 @@
   %p
     = f.label(:realname, "Name:")
     %br
-    = f.text_field(:realname, disabled: @configuration.ldap_enabled?)
+    = f.text_field(:realname, disabled: @configuration.ldap_enabled? && !@displayed_user.ignore_auth_services?)
   %p
     = f.label(:email, "e-Mail:")
     %br
-    = f.text_field(:email, required: true, email: true, disabled: @configuration.ldap_enabled?)
+    = f.text_field(:email, required: true, email: true, disabled: @configuration.ldap_enabled? && !@displayed_user.ignore_auth_services?)
   %p
     = f.collection_check_boxes(:role_ids, Role.global, :id, :title)
   %p
     = f.collection_radio_buttons(:state, user_states_for_edit, :to_s, :to_s)
+  - if Configuration.ldap_enabled? && User.current.is_admin?
+    %p
+      = f.check_box(:ignore_auth_services, {}, checked_value = true)
+      = f.label(:ignore_auth_services, 'Local user')
   %p
     = f.submit("Update")

src/api/app/views/webui/user/index.html.haml

@@ -15,6 +15,9 @@
         %tr
           %td
             User
+          - if Configuration.ldap_enabled?
+            %td
+              Local user
           %td
             State
           %td
@@ -24,6 +27,9 @@
           %tr(id="user-#{valid_xml_id(user.login)}")
             %td
               = user_with_realname_and_icon(user, no_icon: true)
+            - if Configuration.ldap_enabled?
+              %td
+                = user.ignore_auth_services?
             %td
               = user.state
             %td.nowrap

src/api/app/views/webui/user/show.html.haml

@@ -32,14 +32,14 @@
   / edit
   - if @is_displayed_user
     %p
-      - if @configuration.accounts_editable?
+      - if @configuration.accounts_editable?(@displayed_user)
         - if @account_edit_link.present?
           = link_to sprited_text('user_edit', 'Edit your account'), @account_edit_link
           %br/
         - else
           = link_to(sprited_text('user_edit', 'Edit your account'), { controller: 'user', action: 'save_dialog', user: User.current }, {id: 'save_dialog', remote: true})
         %br/
-      - if @configuration.passwords_changable?
+      - if @configuration.passwords_changable?(User.current)
         = link_to(sprited_text('key', 'Change your password'), { controller: 'user', action: 'password_dialog', user: User.current }, {id: 'password_dialog', remote: true})
         %br/
       = link_to(sprited_text('email', 'Change your notifications'), user_notifications_path)

src/api/config/routes.rb

@@ -337,6 +337,7 @@ def self.public_or_about_path?(request)
       post 'user/confirm' => :confirm
       post 'user/lock' => :lock
       post 'user/admin' => :admin
+      post 'user/make_user_local' => :make_user_local
       delete 'user/delete' => :delete
 
       get 'user/autocomplete' => :autocomplete

src/api/spec/controllers/person_controller_spec.rb

@@ -23,34 +23,92 @@
     end
   end
 
+  describe 'GET #get_userinfo' do
+    context 'called by a user' do
+      before do
+        login user
+        get :get_userinfo, params: { login: user.login }
+      end
+
+      it 'shows all user related data' do
+        assert_select 'person' do
+          assert_select 'login', text: user.login
+          assert_select 'email', text: user.email
+          assert_select 'realname', text: user.realname
+          assert_select 'state', text: 'confirmed'
+        end
+      end
+
+      it 'shows not the ignore_auth_services flag' do
+        assert_select 'person' do
+          assert_select 'ignore_auth_services', text: user.ignore_auth_services, count: 0
+        end
+      end
+    end
+
+    context 'called by an admin' do
+      before do
+        login user
+        get :get_userinfo, params: { login: user.login }
+      end
+
+      it 'shows all user related data' do
+        assert_select 'person' do
+          assert_select 'login', text: user.login
+          assert_select 'email', text: user.email
+          assert_select 'realname', text: user.realname
+          assert_select 'state', text: 'confirmed'
+        end
+      end
+
+      it 'shows not the ignore_auth_services flag' do
+        assert_select 'person' do
+          assert_select 'ignore_auth_services', text: user.ignore_auth_services, count: 0
+        end
+      end
+    end
+  end
+
   describe 'POST #post_userinfo' do
+    let!(:old_password_digest) { user.password_digest }
+
     before do
       login user
     end
 
     context 'when using default authentication' do
-      let!(:old_password) { user.password_digest }
-
       before do
         request.env['RAW_POST_DATA'] = 'password_has_changed'
         post :post_userinfo, params: { login: user.login, cmd: 'change_password', format: :xml }
-
-        user.reload
       end
 
       it 'changes the password' do
-        expect(old_password).to_not eq(user.password_digest)
+        expect(old_password_digest).to_not eq(user.reload.password_digest)
       end
     end
 
     context 'when in LDAP mode' do
       before do
+        request.env['RAW_POST_DATA'] = 'password_has_changed'
         stub_const('CONFIG', CONFIG.merge('ldap_mode' => :on))
-        post :post_userinfo, params: { login: user.login, cmd: 'change_password', format: :xml }
       end
 
-      it 'user is not allowed to change their password' do
-        expect(response.header['X-Opensuse-Errorcode']).to eq('change_password_no_permission')
+      context 'and the user is configured to authorize on the LDAP server' do
+        before do
+          post :post_userinfo, params: { login: user.login, cmd: 'change_password', format: :xml }
+        end
+
+        it { expect(response.header['X-Opensuse-Errorcode']).to eq('change_password_no_permission') }
+        it { expect(old_password_digest).to eq(user.reload.password_digest) }
+      end
+
+      context 'and the user is configured to authorize locally' do
+        before do
+          user.update(ignore_auth_services: true)
+          post :post_userinfo, params: { login: user.login, cmd: 'change_password', format: :xml }
+        end
+
+        it { expect(old_password_digest).to_not eq(user.reload.password_digest) }
       end
     end
   end