Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .release-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
14.78.1
14.78.2
2 changes: 1 addition & 1 deletion .ruby-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
3.4.4
3.4.6
2 changes: 1 addition & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ group :default do
# Version 0.1.1 was created from the [develop](https://github.com/sanger/jsonapi-resources/tree/develop) branch
# published, and pinned for Sequencescape compatibility.
# This version is tested and compatible with Rails 7.1/7.2 and Ruby 3.2/3.3.
gem 'sanger-jsonapi-resources', '~> 0.1.1'
gem 'sanger-jsonapi-resources', '~> 0.1.2'

# gem 'sanger-jsonapi-resources', github: 'sanger/jsonapi-resources', branch: 'develop'
gem 'csv', '~> 3.3' # Required by jsonapi-resources, previously part of ruby
Expand Down
12 changes: 6 additions & 6 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ GEM
railties (>= 4.1)
jsonapi-resources-matchers (1.0.0)
jsonapi-resources (>= 0.9.0)
knapsack_pro (8.3.3)
knapsack_pro (8.4.0)
rake
language_server-protocol (3.17.0.5)
launchy (3.1.1)
Expand Down Expand Up @@ -324,11 +324,11 @@ GEM
net-protocol
netrc (0.11.0)
nio4r (2.7.4)
nokogiri (1.18.9-arm64-darwin)
nokogiri (1.18.10-arm64-darwin)
racc (~> 1.4)
nokogiri (1.18.9-x86_64-darwin)
nokogiri (1.18.10-x86_64-darwin)
racc (~> 1.4)
nokogiri (1.18.9-x86_64-linux-gnu)
nokogiri (1.18.10-x86_64-linux-gnu)
racc (~> 1.4)
ostruct (0.6.3)
parallel (1.27.0)
Expand Down Expand Up @@ -511,7 +511,7 @@ GEM
logger
ruby2_keywords (0.0.5)
rubyzip (2.4.1)
sanger-jsonapi-resources (0.1.1)
sanger-jsonapi-resources (0.1.2)
activerecord (>= 4.1)
concurrent-ruby
csv
Expand Down Expand Up @@ -704,7 +704,7 @@ DEPENDENCIES
rubocop-rspec_rails
ruby-prof
ruby-units
sanger-jsonapi-resources (~> 0.1.1)
sanger-jsonapi-resources (~> 0.1.2)
sanger_barcode_format!
sanger_warren
selenium-webdriver (~> 4.1)
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/api/v2/bait_library_layouts_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def preview
private

def respond_with_errors(title, details, status)
status_code = Rack::Utils::SYMBOL_TO_STATUS_CODE[status]
status_code = Rack::Utils.status_code(status)

errors = details.map { |detail| { title: title, detail: detail, code: status_code, status: status_code } }

Expand Down
10 changes: 5 additions & 5 deletions app/controllers/api/v2/concerns/api_key_authenticatable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ module Api
module V2
module Concerns
# Provides the tools needed to confirm that a valid API key was provided for the header X-Sequencescape-Client-Id.
# - Skips the check if the endpoint specifies permissve GETs
# - Skips the check if the endpoint specifies permissive methods names.
# - Where the API key is in the request and exists in among ApiApplication, allow the request to be served.
# - Where the API key is in the request but does not exist, log the attempt and render an unauthorized response.
# - Where the API key is not in the request, respond normally (for now) and log the system that made the request.
# - Where the API key is not in the request, log the attempt and render an unauthorized response.
module ApiKeyAuthenticatable
extend ActiveSupport::Concern

Expand Down Expand Up @@ -74,14 +74,14 @@ def render_unauthorized
# Checks if the current request is a permissive route.
#
# A route is considered permissive if the 'permissive' path parameter is present in the request
# and the HTTP request method is 'GET'.
# and the HTTP request method is specified in the array of permissive methods.
# Path parameters can be defined via defaults next to the route in routes.rb
# e.g. jsonapi_resources :samples, defaults: { permissive: true }
# e.g. jsonapi_resources :samples, defaults: { permissive: [:get, :post] }
#
# @return [Boolean] true if the route is permissive, false otherwise.
def permissive_route
# Use path_parameters as standard parameters are overridable by requesters
request.path_parameters.fetch(:permissive, false) && request.get?
request.path_parameters.fetch(:permissive, []).include?(request.method.downcase.to_sym)
end
end
end
Expand Down
8 changes: 4 additions & 4 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@

jsonapi_resources :barcode_printers
jsonapi_resources :bulk_transfers, except: %i[update]
jsonapi_resources :comments, defaults: { permissive: true }
jsonapi_resources :comments, defaults: { permissive: %i[get post] }
jsonapi_resources :custom_metadatum_collections
jsonapi_resources :labware, defaults: { permissive: true }
jsonapi_resources :labware, defaults: { permissive: %i[get] }
jsonapi_resources :lanes
jsonapi_resources :lot_types
jsonapi_resources :lots
Expand All @@ -57,7 +57,7 @@
jsonapi_resources :primer_panels
jsonapi_resources :projects
jsonapi_resources :purposes
jsonapi_resources :qc_assays, defaults: { permissive: true }
jsonapi_resources :qc_assays, defaults: { permissive: %i[get post] }
jsonapi_resources :qc_files, except: %i[update]
jsonapi_resources :qc_results
jsonapi_resources :qcables
Expand All @@ -78,7 +78,7 @@
jsonapi_resources :submission_templates
jsonapi_resources :submissions, except: %i[update]
jsonapi_resources :tag_group_adapter_types
jsonapi_resources :tag_groups, defaults: { permissive: true }
jsonapi_resources :tag_groups, defaults: { permissive: %i[get] }
jsonapi_resources :tag_sets, only: %i[index show]
jsonapi_resources :tag_layout_templates
jsonapi_resources :tag_layouts, except: %i[update]
Expand Down
10 changes: 5 additions & 5 deletions spec/controllers/api/v2/concerns/api_key_authenticatable_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,19 @@
end

describe '#permissive_route' do
context 'when permissive param is present and request is GET' do
context 'when permissive param includes :get and request method is GET' do
before do
allow(controller.request).to receive_messages(path_parameters: { permissive: true }, get?: true)
allow(controller.request).to receive_messages(path_parameters: { permissive: [:get] }, method: 'GET')
end

it 'returns true' do
expect(controller.send(:permissive_route)).to be true
end
end

context 'when permissive param is present but request is not GET' do
context 'when permissive param includes :get but request method is not GET' do
before do
allow(controller.request).to receive_messages(path_parameters: { permissive: true }, get?: false)
allow(controller.request).to receive_messages(path_parameters: { permissive: [:get] }, method: 'POST')
end

it 'returns false' do
Expand All @@ -29,7 +29,7 @@

context 'when permissive param is not present' do
before do
allow(controller.request).to receive_messages(path_parameters: {}, get?: true)
allow(controller.request).to receive_messages(path_parameters: {}, method: 'GET')
end

it 'returns false' do
Expand Down
27 changes: 23 additions & 4 deletions spec/requests/api/v2/shared_examples/api_key_authenticatable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,33 @@
shared_examples_for 'ApiKeyAuthenticatable' do
let(:client_headers) { { 'User-Agent' => 'Test Agent', 'Origin' => 'Test Origin' } }.freeze
let(:permissive_route) do
Rails.application.routes.recognize_path(base_endpoint, method: :get).fetch(:permissive, false)
Rails.application.routes.recognize_path(base_endpoint, method: :get).fetch(:permissive, []).include?(:get)
end

context 'without an API key' do
it 'gets a success response' do
api_get base_endpoint, headers: client_headers
context 'when feature flag is disabled' do
before { Flipper.disable :y25_442_make_api_key_mandatory }

expect(response).to have_http_status(:success)
it 'gets a success response' do
api_get base_endpoint, headers: client_headers

expect(response).to have_http_status(:success)
end
end

context 'when feature flag is enabled' do
before { Flipper.enable :y25_442_make_api_key_mandatory }

it 'gets an unauthorized response' do
api_get base_endpoint, headers: client_headers

# Permissive routes are successful without API keys
if permissive_route
expect(response).to have_http_status(:success)
else
expect(response).to have_http_status(:unauthorized)
end
end
end

it 'logs the request with client details' do
Expand Down
Loading