Take Apple Authentication out of Beta (#12114)

* Removing :apple_auth feature flag and debugging * Fix tests failing due to nil in Devise initializer * Remove db/schema.rb changes * Devise config tweak * cleaning up * Fix spec + delete debug logs * Removes temporary beta_access_providers spec + remove feature flag DUS * Add test to avoid connecting an existing user with SIWA * Fix merge conflict mistake + more feature flag cleanups
2021-08-03 15:04:00 -06:00 · 2021-08-03 15:04:00 -06:00 · e73afa22c7
commit e73afa22c7
parent 2e52d6ba88
14 changed files with 42 additions and 63 deletions
--- a/app/views/admin/settings/forms/_authentication.html.erb
+++ b/app/views/admin/settings/forms/_authentication.html.erb
@ -159,7 +159,6 @@
                             readonly: true %>
          </div>
          <% authentication_available_providers.each do |provider| %>
-            <% next if provider.provider_name == :apple && !Flipper.enabled?(:apple_auth) %>
            <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") %>
--- a/app/views/shared/authentication/_providers_registration_form.html.erb
+++ b/app/views/shared/authentication/_providers_registration_form.html.erb
@ -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 == :apple && !FeatureFlag.enabled?(:apple_auth) %>
    <%= 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) %>
--- a/app/views/users/_additional_authentication.html.erb
+++ b/app/views/users/_additional_authentication.html.erb
@ -2,10 +2,9 @@
  <div class="crayons-card crayons-card--content-rows block">
    <% authentication_enabled_providers.each do |provider| %>
      <!--
-        TODO: [@forem/oss] Only until we refactor our cookie configurations
-        we'll be able to connect existing accounts with SIWA. This should be
-        done when Apple Auth Provider is no longer in 'Beta':
-        https://github.com/forem/forem/pull/12114
+        INFO: [@forem/oss] We only support creating new accounts and then
+        signing in with them) using Apple Auth. Connecting Forem existing
+        accounts to Apple Auth isn't supported and is why we skip it here.
      -->
      <% next if provider.provider_name == :apple %>
      <% unless @user.authenticated_through?(provider.provider_name) %>
--- a/docs/backend/auth-apple.md
+++ b/docs/backend/auth-apple.md
@ -1,5 +1,5 @@
 ---
-title: Apple Authentication (beta)
+title: Apple Authentication
 ---

 # Sign in with Apple Authentication
@ -12,15 +12,6 @@ retrieve the necessary credentials and an HTTPS supported URL for the callback
 configuration (HTTP won't work). Then you'll need to provide the keys to the
 Rails application.

-#### Beta support
-
-This authentication provider is currently marked as beta. This means it will be
-available but hidden from public access until more thoroughly tested.
-
-If you want to make this feature publicly available (without the state
-parameter) you can enable the `apple_auth` feature flag from the Flipper
-dashboard or the Rails console with `Flipper.enable(:apple_auth)`.
-
 # Apple Developer Portal Configuration

 [Register/Sign in](https://developer.apple.com/account) to your Apple Developer
--- a/lib/data_update_scripts/20210803161945_remove_apple_auth_feature_flag.rb
+++ b/lib/data_update_scripts/20210803161945_remove_apple_auth_feature_flag.rb
@ -0,0 +1,7 @@
+module DataUpdateScripts
+  class RemoveAppleAuthFeatureFlag
+    def run
+      FeatureFlag.remove(:apple_auth)
+    end
+  end
+end
--- a/spec/lib/data_update_scripts/remove_apple_auth_feature_flag_spec.rb
+++ b/spec/lib/data_update_scripts/remove_apple_auth_feature_flag_spec.rb
@ -0,0 +1,20 @@
+require "rails_helper"
+require Rails.root.join(
+  "lib/data_update_scripts/20210803161945_remove_apple_auth_feature_flag.rb",
+)
+
+describe DataUpdateScripts::RemoveAppleAuthFeatureFlag do
+  it "removes the :apple_auth flag" do
+    FeatureFlag.enable(:apple_auth)
+
+    described_class.new.run
+
+    expect(FeatureFlag.exist?(:apple_auth)).to be(false)
+  end
+
+  it "works if the flag is not available" do
+    described_class.new.run
+
+    expect(FeatureFlag.exist?(:apple_auth)).to be(false)
+  end
+end
--- a/spec/requests/registrations_spec.rb
+++ b/spec/requests/registrations_spec.rb
@ -10,7 +10,6 @@ RSpec.describe "Registrations", type: :request do

        Authentication::Providers.enabled.each do |provider_name|
          provider = Authentication::Providers.get!(provider_name)
-          next if provider.provider_name == :apple && !Flipper.enabled?(:apple_auth)

          expect(response.body).to include("Continue with #{provider.official_name}")
        end
--- a/spec/requests/user/user_settings_spec.rb
+++ b/spec/requests/user/user_settings_spec.rb
@ -180,6 +180,17 @@ RSpec.describe "UserSettings", type: :request do

        expect(response.body).to include("Connect GitHub Account")
      end
+
+      it "does not allow to connect an Apple Account to an existing user" do
+        allow(Authentication::Providers).to receive(:enabled).and_return(Authentication::Providers.available)
+        user = create(:user, :with_identity, identities: [:twitter])
+
+        sign_in user
+        get user_settings_path
+
+        expect(response.body).to include("Connect GitHub Account")
+        expect(response.body).not_to include("Connect Apple Account")
+      end
    end

    describe "GitHub repositories" do
--- a/spec/system/authentication/beta_access_providers_spec.rb
+++ b/spec/system/authentication/beta_access_providers_spec.rb
@ -1,37 +0,0 @@
-require "rails_helper"
-
-RSpec.describe "Beta provider access", type: :system do
-  let(:providers) { %w[Facebook GitHub Twitter] }
-  let(:beta_providers) { ["Apple"] }
-
-  before do
-    allow(Settings::Authentication).to receive(:providers).and_return(Authentication::Providers.available)
-  end
-
-  context "when a user tries to sign_up" do
-    it "doesn't render beta providers by default" do
-      visit sign_up_path
-
-      providers.each do |name|
-        expect(page).to have_button("Continue with #{name}")
-      end
-
-      beta_providers.each do |name|
-        expect(page).not_to have_button("Continue with #{name}")
-      end
-    end
-
-    it "renders beta providers when passed in the correct state param" do
-      Flipper.enable(:apple_auth)
-      visit sign_up_path
-
-      providers.each do |name|
-        expect(page).to have_button("Continue with #{name}")
-      end
-
-      beta_providers.each do |name|
-        expect(page).to have_button("Continue with #{name}")
-      end
-    end
-  end
-end
--- a/spec/system/authentication/conditional_registration_spec.rb
+++ b/spec/system/authentication/conditional_registration_spec.rb
@ -16,7 +16,6 @@ RSpec.describe "Conditional registration (ForemWebView)", type: :system do
  end

  before do
-    allow(FeatureFlag).to receive(:enabled?).with(:apple_auth).and_return(true)
    allow(FeatureFlag).to receive(:enabled?).with(:creator_onboarding).and_return(false)
    allow(Settings::Authentication).to receive(:allow_email_password_registration).and_return(true)
  end
--- a/spec/system/authentication/creator_config_edit_spec.rb
+++ b/spec/system/authentication/creator_config_edit_spec.rb
@ -3,9 +3,6 @@ require "rails_helper"
 RSpec.describe "Creator config edit", type: :system, js: true do
  let(:admin) { create(:user, :super_admin) }

-  # Apple auth is in Beta so we need to enable the Feature Flag to test it
-  before { Flipper.enable(:apple_auth) }
-
  context "when a creator browses /admin/customization/config" do
    before do
      sign_in admin
--- a/spec/system/authentication/omniauth_redirect_uri_spec.rb
+++ b/spec/system/authentication/omniauth_redirect_uri_spec.rb
@ -6,9 +6,6 @@ RSpec.describe "Omniauth redirect_uri", type: :system do
  # Avoid messing with other tests by resetting back Settings::General.app_domain
  after { Settings::General.app_domain = test_app_domain }

-  # Apple auth is in Beta so we need to enable the Feature Flag to test it
-  before { Flipper.enable(:apple_auth) }
-
  def provider_redirect_regex(provider_name)
    # URL encoding translates the query params (i.e. colons/slashes/etc)
    %r{
--- a/spec/system/authentication/user_logs_in_with_apple_spec.rb
+++ b/spec/system/authentication/user_logs_in_with_apple_spec.rb
@ -5,7 +5,6 @@ RSpec.describe "Authenticating with Apple", vcr: { cassette_name: "fastly_sloan"

  before do
    omniauth_mock_apple_payload
-    Flipper.enable(:apple_auth)
    allow(Settings::Authentication).to receive(:providers).and_return(Authentication::Providers.available)
  end

--- a/spec/system/link_for_tags_in_posts_in_notifications_spec.rb
+++ b/spec/system/link_for_tags_in_posts_in_notifications_spec.rb
@ -16,7 +16,6 @@ RSpec.describe "Link on tags for post in notifications", type: :system do
    it "shows the sign-with page", js: true do
      Authentication::Providers.enabled.each do |provider_name|
        provider = Authentication::Providers.get!(provider_name)
-        next if provider.provider_name == :apple && !Flipper.enabled?(:apple_auth)

        expect(page).to have_content("Continue with #{provider.official_name}")
      end