diff --git a/app/controllers/graphql_controller.rb b/app/controllers/graphql_controller.rb index faafbc0203fcd108b58e0377ffc74fa858bc81e0..1941920325f2e69ded6800bf186316c0e3db97c3 100644 --- a/app/controllers/graphql_controller.rb +++ b/app/controllers/graphql_controller.rb @@ -255,6 +255,12 @@ def multiplex? end def authorize_access_api! + if current_user.nil? && + request_authenticator.authentication_token_present? && + Feature.enabled?(:invalid_graphql_auth_401) + render_error('Invalid token', status: :unauthorized) + end + return if can?(current_user, :access_api) render_error('API not accessible for user', status: :forbidden) diff --git a/config/feature_flags/development/invalid_graphql_auth_401.yml b/config/feature_flags/development/invalid_graphql_auth_401.yml new file mode 100644 index 0000000000000000000000000000000000000000..668a86cc52fe349f4a737afbd7a896f06234601b --- /dev/null +++ b/config/feature_flags/development/invalid_graphql_auth_401.yml @@ -0,0 +1,8 @@ +--- +name: invalid_graphql_auth_401 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/132149 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/426196 +milestone: '16.5' +type: development +group: group::import and integrate +default_enabled: false diff --git a/lib/gitlab/auth/auth_finders.rb b/lib/gitlab/auth/auth_finders.rb index a715f17ecd6f909a4401135fc36f4b822c17d1a9..25465e73b95d95e7fab56a44dbb1522aee7b7552 100644 --- a/lib/gitlab/auth/auth_finders.rb +++ b/lib/gitlab/auth/auth_finders.rb @@ -32,6 +32,17 @@ module AuthFinders RUNNER_JOB_TOKEN_PARAM = :token PATH_DEPENDENT_FEED_TOKEN_REGEX = /\A#{User::FEED_TOKEN_PREFIX}(\h{64})-(\d+)\z/ + PARAM_TOKEN_KEYS = [ + PRIVATE_TOKEN_PARAM, + JOB_TOKEN_PARAM, + RUNNER_JOB_TOKEN_PARAM + ].map(&:to_s).freeze + HEADER_TOKEN_KEYS = [ + PRIVATE_TOKEN_HEADER, + JOB_TOKEN_HEADER, + DEPLOY_TOKEN_HEADER + ].freeze + # Check the Rails session for valid authentication details def find_user_from_warden current_request.env['warden']&.authenticate if verified_request? @@ -204,6 +215,12 @@ def validate_access_token!(scopes: []) end end + def authentication_token_present? + PARAM_TOKEN_KEYS.intersection(current_request.params.keys).any? || + HEADER_TOKEN_KEYS.intersection(current_request.env.keys).any? || + parsed_oauth_token.present? + end + private def find_user_from_job_bearer_token diff --git a/spec/controllers/graphql_controller_spec.rb b/spec/controllers/graphql_controller_spec.rb index a3a065766536a1f4568cab35665831347b065c82..5c162d7d59e674f507c7ffd5b5e7fa3f196ba9f6 100644 --- a/spec/controllers/graphql_controller_spec.rb +++ b/spec/controllers/graphql_controller_spec.rb @@ -317,6 +317,73 @@ subject { post :execute, params: { query: query, access_token: token.token } } + shared_examples 'invalid token' do + it 'returns 401 with invalid token message' do + subject + + expect(response).to have_gitlab_http_status(:unauthorized) + expect_graphql_errors_to_include('Invalid token') + end + end + + context 'with an invalid token' do + context 'with auth header' do + subject do + request.headers[header] = 'invalid' + post :execute, params: { query: query, user: nil } + end + + context 'with private-token' do + let(:header) { 'Private-Token' } + + it_behaves_like 'invalid token' + end + + context 'with job-token' do + let(:header) { 'Job-Token' } + + it_behaves_like 'invalid token' + end + + context 'with deploy-token' do + let(:header) { 'Deploy-Token' } + + it_behaves_like 'invalid token' + end + end + + context 'with authorization bearer (oauth token)' do + subject do + request.headers['Authorization'] = 'Bearer invalid' + post :execute, params: { query: query, user: nil } + end + + it_behaves_like 'invalid token' + end + + context 'with auth param' do + subject { post :execute, params: { query: query, user: nil }.merge(header) } + + context 'with private_token' do + let(:header) { { private_token: 'invalid' } } + + it_behaves_like 'invalid token' + end + + context 'with job_token' do + let(:header) { { job_token: 'invalid' } } + + it_behaves_like 'invalid token' + end + + context 'with token' do + let(:header) { { token: 'invalid' } } + + it_behaves_like 'invalid token' + end + end + end + context 'when the user is a project bot' do let(:user) { create(:user, :project_bot, last_activity_on: last_activity_on) } diff --git a/spec/lib/gitlab/auth/auth_finders_spec.rb b/spec/lib/gitlab/auth/auth_finders_spec.rb index b0ec46a3a0e20abafc5978e649a44cc6faf9e6d6..95199ae18de643c8fe9a4c6ac8c28efb3c259422 100644 --- a/spec/lib/gitlab/auth/auth_finders_spec.rb +++ b/spec/lib/gitlab/auth/auth_finders_spec.rb @@ -1149,4 +1149,75 @@ def auth_header_with(token) end end end + + describe '#authentication_token_present?' do + subject { authentication_token_present? } + + context 'no auth header/param/oauth' do + before do + request.headers['Random'] = 'Something' + set_param(:random, 'something') + end + + it { is_expected.to be(false) } + end + + context 'with auth header' do + before do + request.headers[header] = 'invalid' + end + + context 'with private-token' do + let(:header) { 'Private-Token' } + + it { is_expected.to be(true) } + end + + context 'with job-token' do + let(:header) { 'Job-Token' } + + it { is_expected.to be(true) } + end + + context 'with deploy-token' do + let(:header) { 'Deploy-Token' } + + it { is_expected.to be(true) } + end + end + + context 'with authorization bearer (oauth token)' do + before do + request.headers['Authorization'] = 'Bearer invalid' + end + + it { is_expected.to be(true) } + end + + context 'with auth param' do + context 'with private_token' do + it 'returns true' do + set_param(:private_token, 'invalid') + + expect(subject).to be(true) + end + end + + context 'with job_token' do + it 'returns true' do + set_param(:job_token, 'invalid') + + expect(subject).to be(true) + end + end + + context 'with token' do + it 'returns true' do + set_param(:token, 'invalid') + + expect(subject).to be(true) + end + end + end + end end diff --git a/spec/requests/api/graphql_spec.rb b/spec/requests/api/graphql_spec.rb index 8a3c5261eb6e9c0bc6dbc14045daebc39c2994ce..0c9a117ed037d7599b2c52f60b234216da96f419 100644 --- a/spec/requests/api/graphql_spec.rb +++ b/spec/requests/api/graphql_spec.rb @@ -282,9 +282,9 @@ it 'does not authenticate user' do post_graphql(query, headers: { 'PRIVATE-TOKEN' => token.token }) - expect(response).to have_gitlab_http_status(:ok) + expect(response).to have_gitlab_http_status(:unauthorized) - expect(graphql_data['echo']).to eq('nil says: Hello world') + expect_graphql_errors_to_include('Invalid token') end end @@ -308,9 +308,9 @@ post_graphql(query, headers: { 'PRIVATE-TOKEN' => token.token }) - expect(response).to have_gitlab_http_status(:ok) + expect(response).to have_gitlab_http_status(:unauthorized) - expect(graphql_data['echo']).to eq('nil says: Hello world') + expect_graphql_errors_to_include('Invalid token') end end end