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

[EOY2020 Issue] Default new forems to Email Authentication only (#11696)

Browse source 
* Set default Auth Providers to empty array

* complete implementation; tests remain

* tests

* fix failing specs authenticator_spec.rb

* Fix failing specs

* fix more failing specs

* fix more failing specs 2

* update failing spec

* Update app/views/admin/configs/_auth_provider_settings.html.erb

Co-authored-by: Vaidehi Joshi <vaidehi.sj@gmail.com>

* Update app/assets/stylesheets/admin.scss

Co-authored-by: Vaidehi Joshi <vaidehi.sj@gmail.com>

Co-authored-by: Vaidehi Joshi <vaidehi.sj@gmail.com>
This commit is contained in:
Arit Amana 2020-12-02 16:49:07 -05:00 • committed by GitHub
parent 7ee8523ba3
commit 42e10e7b42
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
23 changed files with 158 additions and 69 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
app/assets/stylesheets/admin.scss
View file

@ -147,6 +147,10 @@ label {
gap: var(--su-4);
padding-top: var(--su-6);
padding-bottom: var(--su-2);
.auth-warning {
padding: var(--su-2); // Override default crayons notice padding
}
}
.admin-config-checkmark {

7
app/controllers/admin/configs_controller.rb
View file

@ -247,8 +247,13 @@ module Admin
config[:credit_prices_in_cents]&.transform_values!(&:to_i)
end
def provider_keys_missing(entry)
SiteConfig.public_send("#{entry}_key").blank? || SiteConfig.public_send("#{entry}_secret").blank?
end
def invalid_provider_entry(entry)
entry.blank? || helpers.available_providers_array.exclude?(entry)
entry.blank? || helpers.available_providers_array.exclude?(entry) ||
provider_keys_missing(entry)
end
def email_login_disabled_with_one_or_less_auth_providers(enabled_providers)

5
app/helpers/authentication_helper.rb
View file

@ -23,6 +23,11 @@ module AuthenticationHelper
Authentication::Providers.available.map(&:to_s)
end
def provider_keys_configured?(provider)
SiteConfig.public_send("#{provider}_key").present? &&
SiteConfig.public_send("#{provider}_secret").present?
end
def forem_creator_flow_enabled?
FeatureFlag.enabled?(:creator_onboarding) && waiting_on_first_user?
end

10
app/javascript/admin/controllers/config_controller.js
View file

@ -125,15 +125,17 @@ export default class ConfigController extends Controller {
const enabledIndicator = document.getElementById(
`${provider}-enabled-indicator`,
);
document
.getElementById(`${provider}-auth-settings`)
.classList.remove('hidden');
event.target.classList.add('hidden');
if (event.target.dataset.buttonText === 'enable') {
enabledIndicator.classList.add('visible');
event.target.setAttribute('data-enable-auth', 'true');
this.listAuthToBeEnabled();
}
document
.getElementById(`${provider}-auth-settings`)
.classList.remove('hidden');
event.target.classList.add('hidden');
}
disableAuthProvider(event) {

2
app/models/site_config.rb
View file

@ -30,7 +30,7 @@ class SiteConfig < RailsSettings::Base
field :allowed_registration_email_domains, type: :array, default: %w[]
field :display_email_domain_allow_list_publicly, type: :boolean, default: false
field :require_captcha_for_email_password_registration, type: :boolean, default: false
field :authentication_providers, type: :array, default: proc { Authentication::Providers.available }
field :authentication_providers, type: :array, default: %w[]
field :invite_only_mode, type: :boolean, default: false
field :twitter_key, type: :string, default: ApplicationConfig["TWITTER_KEY"]
field :twitter_secret, type: :string, default: ApplicationConfig["TWITTER_SECRET"]

96
app/views/admin/configs/_auth_provider_settings.html.erb
View file

@ -1,48 +1,52 @@
<fieldset class="config-authentication-form">
<div class="crayons-field">
<%= admin_config_label :"#{provider.provider_name}_key" %>
<p class="crayons-field__description">
<%= Constants::SiteConfig::DETAILS[:"#{provider.provider_name}_key"][:description] %>
</p>
<%= f.text_field :"#{provider.provider_name}_key",
class: "crayons-textfield",
value: SiteConfig.public_send("#{provider.provider_name}_key"),
placeholder: Constants::SiteConfig::DETAILS[:"#{provider.provider_name}_key"][:placeholder] %>
</div>
<div class="crayons-field">
<%= admin_config_label :"#{provider.provider_name}_secret" %>
<p class="crayons-field__description">
<%= Constants::SiteConfig::DETAILS[:"#{provider.provider_name}_secret"][:description] %>
<p>
<%= f.text_field :"#{provider.provider_name}_secret",
class: "crayons-textfield",
value: SiteConfig.public_send("#{provider.provider_name}_secret"),
placeholder: Constants::SiteConfig::DETAILS[:"#{provider.provider_name}_secret"][:placeholder] %>
</div>
<div class="flex gap-2">
<% if authentication_enabled_providers.include?(provider) %>
<button
class="crayons-btn crayons-btn--danger"
data-action="click->config#activateAuthProviderModal"
data-tooltip="<%= tooltip_text_email_or_auth_provider_btns %>"
data-auth-provider="<%= provider.provider_name %>"
data-auth-provider-official="<%= provider.official_name %>"
<%= disabled_attr_on_auth_provider_enablebtn %>>
Disable
</button>
<button
class="crayons-btn crayons-btn--secondary"
data-auth-provider="<%= provider.provider_name %>"
data-action="click->config#hideAuthProviderSettings">
Close
</button>
<% else %>
<button
class="crayons-btn crayons-btn--secondary"
data-auth-provider="<%= provider.provider_name %>"
data-action="click->config#disableAuthProvider">
Undo
</button>
<% end %>
</div>
<div
class="crayons-notice crayons-notice--warning auth-warning <%= provider_keys_configured?(provider.provider_name) ? "hidden" : "" %>">
<strong>Note:</strong> This authentication provider will <strong>not</strong> be enabled if the key or secret are missing. Please make sure that you enter your key and secret correctly.
</div>
<div class="crayons-field">
<%= admin_config_label :"#{provider.provider_name}_key" %>
<p class="crayons-field__description">
<%= Constants::SiteConfig::DETAILS[:"#{provider.provider_name}_key"][:description] %>
</p>
<%= f.text_field :"#{provider.provider_name}_key",
class: "crayons-textfield",
value: SiteConfig.public_send("#{provider.provider_name}_key"),
placeholder: Constants::SiteConfig::DETAILS[:"#{provider.provider_name}_key"][:placeholder] %>
</div>
<div class="crayons-field">
<%= admin_config_label :"#{provider.provider_name}_secret" %>
<p class="crayons-field__description">
<%= Constants::SiteConfig::DETAILS[:"#{provider.provider_name}_secret"][:description] %>
<p>
<%= f.text_field :"#{provider.provider_name}_secret",
class: "crayons-textfield",
value: SiteConfig.public_send("#{provider.provider_name}_secret"),
placeholder: Constants::SiteConfig::DETAILS[:"#{provider.provider_name}_secret"][:placeholder] %>
</div>
<div class="flex gap-2">
<% if authentication_enabled_providers.include?(provider) %>
<button
class="crayons-btn crayons-btn--danger"
data-action="click->config#activateAuthProviderModal"
data-tooltip="<%= tooltip_text_email_or_auth_provider_btns %>"
data-auth-provider="<%= provider.provider_name %>"
data-auth-provider-official="<%= provider.official_name %>"
<%= disabled_attr_on_auth_provider_enablebtn %>>
Disable
</button>
<button
class="crayons-btn crayons-btn--secondary"
data-auth-provider="<%= provider.provider_name %>"
data-action="click->config#hideAuthProviderSettings">
Close
</button>
<% else %>
<button
class="crayons-btn crayons-btn--secondary"
data-auth-provider="<%= provider.provider_name %>"
data-action="click->config#disableAuthProvider">
Undo
</button>
<% end %>
</div>
</fieldset>

23
spec/helpers/authentication_helper_spec.rb
View file

@ -38,6 +38,29 @@ RSpec.describe AuthenticationHelper, type: :helper do
end
end
describe "#provider_keys_configured?(provider)" do
let(:provider) { "facebook" }
it "returns true if provider key and secret both present" do
allow(SiteConfig).to receive(:"#{provider}_key").and_return("someKey")
allow(SiteConfig).to receive(:"#{provider}_secret").and_return("someSecret")
expect(provider_keys_configured?(provider)).to be(true)
end
it "returns false if either provider key or secret is missing" do
allow(SiteConfig).to receive(:"#{provider}_key").and_return("someKey")
allow(SiteConfig).to receive(:"#{provider}_secret").and_return("")
expect(provider_keys_configured?(provider)).to be(false)
allow(SiteConfig).to receive(:"#{provider}_key").and_return("")
allow(SiteConfig).to receive(:"#{provider}_secret").and_return("someSecret")
expect(provider_keys_configured?(provider)).to be(false)
end
end
describe "tooltip classes, attributes and content" do
context "when invite-only-mode enabled and no enabled registration options" do
before do

5
spec/models/user_spec.rb
View file

@ -28,7 +28,10 @@ RSpec.describe User, type: :model do
let(:other_user) { create(:user) }
let(:org) { create(:organization) }
before { omniauth_mock_providers_payload }
before do
omniauth_mock_providers_payload
allow(SiteConfig).to receive(:authentication_providers).and_return(Authentication::Providers.available)
end
describe "validations" do
describe "builtin validations" do

1
spec/policies/github_repo_policy_spec.rb
View file

@ -23,6 +23,7 @@ RSpec.describe GithubRepoPolicy, type: :policy do
before do
omniauth_mock_github_payload
allow(SiteConfig).to receive(:authentication_providers).and_return(Authentication::Providers.available)
end
it { is_expected.to permit_actions(%i[index update_or_create]) }

33
spec/requests/admin/configs_spec.rb
View file

@ -75,19 +75,44 @@ RSpec.describe "/admin/config", type: :request do
describe "Authentication" do
it "updates enabled authentication providers" do
enabled = Authentication::Providers.available.first.to_s
post "/admin/config", params: { site_config: { auth_providers_to_enable: enabled },
confirmation: confirmation_message }
post admin_config_path, params: {
site_config: {
"#{enabled}_key": "someKey",
"#{enabled}_secret": "someSecret",
auth_providers_to_enable: enabled
},
confirmation: confirmation_message
}
expect(SiteConfig.authentication_providers).to eq([enabled])
end
it "strips empty elements" do
provider = Authentication::Providers.available.first.to_s
enabled = "#{provider}, '', nil"
post "/admin/config", params: { site_config: { auth_providers_to_enable: enabled },
confirmation: confirmation_message }
post admin_config_path, params: {
site_config: {
"#{provider}_key": "someKey",
"#{provider}_secret": "someSecret",
auth_providers_to_enable: enabled
},
confirmation: confirmation_message
}
expect(SiteConfig.authentication_providers).to eq([provider])
end
it "does not update enabled authentication providers if any associated key missing" do
enabled = Authentication::Providers.available.first.to_s
post admin_config_path, params: {
site_config: {
"#{enabled}_key": "someKey",
"#{enabled}_secret": "",
auth_providers_to_enable: enabled
},
confirmation: confirmation_message
}
expect(SiteConfig.authentication_providers).to eq([])
end
it "enables proper domains to allow list" do
proper_list = "dev.to, forem.com, forem.dev"
post "/admin/config", params: { site_config: { allowed_registration_email_domains: proper_list },

2
spec/requests/github_repos_spec.rb
View file

@ -41,7 +41,7 @@ RSpec.describe "GithubRepos", type: :request do
before do
omniauth_mock_github_payload
allow(SiteConfig).to receive(:authentication_providers).and_return(Authentication::Providers.available)
allow(Github::OauthClient).to receive(:new).and_return(github_client)
end

1
spec/requests/user/user_settings_spec.rb
View file

@ -369,6 +369,7 @@ RSpec.describe "UserSettings", type: :request do
before do
omniauth_mock_providers_payload
allow(SiteConfig).to receive(:authentication_providers).and_return(Authentication::Providers.available)
sign_in user
end

5
spec/services/authentication/authenticator_spec.rb
View file

@ -1,7 +1,10 @@
require "rails_helper"
RSpec.describe Authentication::Authenticator, type: :service do
before { omniauth_mock_providers_payload }
before do
omniauth_mock_providers_payload
allow(SiteConfig).to receive(:authentication_providers).and_return(Authentication::Providers.available)
end
context "when authenticating through an unknown provider" do
it "raises ProviderNotFound" do

6
spec/services/authentication/providers_spec.rb
View file

@ -17,6 +17,8 @@ RSpec.describe Authentication::Providers, type: :service do
end
it "loads the correct provider class" do
allow(SiteConfig).to receive(:authentication_providers).and_return(Authentication::Providers.available)
is_subclass_of = (
described_class.get!(:twitter) < Authentication::Providers::Provider
)
@ -32,10 +34,6 @@ RSpec.describe Authentication::Providers, type: :service do
end
describe ".enabled" do
it "lists all available providers" do
expect(described_class.available).to eq(described_class.enabled)
end
context "when one of the available providers is disabled" do
it "only lists those that remain enabled" do
allow(SiteConfig).to receive(:authentication_providers).and_return(%w[github])

1
spec/services/broadcasts/welcome_notification/generator_spec.rb
View file

@ -19,6 +19,7 @@ RSpec.describe Broadcasts::WelcomeNotification::Generator, type: :service do
allow(Notification).to receive(:send_welcome_notification).and_call_original
allow(User).to receive(:mascot_account).and_return(mascot_account)
allow(SiteConfig).to receive(:staff_user_id).and_return(mascot_account.id)
allow(SiteConfig).to receive(:authentication_providers).and_return(Authentication::Providers.available)
end
it "requires a valid user id" do

5
spec/services/users/delete_spec.rb
View file

@ -1,7 +1,10 @@
require "rails_helper"
RSpec.describe Users::Delete, type: :service do
before { omniauth_mock_github_payload }
before do
omniauth_mock_github_payload
allow(SiteConfig).to receive(:authentication_providers).and_return(Authentication::Providers.available)
end
let(:user) { create(:user, :trusted, :with_identity, identities: ["github"]) }

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

@ -17,6 +17,7 @@ RSpec.describe "Omniauth redirect_uri", type: :system do
end
it "relies on SiteConfig.app_domain to generate correct callbacks_url" do
allow(SiteConfig).to receive(:authentication_providers).and_return(Authentication::Providers.available)
visit sign_up_path
Authentication::Providers.available.each do |provider_name|
provider_auth_url = find("a.crayons-btn--brand-#{provider_name}")["href"]

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

@ -11,6 +11,7 @@ RSpec.describe "Redirects authentication using Referer", type: :system do
before do
omniauth_mock_twitter_payload
allow(SiteConfig).to receive(:authentication_providers).and_return(Authentication::Providers.available)
OmniAuth.config.mock_auth[:twitter].info.email = user.email
end

5
spec/system/authentication/user_logs_in_with_facebook_spec.rb
View file

@ -3,7 +3,10 @@ require "rails_helper"
RSpec.describe "Authenticating with Facebook" do
let(:sign_in_link) { "Continue with Facebook" }
before { omniauth_mock_facebook_payload }
before do
omniauth_mock_facebook_payload
allow(SiteConfig).to receive(:authentication_providers).and_return(Authentication::Providers.available)
end
context "when a user is new" do
context "when using valid credentials" do

5
spec/system/authentication/user_logs_in_with_github_spec.rb
View file

@ -3,7 +3,10 @@ require "rails_helper"
RSpec.describe "Authenticating with GitHub" do
let(:sign_in_link) { "Continue with GitHub" }
before { omniauth_mock_github_payload }
before do
omniauth_mock_github_payload
allow(SiteConfig).to receive(:authentication_providers).and_return(Authentication::Providers.available)
end
context "when a user is new" do
context "when using valid credentials" do

5
spec/system/authentication/user_logs_in_with_twitter_spec.rb
View file

@ -3,7 +3,10 @@ require "rails_helper"
RSpec.describe "Authenticating with Twitter" do
let(:sign_in_link) { "Continue with Twitter" }
before { omniauth_mock_twitter_payload }
before do
omniauth_mock_twitter_payload
allow(SiteConfig).to receive(:authentication_providers).and_return(Authentication::Providers.available)
end
context "when a user is new" do
context "when using valid credentials" do

2
spec/system/homepage/user_visits_homepage_spec.rb
View file

@ -8,7 +8,7 @@ RSpec.describe "User visits a homepage", type: :system do
context "when user hasn't logged in" do
it "shows the sign-in block" do
visit "/"
within ".signin-cta-widget" do
within ".authentication-header__actions" do
expect(page).to have_text("Log in")
expect(page).to have_text("Create new account")
end

2
spec/system/user_logs_in_with_password_spec.rb
View file

@ -11,7 +11,7 @@ RSpec.describe "Authenticating with a password" do
let!(:user) { create(:user, password: password, password_confirmation: password) }
before do
allow(SiteConfig).to receive(:allow_email_password_login).and_return(true)
allow(SiteConfig).to receive(:authentication_providers).and_return(Authentication::Providers.available)
visit sign_up_path
end