Cross-space authentication

Gemfile

@@ -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

Gemfile.lock

@@ -825,6 +825,7 @@ PLATFORMS
 DEPENDENCIES
   active_model_serializers
   activerecord-session_store
+  addressable
   ahoy_matey
   auto_strip_attributes
   better_errors

app/controllers/callbacks_controller.rb

@@ -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
@@ -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
@@ -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

app/controllers/concerns/space_redirect.rb

@@ -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

app/controllers/orcid_controller.rb

@@ -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]
@@ -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
@@ -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
 
@@ -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')

app/controllers/tess_devise/sessions_controller.rb

@@ -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

app/helpers/application_helper.rb

@@ -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

app/helpers/spaces_helper.rb

@@ -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

app/models/space.rb

@@ -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?
 
@@ -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?

app/views/layouts/_login_menu.html.erb

@@ -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">

app/views/users/show.html.erb

@@ -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>

config/application.rb

@@ -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)

config/initializers/session_store.rb

@@ -1,8 +1,10 @@
 # Be sure to restart your server when you modify this file.
-opts = {}
+opts = {
+  domain: :all
+}
 
 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

config/routes.rb

@@ -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

test/controllers/orcid_controller_test.rb

@@ -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
@@ -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

test/controllers/static_controller_test.rb

@@ -712,7 +712,7 @@ class StaticControllerTest < ActionController::TestCase
       end
     end
   end
-
+        
   test 'disabled features do not show as counters on space front page' do
     space = spaces(:plants)
     space.disabled_features = ['materials']
@@ -739,4 +739,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