Add optional on_login_success callback (#90)

This adds a new `on_login_success` configuration option. If this option
is set to a `Proc` and the login was successful, the `Proc` will be
called in the context of the `RpiAuth::AuthController#callback` action,
i.e. `current_user` will be available. This is intended to allow apps to
record successful logins.

I'm not convinced that using `instance_exec` is the most sensible
approach, but I decided it was better to be consistent with the way the
`success_redirect` option is implemented.

If the `Proc` raises an exception, this will be rescued and the
exception will be logged as a warning in the Rails log. This is so that
the login will still complete successfully, even if there was a problem
recording it.

The immediate motivation for adding this is so we can record successful
*student* logins in `experience-cs` in order to be able to calculate the
"active users" metric described in [this issue][1]. Unfortunately while
this information is available in the `profile` app database for regular
users; it's not available for student users.

[1]: https://github.com/RaspberryPiFoundation/experience-cs/issues/727

Author: floehopper

CHANGELOG.md

@@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
+- Add optional `on_login_success` callback (#90)
+
 ### Fixed
 
 ### Removed

README.md

@@ -170,6 +170,10 @@ link_to 'Log out', rpi_auth_logout_path, params: { returnTo: '/thanks-dude' }
 
 This has to be a relative URL, i.e. it has to start with a slash.  This is to ensure there's no open redirect.
 
+### Callback on successful login
+
+If the RpiAuth configuration option `on_login_success` is set to a `Proc`, this will be called in the context of the `RpiAuth::AuthController#callback` action, i.e. `current_user` will be available. This is intended to allow apps to record successful logins.
+
 ### Globbed/catch-all routes
 
 If your app has a catch-all route at the end of the routing table, you must

app/controllers/rpi_auth/auth_controller.rb

@@ -27,6 +27,8 @@ def callback
       auth = request.env['omniauth.auth']
       self.current_user = RpiAuth.user_model.from_omniauth(auth)
 
+      run_login_success_callback
+
       redirect_to ensure_relative_url(login_redirect_path)
     end
 
@@ -53,6 +55,16 @@ def failure
 
     private
 
+    def run_login_success_callback
+      return unless RpiAuth.configuration.on_login_success.is_a?(Proc)
+
+      begin
+        instance_exec(&RpiAuth.configuration.on_login_success)
+      rescue StandardError => e
+        Rails.logger.warn("Caught #{e} while processing on_login_success proc.")
+      end
+    end
+
     def login_redirect_path
       unless RpiAuth.configuration.success_redirect.is_a?(Proc)
         return RpiAuth.configuration.success_redirect || request.env['omniauth.origin']

lib/rpi_auth/configuration.rb

@@ -19,7 +19,8 @@ class Configuration
                   :session_keys_to_persist,
                   :success_redirect,
                   :user_model,
-                  :setup
+                  :setup,
+                  :on_login_success
 
     def initialize
       @bypass_auth = false

spec/dummy/spec/requests/auth_request_spec.rb

@@ -2,6 +2,11 @@
 
 require 'spec_helper'
 
+class Foo
+  cattr_accessor :successful_logins
+  self.successful_logins = []
+end
+
 RSpec.describe 'Authentication' do
   let(:user) do
     User.new(
@@ -311,6 +316,42 @@
           end
         end
       end
+
+      context 'when on_login_success is set as a proc in config' do
+        before do
+          allow(Rails.logger).to receive(:warn).and_call_original
+
+          RpiAuth.configuration.on_login_success = lambda do
+            Rails.logger.warn "Successful login: #{current_user.user_id}"
+          end
+        end
+
+        it 'calls the proc making the current user available' do
+          post '/auth/rpi'
+          follow_redirect!
+
+          expect(Rails.logger).to have_received(:warn).with(
+            "Successful login: #{user.user_id}"
+          )
+        end
+
+        context 'when the proc raises an exception' do
+          before do
+            RpiAuth.configuration.on_login_success = lambda do
+              raise 'boom!'
+            end
+          end
+
+          it 'logs a warning error' do
+            post '/auth/rpi'
+            follow_redirect!
+
+            expect(Rails.logger).to have_received(:warn).with(
+              'Caught boom! while processing on_login_success proc.'
+            )
+          end
+        end
+      end
     end
 
     describe 'and toggling the scope at runtime' do

spec/rpi_auth/configuration_spec.rb

@@ -110,4 +110,8 @@
       it_behaves_like 'sets up the token url defaults'
     end
   end
+
+  it 'sets on_login_success to nil by default' do
+    expect(configuration.on_login_success).to be_nil
+  end
 end