Add latest changes from gitlab-org/security/gitlab@14-0-stable-ee

4 files changed, 176 insertions, 9 deletions

diff --git a/spec/controllers/graphql_controller_spec.rb b/spec/controllers/graphql_controller_spec.rb
index f2d86b1b166..aed97a01a72 100644
--- a/spec/controllers/graphql_controller_spec.rb
+++ b/spec/controllers/graphql_controller_spec.rb
@@ -44,7 +44,7 @@ RSpec.describe GraphqlController do
         expect(response).to have_gitlab_http_status(:ok)
       end
 
-      it 'returns access denied template when user cannot access API' do
+      it 'returns forbidden when user cannot access API' do
         # User cannot access API in a couple of cases
         # * When user is internal(like ghost users)
         # * When user is blocked
@@ -54,7 +54,9 @@ RSpec.describe GraphqlController do
         post :execute
 
         expect(response).to have_gitlab_http_status(:forbidden)
-        expect(response).to render_template('errors/access_denied')
+        expect(json_response).to include(
+          'errors' => include(a_hash_including('message' => /API not accessible/))
+        )
       end
 
       it 'updates the users last_activity_on field' do
diff --git a/spec/policies/global_policy_spec.rb b/spec/policies/global_policy_spec.rb
index e88619b9527..85026ced466 100644
--- a/spec/policies/global_policy_spec.rb
+++ b/spec/policies/global_policy_spec.rb
@@ -239,6 +239,14 @@ RSpec.describe GlobalPolicy do
       it { is_expected.not_to be_allowed(:access_api) }
     end
 
+    context 'with a deactivated user' do
+      before do
+        current_user.deactivate!
+      end
+
+      it { is_expected.not_to be_allowed(:access_api) }
+    end
+
     context 'user with expired password' do
       before do
         current_user.update!(password_expires_at: 2.minutes.ago)
diff --git a/spec/requests/api/graphql_spec.rb b/spec/requests/api/graphql_spec.rb
index a336d74b135..463fca43cb5 100644
--- a/spec/requests/api/graphql_spec.rb
+++ b/spec/requests/api/graphql_spec.rb
@@ -6,6 +6,9 @@ RSpec.describe 'GraphQL' do
   include AfterNextHelpers
 
   let(:query) { graphql_query_for('echo', text: 'Hello world') }
+  let(:mutation) { 'mutation { echoCreate(input: { messages: ["hello", "world"] }) { echoes } }' }
+
+  let_it_be(:user) { create(:user) }
 
   describe 'logging' do
     shared_examples 'logging a graphql query' do
@@ -70,6 +73,139 @@ RSpec.describe 'GraphQL' do
     end
   end
 
+  context 'when executing mutations' do
+    let(:mutation_with_variables) do
+      <<~GQL
+      mutation($a: String!, $b: String!) {
+        echoCreate(input: { messages: [$a, $b] }) { echoes }
+      }
+      GQL
+    end
+
+    context 'with POST' do
+      it 'succeeds' do
+        post_graphql(mutation, current_user: user)
+
+        expect(graphql_data_at(:echo_create, :echoes)).to eq %w[hello world]
+      end
+
+      context 'with variables' do
+        it 'succeeds' do
+          post_graphql(mutation_with_variables, current_user: user, variables: { a: 'Yo', b: 'there' })
+
+          expect(graphql_data_at(:echo_create, :echoes)).to eq %w[Yo there]
+        end
+      end
+    end
+
+    context 'with GET' do
+      it 'fails' do
+        get_graphql(mutation, current_user: user)
+
+        expect(graphql_errors).to include(a_hash_including('message' => /Mutations are forbidden/))
+      end
+
+      context 'with variables' do
+        it 'fails' do
+          get_graphql(mutation_with_variables, current_user: user, variables: { a: 'Yo', b: 'there' })
+
+          expect(graphql_errors).to include(a_hash_including('message' => /Mutations are forbidden/))
+        end
+      end
+    end
+  end
+
+  context 'when executing queries' do
+    context 'with POST' do
+      it 'succeeds' do
+        post_graphql(query, current_user: user)
+
+        expect(graphql_data_at(:echo)).to include 'Hello world'
+      end
+    end
+
+    context 'with GET' do
+      it 'succeeds' do
+        get_graphql(query, current_user: user)
+
+        expect(graphql_data_at(:echo)).to include 'Hello world'
+      end
+    end
+  end
+
+  context 'when selecting a query by operation name' do
+    let(:query) { "query A #{graphql_query_for('echo', text: 'Hello world')}" }
+    let(:mutation) { 'mutation B { echoCreate(input: { messages: ["hello", "world"] }) { echoes } }' }
+
+    let(:combined) { [query, mutation].join("\n\n") }
+
+    context 'with POST' do
+      it 'succeeds when selecting the query' do
+        post_graphql(combined, current_user: user, params: { operationName: 'A' })
+
+        resp = json_response
+
+        expect(resp.dig('data', 'echo')).to include 'Hello world'
+      end
+
+      it 'succeeds when selecting the mutation' do
+        post_graphql(combined, current_user: user, params: { operationName: 'B' })
+
+        resp = json_response
+
+        expect(resp.dig('data', 'echoCreate', 'echoes')).to eq %w[hello world]
+      end
+    end
+
+    context 'with GET' do
+      it 'succeeds when selecting the query' do
+        get_graphql(combined, current_user: user, params: { operationName: 'A' })
+
+        resp = json_response
+
+        expect(resp.dig('data', 'echo')).to include 'Hello world'
+      end
+
+      it 'fails when selecting the mutation' do
+        get_graphql(combined, current_user: user, params: { operationName: 'B' })
+
+        resp = json_response
+
+        expect(resp.dig('errors', 0, 'message')).to include "Mutations are forbidden"
+      end
+    end
+  end
+
+  context 'when batching mutations and queries' do
+    let(:batched) do
+      [
+        { query: "query A #{graphql_query_for('echo', text: 'Hello world')}" },
+        { query: 'mutation B { echoCreate(input: { messages: ["hello", "world"] }) { echoes } }' }
+      ]
+    end
+
+    context 'with POST' do
+      it 'succeeds' do
+        post_multiplex(batched, current_user: user)
+
+        resp = json_response
+
+        expect(resp.dig(0, 'data', 'echo')).to include 'Hello world'
+        expect(resp.dig(1, 'data', 'echoCreate', 'echoes')).to eq %w[hello world]
+      end
+    end
+
+    context 'with GET' do
+      it 'fails with a helpful error message' do
+        get_multiplex(batched, current_user: user)
+
+        resp = json_response
+
+        expect(resp.dig('errors', 0, 'message')).to include "Mutations are forbidden"
+      end
+    end
+  end
+
   context 'with invalid variables' do
     it 'returns an error' do
       post_graphql(query, variables: "This is not JSON")
@@ -80,8 +216,6 @@ RSpec.describe 'GraphQL' do
   end
 
   describe 'authentication', :allow_forgery_protection do
-    let(:user) { create(:user) }
-
     it 'allows access to public data without authentication' do
       post_graphql(query)
 
@@ -109,11 +243,9 @@ RSpec.describe 'GraphQL' do
     context 'with token authentication' do
       let(:token) { create(:personal_access_token) }
 
-      before do
+      it 'authenticates users with a PAT' 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)
@@ -124,6 +256,14 @@ RSpec.describe 'GraphQL' do
         expect(graphql_data['echo']).to eq("\"#{token.user.username}\" says: Hello world")
       end
 
+      it 'prevents access by deactived users' do
+        token.user.deactivate!
+
+        post_graphql(query, headers: { 'PRIVATE-TOKEN' => token.token })
+
+        expect(graphql_errors).to include({ 'message' => /API not accessible/ })
+      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])
diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb
index 4857fa63114..38cf828ca5e 100644
--- a/spec/support/helpers/graphql_helpers.rb
+++ b/spec/support/helpers/graphql_helpers.rb
@@ -400,8 +400,13 @@ module GraphqlHelpers
     post api('/', current_user, version: 'graphql'), params: { _json: queries }, headers: headers
   end
 
-  def post_graphql(query, current_user: nil, variables: nil, headers: {}, token: {})
-    params = { query: query, variables: serialize_variables(variables) }
+  def get_multiplex(queries, current_user: nil, headers: {})
+    path = "/?#{queries.to_query('_json')}"
+    get api(path, current_user, version: 'graphql'), headers: headers
+  end
+
+  def post_graphql(query, current_user: nil, variables: nil, headers: {}, token: {}, params: {})
+    params = params.merge(query: query, variables: serialize_variables(variables))
     post api('/', current_user, version: 'graphql', **token), params: params, headers: headers
 
     return unless graphql_errors
@@ -410,6 +415,18 @@ module GraphqlHelpers
     expect(graphql_errors).not_to include(a_hash_including('message' => 'Internal server error'))
   end
 
+  def get_graphql(query, current_user: nil, variables: nil, headers: {}, token: {}, params: {})
+    vars = "variables=#{CGI.escape(serialize_variables(variables))}" if variables
+    params = params.to_a.map { |k, v| v.to_query(k) }
+    path = ["/?query=#{CGI.escape(query)}", vars, *params].join('&')
+    get api(path, current_user, version: 'graphql', **token), headers: headers
+
+    return unless graphql_errors
+
+    # Errors are acceptable, but not this one:
+    expect(graphql_errors).not_to include(a_hash_including('message' => 'Internal server error'))
+  end
+
   def post_graphql_mutation(mutation, current_user: nil, token: {})
     post_graphql(mutation.query,
                  current_user: current_user,