Skip to content
Open
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
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ gem 'rails', '7.2.2.2'

gem 'active_model_serializers'
gem 'activerecord-session_store'
gem 'addressable'
gem 'ahoy_matey'
gem 'auto_strip_attributes'
gem 'bootsnap', require: false
Expand Down
1 change: 1 addition & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -825,6 +825,7 @@ PLATFORMS
DEPENDENCIES
active_model_serializers
activerecord-session_store
addressable
ahoy_matey
auto_strip_attributes
better_errors
Expand Down
13 changes: 9 additions & 4 deletions app/controllers/callbacks_controller.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# The controller for callback actions
class CallbacksController < Devise::OmniauthCallbacksController
include SpaceRedirect

Devise.omniauth_configs.each do |provider, config|
define_method(provider) do
Expand All @@ -11,6 +12,9 @@ class CallbacksController < Devise::OmniauthCallbacksController

def handle_callback(provider, config)
@user = User.from_omniauth(request.env["omniauth.auth"])
if request.env['omniauth.params'] && request.env['omniauth.params']['space_id']
space = Space.find_by_id(request.env['omniauth.params']['space_id'])
end

if @user.new_record?
# new user
Expand All @@ -27,14 +31,15 @@ def handle_callback(provider, config)

sign_in @user
flash[:notice] = "#{I18n.t('devise.registrations.signed_up')} Please ensure your profile is correct."
redirect_to edit_user_path(@user)
redirect_to_space(edit_user_path(@user), space)
rescue Exception => e
flash[:notice] = "Login failed: #{e.message.to_s}"
redirect_to new_user_session_path
redirect_to_space(new_user_session_path, space)
end
else
sign_in_and_redirect @user
scope = Devise::Mapping.find_scope!(@user)
sign_in(scope, resource, {})
redirect_to_space(after_sign_in_path_for(@user), space)
end
end

end
16 changes: 16 additions & 0 deletions app/controllers/concerns/space_redirect.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
module SpaceRedirect
extend ActiveSupport::Concern

private

def redirect_to_space(path, space)
if space&.is_subdomain?
port_part = ''
port_part = ":#{request.port}" if (request.protocol == "http://" && request.port != 80) ||
(request.protocol == "https://" && request.port != 443)
redirect_to URI.join("#{request.protocol}#{space.host}#{port_part}", path).to_s, allow_other_host: true
else
redirect_to path
end
end
end
13 changes: 10 additions & 3 deletions app/controllers/orcid_controller.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
class OrcidController < ApplicationController
include SpaceRedirect

before_action :orcid_auth_enabled
before_action :authenticate_user!
before_action :set_oauth_client, only: [:authenticate, :callback]
Expand All @@ -9,12 +11,17 @@ class OrcidController < ApplicationController
end

def authenticate
redirect_to @oauth2_client.authorization_uri(scope: '/authenticate'), allow_other_host: true
params = Space.current_space&.default? ? {} : { state: "space_id:#{Space.current_space.id}" }
redirect_to @oauth2_client.authorization_uri(scope: '/authenticate', **params), allow_other_host: true
end

def callback
@oauth2_client.authorization_code = params[:code]
token = Rack::OAuth2::AccessToken::Bearer.new(access_token: @oauth2_client.access_token!)
if params[:state].present?
m = params[:state].match(/space_id:(\d+)/)
space = Space.find_by_id(m[1]) if m
end
orcid = token.access_token&.raw_attributes['orcid']
respond_to do |format|
profile = current_user.profile
Expand All @@ -27,7 +34,7 @@ def callback
else
flash[:error] = t('orcid.authentication_failure')
end
format.html { redirect_to current_user }
format.html { redirect_to_space(user_path(current_user), space) }
end
end

Expand All @@ -38,7 +45,7 @@ def set_oauth_client
@oauth2_client ||= Rack::OAuth2::Client.new(
identifier: config[:client_id],
secret: config[:secret],
redirect_uri: config[:redirect_uri].presence || orcid_callback_url,
redirect_uri: config[:redirect_uri].presence || orcid_callback_url(host: TeSS::Config.base_uri.host),
authorization_endpoint: '/oauth/authorize',
token_endpoint: '/oauth/token',
host: config[:host].presence || (Rails.env.production? ? 'orcid.org' : 'sandbox.orcid.org')
Expand Down
35 changes: 35 additions & 0 deletions app/controllers/tess_devise/sessions_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
class TessDevise::SessionsController < Devise::SessionsController

def create
clear_legacy_cookie

super
end

def destroy
super

clear_legacy_cookie
end

private

def clear_legacy_cookie
# Clean up legacy host-only session cookie
key = Rails.application.config.session_options[:key]
append_set_cookie("#{key}=; path=/; Max-Age=0; HttpOnly; SameSite=Lax")
end

def append_set_cookie(value)
existing = response.headers['Set-Cookie']

case existing
when nil
response.headers['Set-Cookie'] = value
when String
response.headers['Set-Cookie'] = [existing, value]
when Array
existing << value
end
end
end
3 changes: 2 additions & 1 deletion app/helpers/application_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -700,11 +700,12 @@ def theme_path
end

def omniauth_login_link(provider, config)
params = Space.current_space&.default? ? {} : { space_id: Space.current_space.id }
link_to(
t('authentication.omniauth.log_in_with',
provider: config.options[:label] ||
t("authentication.omniauth.providers.#{provider}", default: provider.to_s.titleize)),
omniauth_authorize_path('user', provider),
omniauth_authorize_path('user', provider, **params),
method: :post
)
end
Expand Down
4 changes: 4 additions & 0 deletions app/helpers/spaces_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,8 @@ def space_feature_options
[t("features.#{f}.short"), f]
end
end

def space_supports_omniauth?(space = current_space)
space.nil? || space.default? || space.is_subdomain?(TeSS::Config.base_uri.domain)
end
end
8 changes: 8 additions & 0 deletions app/models/space.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ class Space < ApplicationRecord
has_many :administrator_roles, -> { where(key: :admin) }, class_name: 'SpaceRole'
has_many :administrators, through: :administrator_roles, source: :user, class_name: 'User'

auto_strip_attributes :title, :description, :host

validates :title, presence: true
validates :host, presence: true, uniqueness: true, format: /\A[a-zA-Z0-9\-]+(\.[a-zA-Z0-9\-]+)*\z/i
validates :theme, inclusion: { in: TeSS::Config.themes.keys, allow_blank: true }
validate :disabled_features_valid?

Expand Down Expand Up @@ -65,6 +69,10 @@ def enabled_features
(FEATURES - disabled_features)
end

def is_subdomain?(domain = TeSS::Config.base_uri.domain)
(host == domain || host.ends_with?(".#{domain}"))
end

private

def disabled_features_valid?
Expand Down
6 changes: 4 additions & 2 deletions app/views/layouts/_login_menu.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@
<strong>Log In</strong> <span class="caret"></span>
</a>
<ul class="dropdown-menu dropdown-menu-right">
<% Devise.omniauth_configs.each do |provider, config| -%>
<li class="dropdown-item"><%= omniauth_login_link(provider, config) %></li>
<% if space_supports_omniauth? %>
<% Devise.omniauth_configs.each do |provider, config| -%>
<li class="dropdown-item"><%= omniauth_login_link(provider, config) %></li>
<% end %>
<% end %>

<li class="dropdown-item">
Expand Down
5 changes: 4 additions & 1 deletion app/views/users/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,10 @@
<% else %>
<%= orcid_link(@user.profile) %>
<% end %>
<% if TeSS::Config.orcid_authentication_enabled? && current_user == @user && !@user.profile.orcid_authenticated? %>
<% if TeSS::Config.orcid_authentication_enabled? &&
current_user == @user &&
!@user.profile.orcid_authenticated? &&
space_supports_omniauth? %>
<%= button_to t(@user.profile.orcid.blank? ? 'orcid.link' : 'orcid.authenticate'), authenticate_orcid_path, class: 'btn btn-default' %>
<% end %>
</p>
Expand Down
4 changes: 4 additions & 0 deletions config/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,10 @@ def orcid_authentication_enabled?
Rails.application.config.secrets.orcid[:client_id].present? &&
Rails.application.config.secrets.orcid[:secret].present?
end

def base_uri
@base_uri ||= Addressable::URI.parse(base_url)
end
end

Config = TessConfig.new(tess_config)
Expand Down
6 changes: 4 additions & 2 deletions config/initializers/session_store.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
# Be sure to restart your server when you modify this file.
opts = {}
opts = {
domain: :all
}

Comment on lines +2 to 5
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The session cookie is configured with domain: :all, which relies on Rails’ action_dispatch.tld_length to compute the parent domain correctly. This app doesn’t appear to set tld_length, so deployments on multi-part TLDs (e.g. example.co.uk) can result in an invalid Domain attribute (cookie rejected) and users being unexpectedly logged out across spaces. Consider setting an explicit cookie domain derived from TeSS::Config.base_uri.domain (when present) or configuring action_dispatch.tld_length accordingly.

Suggested change
opts = {
domain: :all
}
opts = {}
if defined?(TeSS::Config) && TeSS::Config.respond_to?(:base_uri) && TeSS::Config.base_uri
base_uri = TeSS::Config.base_uri
cookie_domain =
if base_uri.respond_to?(:domain)
base_uri.domain
elsif base_uri.respond_to?(:host)
base_uri.host
end
opts[:domain] = cookie_domain if cookie_domain.present?
end

Copilot uses AI. Check for mistakes.
if Rails.env.production?
opts = { same_site: :lax, secure: true }
opts.merge!(same_site: :lax, secure: true)
expiry_time = TeSS::Config.login_expires_after
opts[:expire_after] = expiry_time unless expiry_time.blank?
end
Expand Down
7 changes: 4 additions & 3 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,10 @@
# to happen when precompiling assets in the docker build script.
unless Rake.try(:application)&.top_level_tasks&.include? 'assets:precompile'
devise_for :users, :controllers => {
:registrations => 'tess_devise/registrations',
:invitations => 'tess_devise/invitations',
:omniauth_callbacks => 'callbacks'
registrations: 'tess_devise/registrations',
invitations: 'tess_devise/invitations',
sessions: 'tess_devise/sessions',
omniauth_callbacks: 'callbacks'
}
end
#Redirect to users index page after devise user account update
Expand Down
74 changes: 74 additions & 0 deletions test/controllers/orcid_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,23 @@ class OrcidControllerTest < ActionController::TestCase
post :authenticate

assert_redirected_to /https:\/\/sandbox\.orcid\.org\/oauth\/authorize\?.+/
params = Rack::Utils.parse_query(URI.parse(response.location).query)
assert_equal "#{TeSS::Config.base_url}/orcid/callback", params['redirect_uri']
assert_nil params['state']
end

test 'authenticating orcid in space uses root app redirect URI and sets space state' do
plant_space = spaces(:plants)
with_host(plant_space.host) do
sign_in users(:regular_user)

post :authenticate

assert_redirected_to /https:\/\/sandbox\.orcid\.org\/oauth\/authorize\?.+/
params = Rack::Utils.parse_query(URI.parse(response.location).query)
assert_equal "#{TeSS::Config.base_url}/orcid/callback", params['redirect_uri']
assert_equal "space_id:#{plant_space.id}", params['state']
end
end

test 'do not authenticate orcid if user not logged-in' do
Expand Down Expand Up @@ -148,4 +165,61 @@ class OrcidControllerTest < ActionController::TestCase
end
end
end

test 'redirect to subdomain space in callback' do
space = spaces(:astro)
space.update!(host: 'space.example.com')
mock_images
user = users(:regular_user)
assert user.profile.orcid.blank?
sign_in user

VCR.use_cassette('orcid/get_token_free_orcid') do
get :callback, params: { code: '123xyz', state: "space_id:#{space.id}" }
end

profile = user.profile.reload
assert_equal '0009-0006-0987-5702', profile.orcid
assert profile.orcid_authenticated?
assert_redirected_to user_url(user, host: 'space.example.com')
assert response.headers['Location'].starts_with?('http://space.example.com/users/')
assert flash[:error].blank?
end

test 'do not redirect to non-subdomain space in callback' do
space = spaces(:astro)
space.update!(host: 'space.golf.com')
mock_images
user = users(:regular_user)
assert user.profile.orcid.blank?
sign_in user

VCR.use_cassette('orcid/get_token_free_orcid') do
get :callback, params: { code: '123xyz', state: "space_id:#{space.id}" }
end

profile = user.profile.reload
assert_equal '0009-0006-0987-5702', profile.orcid
assert profile.orcid_authenticated?
assert_redirected_to user
refute response.headers['Location'].starts_with?('http://space.golf.com/users/')
assert flash[:error].blank?
end

test 'ignore bad space when redirecting in callback' do
mock_images
user = users(:regular_user)
assert user.profile.orcid.blank?
sign_in user

VCR.use_cassette('orcid/get_token_free_orcid') do
get :callback, params: { code: '123xyz', state: "space_id:banana🍌" }
end

profile = user.profile.reload
assert_equal '0009-0006-0987-5702', profile.orcid
assert profile.orcid_authenticated?
assert_redirected_to user
assert flash[:error].blank?
end
end
22 changes: 22 additions & 0 deletions test/controllers/static_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -712,4 +712,26 @@ class StaticControllerTest < ActionController::TestCase
end
end
end

test 'show omniauth login options when on subdomain space' do
space = spaces(:plants)
space.update!(host: 'plants.example.com')
with_host(space.host) do
get :home
assert_select 'ul.user-options.nav.navbar-nav.navbar-right' do
assert_select "a[href=\"/users/auth/oidc?space_id=#{space.id}\"]", count: 1
end
end
end

test 'do not show omniauth login options when on non-subdomain space' do
space = spaces(:plants)
space.update!(host: 'other-host.com')
with_host(space.host) do
get :home
assert_select 'ul.user-options.nav.navbar-nav.navbar-right' do
assert_select "a[href=\"/users/auth/oidc?space_id=#{space.id}\"]", count: 0
end
end
end
end
Loading