Merge branch 'master' into auto-pipelines-vue

10 files changed, 70 insertions, 23 deletions

diff --git a/lib/api/api.rb b/lib/api/api.rb
index 9d5adffd8f4..6cf6b501021 100644
--- a/lib/api/api.rb
+++ b/lib/api/api.rb
@@ -14,7 +14,11 @@ module API
     end
 
     # Retain 405 error rather than a 500 error for Grape 0.15.0+.
-    # See: https://github.com/ruby-grape/grape/commit/252bfd27c320466ec3c0751812cf44245e97e5de
+    # https://github.com/ruby-grape/grape/blob/a3a28f5b5dfbb2797442e006dbffd750b27f2a76/UPGRADING.md#changes-to-method-not-allowed-routes
+    rescue_from Grape::Exceptions::MethodNotAllowed do |e|
+      error! e.message, e.status, e.headers
+    end
+
     rescue_from Grape::Exceptions::Base do |e|
       error! e.message, e.status, e.headers
     end
diff --git a/lib/api/internal.rb b/lib/api/internal.rb
index db2d18f935d..d235977fbd8 100644
--- a/lib/api/internal.rb
+++ b/lib/api/internal.rb
@@ -28,6 +28,8 @@ module API
 
         protocol = params[:protocol]
 
+        actor.update_last_used_at if actor.is_a?(Key)
+
         access =
           if wiki?
             Gitlab::GitAccessWiki.new(actor, project, protocol, authentication_abilities: ssh_authentication_abilities)
@@ -61,6 +63,8 @@ module API
         status 200
 
         key = Key.find(params[:key_id])
+        key.update_last_used_at
+
         token_handler = Gitlab::LfsToken.new(key)
 
         {
@@ -103,7 +107,9 @@ module API
 
         key = Key.find_by(id: params[:key_id])
 
-        unless key
+        if key
+          key.update_last_used_at
+        else
           return { 'success' => false, 'message' => 'Could not find the given key' }
         end
 
diff --git a/lib/api/users.rb b/lib/api/users.rb
index de07fbf59fc..0db76ec7877 100644
--- a/lib/api/users.rb
+++ b/lib/api/users.rb
@@ -91,10 +91,11 @@ module API
         authenticated_as_admin!
 
         # Filter out params which are used later
-        identity_attrs = params.slice(:provider, :extern_uid)
+        user_params = declared_params(include_missing: false)
+        identity_attrs = user_params.slice(:provider, :extern_uid)
         confirm = params.delete(:confirm)
 
-        user = User.new(declared_params(include_missing: false))
+        user = User.new(user_params.except(:extern_uid, :provider))
         user.skip_confirmation! unless confirm
 
         if identity_attrs.any?
@@ -159,11 +160,7 @@ module API
           end
         end
 
-        # Delete already handled parameters
-        user_params.delete(:extern_uid)
-        user_params.delete(:provider)
-
-        if user.update_attributes(user_params)
+        if user.update_attributes(user_params.except(:extern_uid, :provider))
           present user, with: Entities::UserPublic
         else
           render_validation_error!(user)
diff --git a/lib/ci/ansi2html.rb b/lib/ci/ansi2html.rb
index 229050151d3..c10d3616f31 100644
--- a/lib/ci/ansi2html.rb
+++ b/lib/ci/ansi2html.rb
@@ -105,7 +105,7 @@ module Ci
             break
           elsif s.scan(/</)
             @out << '&lt;'
-          elsif s.scan(/\n/)
+          elsif s.scan(/\r?\n/)
             @out << '<br>'
           else
             @out << s.scan(/./m)
diff --git a/lib/ci/api/api.rb b/lib/ci/api/api.rb
index a6b9beecded..24bb3649a76 100644
--- a/lib/ci/api/api.rb
+++ b/lib/ci/api/api.rb
@@ -8,6 +8,16 @@ module Ci
         rack_response({ 'message' => '404 Not found' }.to_json, 404)
       end
 
+      # Retain 405 error rather than a 500 error for Grape 0.15.0+.
+      # https://github.com/ruby-grape/grape/blob/a3a28f5b5dfbb2797442e006dbffd750b27f2a76/UPGRADING.md#changes-to-method-not-allowed-routes
+      rescue_from Grape::Exceptions::MethodNotAllowed do |e|
+        error! e.message, e.status, e.headers
+      end
+
+      rescue_from Grape::Exceptions::Base do |e|
+        error! e.message, e.status, e.headers
+      end
+
       rescue_from :all do |exception|
         handle_api_exception(exception)
       end
diff --git a/lib/gitlab/ldap/access.rb b/lib/gitlab/ldap/access.rb
index 7e06bd2b0fb..7ed01bf56ca 100644
--- a/lib/gitlab/ldap/access.rb
+++ b/lib/gitlab/ldap/access.rb
@@ -34,21 +34,21 @@ module Gitlab
       def allowed?
         if ldap_user
           unless ldap_config.active_directory
-            user.activate if user.ldap_blocked?
+            unblock_user(user, 'is available again') if user.ldap_blocked?
             return true
           end
 
           # Block user in GitLab if he/she was blocked in AD
           if Gitlab::LDAP::Person.disabled_via_active_directory?(user.ldap_identity.extern_uid, adapter)
-            user.ldap_block
+            block_user(user, 'is disabled in Active Directory')
             false
           else
-            user.activate if user.ldap_blocked?
+            unblock_user(user, 'is not disabled anymore') if user.ldap_blocked?
             true
           end
         else
           # Block the user if they no longer exist in LDAP/AD
-          user.ldap_block
+          block_user(user, 'does not exist anymore')
           false
         end
       end
@@ -64,6 +64,24 @@ module Gitlab
       def ldap_user
         @ldap_user ||= Gitlab::LDAP::Person.find_by_dn(user.ldap_identity.extern_uid, adapter)
       end
+
+      def block_user(user, reason)
+        user.ldap_block
+
+        Gitlab::AppLogger.info(
+          "LDAP account \"#{user.ldap_identity.extern_uid}\" #{reason}, " +
+          "blocking Gitlab user \"#{user.name}\" (#{user.email})"
+        )
+      end
+
+      def unblock_user(user, reason)
+        user.activate
+
+        Gitlab::AppLogger.info(
+          "LDAP account \"#{user.ldap_identity.extern_uid}\" #{reason}, " +
+          "unblocking Gitlab user \"#{user.name}\" (#{user.email})"
+        )
+      end
     end
   end
 end
diff --git a/lib/gitlab/ldap/auth_hash.rb b/lib/gitlab/ldap/auth_hash.rb
index bf4dd9542d5..95378e5a769 100644
--- a/lib/gitlab/ldap/auth_hash.rb
+++ b/lib/gitlab/ldap/auth_hash.rb
@@ -25,7 +25,7 @@ module Gitlab
       end
 
       def get_raw(key)
-        auth_hash.extra[:raw_info][key]
+        auth_hash.extra[:raw_info][key] if auth_hash.extra
       end
 
       def ldap_config
diff --git a/lib/gitlab/ldap/config.rb b/lib/gitlab/ldap/config.rb
index de52ef3fc65..28129198438 100644
--- a/lib/gitlab/ldap/config.rb
+++ b/lib/gitlab/ldap/config.rb
@@ -107,7 +107,7 @@ module Gitlab
       end
 
       def attributes
-        options['attributes']
+        default_attributes.merge(options['attributes'])
       end
 
       def timeout
@@ -130,6 +130,16 @@ module Gitlab
         end
       end
 
+      def default_attributes
+        {
+          'username'    => %w(uid userid sAMAccountName),
+          'email'       => %w(mail email userPrincipalName),
+          'name'        => 'cn',
+          'first_name'  => 'givenName',
+          'last_name'   => 'sn'
+        }
+      end
+
       protected
 
       def base_options
diff --git a/lib/gitlab/ldap/person.rb b/lib/gitlab/ldap/person.rb
index 333f170a484..7084fd1767d 100644
--- a/lib/gitlab/ldap/person.rb
+++ b/lib/gitlab/ldap/person.rb
@@ -28,7 +28,7 @@ module Gitlab
       end
 
       def name
-        attribute_value(:name)
+        attribute_value(:name).first
       end
 
       def uid
@@ -62,14 +62,12 @@ module Gitlab
       # this method looks for 'mail', 'email' and 'userPrincipalName' and
       # returns the first with a value.
       def attribute_value(attribute)
-        attributes = Array(config.attributes[attribute.to_sym])
+        attributes = Array(config.attributes[attribute.to_s])
         selected_attr = attributes.find { |attr| entry.respond_to?(attr) }
 
         return nil unless selected_attr
 
-        # Some LDAP attributes return an array,
-        # even if it is a single value (like 'cn')
-        Array(entry.public_send(selected_attr)).first
+        entry.public_send(selected_attr)
       end
     end
   end
diff --git a/lib/gitlab/metrics/rack_middleware.rb b/lib/gitlab/metrics/rack_middleware.rb
index 91fb0bb317a..d01d47a6a7a 100644
--- a/lib/gitlab/metrics/rack_middleware.rb
+++ b/lib/gitlab/metrics/rack_middleware.rb
@@ -70,8 +70,12 @@ module Gitlab
 
       def tag_endpoint(trans, env)
         endpoint = env[ENDPOINT_KEY]
-        path = endpoint_paths_cache[endpoint.route.request_method][endpoint.route.path]
-        trans.action = "Grape##{endpoint.route.request_method} #{path}"
+
+        # endpoint.route is nil in the case of a 405 response
+        if endpoint.route
+          path = endpoint_paths_cache[endpoint.route.request_method][endpoint.route.path]
+          trans.action = "Grape##{endpoint.route.request_method} #{path}"
+        end
       end
 
       private