From 21a459e5ad23005c525c32755fde833993004dbd Mon Sep 17 00:00:00 2001 From: Igor Drozdov Date: Wed, 1 Oct 2025 13:29:15 +0200 Subject: [PATCH] Fix MCP API scope validation to allow multiple scopes - Changes from exact mcp scope match to inclusion check - Changes GET endpoint status from :not_implemented to :method_not_allowed per protocol - Extend AI_WORKFLOW_SCOPES to contain mcp These changes are prerequisites for the MCP client implementation in Workhorse. --- .../create_composite_oauth_access_token_service.rb | 2 +- .../create_oauth_access_token_service.rb | 13 +++++++++++-- .../services/ai/duo_workflows/onboarding_service.rb | 2 +- ...ate_composite_oauth_access_token_service_spec.rb | 3 +++ .../create_oauth_access_token_service_spec.rb | 10 ++++++++-- .../ai/duo_workflows/onboarding_service_spec.rb | 2 +- lib/api/mcp/base.rb | 4 ++-- spec/requests/api/mcp/base_spec.rb | 6 +++--- 8 files changed, 30 insertions(+), 12 deletions(-) diff --git a/ee/app/services/ai/duo_workflows/create_composite_oauth_access_token_service.rb b/ee/app/services/ai/duo_workflows/create_composite_oauth_access_token_service.rb index 6d276b5c594bac..2b6c5094e84ea4 100644 --- a/ee/app/services/ai/duo_workflows/create_composite_oauth_access_token_service.rb +++ b/ee/app/services/ai/duo_workflows/create_composite_oauth_access_token_service.rb @@ -14,7 +14,7 @@ def initialize(current_user:, organization:, scopes: nil, service_account: nil) @current_user = current_user @organization = organization @service_account = service_account || ai_settings.duo_workflow_service_account_user - @scopes = (scopes || ::Gitlab::Auth::AI_WORKFLOW_SCOPES) + dynamic_user_scope + @scopes = (scopes || (::Gitlab::Auth::AI_WORKFLOW_SCOPES + [::Gitlab::Auth::MCP_SCOPE])) + dynamic_user_scope end def execute diff --git a/ee/app/services/ai/duo_workflows/create_oauth_access_token_service.rb b/ee/app/services/ai/duo_workflows/create_oauth_access_token_service.rb index f4699abf28741a..ba4d9244c702d1 100644 --- a/ee/app/services/ai/duo_workflows/create_oauth_access_token_service.rb +++ b/ee/app/services/ai/duo_workflows/create_oauth_access_token_service.rb @@ -37,7 +37,16 @@ def create_oauth_access_token end def ensure_oauth_application! - return if oauth_application + return if oauth_application&.scopes&.include?(::Gitlab::Auth::MCP_SCOPE) + + scopes = ::Gitlab::Auth::AI_WORKFLOW_SCOPES + [::Gitlab::Auth::MCP_SCOPE] + + # Add missing mcp scope if the application exists + if oauth_application.present? + oauth_application.update!(scopes: scopes) + + return + end should_expire_cache = false @@ -50,7 +59,7 @@ def ensure_oauth_application! application = Authn::OauthApplication.new( name: 'GitLab Duo Agent Platform', redirect_uri: oauth_callback_url, - scopes: ::Gitlab::Auth::AI_WORKFLOW_SCOPES, + scopes: scopes, trusted: true, confidential: false ) diff --git a/ee/app/services/ai/duo_workflows/onboarding_service.rb b/ee/app/services/ai/duo_workflows/onboarding_service.rb index 15ef8110d5adfa..11b6d426e69ea9 100644 --- a/ee/app/services/ai/duo_workflows/onboarding_service.rb +++ b/ee/app/services/ai/duo_workflows/onboarding_service.rb @@ -85,7 +85,7 @@ def find_or_create_oauth_app! Authn::OauthApplication.create!( name: 'GitLab Duo Agent Platform Composite OAuth Application', redirect_uri: oauth_callback_url, - scopes: ::Gitlab::Auth::AI_WORKFLOW_SCOPES + [::Gitlab::Auth::DYNAMIC_USER], + scopes: ::Gitlab::Auth::AI_WORKFLOW_SCOPES + [::Gitlab::Auth::MCP_SCOPE, ::Gitlab::Auth::DYNAMIC_USER], trusted: true, confidential: true ) diff --git a/ee/spec/services/ai/duo_workflows/create_composite_oauth_access_token_service_spec.rb b/ee/spec/services/ai/duo_workflows/create_composite_oauth_access_token_service_spec.rb index 93044357fdf376..63b15b70871180 100644 --- a/ee/spec/services/ai/duo_workflows/create_composite_oauth_access_token_service_spec.rb +++ b/ee/spec/services/ai/duo_workflows/create_composite_oauth_access_token_service_spec.rb @@ -28,6 +28,9 @@ it 'creates a new oauth access token' do expect { response }.to change { OauthAccessToken.count }.by(1) expect(response).to be_success + + oauth_token = OauthAccessToken.last + expect(oauth_token.scopes).to contain_exactly('mcp', 'ai_workflows', "user:#{user.id}") end context 'when service account does not have composite identity enabled' do diff --git a/ee/spec/services/ai/duo_workflows/create_oauth_access_token_service_spec.rb b/ee/spec/services/ai/duo_workflows/create_oauth_access_token_service_spec.rb index 6ff90e82dcc91b..888e27dd9c89c5 100644 --- a/ee/spec/services/ai/duo_workflows/create_oauth_access_token_service_spec.rb +++ b/ee/spec/services/ai/duo_workflows/create_oauth_access_token_service_spec.rb @@ -21,7 +21,7 @@ :oauth_application, owner: nil, id: application_settings.duo_workflow_oauth_application_id, - scopes: :ai_workflows + scopes: [:ai_workflows] ) end @@ -36,6 +36,8 @@ token = execute[:oauth_access_token] expect(token.resource_owner_id).to eq user.id expect(token.application).to eq oauth_application + expect(token.scopes).to contain_exactly('mcp', 'ai_workflows') + expect(oauth_application.reload.scopes).to contain_exactly('mcp', 'ai_workflows') end end @@ -111,7 +113,11 @@ it 'creates a new doorkeeper oauth application' do expect(::Gitlab::CurrentSettings).to receive(:expire_current_application_settings).and_call_original expect { execute }.to change { Authn::OauthApplication.count }.by(1) - expect(application_settings.duo_workflow_oauth_application_id).to eq(Authn::OauthApplication.last&.id) + + application = Authn::OauthApplication.last + + expect(application_settings.duo_workflow_oauth_application_id).to eq(application.id) + expect(application.scopes).to contain_exactly('mcp', 'ai_workflows') end end diff --git a/ee/spec/services/ai/duo_workflows/onboarding_service_spec.rb b/ee/spec/services/ai/duo_workflows/onboarding_service_spec.rb index 990d7be932d5de..eaed04917b04db 100644 --- a/ee/spec/services/ai/duo_workflows/onboarding_service_spec.rb +++ b/ee/spec/services/ai/duo_workflows/onboarding_service_spec.rb @@ -52,7 +52,7 @@ { name: 'GitLab Duo Agent Platform Composite OAuth Application', redirect_uri: Gitlab::Routing.url_helpers.root_url, - scopes: [:ai_workflows, :"user:*"], + scopes: [:ai_workflows, :mcp, :"user:*"], trusted: true, confidential: true } diff --git a/lib/api/mcp/base.rb b/lib/api/mcp/base.rb index 3622fd35b645af..902a5422747c40 100644 --- a/lib/api/mcp/base.rb +++ b/lib/api/mcp/base.rb @@ -42,7 +42,7 @@ class Base < ::API::Base before do authenticate! not_found! unless Feature.enabled?(:mcp_server, current_user) - forbidden! unless access_token&.scopes&.map(&:to_s) == [Gitlab::Auth::MCP_SCOPE.to_s] + forbidden! unless AccessTokenValidationService.new(access_token).include_any_scope?([Gitlab::Auth::MCP_SCOPE]) end helpers do @@ -134,7 +134,7 @@ def format_jsonrpc_response(result) # See: https://modelcontextprotocol.io/specification/2025-06-18/basic/transports#listening-for-messages-from-the-server get do - status :not_implemented + status :method_not_allowed end end end diff --git a/spec/requests/api/mcp/base_spec.rb b/spec/requests/api/mcp/base_spec.rb index e14172504eea20..36b85bd2516846 100644 --- a/spec/requests/api/mcp/base_spec.rb +++ b/spec/requests/api/mcp/base_spec.rb @@ -77,7 +77,7 @@ post api('/mcp', user, oauth_access_token: insufficient_access_token), params: { jsonrpc: '2.0', method: 'initialize', id: '1' } - expect(response).to have_gitlab_http_status(:forbidden) + expect(response).to have_gitlab_http_status(:ok) end end @@ -180,10 +180,10 @@ end end - it 'returns not implemented' do + it 'returns method not allowed' do get api('/mcp', user, oauth_access_token: access_token) - expect(response).to have_gitlab_http_status(:not_implemented) + expect(response).to have_gitlab_http_status(:method_not_allowed) end end end -- GitLab