Commit aef1de57 authored by Stan Hu's avatar Stan Hu

Merge branch 'bvl-graphql-csrf' into 'master'

Allow GraphQL requests without CSRF token

Closes #57237

See merge request gitlab-org/gitlab-ce!25719
parents c35e6603 b623932e
...@@ -3,9 +3,16 @@ ...@@ -3,9 +3,16 @@
class GraphqlController < ApplicationController class GraphqlController < ApplicationController
# Unauthenticated users have access to the API for public data # Unauthenticated users have access to the API for public data
skip_before_action :authenticate_user! 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 :check_graphql_feature_flag!
before_action(only: [:execute]) { authenticate_sessionless_user!(:api) }
def execute def execute
variables = Gitlab::Graphql::Variables.new(params[:variables]).to_h variables = Gitlab::Graphql::Variables.new(params[:variables]).to_h
......
---
title: Allow GraphQL requests without CSRF token
merge_request: 25719
author:
type: fixed
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
# 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
...@@ -79,12 +79,21 @@ module GraphqlHelpers ...@@ -79,12 +79,21 @@ module GraphqlHelpers
attributes = attributes_to_graphql(attributes) attributes = attributes_to_graphql(attributes)
attributes = "(#{attributes})" if attributes.present? attributes = "(#{attributes})" if attributes.present?
<<~QUERY <<~QUERY
#{name}#{attributes} { #{name}#{attributes}
#{fields} #{wrap_fields(fields)}
}
QUERY QUERY
end end
def wrap_fields(fields)
return unless fields.strip.present?
<<~FIELDS
{
#{fields}
}
FIELDS
end
def all_graphql_fields_for(class_name, parent_types = Set.new) def all_graphql_fields_for(class_name, parent_types = Set.new)
type = GitlabSchema.types[class_name.to_s] type = GitlabSchema.types[class_name.to_s]
return "" unless type return "" unless type
...@@ -116,8 +125,8 @@ module GraphqlHelpers ...@@ -116,8 +125,8 @@ module GraphqlHelpers
end.join(", ") end.join(", ")
end end
def post_graphql(query, current_user: nil, variables: nil) def post_graphql(query, current_user: nil, variables: nil, headers: {})
post api('/', current_user, version: 'graphql'), params: { query: query, variables: variables } post api('/', current_user, version: 'graphql'), params: { query: query, variables: variables }, headers: headers
end end
def post_graphql_mutation(mutation, current_user: nil) def post_graphql_mutation(mutation, current_user: nil)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment