Allow GraphQL requests without CSRF token

With this we allow authentication using a session or using personal
access token.

Authentication using a session, and CSRF token makes it easy to play
with GraphQL from the Graphiql endpoint we expose.

But we cannot enforce CSRF validity, otherwise authentication for
regular API clients would fail when they use personal access tokens to
authenticate.

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

changelogs/unreleased/bvl-graphql-csrf.yml

+---
+title: Allow GraphQL requests without CSRF token
+merge_request: 25719
+author:
+type: fixed

spec/controllers/graphql_controller_spec.rb

-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

spec/requests/api/graphql_spec.rb

+# 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