From 2802cc4a411908c522a36b05341a6c098121268a Mon Sep 17 00:00:00 2001 From: "Patrick J. Cherry" Date: Tue, 6 Dec 2022 09:28:09 +0000 Subject: [PATCH 01/11] Refactor Hydra token introspection This moves the Hydra Admin API helper into a separate library, using more idiomatic call for Faraday. It does mean that when testing the auth token has to be given in the headers. --- .env.example | 5 +- .../api/default_projects_controller.rb | 2 +- .../api/projects/images_controller.rb | 2 +- .../api/projects/remixes_controller.rb | 4 +- app/controllers/api/projects_controller.rb | 2 +- app/controllers/api_controller.rb | 16 +++-- app/helpers/oauth_user.rb | 29 ---------- lib/hydra_admin_api.rb | 41 +++++++++++++ spec/lib/hydra_admin_api_spec.rb | 58 +++++++++++++++++++ spec/request/projects/create_spec.rb | 23 +++++--- spec/request/projects/destroy_spec.rb | 12 ++-- spec/request/projects/images_spec.rb | 23 ++++---- spec/request/projects/index_spec.rb | 10 ++-- spec/request/projects/remix_spec.rb | 46 +++++++-------- spec/request/projects/show_spec.rb | 25 ++++---- spec/request/projects/update_spec.rb | 29 ++++++---- spec/support/oauth_user_mock.rb | 6 +- 17 files changed, 215 insertions(+), 118 deletions(-) delete mode 100644 app/helpers/oauth_user.rb create mode 100644 lib/hydra_admin_api.rb create mode 100644 spec/lib/hydra_admin_api_spec.rb diff --git a/.env.example b/.env.example index 6aeceae9..28a957f6 100644 --- a/.env.example +++ b/.env.example @@ -9,8 +9,9 @@ POSTGRES_HOST=changeme POSTGRES_USER=changeme POSTGRES_PASSWORD=changeme -HYDRA_ADMIN_URL=http://docker.for.mac.localhost:9001 -HYDRA_SECRET= +HYDRA_ADMIN_URL=http://host.docker.internal:9002 +HYDRA_ADMIN_API_KEY=test-key + # Add the below to bypass token authentication with hyrdra # BYPASS_AUTH=true diff --git a/app/controllers/api/default_projects_controller.rb b/app/controllers/api/default_projects_controller.rb index 687c7a9b..988e3dfa 100644 --- a/app/controllers/api/default_projects_controller.rb +++ b/app/controllers/api/default_projects_controller.rb @@ -2,7 +2,7 @@ module Api class DefaultProjectsController < ApiController - before_action :require_oauth_user, only: %i[create] + before_action :authorize_user, only: %i[create] def show data = if params[:type] == 'html' diff --git a/app/controllers/api/projects/images_controller.rb b/app/controllers/api/projects/images_controller.rb index e9c849ca..cf2cd472 100644 --- a/app/controllers/api/projects/images_controller.rb +++ b/app/controllers/api/projects/images_controller.rb @@ -3,7 +3,7 @@ module Api module Projects class ImagesController < ApiController - before_action :require_oauth_user + before_action :authorize_user def create @project = Project.find_by!(identifier: params[:project_id]) diff --git a/app/controllers/api/projects/remixes_controller.rb b/app/controllers/api/projects/remixes_controller.rb index 853f6be8..a6ad6a9a 100644 --- a/app/controllers/api/projects/remixes_controller.rb +++ b/app/controllers/api/projects/remixes_controller.rb @@ -3,11 +3,11 @@ module Api module Projects class RemixesController < ApiController - before_action :require_oauth_user + before_action :authorize_user def create result = Project::CreateRemix.call(params: remix_params, - user_id: oauth_user_id, + user_id: current_user, original_project: project) if result.success? diff --git a/app/controllers/api/projects_controller.rb b/app/controllers/api/projects_controller.rb index 9105a2f5..0c2d9cf1 100644 --- a/app/controllers/api/projects_controller.rb +++ b/app/controllers/api/projects_controller.rb @@ -2,7 +2,7 @@ module Api class ProjectsController < ApiController - before_action :require_oauth_user, only: %i[create update index destroy] + before_action :authorize_user, only: %i[create update index destroy] before_action :load_project, only: %i[show update destroy] before_action :load_projects, only: %i[index] load_and_authorize_resource diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb index 13ab4543..5179f837 100644 --- a/app/controllers/api_controller.rb +++ b/app/controllers/api_controller.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true -class ApiController < ActionController::API - include OauthUser +require 'hydra_admin_api' +class ApiController < ActionController::API unless Rails.application.config.consider_all_requests_local rescue_from ActiveRecord::RecordNotFound, with: -> { notfound } rescue_from CanCan::AccessDenied, with: -> { denied } @@ -10,13 +10,17 @@ class ApiController < ActionController::API private - def require_oauth_user - head :unauthorized unless oauth_user_id + def authorize_user + head :unauthorized unless current_user end def current_user - # current_user is required by CanCanCan - oauth_user_id + return @current_user if @current_user + + token = request.headers['Authorization'] + return nil unless token + + @current_user = HydraAdminApi.fetch_oauth_user_id(token:) end def notfound diff --git a/app/helpers/oauth_user.rb b/app/helpers/oauth_user.rb deleted file mode 100644 index 66dd011b..00000000 --- a/app/helpers/oauth_user.rb +++ /dev/null @@ -1,29 +0,0 @@ -# frozen_string_literal: true - -module OauthUser - def oauth_user_id - @oauth_user_id ||= fetch_oauth_user_id - end - - private - - def fetch_oauth_user_id - return nil if request.headers['Authorization'].blank? - - return AUTH_USER_ID if BYPASS_AUTH - - json = hydra_request - json['sub'] - end - - def hydra_request - con = Faraday.new ENV.fetch('HYDRA_ADMIN_URL') - res = con.post do |req| - req.url '/oauth2/introspect' - req.headers['Content-Type'] = 'application/x-www-form-urlencoded' - req.headers['apiKey'] = ENV.fetch('HYDRA_SECRET') - req.body = { token: request.headers['Authorization'] } - end - JSON.parse(res.body) - end -end diff --git a/lib/hydra_admin_api.rb b/lib/hydra_admin_api.rb new file mode 100644 index 00000000..3d88bec4 --- /dev/null +++ b/lib/hydra_admin_api.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require 'faraday' + +class HydraAdminApi + ADMIN_URL = ENV.fetch('HYDRA_ADMIN_URL', 'http://localhost:9002') + ADMIN_API_KEY = ENV.fetch('HYDRA_ADMIN_API_KEY', 'test-key') + + # The "bypass" user ID from + # https://github.com/RaspberryPiFoundation/rpi-auth/blob/main/lib/rpi_auth/engine.rb#L17 + BYPASS_AUTH_USER_ID = 'b6301f34-b970-4d4f-8314-f877bad8b150' + + class << self + def fetch_oauth_user_id(...) + new.fetch_oauth_user_id(...) + end + end + + def fetch_oauth_user_id(token:) + return nil if token.blank? + + return BYPASS_AUTH_USER_ID if ENV.fetch('BYPASS_AUTH', nil) == 'yes' + + response = post('oauth2/introspect', { token: }, { apikey: ADMIN_API_KEY }) + response.body['sub'] + end + + private + + def conn + @conn ||= Faraday.new(ADMIN_URL) do |f| + f.request :url_encoded + f.response :raise_error + f.response :json + end + end + + def post(...) + conn.post(...) + end +end diff --git a/spec/lib/hydra_admin_api_spec.rb b/spec/lib/hydra_admin_api_spec.rb new file mode 100644 index 00000000..52436a6f --- /dev/null +++ b/spec/lib/hydra_admin_api_spec.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +require 'rails_helper' +require 'hydra_admin_api' + +# rubocop:disable RSpec/MultipleMemoizedHelpers +RSpec.describe HydraAdminApi do + let(:hydra_admin_url) { 'https://hydra.com/admin' } + let(:hydra_admin_api_key) { 'secret' } + let(:bypass_auth) { nil } + + around do |example| + ClimateControl.modify(BYPASS_AUTH: bypass_auth) do + example.run + end + end + + before do + stub_const('HydraAdminApi::ADMIN_URL', hydra_admin_url) + stub_const('HydraAdminApi::ADMIN_API_KEY', hydra_admin_api_key) + end + + describe '#fetch_oauth_user_id' do + subject(:response) { described_class.fetch_oauth_user_id(**args) } + + let(:args) { { token: 'abc123' } } + let(:uuid) { SecureRandom.uuid } + let(:stubbed_response) { { active: true, sub: uuid } } + # `active` is a required field in the response; `sub` is the "subject". + # + # See https://www.ory.sh/docs/reference/api#tag/oAuth2/operation/introspectOAuth2Token + + before do + stub_request(:post, "#{hydra_admin_url}/oauth2/introspect") + .with(body: args, headers: { apiKey: hydra_admin_api_key }) + .to_return(status: 200, + body: stubbed_response.to_json, + headers: { content_type: 'application/json' }) + end + + it { is_expected.to eq uuid } + + context 'when the token is not found' do + let(:stubbed_response) { { active: false } } + + it { is_expected.to be_nil } + end + + context 'when BYPASS_AUTH is set' do + let(:bypass_auth) { 'yes' } + + # Default bypass ID from + # https://github.com/RaspberryPiFoundation/rpi-auth/blob/main/lib/rpi_auth/engine.rb#L17 + it { is_expected.to eq 'b6301f34-b970-4d4f-8314-f877bad8b150' } + end + end +end +# rubocop:enable RSpec/MultipleMemoizedHelpers diff --git a/spec/request/projects/create_spec.rb b/spec/request/projects/create_spec.rb index e5612ad2..7c1cb587 100644 --- a/spec/request/projects/create_spec.rb +++ b/spec/request/projects/create_spec.rb @@ -6,8 +6,10 @@ let(:user_id) { 'e0675b6c-dc48-4cd6-8c04-0f7ac05af51a' } let(:project) { create(:project, user_id:) } - describe 'create' do - context 'when auth is correct' do + context 'when auth is correct' do + let(:headers) { { Authorization: 'dummy-token' } } + + context 'when creating project is successful' do before do mock_oauth_user(user_id) @@ -17,7 +19,8 @@ end it 'returns success' do - post '/api/projects' + post '/api/projects', headers: headers + expect(response).to have_http_status(:ok) end end @@ -32,16 +35,18 @@ end it 'returns error' do - post '/api/projects' + post '/api/projects', headers: headers + expect(response).to have_http_status(:internal_server_error) end end + end - context 'when no auth user' do - it 'returns unauthorized' do - post '/api/projects' - expect(response).to have_http_status(:unauthorized) - end + context 'when no token is given' do + it 'returns unauthorized' do + post '/api/projects' + + expect(response).to have_http_status(:unauthorized) end end end diff --git a/spec/request/projects/destroy_spec.rb b/spec/request/projects/destroy_spec.rb index 69a8f82a..82393d45 100644 --- a/spec/request/projects/destroy_spec.rb +++ b/spec/request/projects/destroy_spec.rb @@ -7,6 +7,7 @@ context 'when user is logged in' do let!(:project) { create(:project, user_id:) } + let(:headers) { { Authorization: 'dummy-token' } } before do mock_oauth_user(user_id) @@ -14,13 +15,14 @@ context 'when deleting a project the user owns' do it 'returns success' do - delete "/api/projects/#{project.identifier}" + delete "/api/projects/#{project.identifier}", headers: headers + expect(response).to have_http_status(:ok) end it "deletes user's project" do expect do - delete "/api/projects/#{project.identifier}" + delete "/api/projects/#{project.identifier}", headers: end.to change(Project, :count).by(-1) end end @@ -29,15 +31,17 @@ let(:non_owned_project) { create(:project) } it 'returns forbidden' do - delete "/api/projects/#{non_owned_project.identifier}" + delete "/api/projects/#{non_owned_project.identifier}", headers: headers + expect(response).to have_http_status(:forbidden) end end end - context 'when no user' do + context 'when no token is given' do it 'returns unauthorized' do delete '/api/projects/project-identifier' + expect(response).to have_http_status(:unauthorized) end end diff --git a/spec/request/projects/images_spec.rb b/spec/request/projects/images_spec.rb index deb6a48b..42ebe7b9 100644 --- a/spec/request/projects/images_spec.rb +++ b/spec/request/projects/images_spec.rb @@ -3,8 +3,7 @@ require 'rails_helper' RSpec.describe 'Images requests', type: :request do - let(:user_id) { 'e0675b6c-dc48-4cd6-8c04-0f7ac05af51a' } - let(:project) { create(:project, user_id:) } + let(:project) { create(:project) } let(:image_filename) { 'test_image_1.png' } let(:params) { { images: [fixture_file_upload(image_filename, 'image/png')] } } let(:expected_json) do @@ -20,47 +19,51 @@ describe 'create' do context 'when auth is correct' do + let(:headers) { { Authorization: 'dummy-token' } } + before do - mock_oauth_user(user_id) + mock_oauth_user(project.user_id) end it 'attaches file to project' do - expect { post "/api/projects/#{project.identifier}/images", params: }.to change { project.images.count }.by(1) + expect { post "/api/projects/#{project.identifier}/images", params:, headers: }.to change { project.images.count }.by(1) end it 'returns file list' do - post "/api/projects/#{project.identifier}/images", params: params + post "/api/projects/#{project.identifier}/images", params: params, headers: headers expect(response.body).to eq(expected_json) end it 'returns success response' do - post "/api/projects/#{project.identifier}/images", params: params + post "/api/projects/#{project.identifier}/images", params: params, headers: headers expect(response).to have_http_status(:ok) end it 'returns 404 response if invalid project' do - post '/api/projects/no-such-project/images' + post '/api/projects/no-such-project/images', headers: headers expect(response).to have_http_status(:not_found) end end context 'when authed user is not creator' do + let(:headers) { { Authorization: 'dummy-token' } } + before do mock_oauth_user end it 'returns forbidden response' do - post "/api/projects/#{project.identifier}/images", params: params + post "/api/projects/#{project.identifier}/images", params: params, headers: headers expect(response).to have_http_status(:forbidden) end end - context 'when auth is invalid' do + context 'when auth token is missing' do it 'returns unauthorized' do - post "/api/projects/#{project.identifier}/images" + post "/api/projects/#{project.identifier}/images", headers: headers expect(response).to have_http_status(:unauthorized) end diff --git a/spec/request/projects/index_spec.rb b/spec/request/projects/index_spec.rb index 18cbd62a..5289fdbe 100644 --- a/spec/request/projects/index_spec.rb +++ b/spec/request/projects/index_spec.rb @@ -10,6 +10,8 @@ end context 'when user is logged in' do + let(:headers) { { Authorization: 'dummy-token' } } + before do # create non user projects create_list(:project, 2) @@ -17,24 +19,24 @@ end it 'returns success response' do - get '/api/projects' + get '/api/projects', headers: headers expect(response).to have_http_status(:ok) end it 'returns correct number of projects' do - get '/api/projects' + get '/api/projects', headers: headers returned = JSON.parse(response.body) expect(returned.length).to eq(2) end it 'returns users projects' do - get '/api/projects' + get '/api/projects', headers: headers returned = JSON.parse(response.body) expect(returned.all? { |proj| proj['user_id'] == user_id }).to be(true) end end - context 'when no user' do + context 'when no token is given' do it 'returns unauthorized' do get '/api/projects' expect(response).to have_http_status(:unauthorized) diff --git a/spec/request/projects/remix_spec.rb b/spec/request/projects/remix_spec.rb index d830fb9f..b32d4a55 100644 --- a/spec/request/projects/remix_spec.rb +++ b/spec/request/projects/remix_spec.rb @@ -13,28 +13,28 @@ } end - describe 'create' do + before do + mock_phrase_generation + end + + context 'when auth is correct' do + let(:headers) { { Authorization: 'dummy-token' } } + before do - mock_phrase_generation + mock_oauth_user(user_id) end - context 'when auth is correct' do - before do - mock_oauth_user(user_id) - end - - it 'returns success response' do - post "/api/projects/#{original_project.identifier}/remix", params: { project: project_params } + it 'returns success response' do + post "/api/projects/#{original_project.identifier}/remix", params: { project: project_params }, headers: headers - expect(response).to have_http_status(:ok) - end + expect(response).to have_http_status(:ok) + end - it 'returns 404 response if invalid project' do - project_params[:identifier] = 'no-such-project' - post '/api/projects/no-such-project/remix', params: { project: project_params } + it 'returns 404 response if invalid project' do + project_params[:identifier] = 'no-such-project' + post '/api/projects/no-such-project/remix', params: { project: project_params }, headers: headers - expect(response).to have_http_status(:not_found) - end + expect(response).to have_http_status(:not_found) end context 'when project can not be saved' do @@ -46,24 +46,24 @@ end it 'returns 400' do - post "/api/projects/#{original_project.identifier}/remix", params: { project: project_params } + post "/api/projects/#{original_project.identifier}/remix", params: { project: project_params }, headers: headers expect(response).to have_http_status(:bad_request) end it 'returns error message' do - post "/api/projects/#{original_project.identifier}/remix", params: { project: project_params } + post "/api/projects/#{original_project.identifier}/remix", params: { project: project_params }, headers: headers expect(response.body).to eq({ error: 'Something went wrong' }.to_json) end end + end - context 'when auth is invalid' do - it 'returns unauthorized' do - post "/api/projects/#{original_project.identifier}/remix" + context 'when auth is invalid' do + it 'returns unauthorized' do + post "/api/projects/#{original_project.identifier}/remix" - expect(response).to have_http_status(:unauthorized) - end + expect(response).to have_http_status(:unauthorized) end end end diff --git a/spec/request/projects/show_spec.rb b/spec/request/projects/show_spec.rb index 019d5eb5..16026d54 100644 --- a/spec/request/projects/show_spec.rb +++ b/spec/request/projects/show_spec.rb @@ -14,26 +14,29 @@ image_list: [] }.to_json end + let(:headers) { {} } context 'when user is logged in' do + let(:headers) { { Authorization: 'dummy-token' } } + before do mock_oauth_user(project.user_id) end context 'when loading own project' do it 'returns success response' do - get "/api/projects/#{project.identifier}" + get "/api/projects/#{project.identifier}", headers: headers expect(response).to have_http_status(:ok) end it 'returns json' do - get "/api/projects/#{project.identifier}" + get "/api/projects/#{project.identifier}", headers: headers expect(response.content_type).to eq('application/json; charset=utf-8') end it 'returns the project json' do - get "/api/projects/#{project.identifier}" + get "/api/projects/#{project.identifier}", headers: headers expect(response.body).to eq(project_json) end end @@ -52,13 +55,13 @@ end it 'returns forbidden response' do - get "/api/projects/#{another_project.identifier}" + get "/api/projects/#{another_project.identifier}", headers: headers expect(response).to have_http_status(:forbidden) end it 'does not return the project json' do - get "/api/projects/#{another_project.identifier}" + get "/api/projects/#{another_project.identifier}", headers: headers expect(response.body).not_to include(another_project_json) end end @@ -79,36 +82,36 @@ end it 'returns success response' do - get "/api/projects/#{starter_project.identifier}" + get "/api/projects/#{starter_project.identifier}", headers: headers expect(response).to have_http_status(:ok) end it 'returns json' do - get "/api/projects/#{starter_project.identifier}" + get "/api/projects/#{starter_project.identifier}", headers: headers expect(response.content_type).to eq('application/json; charset=utf-8') end it 'returns the project json' do - get "/api/projects/#{starter_project.identifier}" + get "/api/projects/#{starter_project.identifier}", headers: headers expect(response.body).to eq(starter_project_json) end it 'returns 404 response if invalid project' do - get '/api/projects/no-such-project' + get '/api/projects/no-such-project', headers: headers expect(response).to have_http_status(:not_found) end end context 'when loading an owned project' do it 'returns forbidden response' do - get "/api/projects/#{project.identifier}" + get "/api/projects/#{project.identifier}", headers: headers expect(response).to have_http_status(:forbidden) end it 'does not return the project json' do - get "/api/projects/#{project.identifier}" + get "/api/projects/#{project.identifier}", headers: headers expect(response.body).not_to include(project_json) end end diff --git a/spec/request/projects/update_spec.rb b/spec/request/projects/update_spec.rb index 009ab6d0..7174aef2 100644 --- a/spec/request/projects/update_spec.rb +++ b/spec/request/projects/update_spec.rb @@ -3,11 +3,10 @@ require 'rails_helper' RSpec.describe 'Project update requests', type: :request do - let(:user_id) { 'e0675b6c-dc48-4cd6-8c04-0f7ac05af51a' } - let(:project) { create(:project, user_id:) } + let(:headers) { { Authorization: 'dummy-token' } } context 'when authed user is project creator' do - let(:project) { create(:project, :with_default_component, user_id:) } + let(:project) { create(:project, :with_default_component) } let!(:component) { create(:component, project:) } let(:default_component_params) do project.components.first.attributes.symbolize_keys.slice( @@ -28,16 +27,16 @@ end before do - mock_oauth_user(user_id) + mock_oauth_user(project.user_id) end it 'returns success response' do - put "/api/projects/#{project.identifier}", params: params + put "/api/projects/#{project.identifier}", params: params, headers: headers expect(response).to have_http_status(:ok) end it 'returns updated project json' do - put "/api/projects/#{project.identifier}", params: params + put "/api/projects/#{project.identifier}", params: params, headers: headers expect(response.body).to include('updated component content') end @@ -45,7 +44,7 @@ mock_response = instance_double(OperationResponse) allow(mock_response).to receive(:success?).and_return(true) allow(Project::Update).to receive(:call).and_return(mock_response) - put "/api/projects/#{project.identifier}", params: params + put "/api/projects/#{project.identifier}", params: params, headers: headers expect(Project::Update).to have_received(:call) end @@ -53,7 +52,7 @@ let(:params) { { project: { components: [] } } } it 'returns error response' do - put "/api/projects/#{project.identifier}", params: params + put "/api/projects/#{project.identifier}", params: params, headers: headers expect(response).to have_http_status(:bad_request) end end @@ -64,18 +63,24 @@ let(:params) { { project: { components: [] } } } before do - mock_oauth_user(user_id) + mock_oauth_user end it 'returns forbidden response' do - put "/api/projects/#{project.identifier}", params: params + put "/api/projects/#{project.identifier}", params: params, headers: headers expect(response).to have_http_status(:forbidden) end end - context 'when auth is invalid' do + context 'when auth token is invalid' do + let(:project) { create(:project) } + + before do + allow(HydraAdminApi).to receive(:fetch_oauth_user_id).and_return(nil) + end + it 'returns unauthorized' do - put "/api/projects/#{project.identifier}" + put "/api/projects/#{project.identifier}", headers: headers expect(response).to have_http_status(:unauthorized) end diff --git a/spec/support/oauth_user_mock.rb b/spec/support/oauth_user_mock.rb index 59234bc9..43201987 100644 --- a/spec/support/oauth_user_mock.rb +++ b/spec/support/oauth_user_mock.rb @@ -1,11 +1,11 @@ # frozen_string_literal: true +require 'hydra_admin_api' + module OauthUserMock def mock_oauth_user(user_id = nil) user_id ||= SecureRandom.uuid - # rubocop:disable RSpec/AnyInstance - allow_any_instance_of(OauthUser).to receive(:oauth_user_id).and_return(user_id) - # rubocop:enable RSpec/AnyInstance + allow(HydraAdminApi).to receive(:fetch_oauth_user_id).and_return(user_id) end end From 39634d37dde0d2df0e96e3d34f9d7e6467535ab2 Mon Sep 17 00:00:00 2001 From: "Patrick J. Cherry" Date: Tue, 6 Dec 2022 09:34:15 +0000 Subject: [PATCH 02/11] Add Climate Control gem --- Gemfile | 1 + Gemfile.lock | 2 ++ 2 files changed, 3 insertions(+) diff --git a/Gemfile b/Gemfile index e17e0546..15268453 100644 --- a/Gemfile +++ b/Gemfile @@ -21,6 +21,7 @@ gem 'stimulus-rails' gem 'turbo-rails' group :development, :test do + gem 'climate_control' gem 'dotenv-rails' gem 'factory_bot_rails' gem 'faker' diff --git a/Gemfile.lock b/Gemfile.lock index 88550602..f3e94ce2 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -90,6 +90,7 @@ GEM builder (3.2.4) byebug (11.1.3) cancancan (3.4.0) + climate_control (1.2.0) coderay (1.1.3) concurrent-ruby (1.1.10) crack (0.4.5) @@ -289,6 +290,7 @@ DEPENDENCIES aws-sdk-s3 bootsnap cancancan (~> 3.3) + climate_control dotenv-rails factory_bot_rails faker From 0bf23ec813da9425ca09f55e084d0bfef0514756 Mon Sep 17 00:00:00 2001 From: "Patrick J. Cherry" Date: Tue, 6 Dec 2022 09:41:30 +0000 Subject: [PATCH 03/11] Remove useless return --- app/controllers/api_controller.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb index 5179f837..35201e18 100644 --- a/app/controllers/api_controller.rb +++ b/app/controllers/api_controller.rb @@ -18,8 +18,6 @@ def current_user return @current_user if @current_user token = request.headers['Authorization'] - return nil unless token - @current_user = HydraAdminApi.fetch_oauth_user_id(token:) end From 5269bebb6aee1fd84a94ed71ef894408fe2488e2 Mon Sep 17 00:00:00 2001 From: "Patrick J. Cherry" Date: Tue, 28 Feb 2023 21:19:53 +0000 Subject: [PATCH 04/11] Fix up newer tests --- app/controllers/api_controller.rb | 13 ++++--------- app/controllers/graphql_controller.rb | 7 +------ spec/requests/projects/index_spec.rb | 17 +++++++++-------- spec/requests/projects/update_spec.rb | 6 +++--- 4 files changed, 17 insertions(+), 26 deletions(-) diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb index 35201e18..33f54f74 100644 --- a/app/controllers/api_controller.rb +++ b/app/controllers/api_controller.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true -require 'hydra_admin_api' - class ApiController < ActionController::API + include AuthenticationConcern + unless Rails.application.config.consider_all_requests_local rescue_from ActiveRecord::RecordNotFound, with: -> { notfound } rescue_from CanCan::AccessDenied, with: -> { denied } @@ -10,17 +10,12 @@ class ApiController < ActionController::API private + alias current_user current_user_id + def authorize_user head :unauthorized unless current_user end - def current_user - return @current_user if @current_user - - token = request.headers['Authorization'] - @current_user = HydraAdminApi.fetch_oauth_user_id(token:) - end - def notfound head :not_found end diff --git a/app/controllers/graphql_controller.rb b/app/controllers/graphql_controller.rb index dad3b909..cdc1d834 100644 --- a/app/controllers/graphql_controller.rb +++ b/app/controllers/graphql_controller.rb @@ -1,11 +1,6 @@ # frozen_string_literal: true -class GraphqlController < ApplicationController - include AuthenticationConcern - include ActiveStorage::SetCurrent if Rails.env.development? - - skip_before_action :verify_authenticity_token - +class GraphqlController < ApiController # If accessing from outside this domain, nullify the session # This allows for outside API access while preventing CSRF attacks, # but you'll have to authenticate your user separately diff --git a/spec/requests/projects/index_spec.rb b/spec/requests/projects/index_spec.rb index 4d050fed..41bddeef 100644 --- a/spec/requests/projects/index_spec.rb +++ b/spec/requests/projects/index_spec.rb @@ -5,6 +5,7 @@ RSpec.describe 'Project index requests' do include PaginationLinksMock + let(:headers) { { Authorization: 'dummy-token' } } let(:user_id) { 'e0675b6c-dc48-4cd6-8c04-0f7ac05af51a' } let(:project_keys) { %w[identifier project_type name user_id updated_at] } @@ -20,24 +21,24 @@ end it 'returns success response' do - get '/api/projects' + get '/api/projects', headers: headers expect(response).to have_http_status(:ok) end it 'returns correct number of projects' do - get '/api/projects' + get '/api/projects', headers: headers returned = JSON.parse(response.body) expect(returned.length).to eq(2) end it 'returns users projects' do - get '/api/projects' + get '/api/projects', headers: headers returned = JSON.parse(response.body) expect(returned.all? { |proj| proj['user_id'] == user_id }).to be(true) end it 'returns all keys in response' do - get '/api/projects' + get '/api/projects', headers: headers returned = JSON.parse(response.body) returned.each { |project| expect(project.keys).to eq(project_keys) } end @@ -50,13 +51,13 @@ end it 'returns the default number of projects on the first page' do - get '/api/projects' + get '/api/projects', headers: headers returned = JSON.parse(response.body) expect(returned.length).to eq(8) end it 'returns the next set of projects on the next page' do - get '/api/projects?page=2' + get '/api/projects?page=2', headers: headers returned = JSON.parse(response.body) expect(returned.length).to eq(4) end @@ -66,7 +67,7 @@ next_link = page_links(2, 'next') expected_link_header = [last_link, next_link].join(', ') - get '/api/projects' + get '/api/projects', headers: headers expect(response.headers['Link']).to eq expected_link_header end @@ -75,7 +76,7 @@ prev_link = page_links(1, 'prev') expected_link_header = [first_link, prev_link].join(', ') - get '/api/projects?page=2' + get '/api/projects?page=2', headers: headers expect(response.headers['Link']).to eq expected_link_header end end diff --git a/spec/requests/projects/update_spec.rb b/spec/requests/projects/update_spec.rb index 45709450..8697e691 100644 --- a/spec/requests/projects/update_spec.rb +++ b/spec/requests/projects/update_spec.rb @@ -53,17 +53,17 @@ let(:params) { { project: { name: 'updated project name' } } } it 'returns success response' do - put "/api/projects/#{project.identifier}", params: params + put "/api/projects/#{project.identifier}", params: params, headers: headers expect(response).to have_http_status(:ok) end it 'returns json with updated project properties' do - put "/api/projects/#{project.identifier}", params: params + put "/api/projects/#{project.identifier}", params: params, headers: headers expect(response.body).to include('updated project name') end it 'returns json with previous project components' do - put "/api/projects/#{project.identifier}", params: params + put "/api/projects/#{project.identifier}", params: params, headers: headers expect(response.body).to include(project.components.first.attributes[:content].to_s) end end From 3b34b4178ff5df9c531f3c725cf04191c13eefba Mon Sep 17 00:00:00 2001 From: "Patrick J. Cherry" Date: Tue, 28 Feb 2023 21:25:21 +0000 Subject: [PATCH 05/11] Rubocop --- .rubocop.yml | 2 ++ Gemfile | 1 - spec/lib/hydra_admin_api_spec.rb | 2 -- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index ae85abde..0f1b4ec4 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -25,3 +25,5 @@ RSpec/DescribeClass: - "spec/graphql/queries/**" - "spec/graphql/mutations/**" +RSpec/MultipleMemoizedHelpers: + Max: 8 diff --git a/Gemfile b/Gemfile index 387a67ca..05dfa3c7 100644 --- a/Gemfile +++ b/Gemfile @@ -23,7 +23,6 @@ gem 'rails', '~> 7.0.0' gem 'sentry-rails', '~> 5.5.0' group :development, :test do - gem 'climate_control' gem 'dotenv-rails' gem 'factory_bot_rails' gem 'faker' diff --git a/spec/lib/hydra_admin_api_spec.rb b/spec/lib/hydra_admin_api_spec.rb index 52436a6f..50cadaac 100644 --- a/spec/lib/hydra_admin_api_spec.rb +++ b/spec/lib/hydra_admin_api_spec.rb @@ -3,7 +3,6 @@ require 'rails_helper' require 'hydra_admin_api' -# rubocop:disable RSpec/MultipleMemoizedHelpers RSpec.describe HydraAdminApi do let(:hydra_admin_url) { 'https://hydra.com/admin' } let(:hydra_admin_api_key) { 'secret' } @@ -55,4 +54,3 @@ end end end -# rubocop:enable RSpec/MultipleMemoizedHelpers From e1add51c389ee94b064d814b7eca4775cd22a1b9 Mon Sep 17 00:00:00 2001 From: "Patrick J. Cherry" Date: Tue, 28 Feb 2023 21:35:24 +0000 Subject: [PATCH 06/11] Remove accidental index request spec commit --- spec/request/projects/index_spec.rb | 45 ----------------------------- 1 file changed, 45 deletions(-) delete mode 100644 spec/request/projects/index_spec.rb diff --git a/spec/request/projects/index_spec.rb b/spec/request/projects/index_spec.rb deleted file mode 100644 index 5289fdbe..00000000 --- a/spec/request/projects/index_spec.rb +++ /dev/null @@ -1,45 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe 'Project index requests', type: :request do - let(:user_id) { 'e0675b6c-dc48-4cd6-8c04-0f7ac05af51a' } - - before do - create_list(:project, 2, user_id:) - end - - context 'when user is logged in' do - let(:headers) { { Authorization: 'dummy-token' } } - - before do - # create non user projects - create_list(:project, 2) - mock_oauth_user(user_id) - end - - it 'returns success response' do - get '/api/projects', headers: headers - expect(response).to have_http_status(:ok) - end - - it 'returns correct number of projects' do - get '/api/projects', headers: headers - returned = JSON.parse(response.body) - expect(returned.length).to eq(2) - end - - it 'returns users projects' do - get '/api/projects', headers: headers - returned = JSON.parse(response.body) - expect(returned.all? { |proj| proj['user_id'] == user_id }).to be(true) - end - end - - context 'when no token is given' do - it 'returns unauthorized' do - get '/api/projects' - expect(response).to have_http_status(:unauthorized) - end - end -end From 5fcb93f54397346686fd20b6f5014b2e9faf1a08 Mon Sep 17 00:00:00 2001 From: "Patrick J. Cherry" Date: Tue, 28 Feb 2023 21:36:28 +0000 Subject: [PATCH 07/11] Adjust test description --- spec/requests/projects/index_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/requests/projects/index_spec.rb b/spec/requests/projects/index_spec.rb index 41bddeef..a217078c 100644 --- a/spec/requests/projects/index_spec.rb +++ b/spec/requests/projects/index_spec.rb @@ -81,7 +81,7 @@ end end - context 'when no user' do + context 'when no token is given' do it 'returns unauthorized' do get '/api/projects' expect(response).to have_http_status(:unauthorized) From f1a1e65f9dd0795247945a866b76ccd8dfad4b69 Mon Sep 17 00:00:00 2001 From: "Patrick J. Cherry" Date: Tue, 28 Feb 2023 21:39:28 +0000 Subject: [PATCH 08/11] Remove old env var --- .env.example | 2 -- 1 file changed, 2 deletions(-) diff --git a/.env.example b/.env.example index 31eddfa4..b22d6e16 100644 --- a/.env.example +++ b/.env.example @@ -14,8 +14,6 @@ POSTGRES_PASSWORD=changeme HYDRA_ADMIN_URL=http://host.docker.internal:9002 HYDRA_ADMIN_API_KEY=test-key -HYDRA_SECRET= - SMEE_TUNNEL=https://smee.io/MLq0n9kvAes2vydX # Add the below to bypass token authentication with hyrdra From a63cf88ad1b2de85e8ab8e7daa205929374526c4 Mon Sep 17 00:00:00 2001 From: "Patrick J. Cherry" Date: Wed, 1 Mar 2023 08:40:18 +0000 Subject: [PATCH 09/11] Clarify naming of methods/stubs/concerns --- app/controllers/api_controller.rb | 4 +--- .../{authentication_concern.rb => identifiable.rb} | 14 +++++++++----- spec/rails_helper.rb | 2 +- spec/requests/projects/create_spec.rb | 4 ++-- spec/requests/projects/destroy_spec.rb | 2 +- spec/requests/projects/images_spec.rb | 4 ++-- spec/requests/projects/index_spec.rb | 4 ++-- spec/requests/projects/remix_spec.rb | 4 ++-- spec/requests/projects/show_spec.rb | 2 +- spec/requests/projects/update_spec.rb | 4 ++-- ...{oauth_user_mock.rb => hydra_admin_api_mock.rb} | 4 ++-- 11 files changed, 25 insertions(+), 23 deletions(-) rename app/controllers/concerns/{authentication_concern.rb => identifiable.rb} (54%) rename spec/support/{oauth_user_mock.rb => hydra_admin_api_mock.rb} (71%) diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb index 33f54f74..dd7b4bf3 100644 --- a/app/controllers/api_controller.rb +++ b/app/controllers/api_controller.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class ApiController < ActionController::API - include AuthenticationConcern + include Identifiable unless Rails.application.config.consider_all_requests_local rescue_from ActiveRecord::RecordNotFound, with: -> { notfound } @@ -10,8 +10,6 @@ class ApiController < ActionController::API private - alias current_user current_user_id - def authorize_user head :unauthorized unless current_user end diff --git a/app/controllers/concerns/authentication_concern.rb b/app/controllers/concerns/identifiable.rb similarity index 54% rename from app/controllers/concerns/authentication_concern.rb rename to app/controllers/concerns/identifiable.rb index f20549cb..3cab94eb 100644 --- a/app/controllers/concerns/authentication_concern.rb +++ b/app/controllers/concerns/identifiable.rb @@ -2,15 +2,19 @@ require 'hydra_admin_api' -module AuthenticationConcern +module Identifiable extend ActiveSupport::Concern - def current_user_id - return @current_user_id if @current_user_id - + def identify_user token = request.headers['Authorization'] return nil unless token - @current_user_id = HydraAdminApi.fetch_oauth_user_id(token:) + HydraAdminApi.fetch_oauth_user_id(token:) + end + + def current_user_id + @current_user_id ||= identify_user end + + alias current_user current_user_id end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 8884db96..44b3c4b9 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -83,7 +83,7 @@ config.include GraphqlQueryHelpers, type: :graphql_query config.include PhraseIdentifierMock - config.include OauthUserMock + config.include HydraAdminApiMock, type: :request end Shoulda::Matchers.configure do |config| diff --git a/spec/requests/projects/create_spec.rb b/spec/requests/projects/create_spec.rb index bbd5fed9..43ba6bd8 100644 --- a/spec/requests/projects/create_spec.rb +++ b/spec/requests/projects/create_spec.rb @@ -11,7 +11,7 @@ context 'when creating project is successful' do before do - mock_oauth_user(user_id) + stub_fetch_oauth_user_id(user_id) response = OperationResponse.new response[:project] = project @@ -27,7 +27,7 @@ context 'when creating project fails' do before do - mock_oauth_user(user_id) + stub_fetch_oauth_user_id(user_id) response = OperationResponse.new response[:error] = 'Error creating project' diff --git a/spec/requests/projects/destroy_spec.rb b/spec/requests/projects/destroy_spec.rb index 02f7d87b..958b7db7 100644 --- a/spec/requests/projects/destroy_spec.rb +++ b/spec/requests/projects/destroy_spec.rb @@ -10,7 +10,7 @@ let(:headers) { { Authorization: 'dummy-token' } } before do - mock_oauth_user(user_id) + stub_fetch_oauth_user_id(user_id) end context 'when deleting a project the user owns' do diff --git a/spec/requests/projects/images_spec.rb b/spec/requests/projects/images_spec.rb index 0d19eb56..1e663f21 100644 --- a/spec/requests/projects/images_spec.rb +++ b/spec/requests/projects/images_spec.rb @@ -23,7 +23,7 @@ let(:headers) { { Authorization: 'dummy-token' } } before do - mock_oauth_user(project.user_id) + stub_fetch_oauth_user_id(project.user_id) end it 'attaches file to project' do @@ -53,7 +53,7 @@ let(:headers) { { Authorization: 'dummy-token' } } before do - mock_oauth_user + stub_fetch_oauth_user_id end it 'returns forbidden response' do diff --git a/spec/requests/projects/index_spec.rb b/spec/requests/projects/index_spec.rb index a217078c..9c2051da 100644 --- a/spec/requests/projects/index_spec.rb +++ b/spec/requests/projects/index_spec.rb @@ -17,7 +17,7 @@ before do # create non user projects create_list(:project, 2) - mock_oauth_user(user_id) + stub_fetch_oauth_user_id(user_id) end it 'returns success response' do @@ -47,7 +47,7 @@ context 'when the projects index has pagination' do before do create_list(:project, 10, user_id:) - mock_oauth_user(user_id) + stub_fetch_oauth_user_id(user_id) end it 'returns the default number of projects on the first page' do diff --git a/spec/requests/projects/remix_spec.rb b/spec/requests/projects/remix_spec.rb index 955d3ed1..fd3b8a29 100644 --- a/spec/requests/projects/remix_spec.rb +++ b/spec/requests/projects/remix_spec.rb @@ -21,7 +21,7 @@ let(:headers) { { Authorization: 'dummy-token' } } before do - mock_oauth_user(user_id) + stub_fetch_oauth_user_id(user_id) end it 'returns success response' do @@ -39,7 +39,7 @@ context 'when project can not be saved' do before do - mock_oauth_user(user_id) + stub_fetch_oauth_user_id(user_id) error_response = OperationResponse.new error_response[:error] = 'Something went wrong' allow(Project::CreateRemix).to receive(:call).and_return(error_response) diff --git a/spec/requests/projects/show_spec.rb b/spec/requests/projects/show_spec.rb index dc3a5c9d..dc65dc6b 100644 --- a/spec/requests/projects/show_spec.rb +++ b/spec/requests/projects/show_spec.rb @@ -20,7 +20,7 @@ let(:headers) { { Authorization: 'dummy-token' } } before do - mock_oauth_user(project.user_id) + stub_fetch_oauth_user_id(project.user_id) end context 'when loading own project' do diff --git a/spec/requests/projects/update_spec.rb b/spec/requests/projects/update_spec.rb index 8697e691..dc081d4e 100644 --- a/spec/requests/projects/update_spec.rb +++ b/spec/requests/projects/update_spec.rb @@ -28,7 +28,7 @@ end before do - mock_oauth_user(project.user_id) + stub_fetch_oauth_user_id(project.user_id) end it 'returns success response' do @@ -83,7 +83,7 @@ let(:params) { { project: { components: [] } } } before do - mock_oauth_user + stub_fetch_oauth_user_id end it 'returns forbidden response' do diff --git a/spec/support/oauth_user_mock.rb b/spec/support/hydra_admin_api_mock.rb similarity index 71% rename from spec/support/oauth_user_mock.rb rename to spec/support/hydra_admin_api_mock.rb index 43201987..5de460e6 100644 --- a/spec/support/oauth_user_mock.rb +++ b/spec/support/hydra_admin_api_mock.rb @@ -2,8 +2,8 @@ require 'hydra_admin_api' -module OauthUserMock - def mock_oauth_user(user_id = nil) +module HydraAdminApiMock + def stub_fetch_oauth_user_id(user_id = nil) user_id ||= SecureRandom.uuid allow(HydraAdminApi).to receive(:fetch_oauth_user_id).and_return(user_id) From 766bbea4c0ce8e66bcd1d73c49ff5911d9b44fd3 Mon Sep 17 00:00:00 2001 From: "Patrick J. Cherry" Date: Wed, 1 Mar 2023 09:02:55 +0000 Subject: [PATCH 10/11] Re=add comment --- app/controllers/concerns/identifiable.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/controllers/concerns/identifiable.rb b/app/controllers/concerns/identifiable.rb index 3cab94eb..c6711cd6 100644 --- a/app/controllers/concerns/identifiable.rb +++ b/app/controllers/concerns/identifiable.rb @@ -16,5 +16,6 @@ def current_user_id @current_user_id ||= identify_user end + # current_user is required by CanCanCan alias current_user current_user_id end From 8793c74d174026e1fb686f9e7dfc68f9145b59ef Mon Sep 17 00:00:00 2001 From: "Patrick J. Cherry" Date: Wed, 1 Mar 2023 15:00:16 +0000 Subject: [PATCH 11/11] Add moar graphql spec tests --- app/controllers/graphql_controller.rb | 26 +---- spec/requests/graphql_spec.rb | 136 ++++++++++++++++++++++---- spec/requests/projects/images_spec.rb | 2 +- spec/requests/projects/update_spec.rb | 2 +- spec/support/hydra_admin_api_mock.rb | 4 +- 5 files changed, 124 insertions(+), 46 deletions(-) diff --git a/app/controllers/graphql_controller.rb b/app/controllers/graphql_controller.rb index cdc1d834..84cc9c84 100644 --- a/app/controllers/graphql_controller.rb +++ b/app/controllers/graphql_controller.rb @@ -9,10 +9,6 @@ class GraphqlController < ApiController def execute result = EditorApiSchema.execute(query, variables:, context:, operation_name:) render json: result - rescue StandardError => e - raise e unless Rails.env.development? - - handle_error_in_development(e) end private @@ -33,29 +29,15 @@ def context def variables variables_param = params[:variables] - case params[:variables] + return {} if variables_param.blank? + + case variables_param when String - if variables_param.present? - JSON.parse(variables_param) || {} - else - {} - end - when Hash - variables_param + JSON.parse(variables_param) || {} when ActionController::Parameters variables_param.to_unsafe_hash # GraphQL-Ruby will validate name and type of incoming variables. - when nil - {} else raise ArgumentError, "Unexpected parameter: #{variables_param}" end end - - def handle_error_in_development(error) - logger.error error.message - logger.error error.backtrace.join("\n") - - render json: { errors: [{ message: error.message, backtrace: error.backtrace }], data: {} }, - status: :internal_server_error - end end diff --git a/spec/requests/graphql_spec.rb b/spec/requests/graphql_spec.rb index 867c1ecf..5084b2bf 100644 --- a/spec/requests/graphql_spec.rb +++ b/spec/requests/graphql_spec.rb @@ -3,38 +3,136 @@ require 'rails_helper' RSpec.describe 'POST /graphql' do - subject { response } + subject(:request) { post(graphql_path, as: :json, params:, headers:) } - let(:params) { {} } - let(:headers) { {} } - let(:json_response) { JSON.parse(response.body) } + let(:headers) { nil } + let(:params) { nil } - before { post graphql_path, params:, headers: } + before do + allow(EditorApiSchema).to receive(:execute).and_return({}) + end - it { is_expected.to be_ok } + shared_examples 'no variables are set' do + it 'sets an empty hash for the variables' do + request + expect(EditorApiSchema).to have_received(:execute).with(anything, hash_including(variables: {})) + end + end - it 'returns errors' do - expect(json_response['errors']).to be_a Array + shared_examples 'correctly sets the variables' do + it 'passes them correctly' do + request + expect(EditorApiSchema).to have_received(:execute).with(anything, hash_including(variables:)) + end end - context 'with a query' do - let(:params) { { query: } } - let(:query) { '' } + shared_examples 'an unidentified request' do + it 'sets the current_user_id as nil in the context' do + request + expect(EditorApiSchema).to have_received(:execute).with(anything, hash_including(context: hash_including(current_user_id: nil))) + end + end + + it 'returns OK' do + request + expect(response).to be_ok + end + + it_behaves_like 'an unidentified request' + + it_behaves_like 'no variables are set' - it { is_expected.to be_ok } + it 'returns a JSON response' do + request + expect(response.content_type).to start_with 'application/json;' + end + + context 'when an operationName is given' do + let(:params) { { operationName: operation_name } } + let(:operation_name) { 'testOperation' } - it 'returns errors' do - expect(json_response['errors']).to be_a Array + it 'sets the operationName correctly' do + request + expect(EditorApiSchema).to have_received(:execute).with(anything, hash_including(operation_name:)) end + end - context 'with a valid query' do - let(:query) { '{ node("xyz") }' } + context 'when an Authorization header is supplied' do + let(:headers) { { Authorization: token } } + let(:token) { '' } - it { is_expected.to be_ok } + it_behaves_like 'an unidentified request' - it 'returns data' do - expect(json_response.dig('data', 'node')).to be_nil + context 'with a token' do + let(:token) { 'valid-token' } + + context 'when the token is invalid' do + before do + stub_fetch_oauth_user_id(nil) + end + + it_behaves_like 'an unidentified request' end + + context 'when the token is valid' do + let(:current_user_id) { SecureRandom.uuid } + + before do + stub_fetch_oauth_user_id(current_user_id) + end + + it 'sets the current_user_id in the context' do + request + expect(EditorApiSchema).to have_received(:execute).with(anything, hash_including(context: hash_including(current_user_id:))) + end + end + end + end + + context 'when variables are given' do + let(:params) { { variables: } } + let(:variables) { { 'key' => 'value' } } + + it_behaves_like 'correctly sets the variables' + + context 'when they are a JSON string' do + subject(:request) { post(graphql_path, as: :url_encoded_form, params:) } + + let(:params) { { variables: variables.to_json } } + + it_behaves_like 'correctly sets the variables' + end + + context 'when the params are encoded as url_encoded_form' do + subject(:request) { post(graphql_path, as: :url_encoded_form, params:) } + + it_behaves_like 'correctly sets the variables' + end + + context 'when variables are set to null' do + let(:variables) { 'null' } + + it_behaves_like 'no variables are set' + end + + context 'when variables are an empty string' do + let(:variables) { '' } + + it_behaves_like 'no variables are set' + end + end + + context 'when a query is given' do + let(:query) { '{ query { hello { id } }' } + let(:params) { { query: } } + + before do + allow(EditorApiSchema).to receive(:execute).and_return({}) + end + + it 'passes them correctly' do + request + expect(EditorApiSchema).to have_received(:execute).with(query, anything) end end end diff --git a/spec/requests/projects/images_spec.rb b/spec/requests/projects/images_spec.rb index 1e663f21..795c0e58 100644 --- a/spec/requests/projects/images_spec.rb +++ b/spec/requests/projects/images_spec.rb @@ -53,7 +53,7 @@ let(:headers) { { Authorization: 'dummy-token' } } before do - stub_fetch_oauth_user_id + stub_fetch_oauth_user_id(SecureRandom.uuid) end it 'returns forbidden response' do diff --git a/spec/requests/projects/update_spec.rb b/spec/requests/projects/update_spec.rb index dc081d4e..7b0b1496 100644 --- a/spec/requests/projects/update_spec.rb +++ b/spec/requests/projects/update_spec.rb @@ -83,7 +83,7 @@ let(:params) { { project: { components: [] } } } before do - stub_fetch_oauth_user_id + stub_fetch_oauth_user_id(SecureRandom.uuid) end it 'returns forbidden response' do diff --git a/spec/support/hydra_admin_api_mock.rb b/spec/support/hydra_admin_api_mock.rb index 5de460e6..6b17f519 100644 --- a/spec/support/hydra_admin_api_mock.rb +++ b/spec/support/hydra_admin_api_mock.rb @@ -3,9 +3,7 @@ require 'hydra_admin_api' module HydraAdminApiMock - def stub_fetch_oauth_user_id(user_id = nil) - user_id ||= SecureRandom.uuid - + def stub_fetch_oauth_user_id(user_id) allow(HydraAdminApi).to receive(:fetch_oauth_user_id).and_return(user_id) end end