Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 35 additions & 4 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,42 @@ def flash
protected

def after_sign_in_path_for(resource)
if resource.judge?
judge_dashboard_path
else
root_path
# SAML POSTs from the IdP often arrive without the Rails session cookie
# (SameSite). Prefer RelayState / omniauth.origin, which survive that round-trip.
saml_preserved_return_path ||
safe_internal_redirect_path(stored_location_for(resource)) ||
default_sign_in_path_for(resource)
end

def default_sign_in_path_for(resource)
resource.judge? ? judge_dashboard_path : root_path
end

def saml_preserved_return_path
safe_internal_redirect_path(request.params['RelayState']) ||
safe_internal_redirect_path(request.env['omniauth.origin'])
end

def safe_internal_redirect_path(path)
return if path.blank?

uri = URI.parse(path)
if uri.host.present?
return unless uri.host == request.host

path = [ uri.path, uri.query ].compact.join('?')
end

return unless path.start_with?('/') && !path.start_with?('//')

# Homepage / login origins are not meaningful return destinations; fall
# through to role-based defaults (e.g. judges → judge dashboard).
path_only = path.split('?', 2).first
return if path_only.blank? || path_only == '/' || path_only == root_path

path
rescue URI::InvalidURIError
nil
end

def after_sign_out_path_for(resource_or_scope)
Expand Down
9 changes: 9 additions & 0 deletions app/controllers/concerns/contest_invite_session.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ module ContestInviteSession
extend ActiveSupport::Concern

REDEEMED_SESSION_KEY = :redeemed_contest_instances
PENDING_SESSION_KEY = :pending_contest_invite_token

included do
before_action :load_redeemed_contest_instances_into_current
Expand All @@ -21,6 +22,14 @@ def redeem_contest_instance!(contest_instance)
Current.redeemed_contest_instances = session[REDEEMED_SESSION_KEY]
end

def remember_pending_contest_invite!(contest_instance)
session[PENDING_SESSION_KEY] = contest_instance.access_token
end

def consume_pending_contest_invite_token
session.delete(PENDING_SESSION_KEY)
end

def redeemed_token_for(contest_instance)
Current.redeemed_contest_instances&.dig(contest_instance.id.to_s)
end
Expand Down
16 changes: 13 additions & 3 deletions app/controllers/contest_invites_controller.rb
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
# frozen_string_literal: true

class ContestInvitesController < ApplicationController
skip_before_action :authenticate_user!, only: :show

def show
@contest_instance = ContestInstance.find_by!(access_token: params[:token])

unless current_user.profile
redirect_to new_profile_path, alert: 'Please create your profile before accessing this contest.'
unless user_signed_in?
store_location_for(:user, contest_invite_path(token: params[:token]))
redirect_to root_path, alert: 'Please log in to access this contest.'
return
end

Expand All @@ -14,6 +17,12 @@ def show
return
end

unless current_user.profile
remember_pending_contest_invite!(@contest_instance)
redirect_to new_profile_path, alert: 'Please create your profile before accessing this contest.'
return
end

redeem_contest_instance!(@contest_instance)

unless @contest_instance.open?
Expand All @@ -30,6 +39,7 @@ def show
redirect_to new_entry_path(contest_instance_id: @contest_instance.id),
notice: "Welcome to #{@contest_instance.contest_description.name}."
rescue ActiveRecord::RecordNotFound
redirect_to applicant_dashboard_path, alert: 'Invalid or expired contest invite link.'
redirect_to user_signed_in? ? applicant_dashboard_path : root_path,
alert: 'Invalid or expired contest invite link.'
end
end
8 changes: 7 additions & 1 deletion app/controllers/profiles_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def create

respond_to do |format|
if @profile.save
format.html { redirect_to applicant_dashboard_path, notice: 'Profile was successfully created.' }
format.html { redirect_to profile_created_redirect_path, notice: 'Profile was successfully created.' }
else
format.html { render :new, status: :unprocessable_entity }
end
Expand Down Expand Up @@ -78,6 +78,12 @@ def set_profile
def authorize_profile
authorize @profile
end

def profile_created_redirect_path
token = consume_pending_contest_invite_token
token.present? ? contest_invite_path(token: token) : applicant_dashboard_path
end

# Only allow a list of trusted parameters through.
def profile_params
params.require(:profile).permit(:user_id, :umid, :preferred_first_name, :preferred_last_name, :class_level_id, :school_id, :campus_id,
Expand Down
6 changes: 6 additions & 0 deletions app/helpers/application_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ def redirect_back_or_default(default: root_url)
redirect_to(session.delete(:return_to) || default, anchor: 'top')
end

# Origin passed to SAML so RelayState can restore private-contest (and other) deep links.
def saml_authorize_params
origin = session['user_return_to'].presence
origin ? { origin: origin } : {}
end

def boolean_to_yes_no(value)
if value
content_tag(:span, 'Yes', class: 'text-success fw-bold')
Expand Down
5 changes: 4 additions & 1 deletion app/views/devise/shared/_links.html.erb
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@

<%- if devise_mapping.omniauthable? %>
<%- resource_class.omniauth_providers.each do |provider| %>
<%= button_to "Sign in with U-M account", omniauth_authorize_path(resource_name, provider), class: "btn btn-warning btn-sm mb-4", data: { turbo: false } %><br />
<%= button_to "Sign in with U-M account", omniauth_authorize_path(resource_name, provider),
params: saml_authorize_params,
class: "btn btn-warning btn-sm mb-4",
data: { turbo: false } %><br />
<% end %>
<% end %>
5 changes: 4 additions & 1 deletion app/views/static_pages/home.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@
<% end %>
</div>
<div class="d-grid w-25 ms-6 mt-6 mb-8">
<%= button_to "Login to LSA Evaluate", user_saml_omniauth_authorize_path, class: "btn btn-warning btn-sm mb-4", data: { turbo: "false" } %>
<%= button_to "Login to LSA Evaluate", user_saml_omniauth_authorize_path,
params: saml_authorize_params,
class: "btn btn-warning btn-sm mb-4",
data: { turbo: "false" } %>
</div>
</div>
</div>
Expand Down
3 changes: 3 additions & 0 deletions config/initializers/devise.rb
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,9 @@
person_affiliation: [ 'urn:oid:1.3.6.1.4.1.5923.1.1.1.1' ],
principal_name: [ 'urn:oid:1.3.6.1.4.1.5923.1.1.1.6' ] },
request_attributes: {},
# Carry post-login destination through the IdP via SAML RelayState.
# Session-based Devise return paths are often lost on the ACS POST.
idp_sso_service_url_runtime_params: { origin: :RelayState },
idp_cert_fingerprint: idp_fingerprint,
idp_cert_fingerprint_algorithm: 'http://www.w3.org/2000/09/xmldsig#sha256',
allowed_clock_drift: 10,
Expand Down
56 changes: 56 additions & 0 deletions spec/controllers/application_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# frozen_string_literal: true

require 'rails_helper'

RSpec.describe ApplicationController, type: :controller do
controller do
def index
head :ok
end
end

describe '#after_sign_in_path_for' do
let(:judge) { create(:user, :with_judge_role) }

it 'uses SAML RelayState before the role-based default' do
allow(controller.request).to receive(:params).and_return(
ActionController::Parameters.new('RelayState' => '/c/private-contest-token')
)

expect(controller.send(:after_sign_in_path_for, judge)).to eq('/c/private-contest-token')
end

it 'uses omniauth.origin when RelayState is absent' do
request.env['omniauth.origin'] = '/c/from-omniauth-origin'

expect(controller.send(:after_sign_in_path_for, judge)).to eq('/c/from-omniauth-origin')
end

it 'uses the stored destination before the role-based default' do
session['user_return_to'] = '/c/private-contest-token'

expect(controller.send(:after_sign_in_path_for, judge)).to eq('/c/private-contest-token')
end

it 'rejects external RelayState values' do
allow(controller.request).to receive(:params).and_return(
ActionController::Parameters.new('RelayState' => 'https://evil.example/phish')
)

expect(controller.send(:after_sign_in_path_for, judge)).to eq(judge_dashboard_path)
end

it 'ignores homepage RelayState so judges still land on the dashboard' do
allow(controller.request).to receive(:params).and_return(
ActionController::Parameters.new('RelayState' => '/')
)
request.env['omniauth.origin'] = '/'

expect(controller.send(:after_sign_in_path_for, judge)).to eq(judge_dashboard_path)
end

it 'uses the role-based default without a stored destination' do
expect(controller.send(:after_sign_in_path_for, judge)).to eq(judge_dashboard_path)
end
end
end
15 changes: 15 additions & 0 deletions spec/controllers/contest_invites_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,20 @@
end
end

context 'when the user is not signed in' do
let(:contest_instance) { create(:contest_instance) }

before { sign_out user }

it 'stores the invite path and redirects to login' do
get :show, params: { token: contest_instance.access_token }

expect(response).to redirect_to(root_path)
expect(session['user_return_to']).to eq(contest_invite_path(token: contest_instance.access_token))
expect(flash[:alert]).to match(/log in/i)
end
end

context 'when the user has no profile' do
let(:user_without_profile) { create(:user) }
let(:contest_instance) { create(:contest_instance) }
Expand All @@ -74,6 +88,7 @@
get :show, params: { token: contest_instance.access_token }

expect(response).to redirect_to(new_profile_path)
expect(session[:pending_contest_invite_token]).to eq(contest_instance.access_token)
end
end
end
Expand Down
34 changes: 34 additions & 0 deletions spec/controllers/profiles_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# frozen_string_literal: true

require 'rails_helper'

RSpec.describe ProfilesController, type: :controller do
let(:user) { create(:user) }
let(:contest_instance) { create(:contest_instance) }
let(:profile_attributes) do
attributes_for(:profile).merge(
class_level_id: create(:class_level).id,
school_id: create(:school).id,
campus_id: create(:campus).id
)
end

before { sign_in user }

describe 'POST #create' do
it 'returns to a pending contest invite after creating the profile' do
session[:pending_contest_invite_token] = contest_instance.access_token

post :create, params: { profile: profile_attributes }

expect(response).to redirect_to(contest_invite_path(token: contest_instance.access_token))
expect(session[:pending_contest_invite_token]).to be_nil
end

it 'uses the applicant dashboard when there is no pending invite' do
post :create, params: { profile: profile_attributes }

expect(response).to redirect_to(applicant_dashboard_path)
end
end
end
Loading