diff --git a/app/controllers/api/member_services_controller.rb b/app/controllers/api/member_services_controller.rb index 540088623..a65a47944 100644 --- a/app/controllers/api/member_services_controller.rb +++ b/app/controllers/api/member_services_controller.rb @@ -44,6 +44,8 @@ def authenticate_member_services return end + verify_nonce(nonce) + validator = Api::HMACSignatureValidator.new( secret: Settings.member_services_secret, signature: signature, @@ -56,4 +58,13 @@ def authenticate_member_services return end end + + def verify_nonce(nonce) + if Authentication::Nonce.exists?(nonce: nonce) + render json: { errors: 'The nonce has already been used.' }, status: :unauthorized + nil + else + Authentication::Nonce.create!(nonce: nonce) + end + end end diff --git a/app/models/authentication/nonce.rb b/app/models/authentication/nonce.rb new file mode 100644 index 000000000..67daa1427 --- /dev/null +++ b/app/models/authentication/nonce.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +class Authentication::Nonce < ApplicationRecord + self.table_name = 'authentication_nonces' + validates :nonce, presence: true, uniqueness: true +end diff --git a/db/migrate/20171212093329_create_authentication_nonces.rb b/db/migrate/20171212093329_create_authentication_nonces.rb new file mode 100644 index 000000000..c108282b9 --- /dev/null +++ b/db/migrate/20171212093329_create_authentication_nonces.rb @@ -0,0 +1,9 @@ +class CreateAuthenticationNonces < ActiveRecord::Migration[5.1] + def change + create_table :authentication_nonces do |t| + t.string :nonce + t.timestamps + t.index :nonce + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 7b009d283..2eea5b482 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20171023193604) do +ActiveRecord::Schema.define(version: 20171212093329) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -63,6 +63,13 @@ t.datetime "updated_at", null: false end + create_table "authentication_nonces", force: :cascade do |t| + t.string "nonce" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["nonce"], name: "index_authentication_nonces_on_nonce" + end + create_table "calls", id: :serial, force: :cascade do |t| t.integer "page_id" t.integer "member_id" diff --git a/spec/requests/api/member_services/headers_spec.rb b/spec/requests/api/member_services/headers_spec.rb new file mode 100644 index 000000000..97f5cb881 --- /dev/null +++ b/spec/requests/api/member_services/headers_spec.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe 'API::MemberServices' do + context 'with valid auth headers' do + let(:valid_headers) do + { + 'X-CHAMPAIGN-SIGNATURE' => '2d39dea4bc00ceff1ec1fdf160540400f673e97474b1d197d240b084bd186d34', + 'X-CHAMPAIGN-NONCE' => 'd7b82ede-17f2-4e79-8377-0ad1a1dd8621' + } + end + + let!(:donation) do + create(:payment_braintree_subscription, + subscription_id: 'BraintreeWoohoo', + cancelled_at: nil) + end + + it 'sends back 200 and persists the nonce' do + expect(Authentication::Nonce.exists?(nonce: 'd7b82ede-17f2-4e79-8377-0ad1a1dd8621')).to be false + delete '/api/member_services/recurring_donations/braintree/BraintreeWoohoo', headers: valid_headers + expect(response.status).to eq 200 + expect(Authentication::Nonce.exists?(nonce: 'd7b82ede-17f2-4e79-8377-0ad1a1dd8621')).to be true + end + end + + context 'with invalid auth headers' do + let(:bogus_header) do + { + 'X-CHAMPAIGN-SIGNATURE' => 'olololololo', + 'X-CHAMPAIGN-NONCE' => 'wololo' + } + end + + it 'logs an access violation and sends back status 401' do + error_string = 'Access violation for member services API.' + expect(Rails.logger).to receive(:error).with(error_string) + delete '/api/member_services/recurring_donations/braintree/BraintreeWoohoo', headers: bogus_header + expect(response.status).to eq 401 + expect(response.body).to include('Invalid authentication header') + end + end + + context 'with missing auth headers' do + it 'sends back 401 complains about missing auth headers' do + delete '/api/member_services/recurring_donations/braintree/BraintreeWoohoo' + expect(response.status).to eq 401 + expect(response.body).to include('Missing authentication header or nonce.') + end + end + + context 'with a used nonce' do + let(:valid_headers) do + { + 'X-CHAMPAIGN-SIGNATURE' => '2d39dea4bc00ceff1ec1fdf160540400f673e97474b1d197d240b084bd186d34', + 'X-CHAMPAIGN-NONCE' => 'd7b82ede-17f2-4e79-8377-0ad1a1dd8621' + } + end + + it 'sends back 401 and complains about a nonce that has already been used' do + Authentication::Nonce.create!(nonce: 'd7b82ede-17f2-4e79-8377-0ad1a1dd8621') + delete '/api/member_services/recurring_donations/braintree/BraintreeWoohoo', headers: valid_headers + expect(response.status).to eq 401 + expect(response.body).to include('The nonce has already been used.') + end + end +end diff --git a/spec/requests/api/member_services/invalid_headers_spec.rb b/spec/requests/api/member_services/invalid_headers_spec.rb deleted file mode 100644 index 8a09bdf4a..000000000 --- a/spec/requests/api/member_services/invalid_headers_spec.rb +++ /dev/null @@ -1,30 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -describe 'API::MemberServices' do - context 'with invalid auth headers' do - let(:bogus_header) do - { - 'X-CHAMPAIGN-SIGNATURE' => 'olololololo', - 'X-CHAMPAIGN-NONCE' => 'wololo' - } - end - - it 'logs an access violation and sends back status 401' do - error_string = 'Access violation for member services API.' - expect(Rails.logger).to receive(:error).with(error_string) - delete '/api/member_services/recurring_donations/braintree/BraintreeWoohoo', headers: bogus_header - expect(response.status).to eq 401 - expect(response.body).to include('Invalid authentication header') - end - end - - context 'with missing auth headers' do - it 'complains about missing auth headers' do - delete '/api/member_services/recurring_donations/braintree/BraintreeWoohoo' - expect(response.status).to eq 401 - expect(response.body).to include('Missing authentication header or nonce.') - end - end -end diff --git a/spec/requests/api/member_services/members_spec.rb b/spec/requests/api/member_services/members_spec.rb index 16c9e727b..c67b51a7a 100644 --- a/spec/requests/api/member_services/members_spec.rb +++ b/spec/requests/api/member_services/members_spec.rb @@ -21,7 +21,6 @@ more: '{}') end - # TODO: Move out auth headers specs from recurring_donations_spec.rb into its own file. context 'with valid auth headers' do let(:valid_headers) do {