Skip to content
Merged
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
13 changes: 12 additions & 1 deletion app/controllers/rpi_auth/auth_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
1 change: 1 addition & 0 deletions lib/rpi_auth/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ class Configuration
:identity_url,
:response_type,
:scope,
:session_keys_to_persist,
:success_redirect,
:user_model

Expand Down
10 changes: 10 additions & 0 deletions spec/dummy/app/controllers/sessions_controller.rb
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions spec/dummy/config/initializers/rpi_auth.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions spec/dummy/config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
17 changes: 17 additions & 0 deletions spec/dummy/spec/requests/auth_request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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' }
Expand All @@ -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

Expand Down Expand Up @@ -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' }
Expand Down
9 changes: 9 additions & 0 deletions spec/support/request_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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