diff options
| -rw-r--r-- | app/controllers/graphql_controller.rb | 9 | ||||
| -rw-r--r-- | changelogs/unreleased/bvl-graphql-csrf.yml | 5 | ||||
| -rw-r--r-- | spec/controllers/graphql_controller_spec.rb | 112 | ||||
| -rw-r--r-- | spec/requests/api/graphql_spec.rb | 86 |
4 files changed, 99 insertions, 113 deletions
diff --git a/app/controllers/graphql_controller.rb b/app/controllers/graphql_controller.rb index 3ef03bc9622..e147d32be2e 100644 --- a/app/controllers/graphql_controller.rb +++ b/app/controllers/graphql_controller.rb @@ -3,9 +3,16 @@ class GraphqlController < ApplicationController # Unauthenticated users have access to the API for public data skip_before_action :authenticate_user! - prepend_before_action(only: [:execute]) { authenticate_sessionless_user!(:api) } + + # Allow missing CSRF tokens, this would mean that if a CSRF is invalid or missing, + # the user won't be authenticated but can proceed as an anonymous user. + # + # If a CSRF is valid, the user is authenticated. This makes it easier to play + # around in GraphiQL. + protect_from_forgery with: :null_session, only: :execute before_action :check_graphql_feature_flag! + before_action(only: [:execute]) { authenticate_sessionless_user!(:api) } def execute variables = Gitlab::Graphql::Variables.new(params[:variables]).to_h diff --git a/changelogs/unreleased/bvl-graphql-csrf.yml b/changelogs/unreleased/bvl-graphql-csrf.yml new file mode 100644 index 00000000000..d1e5b56c751 --- /dev/null +++ b/changelogs/unreleased/bvl-graphql-csrf.yml @@ -0,0 +1,5 @@ +--- +title: Allow GraphQL requests without CSRF token +merge_request: 25719 +author: +type: fixed diff --git a/spec/controllers/graphql_controller_spec.rb b/spec/controllers/graphql_controller_spec.rb deleted file mode 100644 index a0f40874db1..00000000000 --- a/spec/controllers/graphql_controller_spec.rb +++ /dev/null @@ -1,112 +0,0 @@ -require 'spec_helper' - -describe GraphqlController do - describe 'execute' do - let(:user) { nil } - - before do - sign_in(user) if user - - run_test_query! - end - - subject { query_response } - - context 'graphql is disabled by feature flag' do - let(:user) { nil } - - before do - stub_feature_flags(graphql: false) - end - - it 'returns 404' do - run_test_query! - - expect(response).to have_gitlab_http_status(404) - end - end - - context 'signed out' do - let(:user) { nil } - - it 'runs the query with current_user: nil' do - is_expected.to eq('echo' => 'nil says: test success') - end - end - - context 'signed in' do - let(:user) { create(:user, username: 'Simon') } - - it 'runs the query with current_user set' do - is_expected.to eq('echo' => '"Simon" says: test success') - end - end - - context 'invalid variables' do - it 'returns an error' do - run_test_query!(variables: "This is not JSON") - - expect(response).to have_gitlab_http_status(422) - expect(json_response['errors'].first['message']).not_to be_nil - end - end - end - - context 'token authentication' do - before do - stub_authentication_activity_metrics(debug: false) - end - - let(:user) { create(:user, username: 'Simon') } - let(:personal_access_token) { create(:personal_access_token, user: user) } - - context "when the 'personal_access_token' param is populated with the personal access token" do - it 'logs the user in' do - expect(authentication_metrics) - .to increment(:user_authenticated_counter) - .and increment(:user_session_override_counter) - .and increment(:user_sessionless_authentication_counter) - - run_test_query!(private_token: personal_access_token.token) - - expect(response).to have_gitlab_http_status(200) - expect(query_response).to eq('echo' => '"Simon" says: test success') - end - end - - context 'when the personal access token has no api scope' do - it 'does not log the user in' do - personal_access_token.update(scopes: [:read_user]) - - run_test_query!(private_token: personal_access_token.token) - - expect(response).to have_gitlab_http_status(200) - - expect(query_response).to eq('echo' => 'nil says: test success') - end - end - - context 'without token' do - it 'shows public data' do - run_test_query! - - expect(query_response).to eq('echo' => 'nil says: test success') - end - end - end - - # Chosen to exercise all the moving parts in GraphqlController#execute - def run_test_query!(variables: { 'text' => 'test success' }, private_token: nil) - query = <<~QUERY - query Echo($text: String) { - echo(text: $text) - } - QUERY - - post :execute, params: { query: query, operationName: 'Echo', variables: variables, private_token: private_token } - end - - def query_response - json_response['data'] - end -end diff --git a/spec/requests/api/graphql_spec.rb b/spec/requests/api/graphql_spec.rb new file mode 100644 index 00000000000..cca87c16f27 --- /dev/null +++ b/spec/requests/api/graphql_spec.rb @@ -0,0 +1,86 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe 'GraphQL' do + include GraphqlHelpers + + let(:query) { graphql_query_for('echo', 'text' => 'Hello world' ) } + + context 'graphql is disabled by feature flag' do + before do + stub_feature_flags(graphql: false) + end + + it 'does not generate a route for GraphQL' do + expect { post_graphql(query) }.to raise_error(ActionController::RoutingError) + end + end + + context 'invalid variables' do + it 'returns an error' do + post_graphql(query, variables: "This is not JSON") + + expect(response).to have_gitlab_http_status(422) + expect(json_response['errors'].first['message']).not_to be_nil + end + end + + context 'authentication', :allow_forgery_protection do + let(:user) { create(:user) } + + it 'allows access to public data without authentication' do + post_graphql(query) + + expect(graphql_data['echo']).to eq('nil says: Hello world') + end + + it 'does not authenticate a user with an invalid CSRF' do + login_as(user) + + post_graphql(query, headers: { 'X-CSRF-Token' => 'invalid' }) + + expect(graphql_data['echo']).to eq('nil says: Hello world') + end + + it 'authenticates a user with a valid session token' do + # Create a session to get a CSRF token from + login_as(user) + get('/') + + post '/api/graphql', params: { query: query }, headers: { 'X-CSRF-Token' => response.session['_csrf_token'] } + + expect(graphql_data['echo']).to eq("\"#{user.username}\" says: Hello world") + end + + context 'token authentication' do + let(:token) { create(:personal_access_token) } + + before do + stub_authentication_activity_metrics(debug: false) + end + + it 'Authenticates users with a PAT' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_sessionless_authentication_counter) + + post_graphql(query, headers: { 'PRIVATE-TOKEN' => token.token }) + + expect(graphql_data['echo']).to eq("\"#{token.user.username}\" says: Hello world") + end + + context 'when the personal access token has no api scope' do + it 'does not log the user in' do + token.update(scopes: [:read_user]) + + post_graphql(query, headers: { 'PRIVATE-TOKEN' => token.token }) + + expect(response).to have_gitlab_http_status(200) + + expect(graphql_data['echo']).to eq('nil says: Hello world') + end + end + end + end +end |