Merge pull request #391 from 18F/feature/audit_auth_failure

Feature/audit auth failure

app/controllers/omniauth_callbacks_controller.rb

@@ -3,13 +3,35 @@ def google_oauth2
     auth = request.env['omniauth.auth']
 
     if user = User.find_from_omniauth(auth)
-      sign_in_and_redirect user, omniauth: true
+      log_success(user)
+      sign_in_and_redirect user
     elsif user = User.create_from_omniauth(auth)
-      sign_in user, omniauth: true
+      log_success(user)
+      sign_in user
       redirect_to new_mobile_recovery_path
     else
+      log_failure(user)
       flash.alert = 'Unable to connect with Google'
       redirect_to new_user_session_path
     end
   end
+
+  def failure
+    log_failure
+    super
+  end
+
+  private
+
+  def provider
+    params[:provider]
+  end
+
+  def log_success(user)
+    UserAction.successful_authentication.create(user: user, data: { authentication_method: provider })
+  end
+
+  def log_failure(user=nil)
+    UserAction.failed_authentication.create(data: { authentication_method: provider })
+  end
 end

app/models/authentication_token.rb

@@ -12,6 +12,8 @@ class AuthenticationToken < ActiveRecord::Base
   attr_accessor :raw
 
   def self.authenticate(user, raw)
+    return nil unless user.present?
+
     digested = Devise.token_generator.digest(self, :token, raw)
     token = user.authentication_tokens.find_by_token(digested)
 

app/models/mobile_confirmation.rb

@@ -16,10 +16,15 @@ def authenticate(raw_token)
     if Devise.secure_compare(digested, self.token)
       self.token = nil
       self.confirm!
+      true
+    else
+      UserAction.failed_authentication.create(data: { authentication_method: 'sms' })
+      false
     end
   end
 
   def confirm!
+    UserAction.successful_authentication.create(data: { authentication_method: 'sms' })
     self.confirmed_at = Time.now
     save!
   end

app/models/user_action.rb

@@ -1,4 +1,10 @@
 class UserAction < ActiveRecord::Base
   belongs_to :user
   belongs_to :record, polymorphic: true
+  serialize :data, JSON
+
+  scope :for, ->(user) { where(user_id: user.id) }
+
+  scope :successful_authentication, -> { where(action: 'successful_authentication') }
+  scope :failed_authentication, -> { where(action: 'failed_authentication') }
 end

config/initializers/audit.rb

@@ -4,10 +4,6 @@
 
 ActionController::Base.around_filter Audit::UserAction::Sweeper.instance
 
-Warden::Manager.after_set_user except: :fetch do |user, auth, opts|
-  ::UserAction.create(action: 'sign_in', user: user)
-end
-
 Warden::Manager.before_logout do |user, auth, opts|
-  ::UserAction.create(action: 'sign_out', user: user)
+  ::UserAction.create(action: 'log_out', user: user)
 end

config/initializers/devise.rb

@@ -233,7 +233,6 @@
   #
   config.warden do |manager|
     manager.failure_app = FailureApp
-    manager.default_strategies(scope: :user).unshift :email_authenticatable
   end
 
   # ==> Mountable engine configurations
@@ -257,7 +256,7 @@
 Warden::Manager.after_fetch do |user, auth, opts|
   request = Rack::Request.new(auth.env)
   params = request.params
-  
+
   if auth.authenticated? && request.get? && !!params["logout"]
     auth.logout
   end
@@ -266,8 +265,8 @@
 Warden::Manager.before_failure do |env, opts|
   uri = URI(opts[:attempted_path])
   params = Rack::Utils.parse_query(uri.query)
-  params.delete('logout')
-  uri.query = params.empty? ? nil : params.to_param
-
-  opts[:attempted_path] = uri.to_s
+  if params.delete('logout')
+    uri.query = params.empty? ? nil : params.to_param
+    opts[:attempted_path] = uri.to_s
+  end
 end

db/migrate/20141001194534_add_data_to_user_action.rb

@@ -0,0 +1,5 @@
+class AddDataToUserAction < ActiveRecord::Migration
+  def change
+    add_column :user_actions, :data, :text
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20140928181620) do
+ActiveRecord::Schema.define(version: 20141001194534) do
 
   create_table "authentication_tokens", force: true do |t|
     t.integer  "user_id"
@@ -180,6 +180,7 @@
     t.string   "action"
     t.string   "remote_ip"
     t.datetime "created_at"
+    t.text     "data"
   end
 
   add_index "user_actions", ["record_id", "record_type"], name: "index_user_actions_on_record_id_and_record_type", using: :btree

lib/email_authenticatable.rb

@@ -22,18 +22,25 @@ def valid?
       def authenticate!
         user = params[:email].present? && User.find_by_email(params[:email])
 
-        @token = AuthenticationToken.authenticate(user, params[:token])
-        if validate(user) { @token.present? }
+        if validate(user) { @token = AuthenticationToken.authenticate(user, params[:token]) }
           session['user_return_to'] = @token.return_to if @token.return_to.present?
           success!(user)
-        else
-          fail!(:invalid_token)
         end
       end
 
       def remember_me?
         !!@token.remember_me
       end
+
+      def success!(user)
+        super
+        UserAction.successful_authentication.create(user: user, data: { authentication_method: 'email' })
+      end
+
+      def fail!(*args)
+        super
+        UserAction.failed_authentication.create(user: user, data: { authentication_method: 'email', message: @message })
+      end
     end
   end
 end

spec/controllers/mobile_recoveries_controller_spec.rb

@@ -48,17 +48,33 @@
 
         expect(confirmation).to be_confirmed
       end
+
+      it 'creates successful_authentication user action' do
+        confirmation = user.profile.create_mobile_confirmation
+        confirmation.send(:generate_token)
+        raw_token = confirmation.raw_token
+        confirmation.save!
+
+        expect { patch :update, mobile_confirmation: { raw_token: raw_token } }.to change(UserAction.successful_authentication, :count).by(1)
+      end
     end
 
     context 'with an invalid token' do
       it 'does not confirm the mobile number' do
-        confirmation = user.profile.create_mobile_confirmation!
+        confirmation = user.profile.create_mobile_confirmation
+
         patch :update, mobile_confirmation: { raw_token: 'foobar' }
         confirmation.reload
 
         expect(flash[:error]).to match(/Please check the number sent to your mobile and re-enter that code/)
         expect(confirmation).to_not be_confirmed
       end
+
+      it 'creates successful_authentication user action' do
+        confirmation = user.profile.create_mobile_confirmation
+
+        expect { patch :update, mobile_confirmation: { raw_token: 'foobar' } }.to change(UserAction.failed_authentication, :count).by(1)
+      end
     end
   end
 

spec/features/devise/sessions/sign_in_spec.rb

@@ -67,25 +67,35 @@
       OmniAuth.config.mock_auth[omniauth_provider] = omniauth_hash
     end
 
+    # make sure a user with this email exists
     before :each, create_user: true do
       FactoryGirl.create(:user, email: email)
     end
 
+    # trick our authentication libraries into failing
+    before :each, authentication_failure: true do
+      allow(AuthenticationToken).to receive(:authenticate) { false }
+      OmniAuth.config.mock_auth[omniauth_provider] = :invalid_credentials
+    end
+
     shared_examples 'sign in' do
       before :each do
         perform_login!
       end
 
-      it 'allows user to authenticate and redirects' do
-        expect(redirect_page).to be_displayed
-      end
-
       it 'allows user to navigate directly to protected pages' do
         target_page.load
         expect(target_page).to be_displayed
       end
     end
 
+    shared_examples 'sign in and redirect' do
+      include_examples 'sign in'
+      it 'allows user to authenticate and redirects' do
+        expect(redirect_page).to be_displayed
+      end
+    end
+
     shared_examples 'sending token' do
       before :each do
         submit_form
@@ -214,28 +224,29 @@ def perform_login!
       context 'for the first time' do
         context 'with email' do
           include_context 'with email'
-          it_behaves_like 'sign in'
+          it_behaves_like 'sign in and redirect'
           it_behaves_like 'sending token'
           it_behaves_like 'remember me'
         end
 
         context 'with google' do
           include_context 'with google'
+          it_behaves_like 'sign in'
           it_behaves_like 'mobile recovery'
         end
       end
 
       context 'with existing user', create_user: true do
         context 'with email' do
           include_context 'with email'
-          it_behaves_like 'sign in'
+          it_behaves_like 'sign in and redirect'
           it_behaves_like 'sending token'
           it_behaves_like 'remember me'
         end
 
         context 'with google' do
           include_context 'with google'
-          it_behaves_like 'sign in'
+          it_behaves_like 'sign in and redirect'
         end
       end
     end