From f4bd1df21ceaa7915ac8e79abe8d4e526b480e4b Mon Sep 17 00:00:00 2001 From: Arit Amana <32520970+msarit@users.noreply.github.com> Date: Mon, 12 Oct 2020 12:54:35 -0400 Subject: [PATCH] [Email/Password Authentication] Allow Admins to enable email/password login (#10745) * Implement Admins ability to allow email login * Write specs for admin allow email login * Fix specs causing builds to fail * Fix spec causing builds to fail * Use dummyimage.com for spec images Co-authored-by: Josh Puetz * Use dummyimage.com * refactor setting of SiteConfig attributes * Address PR review comments * fix indentation * Use "allow to receive and return" approach for other specs Co-authored-by: Josh Puetz --- app/controllers/admin/configs_controller.rb | 1 + app/lib/constants/site_config.rb | 4 +++ app/models/site_config.rb | 1 + app/views/admin/configs/show.html.erb | 5 +++ .../registrations/_registration_form.html.erb | 30 ++---------------- .../authentication/_email_login_form.html.erb | 25 +++++++++++++++ spec/fixtures/files/300x100.svg | 6 ++++ spec/requests/editor_spec.rb | 12 +++++++ spec/requests/registrations_spec.rb | 31 ++++++++++++------- .../articles/user_edits_an_article_spec.rb | 7 +++++ .../system/user_logs_in_with_password_spec.rb | 1 + 11 files changed, 84 insertions(+), 39 deletions(-) create mode 100644 app/views/shared/authentication/_email_login_form.html.erb create mode 100644 spec/fixtures/files/300x100.svg diff --git a/app/controllers/admin/configs_controller.rb b/app/controllers/admin/configs_controller.rb index f97707cff..468c93a01 100644 --- a/app/controllers/admin/configs_controller.rb +++ b/app/controllers/admin/configs_controller.rb @@ -57,6 +57,7 @@ module Admin facebook_secret invite_only_mode allow_email_password_registration + allow_email_password_login primary_brand_color_hex spam_trigger_terms recaptcha_site_key diff --git a/app/lib/constants/site_config.rb b/app/lib/constants/site_config.rb index d73d1e8cc..793b225cd 100644 --- a/app/lib/constants/site_config.rb +++ b/app/lib/constants/site_config.rb @@ -5,6 +5,10 @@ module Constants description: "Can users sign up with only email and password?", placeholder: "" }, + allow_email_password_login: { + description: "Can users login with only email and password?", + placeholder: "" + }, authentication_providers: { description: "How can users sign in?", placeholder: "" diff --git a/app/models/site_config.rb b/app/models/site_config.rb index 66f2065e2..0d3ce0b34 100644 --- a/app/models/site_config.rb +++ b/app/models/site_config.rb @@ -22,6 +22,7 @@ class SiteConfig < RailsSettings::Base # Authentication field :allow_email_password_registration, type: :boolean, default: false + field :allow_email_password_login, type: :boolean, default: false field :authentication_providers, type: :array, default: proc { Authentication::Providers.available } field :invite_only_mode, type: :boolean, default: false field :twitter_key, type: :string, default: ApplicationConfig["TWITTER_KEY"] diff --git a/app/views/admin/configs/show.html.erb b/app/views/admin/configs/show.html.erb index aed095f47..8b2da4c81 100644 --- a/app/views/admin/configs/show.html.erb +++ b/app/views/admin/configs/show.html.erb @@ -126,6 +126,11 @@ <%= f.check_box :allow_email_password_registration, checked: SiteConfig.allow_email_password_registration %>
<%= Constants::SiteConfig::DETAILS[:allow_email_password_registration][:description] %>
+
+ <%= admin_config_label :allow_email_password_login %> + <%= f.check_box :allow_email_password_login, checked: SiteConfig.allow_email_password_login %> +
<%= Constants::SiteConfig::DETAILS[:allow_email_password_login][:description] %>
+
<%= admin_config_label :authentication_providers %> <%= select_tag "site_config[authentication_providers]", diff --git a/app/views/devise/registrations/_registration_form.html.erb b/app/views/devise/registrations/_registration_form.html.erb index 4182247bc..c8a33799a 100644 --- a/app/views/devise/registrations/_registration_form.html.erb +++ b/app/views/devise/registrations/_registration_form.html.erb @@ -26,40 +26,14 @@
<% else %>
- <%# if there are no SSO auths enabled, we don't ask the user if they have a password %> - <% if authentication_enabled_providers.any? %> + <% if SiteConfig.allow_email_password_login %>
Have a password? Continue with your email address
+ <%= render partial: "shared/authentication/email_login_form" %> <% end %> - - <%= form_for(User.new, as: :user, url: session_path(:user)) do |f| %> -
- <%= f.label :email, class: "crayons-field__label" %> - <%= f.email_field :email, autocomplete: "email", class: "crayons-textfield" %> -
- -
- <%= f.label :password, class: "crayons-field__label" %> - <%= f.password_field :password, autocomplete: "current-password", class: "crayons-textfield" %> -
- -
- <%= f.check_box :remember_me, class: "crayons-checkbox" %> - <%= f.label :remember_me, class: "crayons-field__label fw-normal" %> -
- -
- <%= f.submit "Continue", class: "crayons-btn crayons-btn--l w-100" %> -
- <% end %> -

- - I forgot my password - -

<% end %> diff --git a/app/views/shared/authentication/_email_login_form.html.erb b/app/views/shared/authentication/_email_login_form.html.erb new file mode 100644 index 000000000..3d94ca5f0 --- /dev/null +++ b/app/views/shared/authentication/_email_login_form.html.erb @@ -0,0 +1,25 @@ +<%= form_for(User.new, as: :user, url: session_path(:user)) do |f| %> +
+ <%= f.label :email, class: "crayons-field__label" %> + <%= f.email_field :email, autocomplete: "email", class: "crayons-textfield" %> +
+ +
+ <%= f.label :password, class: "crayons-field__label" %> + <%= f.password_field :password, autocomplete: "current-password", class: "crayons-textfield" %> +
+ +
+ <%= f.check_box :remember_me, class: "crayons-checkbox" %> + <%= f.label :remember_me, class: "crayons-field__label fw-normal" %> +
+ +
+ <%= f.submit "Continue", class: "crayons-btn crayons-btn--l w-100" %> +
+<% end %> +

+ + I forgot my password + +

diff --git a/spec/fixtures/files/300x100.svg b/spec/fixtures/files/300x100.svg new file mode 100644 index 000000000..e455796e5 --- /dev/null +++ b/spec/fixtures/files/300x100.svg @@ -0,0 +1,6 @@ + + + + 300×100 + + diff --git a/spec/requests/editor_spec.rb b/spec/requests/editor_spec.rb index f80e795f3..be4d5cd9f 100644 --- a/spec/requests/editor_spec.rb +++ b/spec/requests/editor_spec.rb @@ -7,6 +7,18 @@ RSpec.describe "Editor", type: :request do get new_path expect(response).to have_http_status(:ok) + end + end + + context "when email login is allowed in /admin/config" do + before do + allow(SiteConfig).to receive(:allow_email_password_login).and_return(true) + end + + it "asks the non logged in user to sign in, with email signin enabled" do + get new_path + + expect(response.body).to include("Email") expect(response.body).to include("Password") end end diff --git a/spec/requests/registrations_spec.rb b/spec/requests/registrations_spec.rb index 640c3717e..7a7026712 100644 --- a/spec/requests/registrations_spec.rb +++ b/spec/requests/registrations_spec.rb @@ -15,27 +15,36 @@ RSpec.describe "Registrations", type: :request do end end - it "shows the sign in text for password based authentication" do - get sign_up_path - - expect(response.body).to include("Have a password? Continue with your email address") - end - - it "does not show the password based authentication hint if there are no single sign in options enabled" do + it "only shows the single sign on options if they are present" do allow(Authentication::Providers).to receive(:enabled).and_return([]) get sign_up_path expect(response.body).not_to include("Have a password? Continue with your email address") end + end - it "only shows the single sign on options if they are present" do - allow(Authentication::Providers).to receive(:enabled).and_return([]) + context "when email login is enabled in /admin/config" do + before do + allow(SiteConfig).to receive(:allow_email_password_login).and_return(true) + end + it "shows the sign in text for password based authentication" do get sign_up_path - expect(response.body).to include("Password") - expect(response.body).not_to include("Continue with") + expect(response.body).to include("Have a password? Continue with your email address") + end + end + + context "when email login is disabled in /admin/config" do + before do + allow(SiteConfig).to receive(:allow_email_password_login).and_return(false) + end + + it "does not show the sign in text for password based authentication" do + get sign_up_path + + expect(response.body).not_to include("Have a password? Continue with your email address") end end diff --git a/spec/system/articles/user_edits_an_article_spec.rb b/spec/system/articles/user_edits_an_article_spec.rb index 0b4fc8f01..2b9242a83 100644 --- a/spec/system/articles/user_edits_an_article_spec.rb +++ b/spec/system/articles/user_edits_an_article_spec.rb @@ -4,8 +4,15 @@ RSpec.describe "Editing with an editor", type: :system, js: true do let(:template) { file_fixture("article_published.txt").read } let(:user) { create(:user) } let(:article) { create(:article, user: user, body_markdown: template) } + let(:svg_image) { file_fixture("300x100.svg").read } before do + allow(SiteConfig).to receive(:main_social_image).and_return("https://dummyimage.com/800x600.jpg") + allow(SiteConfig).to receive(:logo_png).and_return("https://dummyimage.com/800x600.png") + allow(SiteConfig).to receive(:mascot_image_url).and_return("https://dummyimage.com/800x600.jpg") + allow(SiteConfig).to receive(:suggested_tags).and_return("coding, beginners") + allow(SiteConfig).to receive(:suggested_users).and_return("romagueramica") + allow(SiteConfig).to receive(:logo_svg).and_return(svg_image) sign_in user end diff --git a/spec/system/user_logs_in_with_password_spec.rb b/spec/system/user_logs_in_with_password_spec.rb index 88a7f8b95..19573cd41 100644 --- a/spec/system/user_logs_in_with_password_spec.rb +++ b/spec/system/user_logs_in_with_password_spec.rb @@ -11,6 +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) visit sign_up_path end