Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LG-13937 Validate secret token for socure DocV webhook #11118

Merged
merged 15 commits into from
Aug 26, 2024
33 changes: 32 additions & 1 deletion app/controllers/socure_webhook_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,37 @@ class SocureWebhookController < ApplicationController
skip_before_action :verify_authenticity_token

def create
render json: { message: 'Got here.' }
if token_valid?
render json: { message: 'Secret token is valid.' }
else
render status: :unauthorized, json: { message: 'Invalid secret token.' }
end
Comment on lines +7 to +11
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will we be doing more validation in the future? Should we start doing form validation pattern?

Suggested change
if token_valid?
render json: { message: 'Secret token is valid.' }
else
render status: :unauthorized, json: { message: 'Invalid secret token.' }
end
form = SocureWebhookValidationForm.new
result = form.submit(headers: request.headers)
if result.success?
render json: { message: 'Secret token is valid.' }
else
render status: :unauthorized, json: { message: result.first_error_message }
end

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we will be; my understanding is that we'll use the webhook just for status updates on submitted requests, and the secret token is just an anti-ddos measure.

Certainly, if we do end up using information from the webhook, then we should follow the form validation pattern we already have.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, the actual PII data from Socure will be acquired through a separate API call to them.

end

private

def token_valid?
authorization_header = request.headers['Authorization']&.split&.last

return false if authorization_header.nil?

verify_current_key(authorization_header: authorization_header) ||
verify_queue(authorization_header: authorization_header)
end

def verify_current_key(authorization_header:)
ActiveSupport::SecurityUtils.secure_compare(
authorization_header,
IdentityConfig.store.socure_webhook_secret_key,
)
end

def verify_queue(authorization_header:)
IdentityConfig.store.socure_webhook_secret_key_queue.any? do |key|
ActiveSupport::SecurityUtils.secure_compare(
authorization_header,
key,
)
end
end
end
6 changes: 6 additions & 0 deletions config/application.yml.default
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,8 @@ sign_in_user_id_per_ip_attempt_window_exponential_factor: 1.1
sign_in_user_id_per_ip_attempt_window_in_minutes: 720
sign_in_user_id_per_ip_attempt_window_max_minutes: 43_200
sign_in_user_id_per_ip_max_attempts: 50
socure_webhook_secret_key: ''
socure_webhook_secret_key_queue: '[]'
sp_handoff_bounce_max_seconds: 2
sp_issuer_user_counts_report_configs: '[]'
team_ada_email: ''
Expand Down Expand Up @@ -425,6 +427,8 @@ development:
show_unsupported_passkey_platform_authentication_setup: true
sign_in_recaptcha_score_threshold: 0.3
skip_encryption_allowed_list: '["urn:gov:gsa:SAML:2.0.profiles:sp:sso:localhost"]'
socure_webhook_secret_key: 'secret-key'
socure_webhook_secret_key_queue: '["old-key-one", "old-key-two"]'
state_tracking_enabled: true
telephony_adapter: test
use_dashboard_service_providers: true
Expand Down Expand Up @@ -549,6 +553,8 @@ test:
session_encryption_key: 27bad3c25711099429c1afdfd1890910f3b59f5a4faec1c85e945cb8b02b02f261ba501d99cfbb4fab394e0102de6fecf8ffe260f322f610db3e96b2a775c120
short_term_phone_otp_max_attempts: 100
skip_encryption_allowed_list: '[]'
socure_webhook_secret_key: 'secret-key'
socure_webhook_secret_key_queue: '["old-key-one", "old-key-two"]'
state_tracking_enabled: true
team_ada_email: 'ada@example.com'
team_all_login_emails: '["b@example.com", "c@example.com"]'
Expand Down
2 changes: 2 additions & 0 deletions lib/identity_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,8 @@ def self.store
config.add(:sign_in_user_id_per_ip_max_attempts, type: :integer)
config.add(:sign_in_recaptcha_score_threshold, type: :float)
config.add(:skip_encryption_allowed_list, type: :json)
config.add(:socure_webhook_secret_key, type: :string)
config.add(:socure_webhook_secret_key_queue, type: :json)
config.add(:sp_handoff_bounce_max_seconds, type: :integer)
config.add(:sp_issuer_user_counts_report_configs, type: :json)
config.add(:state_tracking_enabled, type: :boolean)
Expand Down
34 changes: 33 additions & 1 deletion spec/controllers/socure_webhook_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,41 @@

RSpec.describe SocureWebhookController do
describe 'POST /api/webhooks/socure/event' do
it 'returns OK' do
let(:socure_secret_key) { 'this-is-a-secret' }
let(:socure_secret_key_queue) { ['this-is-an-old-secret', 'this-is-an-older-secret'] }

before do
allow(IdentityConfig.store).to receive(:socure_webhook_secret_key).
and_return(socure_secret_key)
allow(IdentityConfig.store).to receive(:socure_webhook_secret_key_queue).
and_return(socure_secret_key_queue)
end

it 'returns OK with a correct secret key' do
request.headers['Authorization'] = socure_secret_key
post :create

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

it 'returns OK with an older secret key' do
request.headers['Authorization'] = socure_secret_key_queue.last
post :create

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

it 'returns unauthorized with a bad secret key' do
request.headers['Authorization'] = 'ABC123'
post :create

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

it 'returns unauthorized with no secret key' do
post :create

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