diff --git a/Gemfile b/Gemfile index 1bdeee2f3..07b487b1f 100644 --- a/Gemfile +++ b/Gemfile @@ -39,6 +39,7 @@ gem 'oai' gem 'omniauth_openid_connect' gem 'pg' gem 'private_address_check' +gem 'public_suffix' gem 'public_activity' gem 'pundit' gem 'rack-cors', require: 'rack/cors' diff --git a/Gemfile.lock b/Gemfile.lock index 84e4be48b..4cf2c6e7f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -363,7 +363,8 @@ GEM loofah (2.25.2) crass (~> 1.0.2) nokogiri (>= 1.12.0) - mail (2.8.1) + mail (2.9.1) + logger mini_mime (>= 0.1.1) net-imap net-pop @@ -402,7 +403,7 @@ GEM uri (>= 0.11.1) net-http-persistent (4.0.8) connection_pool (>= 2.2.4, < 4) - net-imap (0.6.4.1) + net-imap (0.6.6) date net-protocol net-pop (0.1.2) @@ -904,6 +905,7 @@ DEPENDENCIES private_address_check pry-byebug public_activity + public_suffix puma pundit rack-cors diff --git a/app/controllers/concerns/space_redirect.rb b/app/controllers/concerns/space_redirect.rb index 24f6783e8..5dab181be 100644 --- a/app/controllers/concerns/space_redirect.rb +++ b/app/controllers/concerns/space_redirect.rb @@ -4,7 +4,7 @@ module SpaceRedirect private def redirect_to_space(path, space) - if space&.is_subdomain? + if space&.valid_login_domain?(request.host) port_part = '' port_part = ":#{request.port}" if (request.protocol == "http://" && request.port != 80) || (request.protocol == "https://" && request.port != 443) diff --git a/app/controllers/orcid_controller.rb b/app/controllers/orcid_controller.rb index a0332dbce..c8a40e514 100644 --- a/app/controllers/orcid_controller.rb +++ b/app/controllers/orcid_controller.rb @@ -42,10 +42,12 @@ def callback def set_oauth_client config = Rails.application.config.secrets.orcid + redirect_uris = Array(config[:redirect_uri].presence || orcid_callback_url(host: TeSS::Config.base_uri.host)) + @oauth2_client ||= Rack::OAuth2::Client.new( identifier: config[:client_id], secret: config[:secret], - redirect_uri: config[:redirect_uri].presence || orcid_callback_url(host: TeSS::Config.base_uri.host), + redirect_uri: TessOmniauthRedirectUris.resolve_for_host(redirect_uris, request.host), authorization_endpoint: '/oauth/authorize', token_endpoint: '/oauth/token', host: config[:host].presence || (Rails.env.production? ? 'orcid.org' : 'sandbox.orcid.org') diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index e5f9ca7c1..f390e6f15 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -705,15 +705,14 @@ def theme_path "themes/#{params[:theme_preview] || current_space&.theme || TeSS::Config.site['default_theme'] || 'default'}" end - def omniauth_login_link(provider, config) + def omniauth_login_link(provider, config, html_options = {}) 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, **params), - method: :post - ) + label = t('authentication.omniauth.log_in_with', + provider: config.options[:label] || + t("authentication.omniauth.providers.#{provider}", default: provider.to_s.titleize)) + + link_to(capture { block_given? ? yield : label }, omniauth_authorize_path('user', provider, **params), + { method: :post }.merge(html_options)) end def per_page_options_for_select diff --git a/app/helpers/spaces_helper.rb b/app/helpers/spaces_helper.rb index 5bc7dd50c..3cc7b0446 100644 --- a/app/helpers/spaces_helper.rb +++ b/app/helpers/spaces_helper.rb @@ -20,7 +20,27 @@ def space_feature_options end end + def omniauth_providers_for_space(space = current_space) + host = space.try(:host) || TeSS::Config.base_uri.host + + Devise.omniauth_configs.select do |_provider, config| + Array(config.options[:redirect_uris]).any? do |uri| + TessOmniauthRedirectUris.valid_login_domain?(URI.parse(uri).host, host) + end + end + end + def space_supports_omniauth?(space = current_space) - space.nil? || space.default? || space.is_subdomain?(TeSS::Config.base_uri.domain) + omniauth_providers_for_space(space).any? + end + + def space_supports_orcid_auth?(space = current_space) + host = space.try(:host) || TeSS::Config.base_uri.host + config = Rails.application.config.secrets.orcid + redirect_uris = Array(config[:redirect_uri].presence || "#{TeSS::Config.base_url.chomp('/')}/orcid/callback") + + redirect_uris.any? do |uri| + TessOmniauthRedirectUris.valid_login_domain?(URI.parse(uri).host, host) + end end end diff --git a/app/models/space.rb b/app/models/space.rb index bdcb6088b..7929a6bd8 100644 --- a/app/models/space.rb +++ b/app/models/space.rb @@ -146,15 +146,15 @@ def enabled_features (FEATURES - disabled_features) end - # Checks whether this space's host is the given domain, or a subdomain of - # it. + # Checks whether a cookie set for the given domain can be read by this + # space's host. # - # domain:: the domain to compare against; defaults to - # TeSS::Config.base_uri.domain. + # login_host:: the domain to compare against; defaults to + # TeSS::Config.base_uri.host. # # Returns:: +true+ or +false+. - def is_subdomain?(domain = TeSS::Config.base_uri.domain) - (host == domain || host.ends_with?(".#{domain}")) + def valid_login_domain?(login_host = TeSS::Config.base_uri.host) + TessOmniauthRedirectUris.valid_login_domain?(host, login_host) end # Equality by id: two Space instances are equal if they are both Space diff --git a/app/views/devise/sessions/_omniauth_options.html.erb b/app/views/devise/sessions/_omniauth_options.html.erb index 68745c4c7..b7d48974c 100644 --- a/app/views/devise/sessions/_omniauth_options.html.erb +++ b/app/views/devise/sessions/_omniauth_options.html.erb @@ -1,14 +1,14 @@ <% if devise_mapping.omniauthable? -%> - <% Devise.omniauth_configs.each do |provider, config| -%> - <%= link_to(omniauth_authorize_path(resource_name, provider), method: :post, - class: config.options[:logo] ? '' : - TeSS::Config.feature["login_through_oidc_only"] ? 'btn btn-default btn-lg btn-oidc-only' : 'btn btn-default') do %> + <% omniauth_providers_for_space.each do |provider, config| -%> + <%= omniauth_login_link(provider, config, + class: config.options[:logo] ? '' : + TeSS::Config.feature["login_through_oidc_only"] ? 'btn btn-default btn-lg btn-oidc-only' : 'btn btn-default') do %> <% if config.options[:logo].present? %> <%= image_tag(config.options[:logo], class: "omniauth-logo omniauth-#{provider}") -%> <% else %> <%= t('authentication.omniauth.log_in_with', provider: config.options[:label] || t("authentication.omniauth.providers.#{provider}", default: provider.to_s.titleize)) -%> <% end %> - <% end -%> + <% end %> <% end -%> <% end -%> diff --git a/app/views/devise/sessions/new.html.erb b/app/views/devise/sessions/new.html.erb index a8518c381..47a6a9aa0 100644 --- a/app/views/devise/sessions/new.html.erb +++ b/app/views/devise/sessions/new.html.erb @@ -5,7 +5,7 @@ <% end %>
- <% if resource_class.omniauth_providers.any? && devise_mapping.omniauthable? %> + <% if space_supports_omniauth? && devise_mapping.omniauthable? %>
<%= t('authentication.omniauth.title') %>

<%= t('authentication.omniauth.description') %>

<%= render partial: 'devise/sessions/omniauth_options' %> diff --git a/app/views/layouts/_login_menu.html.erb b/app/views/layouts/_login_menu.html.erb index f216f5b2e..a5ef4630e 100644 --- a/app/views/layouts/_login_menu.html.erb +++ b/app/views/layouts/_login_menu.html.erb @@ -1,6 +1,6 @@ -<% if TeSS::Config.feature["login_through_oidc_only"] && Devise.omniauth_configs.size == 1 %> +<% if TeSS::Config.feature["login_through_oidc_only"] && omniauth_providers_for_space.size == 1 %>
  • - <% provider, config = Devise.omniauth_configs.first %> + <% provider, config = omniauth_providers_for_space.first %> <%= omniauth_login_link(provider, config) %>
  • <% else %> @@ -10,7 +10,7 @@