maverick/docbrown
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Enable Forem (Passport) Auth (#15141)

Browse source 
* Enable Forem (Passport) Auth

* Remove feature flag DUS

* Add prompt for Forem Passport in admin

* Add spec & fix broken one

* Link to docs instead of passport site
This commit is contained in:
Fernando Valverde 2021-11-23 14:59:00 -06:00 • committed by GitHub
parent cbff6cb92f
commit c384755bae
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 102 additions and 47 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

17
app/helpers/authentication_helper.rb
View file

@ -71,16 +71,27 @@ module AuthenticationHelper
request.referer&.include?(new_user_registration_path)
end
def display_social_login?
# Returns true when a social authentication provider should be displayed in
# `/enter` page. This method serves the need to comply with platform specific
# restrictions, such as:
# - Within mobile apps we must hide social auth providers unless SIWA is enabled
# - Forem authentication doesn't need to comply with the above (it's the exception)
def display_social_login?(provider_name = nil)
return true if Authentication::Providers.enabled.include?(:apple)
return true if request.user_agent.to_s.match?(/Android/i)
return true if provider_name == :forem && Authentication::Providers.enabled.include?(:forem)
# Don't display (return false) if UserAgent includes ForemWebview - iOS only
request.user_agent.to_s.exclude?("ForemWebView")
end
# Display the fallback message if we can't register with email and at the same time can't display the social options.
# Display the fallback message (return true) if there is no way to register
# (on mobile apps). This only happens when all of the following are true:
# - We're on the "Create account" page
# - Email+Password registration is disabled
# - There are no social options enabled
# - This happens on mobile apps because of platform specific issues
def display_registration_fallback?(state)
state == "new-user" && !Settings::Authentication.allow_email_password_registration && !display_social_login?
state == "new-user" && !Settings::Authentication.allow_email_password_registration && !display_social_login?(:forem)
end
end

1
app/views/admin/settings/forms/_authentication.html.erb
View file

@ -169,7 +169,6 @@
readonly: true %>
</div>
<% authentication_available_providers.each do |provider| %>
<% next if provider.provider_name == :forem && !FeatureFlag.enabled?(:forem_passport) %>
<section class="config-authentication__item mb-0">
<figure class="config-authentication__item--icon">
<%= inline_svg_tag("#{provider.provider_name}.svg", class: "crayons-icon", aria: true, title: "#{provider.official_name} logo") %>

7
app/views/admin/settings/forms/authentication/_auth_provider_settings.html.erb
View file

@ -20,6 +20,13 @@
placeholder: Constants::Settings::Authentication::DETAILS[:"#{provider.provider_name}_secret"][:placeholder] %>
</div>
<div class="crayons-btn-actions">
<% if provider.provider_name == :forem && !authentication_provider_enabled?(provider) %>
<div class="crayons-field">
<a href="https://developers.forem.com/backend/auth-forem" class="crayons-btn" target="_blank">
Get started here
</a>
</div>
<% end %>
<% if authentication_provider_enabled?(provider) %>
<button
class="crayons-btn crayons-btn--danger"

3
app/views/shared/authentication/_providers_registration_form.html.erb
View file

@ -1,7 +1,6 @@
<div class="registration__actions-providers">
<% authentication_enabled_providers.each do |provider| %>
<% next unless display_social_login? %>
<% next if provider.provider_name == :forem && !FeatureFlag.enabled?(:forem_passport) %>
<% next unless display_social_login?(provider.provider_name) %>
<%= form_with url: provider.sign_in_path(state: "navbar_basic"), class: "flex w-100", local: true do |f| %>
<%= f.button type: :submit, class: "crayons-btn crayons-btn--l crayons-btn--brand-#{provider.provider_name} crayons-btn--icon-left grow-1 whitespace-nowrap" do %>
<%= inline_svg_tag("#{provider.provider_name}.svg", aria_hidden: true, class: "crayons-icon", title: provider.provider_name) %>

7
lib/data_update_scripts/20211020145958_remove_passport_feature_flag.rb Normal file
View file

@ -0,0 +1,7 @@
module DataUpdateScripts
class RemovePassportFeatureFlag
def run
FeatureFlag.remove(:forem_passport)
end
end
end

19
spec/helpers/authentication_helper_spec.rb
View file

@ -98,6 +98,8 @@ RSpec.describe AuthenticationHelper, type: :helper do
end
describe "#display_social_login?" do
let(:twitter_provider_name) { :twitter }
let(:forem_provider_name) { :forem }
let(:mobile_browser_ua) { "Mozilla/5.0 (iPhone) AppleWebKit/605.1.15 (KHTML, like Gecko) Mobile/15E148" }
let(:android_foremwebview_ua) do
"Mozilla/5.0 (Linux; Android 10; SM-A217M) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/88.0 ForemWebView/1.0"
@ -113,10 +115,10 @@ RSpec.describe AuthenticationHelper, type: :helper do
it "responds with true regardless if the Apple Auth is enabled" do
allow(Authentication::Providers).to receive(:enabled).and_return(%i[apple twitter])
expect(helper.display_social_login?).to be true
expect(helper.display_social_login?(twitter_provider_name)).to be true
allow(Authentication::Providers).to receive(:enabled).and_return([:twitter])
expect(helper.display_social_login?).to be true
expect(helper.display_social_login?(twitter_provider_name)).to be true
end
end
@ -127,12 +129,17 @@ RSpec.describe AuthenticationHelper, type: :helper do
it "responds with true when Apple Auth is enabled" do
allow(Authentication::Providers).to receive(:enabled).and_return(%i[apple twitter])
expect(helper.display_social_login?).to be true
expect(helper.display_social_login?(twitter_provider_name)).to be true
end
it "responds with false when Apple Auth isn't enabled" do
allow(Authentication::Providers).to receive(:enabled).and_return([:twitter])
expect(helper.display_social_login?).to be false
expect(helper.display_social_login?(twitter_provider_name)).to be false
end
it "responds with true when Apple Auth isn't enabled but requested Forem Auth (the exception)" do
allow(Authentication::Providers).to receive(:enabled).and_return(%i[twitter forem])
expect(helper.display_social_login?(forem_provider_name)).to be true
end
end
@ -143,10 +150,10 @@ RSpec.describe AuthenticationHelper, type: :helper do
it "responds with true regardless of Apple Auth being enabled" do
allow(Authentication::Providers).to receive(:enabled).and_return(%i[apple twitter])
expect(helper.display_social_login?).to be true
expect(helper.display_social_login?(twitter_provider_name)).to be true
allow(Authentication::Providers).to receive(:enabled).and_return([:twitter])
expect(helper.display_social_login?).to be true
expect(helper.display_social_login?(twitter_provider_name)).to be true
end
end
end

20
spec/lib/data_update_scripts/remove_passport_feature_flag_spec.rb Normal file
View file

@ -0,0 +1,20 @@
require "rails_helper"
require Rails.root.join(
"lib/data_update_scripts/20211020145958_remove_passport_feature_flag.rb",
)
describe DataUpdateScripts::RemovePassportFeatureFlag do
it "removes the :forem_passport flag" do
FeatureFlag.enable(:forem_passport)
described_class.new.run
expect(FeatureFlag.exist?(:forem_passport)).to be(false)
end
it "works if the flag is not available" do
described_class.new.run
expect(FeatureFlag.exist?(:forem_passport)).to be(false)
end
end

1
spec/models/user_spec.rb
View file

@ -423,7 +423,6 @@ RSpec.describe User, type: :model do
let(:user) { create(:user) }
before do
allow(FeatureFlag).to receive(:enabled?).with(:forem_passport).and_return(true)
omniauth_mock_providers_payload
end

2
spec/services/authentication/providers/forem_spec.rb
View file

@ -1,8 +1,6 @@
require "rails_helper"
RSpec.describe Authentication::Providers::Forem, type: :service do
before { allow(FeatureFlag).to receive(:enabled?).with(:forem_passport).and_return(true) }
describe ".authentication_path" do
it "returns the correct authentication path" do
expected_path = Rails.application.routes.url_helpers.user_forem_omniauth_authorize_path

56
spec/system/authentication/conditional_registration_spec.rb
View file

@ -4,7 +4,8 @@ require "rails_helper"
# ForemWebView contexts when Apple Auth isn't enabled
RSpec.describe "Conditional registration (ForemWebView)", type: :system do
let(:all_providers) { Authentication::Providers.available }
let(:all_providers_except_apple) { Authentication::Providers.available - [:apple] }
let(:all_providers_except_apple) { Authentication::Providers.available - %i[apple] }
let(:all_providers_minus_apple_forem) { Authentication::Providers.available - %i[apple forem] }
let(:mobile_browser_ua) { "Mozilla/5.0 (iPhone) AppleWebKit/605.1.15 (KHTML, like Gecko) Mobile/15E148" }
let(:foremwebview_ua) do
"Mozilla/5.0 (iPhone) AppleWebKit/605.1.15 (KHTML, like Gecko) Mobile/15E148 ForemWebView/1.0"
@ -87,22 +88,45 @@ RSpec.describe "Conditional registration (ForemWebView)", type: :system do
expect(page).to have_text("Have a password? Continue with your email address")
end
it "renders the fallback message when Apple Auth and email registration aren't enabled" do
# Only renders email option because Apple Auth isn't available for registration
allow(Settings::Authentication).to receive(:providers).and_return(all_providers_except_apple)
allow(Settings::Authentication).to receive(:allow_email_password_registration).and_return(false)
visit sign_up_path(state: "new-user")
expect(page).not_to have_text("Sign up with Apple")
expect(page).not_to have_text("Sign up with GitHub")
expect(page).not_to have_text("Sign up with Email")
expect(page).to have_text("Sorry to be a bummer...")
expect(page).to have_text(flow_b_fallback_text)
context "when Apple Auth and email registration aren't enabled" do
before do
allow(Settings::Authentication).to receive(:allow_email_password_registration).and_return(false)
end
# Only renders email option because Apple Auth isn't available for login
visit sign_up_path
expect(page).not_to have_text("Continue with Apple")
expect(page).not_to have_text("Continue with GitHub")
expect(page).to have_text("Have a password? Continue with your email address")
it "renders the fallback message if Forem Auth is also disabled" do
# Only renders email option because Apple Auth isn't available for registration
allow(Settings::Authentication).to receive(:providers).and_return(all_providers_minus_apple_forem)
visit sign_up_path(state: "new-user")
expect(page).not_to have_text("Sign up with Apple")
expect(page).not_to have_text("Sign up with GitHub")
expect(page).not_to have_text("Sign up with Forem")
expect(page).to have_text("Sorry to be a bummer...")
expect(page).to have_text(flow_b_fallback_text)
# Only renders email option because Apple Auth isn't available for login
visit sign_up_path
expect(page).not_to have_text("Continue with Apple")
expect(page).not_to have_text("Continue with GitHub")
expect(page).to have_text("Have a password? Continue with your email address")
end
it "doesn't render the fallback because Forem Auth is enabled" do
# Only renders email option because Apple Auth isn't available for registration
allow(Settings::Authentication).to receive(:providers).and_return(all_providers_except_apple)
visit sign_up_path(state: "new-user")
expect(page).not_to have_text("Sign up with GitHub")
expect(page).to have_text("Sign up with Forem")
expect(page).not_to have_text("Sorry to be a bummer...")
expect(page).not_to have_text(flow_b_fallback_text)
# Only renders email option because Apple Auth isn't available for login
visit sign_up_path
expect(page).not_to have_text("Continue with Apple")
expect(page).not_to have_text("Continue with GitHub")
expect(page).to have_text("Continue with Forem")
expect(page).to have_text("Have a password? Continue with your email address")
end
end
end
end

1
spec/system/authentication/creator_config_edit_spec.rb
View file

@ -7,7 +7,6 @@ RSpec.describe "Creator config edit", type: :system, js: true do
before do
sign_in admin
allow(ForemInstance).to receive(:private?).and_return(false)
allow(FeatureFlag).to receive(:enabled?).with(:forem_passport).and_return(true)
end
it "presents all available OAuth providers" do

1
spec/system/authentication/omniauth_redirect_uri_spec.rb
View file

@ -18,7 +18,6 @@ RSpec.describe "Omniauth redirect_uri", type: :system do
it "relies on Settings::General.app_domain to generate correct callbacks_url" do
allow(Settings::Authentication).to receive(:providers).and_return(Authentication::Providers.available)
allow(FeatureFlag).to receive(:enabled?).with(:forem_passport).and_return(true)
visit sign_up_path
Authentication::Providers.available.each do |provider_name|
provider_auth_button = find("button.crayons-btn--brand-#{provider_name}")

14
spec/system/authentication/user_logs_in_with_forem_spec.rb
View file

@ -4,24 +4,10 @@ RSpec.describe "Authenticating with Forem" do
let(:sign_in_link) { "Continue with Forem" }
before do
allow(FeatureFlag).to receive(:enabled?).with(:forem_passport).and_return(true)
omniauth_mock_forem_payload
allow(Settings::Authentication).to receive(:providers).and_return(Authentication::Providers.available)
end
describe "FeatureFlag hides the Forem Passport auth" do
it "shows Forem auth when enabled" do
visit sign_up_path
expect(page).to have_text(sign_in_link)
end
it "doesn't show the Forem auth when disabled" do
allow(FeatureFlag).to receive(:enabled?).with(:forem_passport).and_return(false)
visit sign_up_path
expect(page).not_to have_text(sign_in_link)
end
end
context "when a user is new" do
context "when using valid credentials" do
it "creates a new user" do