Evaluate registration and login as anonymous-then-authenticated flows

app/controllers/users/registrations_controller.rb

@@ -5,59 +5,65 @@ 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.
       'allow'
     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

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

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 (<code>$registration</code>); a denied verdict rolls the sign-up back.
+    &mdash; registrations are filtered before the account is created
+    (<code>$registration / $attempted</code>); a denied verdict blocks the
+    sign-up, and an invalid attempt (e.g. an email already taken) is reported to
+    %code filter
+    as <code>$failed</code>.
   %li
     %strong Login
-    &mdash; successful logins are scored with the
+    &mdash; the attempt is filtered first (<code>$login / $attempted</code>),
+    then a successful login is scored with the
     %code risk
-    endpoint; failed logins go to
+    endpoint (<code>$succeeded</code>) while a wrong password or unknown user
+    goes to
     %code filter
-    and the verdict can allow, challenge or deny the user.
+    (<code>$failed</code>); the verdict can allow, challenge or deny the user.
   %li
     %strong Logout, profile updates, custom events & password reset
     &mdash; recorded with the non-blocking

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)

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