Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow specific session keys to bypass reset_session #63

Merged
merged 14 commits into from
Jan 17, 2024
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)
grega marked this conversation as resolved.
Show resolved Hide resolved
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
Loading