[#41480] respond with 401 on invalid bearer tokens

- https://community.openproject.org/work_packages/41480
- amend strategy for doorkeeper to react on all bearer tokens
- added ouath request spec for valid, invalid and revoked token

docker-compose.yml

@@ -57,6 +57,8 @@ x-op-backend: &backend
 services:
   backend:
     <<: *backend
+    tty: true
+    stdin_open: true
     command: run-app
     ports:
       - "3000:3000"
@@ -94,6 +96,8 @@ services:
     stop_grace_period: "3s"
     volumes:
       - "pgdata:/var/lib/postgresql/data"
+    ports:
+      - "54322:5432"
     environment:
       POSTGRES_USER: ${DB_USERNAME:-postgres}
       POSTGRES_PASSWORD: ${DB_PASSWORD:-postgres}

docker/dev/backend/scripts/run-test

@@ -3,6 +3,9 @@
 set -e
 set -u
 
+mkdir -p public/assets
+mkdir -p tmp/cache
+
 chown -R "$USER:$USER" public/assets
 chown -R "$USER:$USER" tmp/cache
 

lib/open_project/authentication/strategies/warden/doorkeeper_oauth.rb

@@ -9,15 +9,20 @@ module OpenProject
         #
         class DoorkeeperOAuth < ::Warden::Strategies::Base
           def valid?
-            @token = ::Doorkeeper::OAuth::Token.authenticate(decorated_request, *Doorkeeper.configuration.access_token_methods)
-            @token&.accessible? && @token&.acceptable?(scope)
+            bearer_token = ::Doorkeeper::OAuth::Token.from_request(decorated_request,
+                                                                   *Doorkeeper.configuration.access_token_methods)
+            bearer_token != nil
           end
 
           def authenticate!
-            if @token.resource_owner_id.nil?
-              authenticate_client_credentials(@token)
+            token = ::Doorkeeper::OAuth::Token.authenticate(decorated_request,
+                                                            *Doorkeeper.configuration.access_token_methods)
+            return fail!('invalid token') unless token&.acceptable?(scope)
+
+            if token.resource_owner_id.nil?
+              authenticate_client_credentials(token)
             else
-              authenticate_user(@token.resource_owner_id)
+              authenticate_user(token.resource_owner_id)
             end
           end
 

spec/factories/oauth_access_token_factory.rb

@@ -0,0 +1,48 @@
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2022 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+FactoryBot.define do
+  factory :oauth_access_token, class: '::Doorkeeper::AccessToken' do
+    transient do
+      resource_owner { create :user }
+    end
+
+    after(:build) do |token, evaluator|
+      token.resource_owner_id = evaluator.resource_owner.id
+    end
+
+    application factory: :oauth_application
+
+    expires_in { 2.hours }
+    scopes { 'api_v3' }
+
+    factory :clientless_access_token do
+      application { nil }
+    end
+  end
+end

spec/requests/api/v3/authentication_spec.rb

@@ -29,10 +29,48 @@
 require 'spec_helper'
 
 describe API::V3, type: :request do
-  describe 'basic auth' do
-    let(:user)     { create :user }
-    let(:resource) { "/api/v3/projects" }
+  let(:resource) { "/api/v3/projects" }
+  let(:user) { create :user }
 
+  describe 'oauth' do
+    let(:oauth_access_token) { '' }
+
+    before do
+      login_as user
+
+      header 'Authorization', "Bearer #{oauth_access_token}"
+
+      get resource
+    end
+
+    context 'with a valid access token' do
+      let(:token) { create :oauth_access_token, resource_owner: user }
+      let(:oauth_access_token) { token.plaintext_token }
+
+      it 'authenticates successfully' do
+        expect(last_response.status).to eq 200
+      end
+    end
+
+    context 'with an invalid access token' do
+      let(:oauth_access_token) { '1337' }
+
+      it 'returns unauthorized' do
+        expect(last_response.status).to eq 401
+      end
+    end
+
+    context 'with an expired access token' do
+      let(:token) { create :oauth_access_token, resource_owner: user, revoked_at: DateTime.now }
+      let(:oauth_access_token) { token.plaintext_token }
+
+      it 'returns unauthorized' do
+        expect(last_response.status).to eq 401
+      end
+    end
+  end
+
+  describe 'basic auth' do
     let(:response_401) do
       {
         '_type' => 'Error',
@@ -224,7 +262,7 @@ describe API::V3, type: :request do
           let(:password) { 'olooleol' }
 
           let(:api_user) { create :user, login: 'user_account' }
-          let(:api_key)  { create :api_token, user: api_user }
+          let(:api_key) { create :api_token, user: api_user }
 
           before do
             config = { user: 'global_account', password: 'global_password' }