Merge branch '18302-use-rails-cookie-in-api' into 'master'

Allow the Rails cookie to be used for API authentication

Makes the Rails cookie into a valid authentication token for the Grape
API, and uses it instead of token authentication in frontend code that
uses the API.

Rendering the private token into client-side javascript is a security
risk; it may be stolen through XSS or other attacks. In general,
re-using API code in the frontend is more desirable than implementing
endless actions that return JSON.

Closes #18302

See merge request !1995

Signed-off-by: Rémy Coutable <remy@rymai.me>

7 files changed, 73 insertions, 58 deletions

diff --git a/CHANGELOG b/CHANGELOG
index f9034abfca4..0299c805f90 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,5 +1,8 @@
 Please view this file on the master branch, on stable branches it's out of date.
 
+v 8.10.10
+  - Allow the Rails cookie to be used for API authentication.
+
 v 8.10.9
   - Exclude some pending or inactivated rows in Member scopes.
 
diff --git a/app/assets/javascripts/api.js.coffee b/app/assets/javascripts/api.js.coffee
index 89b0ac697ed..19c59df8a9d 100644
--- a/app/assets/javascripts/api.js.coffee
+++ b/app/assets/javascripts/api.js.coffee
@@ -15,8 +15,6 @@
 
     $.ajax(
       url: url
-      data:
-        private_token: gon.api_token
       dataType: "json"
     ).done (group) ->
       callback(group)
@@ -29,7 +27,6 @@
     $.ajax(
       url: url
       data:
-        private_token: gon.api_token
         search: query
         per_page: 20
       dataType: "json"
@@ -43,7 +40,6 @@
     $.ajax(
       url: url
       data:
-        private_token: gon.api_token
         search: query
         per_page: 20
       dataType: "json"
@@ -57,7 +53,6 @@
     $.ajax(
       url: url
       data:
-        private_token: gon.api_token
         search: query
         order_by: order
         per_page: 20
@@ -69,7 +64,6 @@
     url = Api.buildUrl(Api.labelsPath)
     url = url.replace(':id', project_id)
 
-    data.private_token = gon.api_token
     $.ajax(
       url: url
       type: "POST"
@@ -88,7 +82,6 @@
     $.ajax(
       url: url
       data:
-        private_token: gon.api_token
         search: query
         per_page: 20
       dataType: "json"
diff --git a/doc/api/README.md b/doc/api/README.md
index d1e6c54c521..81d0e3e6451 100644
--- a/doc/api/README.md
+++ b/doc/api/README.md
@@ -47,11 +47,12 @@ The following documentation is for the [internal CI API](ci/README.md):
 
 ## Authentication
 
-All API requests require authentication via a token. There are three types of tokens
-available: private tokens, OAuth 2 tokens, and personal access tokens.
+All API requests require authentication via a session cookie or token. There are
+three types of tokens available: private tokens, OAuth 2 tokens, and personal
+access tokens.
 
-If a token is invalid or omitted, an error message will be returned with
-status code `401`:
+If authentication information is invalid or omitted, an error message will be
+returned with status code `401`:
 
 ```json
 {
@@ -90,6 +91,13 @@ that needs access to the GitLab API.
 Once you have your token, pass it to the API using either the `private_token`
 parameter or the `PRIVATE-TOKEN` header.
 
+
+### Session cookie
+
+When signing in to GitLab as an ordinary user, a `_gitlab_session` cookie is
+set. The API will use this cookie for authentication if it is present, but using
+the API to generate a new session cookie is currently not supported.
+
 ## Basic Usage
 
 API requests should be prefixed with `api` and the API version. The API version
diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb
index 7e67edb203a..8cc7a26f1fa 100644
--- a/lib/api/api_guard.rb
+++ b/lib/api/api_guard.rb
@@ -33,46 +33,29 @@ module API
       #
       # If the token is revoked, then it raises RevokedError.
       #
-      # If the token is not found (nil), then it raises TokenNotFoundError.
+      # If the token is not found (nil), then it returns nil
       #
       # Arguments:
       #
       #   scopes: (optional) scopes required for this guard.
       #           Defaults to empty array.
       #
-      def doorkeeper_guard!(scopes: [])
-        if (access_token = find_access_token).nil?
-          raise TokenNotFoundError
-
-        else
-          case validate_access_token(access_token, scopes)
-          when Oauth2::AccessTokenValidationService::INSUFFICIENT_SCOPE
-            raise InsufficientScopeError.new(scopes)
-          when Oauth2::AccessTokenValidationService::EXPIRED
-            raise ExpiredError
-          when Oauth2::AccessTokenValidationService::REVOKED
-            raise RevokedError
-          when Oauth2::AccessTokenValidationService::VALID
-            @current_user = User.find(access_token.resource_owner_id)
-          end
-        end
-      end
-
       def doorkeeper_guard(scopes: [])
-        if access_token = find_access_token
-          case validate_access_token(access_token, scopes)
-          when Oauth2::AccessTokenValidationService::INSUFFICIENT_SCOPE
-            raise InsufficientScopeError.new(scopes)
+        access_token = find_access_token
+        return nil unless access_token
+
+        case validate_access_token(access_token, scopes)
+        when Oauth2::AccessTokenValidationService::INSUFFICIENT_SCOPE
+          raise InsufficientScopeError.new(scopes)
 
-          when Oauth2::AccessTokenValidationService::EXPIRED
-            raise ExpiredError
+        when Oauth2::AccessTokenValidationService::EXPIRED
+          raise ExpiredError
 
-          when Oauth2::AccessTokenValidationService::REVOKED
-            raise RevokedError
+        when Oauth2::AccessTokenValidationService::REVOKED
+          raise RevokedError
 
-          when Oauth2::AccessTokenValidationService::VALID
-            @current_user = User.find(access_token.resource_owner_id)
-          end
+        when Oauth2::AccessTokenValidationService::VALID
+          @current_user = User.find(access_token.resource_owner_id)
         end
       end
 
@@ -96,19 +79,6 @@ module API
     end
 
     module ClassMethods
-      # Installs the doorkeeper guard on the whole Grape API endpoint.
-      #
-      # Arguments:
-      #
-      #   scopes: (optional) scopes required for this guard.
-      #           Defaults to empty array.
-      #
-      def guard_all!(scopes: [])
-        before do
-          guard! scopes: scopes
-        end
-      end
-
       private
 
       def install_error_responders(base)
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index d6e4eb2afd7..28225281fee 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -16,13 +16,30 @@ module API
       nil
     end
 
+    def private_token
+      params[PRIVATE_TOKEN_PARAM] || env[PRIVATE_TOKEN_HEADER]
+    end
+
+    def warden
+      env['warden']
+    end
+
+    # Check the Rails session for valid authentication details
+    def find_user_from_warden
+      warden ? warden.authenticate : nil
+    end
+
     def find_user_by_private_token
-      token_string = (params[PRIVATE_TOKEN_PARAM] || env[PRIVATE_TOKEN_HEADER]).to_s
-      User.find_by_authentication_token(token_string) || User.find_by_personal_access_token(token_string)
+      token = private_token
+      return nil unless token.present?
+
+      User.find_by_authentication_token(token) || User.find_by_personal_access_token(token)
     end
 
     def current_user
-      @current_user ||= (find_user_by_private_token || doorkeeper_guard)
+      @current_user ||= find_user_by_private_token
+      @current_user ||= doorkeeper_guard
+      @current_user ||= find_user_from_warden
 
       unless @current_user && Gitlab::UserAccess.new(@current_user).allowed?
         return nil
diff --git a/lib/gitlab/gon_helper.rb b/lib/gitlab/gon_helper.rb
index c5a11148d33..2c21804fe7a 100644
--- a/lib/gitlab/gon_helper.rb
+++ b/lib/gitlab/gon_helper.rb
@@ -11,7 +11,6 @@ module Gitlab
 
       if current_user
         gon.current_user_id = current_user.id
-        gon.api_token = current_user.private_token
       end
     end
   end
diff --git a/spec/requests/api/api_helpers_spec.rb b/spec/requests/api/api_helpers_spec.rb
index 831889afb6c..90a26d56a90 100644
--- a/spec/requests/api/api_helpers_spec.rb
+++ b/spec/requests/api/api_helpers_spec.rb
@@ -35,11 +35,36 @@ describe API::Helpers, api: true do
     params.delete(API::Helpers::SUDO_PARAM)
   end
 
+  def warden_authenticate_returns(value)
+    warden = double("warden", authenticate: value)
+    env['warden'] = warden
+  end
+
+  def doorkeeper_guard_returns(value)
+    allow_any_instance_of(self.class).to receive(:doorkeeper_guard){ value }
+  end
+
   def error!(message, status)
     raise Exception
   end
 
   describe ".current_user" do
+    subject { current_user }
+
+    describe "when authenticating via Warden" do
+      before { doorkeeper_guard_returns false }
+
+      context "fails" do
+        it { is_expected.to be_nil }
+      end
+
+      context "succeeds" do
+        before { warden_authenticate_returns user }
+
+        it { is_expected.to eq(user) }
+      end
+    end
+
     describe "when authenticating using a user's private token" do
       it "should return nil for an invalid token" do
         env[API::Helpers::PRIVATE_TOKEN_HEADER] = 'invalid token'