Skip to content

Commit

Permalink
Accept the Authorization header
Browse files Browse the repository at this point in the history
Log a honeybadger notification when we get the old X-Auth header
Fixes #320
  • Loading branch information
jcoyne committed Sep 12, 2019
1 parent 96003dc commit 4ab67a2
Show file tree
Hide file tree
Showing 14 changed files with 64 additions and 46 deletions.
11 changes: 8 additions & 3 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ class ApplicationController < ActionController::API

# Since Basic auth was already using the Authorization header, we used something
# non-standard:
TOKEN_HEADER = 'X-Auth'
OLD_TOKEN_HEADER = 'X-Auth'
TOKEN_HEADER = 'Authorization'

private

Expand All @@ -25,6 +26,9 @@ def check_auth_token
return render json: { error: 'Not Authorized' }, status: :unauthorized unless token

Honeybadger.context(invoked_by: token[:sub])
return unless request.headers[OLD_TOKEN_HEADER]

Honeybadger.notify("Deprecated authentication header '#{OLD_TOKEN_HEADER}' was provided, but '#{TOKEN_HEADER}' is expected")
end

def decoded_auth_token
Expand All @@ -37,9 +41,10 @@ def decoded_auth_token
end

def http_auth_header
return if request.headers[TOKEN_HEADER].blank?
return if request.headers[OLD_TOKEN_HEADER].blank? && request.headers[TOKEN_HEADER].blank?

request.headers[TOKEN_HEADER].split(' ').last
field = request.headers[TOKEN_HEADER] || request.headers[OLD_TOKEN_HEADER]
field.split(' ').last
end

def proxy_faraday_response(response)
Expand Down
8 changes: 4 additions & 4 deletions spec/requests/add_release_tags_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
it 'adds a release tag' do
post '/v1/objects/druid:1234/release_tags',
params: %( {"to":"searchworks","who":"carrickr","what":"self","release":false} ),
headers: { 'X-Auth' => "Bearer #{jwt}" }
headers: { 'Authorization' => "Bearer #{jwt}" }

expect(ReleaseTags).to have_received(:create)
.with(Dor::Item, release: false, to: 'searchworks', who: 'carrickr', what: 'self')
Expand All @@ -31,7 +31,7 @@
it 'adds a release tag' do
post '/v1/objects/druid:1234/release_tags',
params: %( {"to":"searchworks","who":"carrickr","what":"self","release":true} ),
headers: { 'X-Auth' => "Bearer #{jwt}" }
headers: { 'Authorization' => "Bearer #{jwt}" }

expect(ReleaseTags).to have_received(:create)
.with(Dor::Item, release: true, to: 'searchworks', who: 'carrickr', what: 'self')
Expand All @@ -44,7 +44,7 @@
it 'returns an error' do
post '/v1/objects/druid:1234/release_tags',
params: %( {"to":"searchworks","who":"carrickr","what":"self","release":"seven"} ),
headers: { 'X-Auth' => "Bearer #{jwt}" }
headers: { 'Authorization' => "Bearer #{jwt}" }
expect(response.status).to eq(400)
end
end
Expand All @@ -53,7 +53,7 @@
it 'returns an error' do
post '/v1/objects/druid:1234/release_tags',
params: %( {"to":"searchworks","who":"carrickr","what":"self"} ),
headers: { 'X-Auth' => "Bearer #{jwt}" }
headers: { 'Authorization' => "Bearer #{jwt}" }
expect(response.status).to eq(400)
end
end
Expand Down
15 changes: 14 additions & 1 deletion spec/requests/authorization_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,27 @@
end
end

context 'with a bearer token' do
context 'with a bearer token in the old field' do
let(:payload) { { sub: 'argo' } }
let(:jwt) { JWT.encode(payload, Settings.dor.hmac_secret, 'HS256') }

it 'Logs tokens to honeybadger' do
get '/v1/objects/druid:mk420bs7601/versions/current',
headers: { 'X-Auth' => "Bearer #{jwt}" }
expect(response.body).to eq '5'
expect(Honeybadger).to have_received(:notify).with("Deprecated authentication header 'X-Auth' was provided, but 'Authorization' is expected")
expect(Honeybadger).to have_received(:context).with(invoked_by: 'argo')
end
end

context 'with a bearer token' do
let(:payload) { { sub: 'argo' } }
let(:jwt) { JWT.encode(payload, Settings.dor.hmac_secret, 'HS256') }

it 'Logs tokens to honeybadger' do
get '/v1/objects/druid:mk420bs7601/versions/current',
headers: { 'Authorization' => "Bearer #{jwt}" }
expect(response.body).to eq '5'
expect(Honeybadger).not_to have_received(:notify)
expect(Honeybadger).to have_received(:context).with(invoked_by: 'argo')
end
Expand Down
24 changes: 12 additions & 12 deletions spec/requests/batch_create_virtual_objects_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
it 'creates a virtual object out of the parent object and all child objects' do
post '/v1/virtual_objects',
params: { virtual_objects: [{ parent_id: parent_id, child_ids: [child1_id, child2_id] }] },
headers: { 'X-Auth' => "Bearer #{jwt}" }
headers: { 'Authorization' => "Bearer #{jwt}" }
expect(service).to have_received(:add).with(child_druids: [child1_id, child2_id])
expect(response).to be_successful
end
Expand All @@ -35,7 +35,7 @@
it 'renders an error' do
post '/v1/virtual_objects',
params: { virtual_objects: [{ parent_id: parent_id, child_ids: [child1_id, child2_id] }] },
headers: { 'X-Auth' => "Bearer #{jwt}" }
headers: { 'Authorization' => "Bearer #{jwt}" }
expect(service).to have_received(:add).with(child_druids: [child1_id, child2_id])
expect(response).to be_unprocessable
expect(response.body).to eq '{"errors":[{"druid:mk420bs7601":["versioning is messed up"]}]}'
Expand All @@ -50,7 +50,7 @@
it 'renders an error' do
post '/v1/virtual_objects',
params: { virtual_objects: [{ parent_id: parent_id, child_ids: [child1_id, child2_id] }] },
headers: { 'X-Auth' => "Bearer #{jwt}" }
headers: { 'Authorization' => "Bearer #{jwt}" }
expect(service).to have_received(:add).with(child_druids: [child1_id, child2_id])
expect(response).to be_unprocessable
expect(response.body).to eq '{"errors":[{"druid:mk420bs7601":["Item druid:child2 is not open for modification"]}]}'
Expand All @@ -61,7 +61,7 @@
it 'renders an error' do
post '/v1/virtual_objects',
params: { title: 'New name' },
headers: { 'X-Auth' => "Bearer #{jwt}" }
headers: { 'Authorization' => "Bearer #{jwt}" }
expect(service).not_to have_received(:add)
expect(response).to be_bad_request
json = JSON.parse(response.body)
Expand All @@ -73,7 +73,7 @@
it 'renders an error' do
post '/v1/virtual_objects',
params: { virtual_objects: child1_id },
headers: { 'X-Auth' => "Bearer #{jwt}" }
headers: { 'Authorization' => "Bearer #{jwt}" }
expect(service).not_to have_received(:add)
expect(response).to be_bad_request
json = JSON.parse(response.body)
Expand All @@ -85,7 +85,7 @@
it 'renders an error' do
post '/v1/virtual_objects',
params: { virtual_objects: [] },
headers: { 'X-Auth' => "Bearer #{jwt}" }
headers: { 'Authorization' => "Bearer #{jwt}" }
expect(service).not_to have_received(:add)
expect(response).to be_bad_request
json = JSON.parse(response.body)
Expand All @@ -97,7 +97,7 @@
it 'renders an error' do
post '/v1/virtual_objects',
params: { virtual_objects: [{ child_ids: ['foo'] }] },
headers: { 'X-Auth' => "Bearer #{jwt}" }
headers: { 'Authorization' => "Bearer #{jwt}" }
expect(service).not_to have_received(:add)
expect(response).to be_bad_request
json = JSON.parse(response.body)
Expand All @@ -109,7 +109,7 @@
it 'renders an error' do
post '/v1/virtual_objects',
params: { virtual_objects: [{ parent_id: '' }] },
headers: { 'X-Auth' => "Bearer #{jwt}" }
headers: { 'Authorization' => "Bearer #{jwt}" }
expect(service).not_to have_received(:add)
expect(response).to be_bad_request
json = JSON.parse(response.body)
Expand All @@ -121,7 +121,7 @@
it 'renders an error' do
post '/v1/virtual_objects',
params: { virtual_objects: [{ parent_id: 'foo' }] },
headers: { 'X-Auth' => "Bearer #{jwt}" }
headers: { 'Authorization' => "Bearer #{jwt}" }
expect(service).not_to have_received(:add)
expect(response).to be_bad_request
json = JSON.parse(response.body)
Expand All @@ -133,7 +133,7 @@
it 'renders an error' do
post '/v1/virtual_objects',
params: { virtual_objects: [{ parent_id: 'foo', child_ids: '' }] },
headers: { 'X-Auth' => "Bearer #{jwt}" }
headers: { 'Authorization' => "Bearer #{jwt}" }
expect(service).not_to have_received(:add)
expect(response).to be_bad_request
json = JSON.parse(response.body)
Expand All @@ -145,7 +145,7 @@
it 'renders an error' do
post '/v1/virtual_objects',
params: { virtual_objects: [{ parent_id: 'foo', child_ids: [] }] },
headers: { 'X-Auth' => "Bearer #{jwt}" }
headers: { 'Authorization' => "Bearer #{jwt}" }
expect(service).not_to have_received(:add)
expect(response).to be_bad_request
json = JSON.parse(response.body)
Expand All @@ -157,7 +157,7 @@
it 'renders an error' do
post '/v1/virtual_objects',
params: { virtual_objects: [{ parent_id: 'foo', child_ids: ['foo', 'bar', ''] }] },
headers: { 'X-Auth' => "Bearer #{jwt}" }
headers: { 'Authorization' => "Bearer #{jwt}" }
expect(service).not_to have_received(:add)
expect(response).to be_bad_request
json = JSON.parse(response.body)
Expand Down
2 changes: 1 addition & 1 deletion spec/requests/collections_for_object_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
describe 'as used by WAS crawl seed registration' do
it 'returns (at a minimum) the identifiers of the collections ' do
get '/v1/objects/druid:mk420bs7601/query/collections',
headers: { 'X-Auth' => "Bearer #{jwt}" }
headers: { 'Authorization' => "Bearer #{jwt}" }
expect(response).to be_successful
expect(response.body).to eq '{"collections":[{"externalIdentifier":"druid:999123","type":"collection","label":"collection #1"}]}'
end
Expand Down
8 changes: 4 additions & 4 deletions spec/requests/metadata_refresh_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

it 'updates the metadata and saves the changes' do
post '/v1/objects/druid:mk420bs7601/refresh_metadata',
headers: { 'X-Auth' => "Bearer #{jwt}" }
headers: { 'Authorization' => "Bearer #{jwt}" }
expect(response).to be_successful
expect(RefreshMetadataAction).to have_received(:run).with(object)
expect(object).to have_received(:save)
Expand All @@ -38,7 +38,7 @@

it 'returns a 500 error' do
post '/v1/objects/druid:mk420bs7601/refresh_metadata',
headers: { 'X-Auth' => "Bearer #{jwt}" }
headers: { 'Authorization' => "Bearer #{jwt}" }
expect(response.status).to eq(500)
expect(response.body).to eq('Incomplete response received from Symphony for 666 - expected 0 bytes but got 2')
end
Expand All @@ -51,7 +51,7 @@

it 'returns a 500 error' do
post '/v1/objects/druid:mk420bs7601/refresh_metadata',
headers: { 'X-Auth' => "Bearer #{jwt}" }
headers: { 'Authorization' => "Bearer #{jwt}" }
expect(response.status).to eq(500)
expect(response.body).to eq('Record not found in Symphony: 666')
end
Expand All @@ -75,7 +75,7 @@

it 'returns a 500 error' do
post '/v1/objects/druid:mk420bs7601/refresh_metadata',
headers: { 'X-Auth' => "Bearer #{jwt}" }
headers: { 'Authorization' => "Bearer #{jwt}" }
expect(response.status).to eq(500)
expect(response.body).to match(/^Got HTTP Status-Code 403 retrieving 666 from Symphony:.*Something somewhere went wrong./)
end
Expand Down
4 changes: 2 additions & 2 deletions spec/requests/metadata_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
describe 'dublin core' do
it 'returns the DC xml' do
get '/v1/objects/druid:mk420bs7601/metadata/dublin_core',
headers: { 'X-Auth' => "Bearer #{jwt}" }
headers: { 'Authorization' => "Bearer #{jwt}" }
expect(response).to be_successful
expect(response.body).to include '<dc:title>Hello</dc:title>'
end
Expand All @@ -24,7 +24,7 @@
describe 'descriptive' do
it 'returns the DC xml' do
get '/v1/objects/druid:mk420bs7601/metadata/descriptive',
headers: { 'X-Auth' => "Bearer #{jwt}" }
headers: { 'Authorization' => "Bearer #{jwt}" }
expect(response).to be_successful
expect(response.body).to be_equivalent_to <<~XML
<mods xmlns="http://www.loc.gov/mods/v3" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="3.6" xsi:schemaLocation="http://www.loc.gov/mods/v3 http://www.loc.gov/standards/mods/v3/mods-3-6.xsd">
Expand Down
4 changes: 2 additions & 2 deletions spec/requests/notify_goobi_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
end

it 'notifies goobi of a new registration by making a web service call' do
post '/v1/objects/druid:1234/notify_goobi', headers: { 'X-Auth' => "Bearer #{jwt}" }
post '/v1/objects/druid:1234/notify_goobi', headers: { 'Authorization' => "Bearer #{jwt}" }

expect(response.status).to eq(201)
end
Expand All @@ -36,7 +36,7 @@
end

it 'returns the conflict code' do
post '/v1/objects/druid:1234/notify_goobi', headers: { 'X-Auth' => "Bearer #{jwt}" }
post '/v1/objects/druid:1234/notify_goobi', headers: { 'Authorization' => "Bearer #{jwt}" }
expect(response.status).to eq(409)
expect(response.body).to eq('conflict')
end
Expand Down
4 changes: 2 additions & 2 deletions spec/requests/publish_object_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
end

it 'returns a 409 error with location header' do
post '/v1/objects/druid:1234/publish', headers: { 'X-Auth' => "Bearer #{jwt}" }
post '/v1/objects/druid:1234/publish', headers: { 'Authorization' => "Bearer #{jwt}" }
expect(response.status).to eq(500)
expect(response.body).to eq(error_message)
end
Expand All @@ -31,7 +31,7 @@
end

it 'calls PublishMetadataService and returns 201' do
post '/v1/objects/druid:1234/publish', headers: { 'X-Auth' => "Bearer #{jwt}" }
post '/v1/objects/druid:1234/publish', headers: { 'Authorization' => "Bearer #{jwt}" }

expect(PublishMetadataService).to have_received(:publish)
expect(response.status).to eq(201)
Expand Down
10 changes: 5 additions & 5 deletions spec/requests/register_object_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
end

it 'returns a 409 error with location header' do
post '/v1/objects', headers: { 'X-Auth' => "Bearer #{jwt}" }
post '/v1/objects', headers: { 'Authorization' => "Bearer #{jwt}" }
expect(response.status).to eq(409)
expect(response.headers['Location']).to match(%r{/fedora/objects/druid:existing123obj})
end
Expand All @@ -31,7 +31,7 @@
end

it 'returns a 422 error' do
post '/v1/objects', headers: { 'X-Auth' => "Bearer #{jwt}" }
post '/v1/objects', headers: { 'Authorization' => "Bearer #{jwt}" }
expect(response.status).to eq(422)
expect(response.body).to eq(errmsg)
end
Expand All @@ -45,7 +45,7 @@
end

it 'returns a 500 error' do
post '/v1/objects', headers: { 'X-Auth' => "Bearer #{jwt}" }
post '/v1/objects', headers: { 'Authorization' => "Bearer #{jwt}" }
expect(response.status).to eq(500)
expect(response.body).to eq(errmsg)
end
Expand All @@ -59,7 +59,7 @@
end

it 'returns a 404 error' do
post '/v1/objects', headers: { 'X-Auth' => "Bearer #{jwt}" }
post '/v1/objects', headers: { 'Authorization' => "Bearer #{jwt}" }
expect(response.status).to eq(404)
expect(response.body).to eq(errmsg)
end
Expand All @@ -71,7 +71,7 @@
end

it 'registers the object with the registration service' do
post '/v1/objects', headers: { 'X-Auth' => "Bearer #{jwt}" }
post '/v1/objects', headers: { 'Authorization' => "Bearer #{jwt}" }

expect(response.body).to eq 'druid:xyz'
expect(RegistrationService).to have_received(:create_from_request)
Expand Down
2 changes: 1 addition & 1 deletion spec/requests/show_object_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

it 'returns the object' do
get '/v1/objects/druid:mk420bs7601',
headers: { 'X-Auth' => "Bearer #{jwt}" }
headers: { 'Authorization' => "Bearer #{jwt}" }
expect(response).to be_successful
expect(response.body).to eq '{"externalIdentifier":"druid:1234","type":"object","label":"foo"}'
end
Expand Down
8 changes: 4 additions & 4 deletions spec/requests/update_embargo_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
context 'without the :embargo_date param' do
it 'returns HTTP 400' do
patch '/v1/objects/druid:mk420bs7601/embargo',
headers: { 'X-Auth' => "Bearer #{jwt}" }
headers: { 'Authorization' => "Bearer #{jwt}" }
expect(response.status).to eq(400)
expect(response.body).to eq('{"errors":[{"title":"bad request","detail":"param is missing or the value is empty: embargo_date"}]}')
end
Expand All @@ -27,7 +27,7 @@
it 'returns HTTP 400' do
patch '/v1/objects/druid:mk420bs7601/embargo',
params: { embargo_date: '2100-01-01' },
headers: { 'X-Auth' => "Bearer #{jwt}" }
headers: { 'Authorization' => "Bearer #{jwt}" }
expect(events_datastream).not_to have_received(:add_event)
expect(response.status).to eq(400)
expect(response.body).to eq('{"errors":[{"title":"bad request","detail":"param is missing or the value is empty: requesting_user"}]}')
Expand All @@ -45,7 +45,7 @@
it 'hits the Dor::EmbargoService and returns HTTP 422' do
patch '/v1/objects/druid:mk420bs7601/embargo',
params: { embargo_date: '2100-01-01', requesting_user: 'mjg' },
headers: { 'X-Auth' => "Bearer #{jwt}" }
headers: { 'Authorization' => "Bearer #{jwt}" }
expect(mock_embargo_service).to have_received(:update).once
expect(events_datastream).not_to have_received(:add_event)
expect(response.status).to eq(422)
Expand All @@ -62,7 +62,7 @@
it 'hits the Dor::EmbargoService and returns HTTP 204' do
patch '/v1/objects/druid:mk420bs7601/embargo',
params: { embargo_date: '2100-01-01', requesting_user: 'mjg' },
headers: { 'X-Auth' => "Bearer #{jwt}" }
headers: { 'Authorization' => "Bearer #{jwt}" }
expect(mock_embargo_service).to have_received(:update).with(Date.parse('2100-01-01'))
expect(events_datastream).to have_received(:add_event).with(
'Embargo',
Expand Down

0 comments on commit 4ab67a2

Please sign in to comment.