From 1912ffdc46c4b2d051ed4a3932a87126e2ac2a34 Mon Sep 17 00:00:00 2001 From: Arit Amana <32520970+msarit@users.noreply.github.com> Date: Wed, 28 Oct 2020 13:37:11 -0400 Subject: [PATCH] [Team Email Login] Refactor the Email Authentication flow (#11090) * Hooked "Enable" button to hidden checkbox * Hooked "Close" button to close email settings and show "Enable/Edit" button * Additional hookups * Start building generalized Modal blocks * Everything hooked up except styling and a few Qs * last of the hookups; ensure logic flow * clean up * specs to cover email auth refactor * Fix bug surfaced by Vaidehi * Incorporate PR feedback * prevent email auth disable if invite-only-mode * adjust emailAuthModal body text * Sundry improvements * Last-mile tweaks * Trying to get 3rd party auth deselect to work * delete unnecssary function * remove superfluous comment * Move inline styling into CSS file * Incorporate PR feedback * Incorporate more PR feedback * Make Confirm btn intent clearer * Add TODO comment --- app/assets/stylesheets/admin.scss | 12 ++ app/controllers/admin/configs_controller.rb | 16 ++- app/helpers/authentication_helper.rb | 20 +++ .../admin/controllers/config_controller.js | 106 ++++++++++++++- app/lib/constants/site_config.rb | 10 +- app/models/site_config.rb | 3 +- app/views/admin/configs/show.html.erb | 122 ++++++++++++------ .../_providers_registration_form.html.erb | 2 +- spec/requests/admin/configs_spec.rb | 15 +++ 9 files changed, 248 insertions(+), 58 deletions(-) diff --git a/app/assets/stylesheets/admin.scss b/app/assets/stylesheets/admin.scss index 27d64db9b..33a1cd41f 100644 --- a/app/assets/stylesheets/admin.scss +++ b/app/assets/stylesheets/admin.scss @@ -125,3 +125,15 @@ label { flex-direction: column; gap: var(--su-4); } + +.admin-config-checkmark { + width: var(--su-4); + height: var(--su-4); + background-color: var(--accent-success); + border-radius: 100%; + color: var(--base-inverted); +} + +.cursor-disabled { + cursor: not-allowed; +} diff --git a/app/controllers/admin/configs_controller.rb b/app/controllers/admin/configs_controller.rb index 7dfff7fc4..123af1bc4 100644 --- a/app/controllers/admin/configs_controller.rb +++ b/app/controllers/admin/configs_controller.rb @@ -118,8 +118,7 @@ module Admin facebook_key facebook_secret invite_only_mode - allow_email_password_registration - allow_email_password_login + allow_both_email_signup_and_login require_captcha_for_email_password_registration primary_brand_color_hex spam_trigger_terms @@ -153,6 +152,8 @@ module Admin end end + toggle_email_password_authentication + redirect_to admin_config_path, notice: "Site configuration was successfully updated." end @@ -211,6 +212,17 @@ module Admin config[:credit_prices_in_cents]&.transform_values!(&:to_i) end + def toggle_email_password_authentication + if SiteConfig.allow_both_email_signup_and_login + SiteConfig.allow_email_password_registration = true + SiteConfig.allow_email_password_login = true + else + SiteConfig.allow_email_password_registration = false + SiteConfig.allow_email_password_login = false + SiteConfig.invite_only_mode = false + end + end + # Validations def brand_contrast_too_low hex = params.dig(:site_config, :primary_brand_color_hex) diff --git a/app/helpers/authentication_helper.rb b/app/helpers/authentication_helper.rb index c62dab3d3..a96a5130b 100644 --- a/app/helpers/authentication_helper.rb +++ b/app/helpers/authentication_helper.rb @@ -32,4 +32,24 @@ module AuthenticationHelper def waiting_on_first_user? SiteConfig.waiting_on_first_user end + + def disable_email_tooltip_class + SiteConfig.invite_only_mode || authentication_enabled_providers.none? ? "crayons-tooltip" : "" + end + + def disable_email_tooltip_content + SiteConfig.invite_only_mode || authentication_enabled_providers.none? ? disable_email_auth_tooltip_text : "" + end + + def disable_button_class + SiteConfig.invite_only_mode || authentication_enabled_providers.none? ? "disabled" : "" + end + + def disable_email_auth_tooltip_text + if SiteConfig.invite_only_mode + "You cannot do this until you disable Invite Only Mode" + elsif authentication_enabled_providers.none? + "You cannot do this until you enable at least one other registration option" + end + end end diff --git a/app/javascript/admin/controllers/config_controller.js b/app/javascript/admin/controllers/config_controller.js index d7b3fd304..47a39ce9f 100644 --- a/app/javascript/admin/controllers/config_controller.js +++ b/app/javascript/admin/controllers/config_controller.js @@ -1,11 +1,63 @@ import { Controller } from 'stimulus'; const recaptchaFields = document.querySelector('#recaptchaContainer'); +const emailSigninAndLoginCheckbox = document.querySelector( + '#email-signup-and-login-checkbox', +); +const emailAuthSettingsSection = document.querySelector( + '#email-auth-settings-section', +); +const modalAnchor = document.querySelector('.admin-config-modal-anchor'); +const emailAuthModalTitle = 'Disable email address registration'; +// TODO: Remove the sentence "You must update site config to save this action!" +// once we build more robust flow for Admin/Config +const emailAuthModalBody = + '

If you disable email address as a registration option, people cannot create an account with their email address.


However, people who have already created an account using their email address can continue to login.


You must update site config to save this action!

'; + +const adminConfigModal = ( + title, + body, + confirmBtnText, + confirmBtnAction, + cancelBtnText, + cancelBtnAction, +) => ` +
+
+
+

${title}

+ +
+
+ ${body} +
+ + +
+
+
+
+
+`; export default class ConfigController extends Controller { static targets = [ 'authenticationProviders', 'collectiveNoun', + 'emailAuthSettingsBtn', + 'inviteOnlyMode', 'requireCaptchaForEmailPasswordRegistration', ]; @@ -29,9 +81,59 @@ export default class ConfigController extends Controller { toggleGoogleRecaptchaFields() { if (this.requireCaptchaForEmailPasswordRegistrationTarget.checked) { - recaptchaFields.classList.remove('collapse'); + recaptchaFields.classList.remove('hidden'); } else { - recaptchaFields.classList.add('collapse'); + recaptchaFields.classList.add('hidden'); } } + + enableOrEditEmailAuthSettings() { + event.preventDefault(); + if (this.emailAuthSettingsBtnTarget.dataset.buttonText === 'enable') { + emailSigninAndLoginCheckbox.checked = true; + } + this.emailAuthSettingsBtnTarget.classList.add('hidden'); + emailAuthSettingsSection.classList.remove('hidden'); + } + + hideEmailAuthSettings() { + event.preventDefault(); + this.emailAuthSettingsBtnTarget.classList.remove('hidden'); + emailAuthSettingsSection.classList.add('hidden'); + } + + activateEmailAuthModal() { + event.preventDefault(); + modalAnchor.innerHTML = adminConfigModal( + emailAuthModalTitle, + emailAuthModalBody, + 'Confirm', + 'disableEmailAuthFromModal', + 'Cancel', + 'closeAdminConfigModal', + ); + if (document.querySelector('.crayons-modal')) { + window.scrollTo(0, 0); + document.body.style.height = '100vh'; + document.body.style.overflowY = 'hidden'; + } + } + + closeAdminConfigModal() { + modalAnchor.innerHTML = ''; + document.body.style.height = 'inherit'; + document.body.style.overflowY = 'inherit'; + } + + disableEmailAuthFromModal() { + event.preventDefault(); + emailSigninAndLoginCheckbox.checked = false; + this.closeAdminConfigModal(); + } + + disableEmailAuth() { + event.preventDefault(); + emailSigninAndLoginCheckbox.checked = false; + this.hideEmailAuthSettings(); + } } diff --git a/app/lib/constants/site_config.rb b/app/lib/constants/site_config.rb index 87aaec400..ac16f8437 100644 --- a/app/lib/constants/site_config.rb +++ b/app/lib/constants/site_config.rb @@ -4,14 +4,6 @@ module Constants SVG_PLACEHOLDER = "".freeze DETAILS = { - allow_email_password_registration: { - description: "People can sign up using their email and password", - placeholder: "" - }, - allow_email_password_login: { - description: "People can login using their email and password", - placeholder: "" - }, require_captcha_for_email_password_registration: { description: "People will be required to fill out a captcha when they're creating a new account in your community", @@ -155,7 +147,7 @@ module Constants placeholder: "0" }, invite_only_mode: { - description: "Only users invited by email can join this community.", + description: "Only users invited by email can join this community. To use this, you must enable email address registration.", placeholder: "" }, jobs_url: { diff --git a/app/models/site_config.rb b/app/models/site_config.rb index 8831e4b20..0b5602eb1 100644 --- a/app/models/site_config.rb +++ b/app/models/site_config.rb @@ -25,8 +25,9 @@ class SiteConfig < RailsSettings::Base field :video_encoder_key, type: :string # Authentication - field :allow_email_password_registration, type: :boolean, default: false + field :allow_email_password_registration, type: :boolean, default: true field :allow_email_password_login, type: :boolean, default: true + field :allow_both_email_signup_and_login, type: :boolean, default: true field :require_captcha_for_email_password_registration, type: :boolean, default: false field :authentication_providers, type: :array, default: proc { Authentication::Providers.available } field :invite_only_mode, type: :boolean, default: false diff --git a/app/views/admin/configs/show.html.erb b/app/views/admin/configs/show.html.erb index 5e3e01b7d..5bb6bf155 100644 --- a/app/views/admin/configs/show.html.erb +++ b/app/views/admin/configs/show.html.erb @@ -141,7 +141,11 @@
- <%= f.check_box :invite_only_mode, checked: SiteConfig.invite_only_mode, data: { action: "config#disableTargetField", disable_target: "authenticationProviders" }, class: "crayons-checkbox" %> + <%= f.check_box :invite_only_mode, + checked: SiteConfig.invite_only_mode, + disabled: !SiteConfig.allow_both_email_signup_and_login, + data: { action: "config#disableAuthenticationOptions", target: "config.inviteOnlyMode" }, + class: "crayons-checkbox #{!SiteConfig.allow_both_email_signup_and_login ? 'cursor-disabled' : ''}" %>
<%= admin_config_label :invite_only_mode %> <%= admin_config_description Constants::SiteConfig::DETAILS[:invite_only_mode][:description] %> @@ -150,55 +154,45 @@
-

- Authentication methods +

+ Registration options

-
- <%= admin_config_label :authentication_providers %> -

- <%= Constants::SiteConfig::DETAILS[:authentication_providers][:description] %> -

- <%= select_tag "site_config[authentication_providers]", - options_from_collection_for_select( - authentication_available_providers, - :provider_name, - :official_name, - authentication_enabled_providers.map(&:provider_name), - ), - multiple: true, - class: "form-control selectpicker", - data: { target: "config.authenticationProviders" } %> -
+

+ How can people create an account on your community? +

- <%= inline_svg_tag("email.svg", class: "crayons-icon", aria: true, title: "Twitter logo") %> + <%= inline_svg_tag("email.svg", class: "crayons-icon", aria: true, title: "Email icon") %>
-
-

- Email address and password -

+
+
+ + <% if SiteConfig.allow_both_email_signup_and_login %> +
+ <%= inline_svg_tag("checkmark.svg", aria: true, class: "crayons-icon admin-config-checkmark", title: "Checkmark") %> + Enabled +
+ <% end %> +
+
-
+
+
+ <%= admin_config_label :authentication_providers %> +

+ <%= Constants::SiteConfig::DETAILS[:authentication_providers][:description] %> +

+ <%= select_tag "site_config[authentication_providers]", + options_from_collection_for_select( + authentication_available_providers, + :provider_name, + :official_name, + authentication_enabled_providers.map(&:provider_name), + ), + multiple: true, + class: "form-control selectpicker", + data: { target: "config.authenticationProviders" } %> +
+
<%= inline_svg_tag("twitter.svg", class: "crayons-icon", aria: true, title: "Twitter logo") %> @@ -1338,4 +1373,5 @@
+
diff --git a/app/views/shared/authentication/_providers_registration_form.html.erb b/app/views/shared/authentication/_providers_registration_form.html.erb index 94f418ac2..a5cbd3316 100644 --- a/app/views/shared/authentication/_providers_registration_form.html.erb +++ b/app/views/shared/authentication/_providers_registration_form.html.erb @@ -8,7 +8,7 @@ <%= params[:state] == "new-user" ? "Sign up" : "Continue" %> with <%= provider.official_name %> <% end %> - <% if params[:state] == "new-user" && SiteConfig.allow_email_password_registration %> + <% if params[:state] == "new-user" && SiteConfig.allow_email_password_registration && !SiteConfig.invite_only_mode %> <%= link_to "#{inline_svg_tag('email.svg', aria: true, class: 'crayons-icon', title: 'email')}Sign up with Email".html_safe, request.params.merge(state: "email_signup").except("i"), class: "crayons-btn crayons-btn--l crayons-btn--brand-email crayons-btn--icon-left whitespace-nowrap", diff --git a/spec/requests/admin/configs_spec.rb b/spec/requests/admin/configs_spec.rb index 479b6a75d..66f5122e7 100644 --- a/spec/requests/admin/configs_spec.rb +++ b/spec/requests/admin/configs_spec.rb @@ -87,6 +87,21 @@ RSpec.describe "/admin/config", type: :request do confirmation: confirmation_message } expect(SiteConfig.authentication_providers).to eq([provider]) end + + it "enables email authentication" do + post "/admin/config", params: { site_config: { allow_both_email_signup_and_login: true }, + confirmation: confirmation_message } + expect(SiteConfig.allow_email_password_registration).to be(true) + expect(SiteConfig.allow_email_password_login).to be(true) + end + + it "disables email authentication and invite-only mode" do + post "/admin/config", params: { site_config: { allow_both_email_signup_and_login: false }, + confirmation: confirmation_message } + expect(SiteConfig.allow_email_password_registration).to be(false) + expect(SiteConfig.allow_email_password_login).to be(false) + expect(SiteConfig.invite_only_mode).to be(false) + end end describe "Community Content" do