diff --git a/CHANGELOG.md b/CHANGELOG.md index 1794344..16ff96a 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/README.md b/README.md index 8bf588a..741ac8d 100644 --- a/README.md +++ b/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 diff --git a/app/controllers/rpi_auth/auth_controller.rb b/app/controllers/rpi_auth/auth_controller.rb index 9026361..25d1cee 100644 --- a/app/controllers/rpi_auth/auth_controller.rb +++ b/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) diff --git a/lib/rpi_auth/configuration.rb b/lib/rpi_auth/configuration.rb index 2df8a37..79f4310 100644 --- a/lib/rpi_auth/configuration.rb +++ b/lib/rpi_auth/configuration.rb @@ -16,6 +16,7 @@ class Configuration :identity_url, :response_type, :scope, + :session_keys_to_persist, :success_redirect, :user_model diff --git a/spec/dummy/app/controllers/sessions_controller.rb b/spec/dummy/app/controllers/sessions_controller.rb new file mode 100644 index 0000000..69a1654 --- /dev/null +++ b/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 diff --git a/spec/dummy/config/initializers/rpi_auth.rb b/spec/dummy/config/initializers/rpi_auth.rb index d8aefd5..d3355c4 100644 --- a/spec/dummy/config/initializers/rpi_auth.rb +++ b/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 diff --git a/spec/dummy/config/routes.rb b/spec/dummy/config/routes.rb index b2236aa..2fff33c 100644 --- a/spec/dummy/config/routes.rb +++ b/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 diff --git a/spec/dummy/spec/requests/auth_request_spec.rb b/spec/dummy/spec/requests/auth_request_spec.rb index f4ec330..5eae3b2 100644 --- a/spec/dummy/spec/requests/auth_request_spec.rb +++ b/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' } diff --git a/spec/support/request_helpers.rb b/spec/support/request_helpers.rb index ff22e24..6356a53 100644 --- a/spec/support/request_helpers.rb +++ b/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