diff --git a/app/controllers/users/registrations_controller.rb b/app/controllers/users/registrations_controller.rb index 51595cc..e156406 100644 --- a/app/controllers/users/registrations_controller.rb +++ b/app/controllers/users/registrations_controller.rb @@ -5,41 +5,44 @@ module Users class RegistrationsController < Devise::RegistrationsController layout 'devise' - # Sign up with Castle risk assessment. + # Sign up with Castle filtering. A registration is anonymous activity, so the + # attempt is filtered before the account is created. # @note A 'challenge' verdict is treated as 'allow' here; a real app would - # step up to MFA. 'deny' rolls the registration back. + # step up to MFA. 'deny' blocks the sign-up before the account is created. def create build_resource(sign_up_params) - if resource.save - if evaluate_registration(resource) == 'deny' - resource.destroy - flash[:error] = t('.access_denied') - redirect_to new_user_registration_url - else - sign_up(resource_name, resource) - set_flash_message! :notice, :signed_up - respond_with resource, location: after_sign_up_path_for(resource) - end - else + unless resource.valid? track_failed_registration clean_up_passwords resource set_minimum_password_length respond_with resource + return end + + if evaluate_registration_attempt == 'deny' + flash[:error] = t('.access_denied') + redirect_to new_user_registration_url + return + end + + resource.save + sign_up(resource_name, resource) + set_flash_message! :notice, :signed_up + respond_with resource, location: after_sign_up_path_for(resource) end private - # Sends a successful registration to the risk endpoint and returns the verdict. - # @param user [User] + # Filters the registration attempt while the visitor is still anonymous, + # before the account is created (so the email goes in params). # @return [String] the Castle policy action: 'allow', 'challenge' or 'deny' - def evaluate_registration(user) - castle.risk( + def evaluate_registration_attempt + castle.filter( type: '$registration', - status: '$succeeded', + status: '$attempted', request_token: castle_request_token, - user: { id: user.id, email: user.email } + params: { email: resource.email } ).dig(:policy, :action) rescue Castle::Error # Never block a sign-up because Castle is unhappy with the request. @@ -47,17 +50,20 @@ def evaluate_registration(user) end # Reports an invalid registration attempt (e.g. an email already taken) to - # the filter endpoint. + # the filter endpoint, resolving any existing user via matching_user_id. def track_failed_registration email = sign_up_params[:email] + matching_user = User.find_by(email: email) - castle.filter( + options = { type: '$registration', status: '$failed', request_token: castle_request_token, - user: { email: email }, params: { email: email } - ) + } + options[:matching_user_id] = matching_user.id if matching_user + + castle.filter(**options) rescue Castle::Error nil end diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index adb0858..6247111 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -8,10 +8,18 @@ class SessionsController < Devise::SessionsController # Key that is used in Devise for user authentication AUTHENTICATION_KEY = 'email' - # Sign in with Castle risk assessment. + # Sign in with Castle. The attempt is filtered first while the visitor is + # still anonymous; a successful login is then risk-assessed, reusing the + # same request token. # @note A 'challenge' verdict is treated as 'allow' here; a real app would # step up to MFA. 'deny' blocks the login. def create + if filter_login_attempt == 'deny' + flash[:error] = t('.access_denied') + redirect_to new_user_session_url + return + end + if warden.authenticate(auth_options) if evaluate_login(current_user) == 'deny' warden.logout @@ -43,20 +51,40 @@ def destroy private - # Takes the request form data (login and password) and tries to find the user for which the - # authentication process failed and reports a failed login to the filter endpoint. + # The submitted login email (anonymous form data, before authentication). + def login_email + params.dig(:user, AUTHENTICATION_KEY) + end + + # Filters the login attempt while the visitor is still anonymous, before the + # credentials are checked (so the email goes in params). + # @return [String] the Castle policy action: 'allow', 'challenge' or 'deny' + def filter_login_attempt + castle.filter( + type: '$login', + status: '$attempted', + request_token: castle_request_token, + params: { email: login_email } + ).dig(:policy, :action) + rescue Castle::Error + 'allow' + end + + # Reports a failed login to the filter endpoint, resolving any existing user + # via matching_user_id. def track_failed_login - user_params = params.fetch('user') { {} }.except(*Rails.application.config.filter_parameters) - email = user_params[AUTHENTICATION_KEY] + email = login_email user = User.find_by(AUTHENTICATION_KEY => email) - castle.filter( + options = { type: '$login', status: '$failed', request_token: castle_request_token, - user: { id: user&.id, email: email }, params: { email: email } - ) + } + options[:matching_user_id] = user.id if user + + castle.filter(**options) rescue Castle::Error nil end diff --git a/app/views/main/index.html.haml b/app/views/main/index.html.haml index 3f24349..02c2326 100644 --- a/app/views/main/index.html.haml +++ b/app/views/main/index.html.haml @@ -13,16 +13,20 @@ %ul.prose-list.list-disc.pl-5 %li %strong Sign up - — new registrations are scored with the - %code risk - endpoint ($registration); a denied verdict rolls the sign-up back. + — registrations are filtered before the account is created + ($registration / $attempted); a denied verdict blocks the + sign-up, and an invalid attempt (e.g. an email already taken) is reported to + %code filter + as $failed. %li %strong Login - — successful logins are scored with the + — the attempt is filtered first ($login / $attempted), + then a successful login is scored with the %code risk - endpoint; failed logins go to + endpoint ($succeeded) while a wrong password or unknown user + goes to %code filter - and the verdict can allow, challenge or deny the user. + ($failed); the verdict can allow, challenge or deny the user. %li %strong Logout, profile updates, custom events & password reset — recorded with the non-blocking diff --git a/spec/controllers/users/registrations_controller_spec.rb b/spec/controllers/users/registrations_controller_spec.rb index ca1776f..acbcff4 100644 --- a/spec/controllers/users/registrations_controller_spec.rb +++ b/spec/controllers/users/registrations_controller_spec.rb @@ -18,7 +18,7 @@ end context 'when the registration is allowed' do - before { allow(controller.castle).to receive(:risk).and_return(policy: { action: 'allow' }) } + before { allow(controller.castle).to receive(:filter).and_return(policy: { action: 'allow' }) } it 'creates a new user' do expect { post :create, params: params }.to change(User, :count).by(1) @@ -31,22 +31,22 @@ expect(response).to redirect_to root_path end - it 'scores the registration with the risk endpoint' do + it 'filters the registration attempt before creating the account' do post :create, params: params - expect(controller.castle).to have_received(:risk).with( + expect(controller.castle).to have_received(:filter).with( type: '$registration', - status: '$succeeded', + status: '$attempted', request_token: nil, - user: hash_including(email: email) + params: { email: email } ) end end context 'when the registration is denied' do - before { allow(controller.castle).to receive(:risk).and_return(policy: { action: 'deny' }) } + before { allow(controller.castle).to receive(:filter).and_return(policy: { action: 'deny' }) } - it 'rolls the registration back' do + it 'does not create the account' do expect { post :create, params: params }.not_to change(User, :count) end @@ -57,8 +57,8 @@ end end - context 'when Castle raises during risk assessment' do - before { allow(controller.castle).to receive(:risk).and_raise(Castle::Error) } + context 'when Castle raises while filtering the attempt' do + before { allow(controller.castle).to receive(:filter).and_raise(Castle::Error) } it 'fails open and keeps the user' do expect { post :create, params: params }.to change(User, :count).by(1) diff --git a/spec/controllers/users/sessions_controller_spec.rb b/spec/controllers/users/sessions_controller_spec.rb index 257df22..86b565f 100644 --- a/spec/controllers/users/sessions_controller_spec.rb +++ b/spec/controllers/users/sessions_controller_spec.rb @@ -24,7 +24,8 @@ before do # Since the expectations are handled after the redirect for invalid, we don't have a way # to reference the "future" castle object, so we have to stub all the instances - allow_any_instance_of(controller.castle.class).to receive(:filter) + allow_any_instance_of(controller.castle.class) + .to receive(:filter).and_return(policy: { action: 'allow' }) post :create, params: { user: { email: user.email, password: rand.to_s } } end @@ -42,7 +43,27 @@ end end + context 'when the attempt is filtered out' do + before do + allow(controller.castle).to receive(:filter).and_return(policy: { action: 'deny' }) + allow(controller.castle).to receive(:risk) + post :create, params: { user: { email: user.email, password: password } } + end + + it { expect(response).to redirect_to new_user_session_path } + it { expect(flash['error']).to eq I18n.t('users.sessions.create.access_denied') } + it { expect(controller.castle).not_to have_received(:risk) } + end + context 'when login succeeded' do + let(:filter_args) do + { + type: '$login', + status: '$attempted', + request_token: nil, + params: { email: user.email } + } + end let(:risk_args) do { type: '$login', @@ -53,6 +74,7 @@ end before do + allow(controller.castle).to receive(:filter).and_return(policy: { action: 'allow' }) allow(controller.castle).to receive(:risk).and_return(verdict) post :create, params: { user: { email: user.email, password: password } } end @@ -61,6 +83,7 @@ let(:verdict) { { policy: { action: 'allow' } } } it { expect(response).to redirect_to root_path } + it { expect(controller.castle).to have_received(:filter).with(filter_args) } it { expect(controller.castle).to have_received(:risk).with(risk_args) } end @@ -83,6 +106,7 @@ context 'when Castle raises during risk assessment' do before do + allow(controller.castle).to receive(:filter).and_return(policy: { action: 'allow' }) allow(controller.castle).to receive(:risk).and_raise(Castle::Error) post :create, params: { user: { email: user.email, password: password } } end