Allow specific session keys to bypass reset_session (#63)

Adds `session_keys_to_persist` config option in order to carry over
specific session keys from pre-log in to post-log in

(since `reset_session` is called post-log in in order to prevent session
fixation:
https://guides.rubyonrails.org/security.html#session-fixation-countermeasures)

Author: grega

CHANGELOG.md

@@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+- Adds `session_keys_to_persist` config option to allow for specific session keys to be persisted across logins (since logging in will reset the session: https://guides.rubyonrails.org/security.html#session-fixation-countermeasures)
+
 ## [v3.4.0]
 
 - Removes `v1_signup` param as it is no longer required (https://github.com/RaspberryPiFoundation/profile/pull/1512)

README.md

@@ -33,6 +33,7 @@ RpiAuth.configure do |config|
   config.identity_url = 'http://localhost:3002'        # The url for the profile instance being used for auth
   config.user_model = 'User'                           # The name of the user model in the host app being used, use the name as a string, not the model itself
   config.scope = 'openid email profile force-consent'  # The required OIDC scopes
+  config.session_keys_to_persist = 'foo bar'           # Optional: any session keys to persist across sessions (as the session is reset upon log in)
   config.success_redirect = '/'                        # After succesful login the route the user should be redirected to; this will override redirecting the user back to where they were when they started the log in / sign up flow (via `omniauth.origin`), so should be used rarely / with caution.  This can be a string or a proc, which is executed in the context of the RpiAuth::AuthController.
   config.bypass_auth = false                           # Should auth be bypassed and a default user logged in
 end
@@ -130,7 +131,7 @@ registration form, then you should supply a parameter called `returnTo` which
 is then used to redirect after log in/sign up are successful.
 
 ```ruby
-button_to 'Log in to start registraion', rpi_auth_login_path, params: { returnTo: '/registration_form' }
+button_to 'Log in to start registration', rpi_auth_login_path, params: { returnTo: '/registration_form' }
 ```
 
 If this parameter is missing [Omniauth uses the HTTP Referer

app/controllers/rpi_auth/auth_controller.rb

@@ -10,9 +10,20 @@ class AuthController < ActionController::Base
 
     def callback
       # Prevent session fixation. If the session has been initialized before
-      # this, and we need to keep the data, then we should copy values over.
+      # this, and certain data needs to be persisted, then the client should
+      # pass the keys via config.session_keys_to_persist
+      old_session = session.to_hash
+
       reset_session
 
+      keys_to_persist = RpiAuth.configuration.session_keys_to_persist
+
+      unless keys_to_persist.nil? || keys_to_persist.empty?
+        keys_to_persist.split.each do |key|
+          session[key] = old_session[key]
+        end
+      end
+
       auth = request.env['omniauth.auth']
       self.current_user = RpiAuth.user_model.from_omniauth(auth)
 

lib/rpi_auth/configuration.rb

@@ -16,6 +16,7 @@ class Configuration
                   :identity_url,
                   :response_type,
                   :scope,
+                  :session_keys_to_persist,
                   :success_redirect,
                   :user_model
 

spec/dummy/app/controllers/sessions_controller.rb

@@ -0,0 +1,10 @@
+# Used to modify session variables within request tests
+class SessionsController < ApplicationController
+  def create
+    vars = params.permit(session_vars: {})
+    vars[:session_vars].each do |var, value|
+      session[var] = value
+    end
+    head :created
+  end
+end

spec/dummy/config/initializers/rpi_auth.rb

@@ -6,6 +6,7 @@
   config.brand = 'codeclub'
   config.host_url = 'http://localhost:3009'
   config.identity_url = 'http://localhost:3002'
+  config.session_keys_to_persist = 'foo bar'
   config.user_model = 'User'
 
   # Redurect to the next URL

spec/dummy/config/routes.rb

@@ -2,6 +2,8 @@
   # For details on the DSL available within this file, see https://guides.rubyonrails.org/routing.html
   root to: 'home#show'
 
+  resource :session, only: %i[create]
+
   # Make sure we don't match auth routes
   get '/*slug', to: 'home#show', constraints: { slug: /(?!(rpi_)?auth\/).*/ }
 end

spec/dummy/spec/requests/auth_request_spec.rb

@@ -17,6 +17,7 @@
 
   let(:bypass_auth) { false }
   let(:identity_url) { 'https://my.example.com' }
+  let(:session_keys_to_persist) {}
   # We need to make sure we match the hostname Rails uses in test requests
   # here, otherwise `returnTo` redirects will fail after login/logout.
   let(:host_url) { 'https://www.example.com' }
@@ -29,6 +30,7 @@
     # This would normally be in the initializer, but because we're toggling the
     # option on or off, we need to explicitly call it here.
     RpiAuth.configuration.enable_auth_bypass
+    RpiAuth.configuration.session_keys_to_persist = session_keys_to_persist
     OmniAuth.config.test_mode = true
   end
 
@@ -180,6 +182,21 @@
         expect(session.id).not_to eq previous_id
       end
 
+      context 'when session_keys_to_persist is set' do
+        let(:session_keys_to_persist) { 'foo' }
+
+        it 'persists provided session keys on login' do
+          set_session(foo: 'bar')
+          post '/auth/rpi'
+          previous_foo = session[:foo]
+
+          expect(response).to redirect_to('/rpi_auth/auth/callback')
+          follow_redirect!
+
+          expect(session[:foo]).to eq previous_foo
+        end
+      end
+
       context 'when having visited a page first' do
         it 'redirects back to the original page' do
           post '/auth/rpi', headers: { Referer: 'http://www.example.com/foo#foo' }

spec/support/request_helpers.rb

@@ -13,4 +13,13 @@ def sign_in(user)
     post '/auth/rpi'
     follow_redirect!
   end
+
+  def set_session(vars = {})
+    post session_path, params: { session_vars: vars }
+    expect(response).to have_http_status(:created)
+
+    vars.each_key do |var|
+      expect(session[var]).to be_present
+    end
+  end
 end