diff --git a/Gemfile b/Gemfile index f7aef2fe02bd7..e9a36324d6e83 100644 --- a/Gemfile +++ b/Gemfile @@ -73,7 +73,7 @@ gem "omniauth-apple", "~> 1.0" # OmniAuth strategy for Sign In with Apple gem "omniauth-facebook", "~> 9.0" # OmniAuth strategy for Facebook gem "omniauth-github", "~> 2.0" # OmniAuth strategy for GitHub gem "omniauth-google-oauth2", "~> 1.0" # OmniAuth strategy for Google OAuth2 -gem "omniauth-mlh", "~> 4.1" +gem "omniauth-mlh", "~> 4.2" gem "omniauth-rails_csrf_protection", "~> 2.0" # Provides CSRF protection on OmniAuth request endpoint on Rails application. gem "omniauth-twitter", "~> 1.4" # OmniAuth strategy for Twitter gem "parallel", "~> 1.22" # Run any kind of code in parallel processes diff --git a/Gemfile.lock b/Gemfile.lock index f542d0235f4a3..38c9333d65c39 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -675,7 +675,7 @@ GEM oauth2 (~> 2.0) omniauth (~> 2.0) omniauth-oauth2 (~> 1.8) - omniauth-mlh (4.1.0) + omniauth-mlh (4.2.0) oauth2 (~> 2.0.9) omniauth (~> 2.1.1) omniauth-oauth2 (~> 1.8.0) @@ -1207,7 +1207,7 @@ DEPENDENCIES omniauth-facebook (~> 9.0) omniauth-github (~> 2.0) omniauth-google-oauth2 (~> 1.0) - omniauth-mlh (~> 4.1) + omniauth-mlh (~> 4.2) omniauth-rails_csrf_protection (~> 2.0) omniauth-twitter (~> 1.4) parallel (~> 1.22) diff --git a/app/errors/authentication/errors.rb b/app/errors/authentication/errors.rb index c1860e6897669..a65deb6ae4cf7 100644 --- a/app/errors/authentication/errors.rb +++ b/app/errors/authentication/errors.rb @@ -20,5 +20,20 @@ def message # Raised when we find an email that's from a spammy domain. class SpammyEmailDomain < Error end + + class Ineligible < Error + def message + I18n.t("services.authentication.authenticator.account_not_eligible") + end + end + + class AccountSwitchConfirmation < Error + attr_reader :target_user + + def initialize(target_user) + @target_user = target_user + super("account_switch_confirmation") + end + end end end diff --git a/app/services/authentication/authenticator.rb b/app/services/authentication/authenticator.rb index a45e7c7e405a2..364abaca43597 100644 --- a/app/services/authentication/authenticator.rb +++ b/app/services/authentication/authenticator.rb @@ -41,6 +41,7 @@ def call refresh_identity(identity, linked_identity) return current_user end + guard_account_switch!(identity) if current_user # These variables need to be set outside of the scope of the # transaction in order to be used after the transaction is completed. @@ -169,6 +170,23 @@ def repoint_identity(incoming_identity, linked_identity) nil end + def guard_account_switch!(identity) + candidate = identity.user || verified_email_user + return if candidate.nil? || candidate == current_user + + raise ::Authentication::Errors::Ineligible if candidate.spam_or_suspended? + + raise ::Authentication::Errors::AccountSwitchConfirmation.new(candidate) + end + + def verified_email_user + email = provider.user_email + return nil if email.blank? + + user = User.find_by(email: email) + user&.confirmed? ? user : nil + end + def proper_user(identity) if current_user Rails.logger.debug { "Current user exists: #{current_user.id}" } diff --git a/app/views/liquids/_youtube.html.erb b/app/views/liquids/_youtube.html.erb index d1a37c670e5c0..4fff4f5ff396c 100644 --- a/app/views/liquids/_youtube.html.erb +++ b/app/views/liquids/_youtube.html.erb @@ -4,7 +4,7 @@ src="https://www.youtube.com/embed/<%= id %>" width="<%= local_assigns[:width] || 315 %>" height="<%= local_assigns[:height] || 560 %>" - style="width: 100%; aspect-ratio: 9 / 16;" + style="width: 100%; height: auto; aspect-ratio: 9 / 16;" allowfullscreen loading="lazy"> @@ -14,7 +14,7 @@ src="https://www.youtube.com/embed/<%= id %>" width="<%= local_assigns[:width] || 710 %>" height="<%= local_assigns[:height] || 399 %>" - style="width: 100%; aspect-ratio: 16 / 9;" + style="width: 100%; height: auto; aspect-ratio: 16 / 9;" allowfullscreen loading="lazy"> diff --git a/config/locales/services/en.yml b/config/locales/services/en.yml index f22f89fc8a2f6..2a69d5b90f8da 100644 --- a/config/locales/services/en.yml +++ b/config/locales/services/en.yml @@ -8,6 +8,7 @@ en: authentication: authenticator: not_allowed: Sorry, but the domain for your email address is not allowed. This Forem may have limited signup to only specified email domains or blocked this specific domain from joining. + account_not_eligible: Sorry, this account is not eligible to sign in. Please contact the community staff if you believe this is a mistake. providers: apple: name: "%{first} %{last}" diff --git a/config/locales/services/fr.yml b/config/locales/services/fr.yml index 7c90d12cec841..657f984a60733 100644 --- a/config/locales/services/fr.yml +++ b/config/locales/services/fr.yml @@ -7,6 +7,7 @@ fr: unpublished_video: Vidéo non publiée ~ %{rand} authentication: authenticator: + account_not_eligible: "Désolé, ce compte ne peut pas se connecter. Veuillez contacter l’équipe de la communauté si vous pensez qu’il s’agit d’une erreur." not_allowed: Désolé, mais le domaine de votre adresse e-mail n'est pas autorisé. Ce Forem a peut-être limité l'inscription à certains domaines de messagerie ou bloqué l'accès à ce domaine spécifique. providers: apple: diff --git a/config/locales/services/pt.yml b/config/locales/services/pt.yml index 583cd77fc9076..da62b3c49219f 100644 --- a/config/locales/services/pt.yml +++ b/config/locales/services/pt.yml @@ -7,6 +7,7 @@ pt: unpublished_video: Vídeo Não Publicado ~ %{rand} authentication: authenticator: + account_not_eligible: "Desculpe, esta conta não está autorizada a entrar. Entre em contato com a equipe da comunidade se você acredita que isso é um engano." not_allowed: Desculpe, mas o domínio do seu endereço de e-mail não é permitido. Este Forem pode ter limitado o cadastro apenas a domínios de e-mail específicos ou bloqueado este domínio específico de participar. providers: apple: diff --git a/spec/liquid_tags/descript_tag_spec.rb b/spec/liquid_tags/descript_tag_spec.rb index a2cdccb553e23..435b872c70266 100644 --- a/spec/liquid_tags/descript_tag_spec.rb +++ b/spec/liquid_tags/descript_tag_spec.rb @@ -26,6 +26,14 @@ def generate_embed(url) end describe "rendering" do + before do + allow(Addrinfo).to receive(:getaddrinfo).and_call_original + %w[www.share.descript.com share.descript.com descript.com].each do |host| + allow(Addrinfo).to receive(:getaddrinfo).with(host, nil, nil, :STREAM) + .and_return([instance_double(Addrinfo, ip_address: "34.95.113.47")]) + end + end + it "returns StandardError for invalid Descript URL", :aggregate_failures do invalid_descript_urls.each do |invalid_url| stub_network_request(url: invalid_url, status_code: 404) diff --git a/spec/liquid_tags/youtube_tag_spec.rb b/spec/liquid_tags/youtube_tag_spec.rb index 9dca95c43596c..6279a005f96c1 100644 --- a/spec/liquid_tags/youtube_tag_spec.rb +++ b/spec/liquid_tags/youtube_tag_spec.rb @@ -50,14 +50,14 @@ def generate_tag(input) it "uses a vertical aspect ratio for YouTube Shorts" do result = generate_tag("https://www.youtube.com/shorts/#{valid_id}") - expect(result).to include("aspect-ratio: 9 / 16") + expect(result).to include("width: 100%; height: auto; aspect-ratio: 9 / 16") expect(result).to include('width="315"') expect(result).to include('height="560"') end it "uses a horizontal aspect ratio for regular videos" do result = generate_tag("https://www.youtube.com/watch?v=#{valid_id}") - expect(result).to include("aspect-ratio: 16 / 9") + expect(result).to include("width: 100%; height: auto; aspect-ratio: 16 / 9") expect(result).to include('width="710"') expect(result).to include('height="399"') end diff --git a/spec/services/authentication/authenticator_account_switch_spec.rb b/spec/services/authentication/authenticator_account_switch_spec.rb new file mode 100644 index 0000000000000..d41ca8dabd2db --- /dev/null +++ b/spec/services/authentication/authenticator_account_switch_spec.rb @@ -0,0 +1,79 @@ +require "rails_helper" + +RSpec.describe Authentication::Authenticator, type: :service do + let(:current_user) { create(:user) } + + def mlh_payload(uid:, email:) + OmniAuth::AuthHash.new( + provider: "mlh", + uid: uid, + info: OmniAuth::AuthHash::InfoHash.new(email: email, name: "MLH User"), + credentials: OmniAuth::AuthHash.new(token: "tok_#{uid}", secret: "sec"), + extra: { raw_info: { created_at: 2.years.ago.iso8601 } }, + ) + end + + before do + omniauth_mock_mlh_payload + allow(ForemStatsClient).to receive(:increment) + allow(Settings::Authentication).to receive(:providers).and_return(Authentication::Providers.available) + end + + context "when a different user is signed in and the incoming identity resolves elsewhere" do + it "raises AccountSwitchConfirmation carrying the resolved target (exact verified email)" do + target = create(:user) + payload = mlh_payload(uid: "core-switch-1", email: target.email) + + expect do + expect do + described_class.call(payload, current_user: current_user) + end.to raise_error( + Authentication::Errors::AccountSwitchConfirmation, + ) { |error| expect(error.target_user).to eq(target) } + end.not_to change(Identity, :count) + end + + it "raises AccountSwitchConfirmation carrying the resolved target (uid ownership)" do + target = create(:user) + create(:identity, user: target, provider: "mlh", uid: "core-switch-2") + payload = mlh_payload(uid: "core-switch-2", email: "nobody-else@example.com") + + expect do + described_class.call(payload, current_user: current_user) + end.to raise_error( + Authentication::Errors::AccountSwitchConfirmation, + ) { |error| expect(error.target_user).to eq(target) } + end + + it "fails closed without attaching when the resolved account is suspended" do + target = create(:user) + target.add_role(:suspended) + payload = mlh_payload(uid: "core-switch-3", email: target.email) + + expect do + expect do + described_class.call(payload, current_user: current_user) + end.to raise_error(Authentication::Errors::Ineligible) + end.not_to change(Identity, :count) + end + end + + context "when the signed-in user resolves as the identity's owner themselves" do + it "attaches normally without requiring confirmation" do + payload = mlh_payload(uid: "core-self-1", email: current_user.email) + + expect(described_class.call(payload, current_user: current_user)).to eq(current_user) + expect(current_user.identities.find_by(provider: "mlh").uid).to eq("core-self-1") + end + end + + context "when the incoming unclaimed identity resolves to nobody" do + it "keeps the normal attach-to-current-user flow" do + payload = mlh_payload(uid: "core-nobody-1", email: "unclaimed@example.com") + + expect do + expect(described_class.call(payload, current_user: current_user)).to eq(current_user) + end.to change(Identity, :count).by(1) + end + end +end diff --git a/spec/services/liquid_embed_extractor_spec.rb b/spec/services/liquid_embed_extractor_spec.rb index 719a55eb6ab38..40abbc07178a4 100644 --- a/spec/services/liquid_embed_extractor_spec.rb +++ b/spec/services/liquid_embed_extractor_spec.rb @@ -74,8 +74,8 @@ end it "correctly resolves internal DEV Article links wrapped in general UnifiedEmbeds into native polymorphic relationships" do - dev_article = create(:article, title: "Test Article") - dev_article.user.update!(username: "testuser") + user = create(:user, username: "testuser") + dev_article = create(:article, user: user, title: "Test Article") domain = Settings::General.app_domain || "localhost:3000" article_url = "http://#{domain}/testuser/#{dev_article.slug}" diff --git a/vendor/cache/omniauth-mlh-4.1.0.gem b/vendor/cache/omniauth-mlh-4.1.0.gem deleted file mode 100644 index 49a5ada30444e..0000000000000 Binary files a/vendor/cache/omniauth-mlh-4.1.0.gem and /dev/null differ diff --git a/vendor/cache/omniauth-mlh-4.2.0.gem b/vendor/cache/omniauth-mlh-4.2.0.gem new file mode 100644 index 0000000000000..32afc61464c53 Binary files /dev/null and b/vendor/cache/omniauth-mlh-4.2.0.gem differ