Merge branch 'jhyson/doorkeeper_upgrade_5' into 'master'

Upgrade Doorkeeper gem to 5.0.2

See merge request gitlab-org/gitlab!21173

Gemfile

@@ -26,8 +26,8 @@ gem 'marginalia', '~> 1.8.0'
 
 # Authentication libraries
 gem 'devise', '~> 4.6'
-gem 'doorkeeper', '~> 4.4.3'
-gem 'doorkeeper-openid_connect', '~> 1.5'
+gem 'doorkeeper', '~> 5.0.2'
+gem 'doorkeeper-openid_connect', '~> 1.6.3'
 gem 'omniauth', '~> 1.8'
 gem 'omniauth-auth0', '~> 2.0.0'
 gem 'omniauth-azure-oauth2', '~> 0.0.9'

Gemfile.lock

@@ -243,10 +243,10 @@ GEM
     docile (1.3.1)
     domain_name (0.5.20180417)
       unf (>= 0.0.5, < 1.0.0)
-    doorkeeper (4.4.3)
+    doorkeeper (5.0.2)
       railties (>= 4.2)
-    doorkeeper-openid_connect (1.5.0)
-      doorkeeper (~> 4.3)
+    doorkeeper-openid_connect (1.6.3)
+      doorkeeper (>= 5.0, < 5.2)
       json-jwt (~> 1.6)
     ed25519 (1.2.4)
     elasticsearch (6.8.0)
@@ -1197,8 +1197,8 @@ DEPENDENCIES
   diff_match_patch (~> 0.1.0)
   diffy (~> 3.1.0)
   discordrb-webhooks-blackst0ne (~> 3.3)
-  doorkeeper (~> 4.4.3)
-  doorkeeper-openid_connect (~> 1.5)
+  doorkeeper (~> 5.0.2)
+  doorkeeper-openid_connect (~> 1.6.3)
   ed25519 (~> 1.2)
   elasticsearch-api (~> 6.8)
   elasticsearch-model (~> 6.1)

app/controllers/oauth/applications_controller.rb

@@ -8,6 +8,10 @@ class Oauth::ApplicationsController < Doorkeeper::ApplicationsController
   include Gitlab::Experimentation::ControllerConcern
   include InitializesCurrentUserMode
 
+  # Defined by the `Doorkeeper::ApplicationsController` and is redundant as we call `authenticate_user!` below. Not
+  # defining or skipping this will result in a `403` response to all requests.
+  skip_before_action :authenticate_admin!
+
   prepend_before_action :verify_user_oauth_applications_enabled, except: :index
   prepend_before_action :authenticate_user!
   before_action :add_gon_variables

app/controllers/oauth/token_info_controller.rb

+# frozen_string_literal: true
+
+class Oauth::TokenInfoController < Doorkeeper::TokenInfoController
+  def show
+    if doorkeeper_token && doorkeeper_token.accessible?
+      token_json = doorkeeper_token.as_json
+
+      # maintain backwards compatibility
+      render json: token_json.merge(
+        'scopes'             => token_json[:scope],
+        'expires_in_seconds' => token_json[:expires_in]
+      ), status: :ok
+    else
+      error = Doorkeeper::OAuth::ErrorResponse.new(name: :invalid_request)
+      response.headers.merge!(error.headers)
+      render json: error.body, status: error.status
+    end
+  end
+end

changelogs/unreleased/jhyson-doorkeeper_upgrade_5.yml

+---
+title: Upgrade Doorkeeper to 5.0.2
+merge_request: 21173
+author:
+type: security

config/initializers/doorkeeper.rb

@@ -113,53 +113,3 @@ Doorkeeper.configure do
 
   base_controller '::Gitlab::BaseDoorkeeperController'
 end
-
-# Monkey patch to avoid creating new applications if the scope of the
-# app created does not match the complete list of scopes of the configured app.
-# It also prevents the OAuth authorize application window to appear every time.
-
-# Remove after we upgrade the doorkeeper gem from version 4.x
-if Doorkeeper.gem_version > Gem::Version.new('5.0.0')
-  raise "Doorkeeper was upgraded, please remove the monkey patch in #{__FILE__}"
-end
-
-module Doorkeeper
-  module AccessTokenMixin
-    module ClassMethods
-      def matching_token_for(application, resource_owner_or_id, scopes)
-        resource_owner_id =
-          if resource_owner_or_id.respond_to?(:to_key)
-            resource_owner_or_id.id
-          else
-            resource_owner_or_id
-          end
-
-        tokens = authorized_tokens_for(application.try(:id), resource_owner_id)
-        tokens.detect do |token|
-          scopes_match?(token.scopes, scopes, application.try(:scopes))
-        end
-      end
-
-      def scopes_match?(token_scopes, param_scopes, app_scopes)
-        return true if token_scopes.empty? && param_scopes.empty?
-
-        (token_scopes.sort == param_scopes.sort) &&
-          Doorkeeper::OAuth::Helpers::ScopeChecker.valid?(
-            param_scopes.to_s,
-            Doorkeeper.configuration.scopes,
-            app_scopes)
-      end
-
-      def authorized_tokens_for(application_id, resource_owner_id)
-        ordered_by(:created_at, :desc)
-          .where(application_id: application_id,
-                 resource_owner_id: resource_owner_id,
-                 revoked_at: nil)
-      end
-
-      def last_authorized_token_for(application_id, resource_owner_id)
-        authorized_tokens_for(application_id, resource_owner_id).first
-      end
-    end
-  end
-end

config/routes.rb

@@ -24,7 +24,8 @@ Rails.application.routes.draw do
   use_doorkeeper do
     controllers applications: 'oauth/applications',
                 authorized_applications: 'oauth/authorized_applications',
-                authorizations: 'oauth/authorizations'
+                authorizations: 'oauth/authorizations',
+                token_info: 'oauth/token_info'
   end
 
   # This prefixless path is required because Jira gets confused if we set it up with a path

doc/api/oauth2.md

@@ -102,7 +102,7 @@ CAUTION: **Important:**
 Avoid using this flow for applications that store data outside of the GitLab
 instance. If you do, make sure to verify `application id` associated with the
 access token before granting access to the data
-(see [`/oauth/token/info`](https://github.com/doorkeeper-gem/doorkeeper/wiki/API-endpoint-descriptions-and-examples#get----oauthtokeninfo)).
+(see [`/oauth/token/info`](#retrieving-the-token-info)).
 
 Unlike the web flow, the client receives an `access token` immediately as a
 result of the authorization request. The flow does not use the client secret
@@ -212,3 +212,34 @@ or you can put the token to the Authorization header:
 ```
 curl --header "Authorization: Bearer OAUTH-TOKEN" https://gitlab.example.com/api/v4/user
 ```
+
+## Retrieving the Token Info
+
+To verify the details of a token you can call the `token/info` endpoint. This is provided from the doorkeeper gem (see [`/oauth/token/info`](https://github.com/doorkeeper-gem/doorkeeper/wiki/API-endpoint-descriptions-and-examples#get----oauthtokeninfo)).
+
+You will need to supply the access token, either as a parameter
+
+```
+GET https://gitlab.example.com/oauth/token/info?access_token=OAUTH-TOKEN
+```
+
+Or in the Authorization header:
+
+```
+curl --header "Authorization: Bearer OAUTH-TOKEN" https://gitlab.example.com/oauth/token/info
+```
+
+You will receive the following in response:
+
+```json
+{
+    "resource_owner_id": 1,
+    "scope": ["api"],
+    "expires_in": null,
+    "application": {"uid": "1cb242f495280beb4291e64bee2a17f330902e499882fe8e1e2aa875519cab33"},
+    "created_at": 1575890427
+}
+```
+
+CAUTION: **Deprecated fields:**
+The fields `scopes` and `expires_in_seconds` are also included in the response. They are aliases for `scope` and `expires_in` respectively and have been included to prevent breaking changes introduced in [doorkeeper 5.0.2](https://github.com/doorkeeper-gem/doorkeeper/wiki/Migration-from-old-versions#from-4x-to-5x). Please don't rely on these fields as they will be removed in a later release.

spec/controllers/oauth/token_info_controller_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Oauth::TokenInfoController do
+  describe '#show' do
+    context 'when the user is not authenticated' do
+      it 'responds with a 401' do
+        get :show
+
+        expect(response.status).to eq 401
+        expect(JSON.parse(response.body)).to include('error' => 'invalid_request')
+      end
+    end
+
+    context 'when the request is valid' do
+      let(:application) { create(:oauth_application, scopes: 'api') }
+      let(:access_token) do
+        create(:oauth_access_token, expires_in: 5.minutes, application: application)
+      end
+
+      it 'responds with the token info' do
+        get :show, params: { access_token: access_token.token }
+
+        expect(response.status).to eq 200
+        expect(JSON.parse(response.body)).to eq(
+          'scope'              => %w[api],
+          'scopes'             => %w[api],
+          'created_at'         => access_token.created_at.to_i,
+          'expires_in'         => access_token.expires_in,
+          'application'        => { 'uid' => application.uid },
+          'resource_owner_id'  => access_token.resource_owner_id,
+          'expires_in_seconds' => access_token.expires_in
+        )
+      end
+    end
+
+    context 'when the doorkeeper_token is not recognised' do
+      it 'responds with a 401' do
+        get :show, params: { access_token: 'unknown_token' }
+
+        expect(response.status).to eq 401
+        expect(JSON.parse(response.body)).to include('error' => 'invalid_request')
+      end
+    end
+
+    context 'when the token is expired' do
+      let(:access_token) do
+        create(:oauth_access_token, created_at: 2.days.ago, expires_in: 10.minutes)
+      end
+
+      it 'responds with a 401' do
+        get :show, params: { access_token: access_token.token }
+
+        expect(response.status).to eq 401
+        expect(JSON.parse(response.body)).to include('error' => 'invalid_request')
+      end
+    end
+
+    context 'when the token is revoked' do
+      let(:access_token) { create(:oauth_access_token, revoked_at: 2.days.ago) }
+
+      it 'responds with a 401' do
+        get :show, params: { access_token: access_token.token }
+
+        expect(response.status).to eq 401
+        expect(JSON.parse(response.body)).to include('error' => 'invalid_request')
+      end
+    end
+  end
+end