Remove password confirmation field (#11388)

* Remove password confirmation field * Bring initial implementation for PasswordToggler JS class Comes from #9490 Made by @microstudi * Adapt PasswordToggler to redesign and call it * Change the callers of password_field method so they all use the partial * Add spec * Migrate PasswordToggler to vanilla JS * Fix specs * Fix accessibility errors * Refactor and clean-up javascript code * Don't wrap the password label with the input and the button * Fix linter offense * Fix bug when there's an error message * Prevent submission on enter * Show label with required indicator (asterisk and a11y goodies) * Fix button paddings when there's an error * Fix failing spec when not showing help text on failed password reset * Show required password on new session form * Clean-up `password_required?` method Suggested by code review Co-authored-by: Alexandru Emil Lupu <contact@alecslupu.ro> --------- Co-authored-by: Alexandru Emil Lupu <contact@alecslupu.ro>
decidim · Sep 10, 2023 · 1f4e94d · 1f4e94d
1 parent 08f506b
commit 1f4e94d
Show file tree

Hide file tree

Showing 50 changed files with 205 additions and 183 deletions.
diff --git a/decidim-admin/spec/shared/manage_impersonations_examples.rb b/decidim-admin/spec/shared/manage_impersonations_examples.rb
@@ -256,7 +256,6 @@
 
       within "form.new_user" do
         fill_in :invitation_user_password, with: "decidim123456789"
-        fill_in :invitation_user_password_confirmation, with: "decidim123456789"
         check :invitation_user_tos_agreement
         find("*[type=submit]").click
       end

diff --git a/decidim-admin/spec/system/admin_invite_spec.rb b/decidim-admin/spec/system/admin_invite_spec.rb
@@ -43,7 +43,6 @@
       within "form.new_user" do
         fill_in :invitation_user_nickname, with: "caballo_loco"
         fill_in :invitation_user_password, with: "decidim123456789"
-        fill_in :invitation_user_password_confirmation, with: "decidim123456789"
         check :invitation_user_tos_agreement
         find("*[type=submit]").click
       end

diff --git a/decidim-admin/spec/system/admin_passwords_spec.rb b/decidim-admin/spec/system/admin_passwords_spec.rb
@@ -22,7 +22,6 @@
       expect(page).to have_content("Admin users need to change their password every 90 days")
       expect(page).to have_content("Password change")
       fill_in :password_user_password, with: new_password
-      fill_in :password_user_password_confirmation, with: new_password
       click_button "Change my password"
       expect(page).to have_css("[data-alert-box].success")
       expect(page).to have_content("Password successfully updated")
@@ -49,7 +48,6 @@
         manual_login(user.email, password)
         expect(page).to have_content("Password change")
         fill_in :password_user_password, with: new_password
-        fill_in :password_user_password_confirmation, with: new_password
         click_button "Change my password"
         expect(page).to have_css(".callout.success")
         expect(page).to have_current_path(decidim_admin.root_path)

diff --git a/decidim-assemblies/lib/decidim/assemblies/participatory_space.rb b/decidim-assemblies/lib/decidim/assemblies/participatory_space.rb
@@ -136,7 +136,6 @@
           name: Faker::Name.name,
           nickname: Faker::Twitter.unique.screen_name,
           password: "decidim123456789",
-          password_confirmation: "decidim123456789",
           organization:,
           confirmed_at: Time.current,
           locale: I18n.default_locale,

diff --git a/decidim-conferences/lib/decidim/conferences/participatory_space.rb b/decidim-conferences/lib/decidim/conferences/participatory_space.rb
@@ -102,7 +102,6 @@
           name: Faker::Name.name,
           nickname: Faker::Twitter.unique.screen_name,
           password: "decidim123456789",
-          password_confirmation: "decidim123456789",
           organization:,
           confirmed_at: Time.current,
           locale: I18n.default_locale,

diff --git a/decidim-core/app/commands/decidim/create_omniauth_registration.rb b/decidim-core/app/commands/decidim/create_omniauth_registration.rb
@@ -64,7 +64,6 @@ def create_or_find_user
         @user.nickname = form.normalized_nickname
         @user.newsletter_notifications_at = nil
         @user.password = generated_password
-        @user.password_confirmation = generated_password
         if form.avatar_url.present?
           url = URI.parse(form.avatar_url)
           filename = File.basename(url.path)

diff --git a/decidim-core/app/commands/decidim/create_registration.rb b/decidim-core/app/commands/decidim/create_registration.rb
@@ -40,7 +40,6 @@ def create_user
         name: form.name,
         nickname: form.nickname,
         password: form.password,
-        password_confirmation: form.password_confirmation,
         password_updated_at: Time.current,
         organization: form.current_organization,
         tos_agreement: form.tos_agreement,

diff --git a/decidim-core/app/commands/decidim/update_account.rb b/decidim-core/app/commands/decidim/update_account.rb
@@ -24,7 +24,7 @@ def call
         notify_followers
         broadcast(:ok, @user.unconfirmed_email.present?)
       else
-        [:avatar, :password, :password_confirmation].each do |key|
+        [:avatar, :password].each do |key|
           @form.errors.add key, @user.errors[key] if @user.errors.has_key? key
         end
         broadcast(:invalid)
@@ -54,7 +54,6 @@ def update_password
       return if @form.password.blank?
 
       @user.password = @form.password
-      @user.password_confirmation = @form.password_confirmation
       @user.password_updated_at = Time.current
     end
 

diff --git a/decidim-core/app/commands/decidim/update_password.rb b/decidim-core/app/commands/decidim/update_password.rb
@@ -16,7 +16,6 @@ def call
       return broadcast(:invalid) if form.invalid?
 
       user.password = form.password
-      user.password_confirmation = form.password
       user.password_updated_at = Time.current
 
       if user.save

diff --git a/decidim-core/app/forms/decidim/account_form.rb b/decidim-core/app/forms/decidim/account_form.rb
@@ -13,7 +13,6 @@ class AccountForm < Form
     attribute :nickname
     attribute :email
     attribute :password
-    attribute :password_confirmation
     attribute :avatar, Decidim::Attributes::Blob
     attribute :remove_avatar, Boolean, default: false
     attribute :personal_url
@@ -24,9 +23,7 @@ class AccountForm < Form
     validates :nickname, presence: true, format: { with: Decidim::User::REGEXP_NICKNAME }
 
     validates :nickname, length: { maximum: Decidim::User.nickname_max_length, allow_blank: true }
-    validates :password, confirmation: true
     validates :password, password: { name: :name, email: :email, username: :nickname }, if: -> { password.present? }
-    validates :password_confirmation, presence: true, if: :password_present
     validates :avatar, passthru: { to: Decidim::User }
 
     validate :unique_email
@@ -45,10 +42,6 @@ def personal_url
 
     private
 
-    def password_present
-      password.present?
-    end
-
     def unique_email
       return true if Decidim::UserBaseEntity.where(
         organization: context.current_organization,

diff --git a/decidim-core/app/forms/decidim/password_form.rb b/decidim-core/app/forms/decidim/password_form.rb
@@ -3,8 +3,7 @@
 module Decidim
   class PasswordForm < Form
     attribute :password
-    attribute :password_confirmation
 
-    validates :password, confirmation: true
+    validates :password, presence: true
   end
 end
diff --git a/decidim-core/app/forms/decidim/registration_form.rb b/decidim-core/app/forms/decidim/registration_form.rb
@@ -9,17 +9,14 @@ class RegistrationForm < Form
     attribute :nickname, String
     attribute :email, String
     attribute :password, String
-    attribute :password_confirmation, String
     attribute :newsletter, Boolean
     attribute :tos_agreement, Boolean
     attribute :current_locale, String
 
     validates :name, presence: true, format: { with: Decidim::User::REGEXP_NAME }
     validates :nickname, presence: true, format: { with: Decidim::User::REGEXP_NICKNAME }, length: { maximum: Decidim::User.nickname_max_length }
     validates :email, presence: true, "valid_email_2/email": { disposable: true }
-    validates :password, confirmation: true
-    validates :password, password: { name: :name, email: :email, username: :nickname }
-    validates :password_confirmation, presence: true
+    validates :password, presence: true, password: { name: :name, email: :email, username: :nickname }
     validates :tos_agreement, allow_nil: false, acceptance: true
 
     validate :email_unique_in_organization

diff --git a/decidim-core/app/helpers/decidim/passwords_helper.rb b/decidim-core/app/helpers/decidim/passwords_helper.rb
@@ -25,6 +25,7 @@ def password_field_options_for(user)
       {
         autocomplete: "new-password",
         required: true,
+        label: false,
         help_text:,
         minlength: min_length,
         maxlength: ::PasswordValidator::MAX_LENGTH,

diff --git a/decidim-core/app/packs/src/decidim/password_toggler.js b/decidim-core/app/packs/src/decidim/password_toggler.js
@@ -0,0 +1,121 @@
+import icon from "src/decidim/redesigned_icon"
+
+export default class PasswordToggler {
+  constructor(password) {
+    this.password = password;
+    this.input = this.password.querySelector('input[type="password"]');
+    this.form = this.input.closest("form");
+    this.texts = {
+      showPassword: this.password.getAttribute("data-show-password") || "Show password",
+      hidePassword: this.password.getAttribute("data-hide-password") || "Hide password",
+      hiddenPassword: this.password.getAttribute("data-hidden-password") || "Your password is hidden",
+      shownPassword: this.password.getAttribute("data-shown-password") || "Your password is shown"
+    }
+    this.icons = {
+      show: icon("eye-line"),
+      hide: icon("eye-off-line")
+    }
+    this.buttonClass = {
+      onValid: "mt-5",
+      onInvalid: "mb-2"
+    }
+  }
+
+  // Call init() to hide the password confirmation and add a "view password" inline button
+  init() {
+    this.createControls();
+    this.button.addEventListener("click", (evt) => {
+      this.toggleVisibiliy(evt);
+    });
+    // to prevent browsers trying to use autocomplete, turn the type back to password before submitting
+    this.form.addEventListener("submit", () => {
+      this.hidePassword();
+    });
+
+    // to fix the button margin if there are or not errors
+    // as foundation abide needs jQuery, we need to do it with 'on' instead of 'addEventListener'
+    $(this.input).on("invalid.zf.abide", () => {
+      this.button.classList.remove(this.buttonClass.onValid);
+      this.button.classList.add(this.buttonClass.onInvalid);
+    });
+
+    $(this.input).on("valid.zf.abide", () => {
+      this.button.classList.add(this.buttonClass.onValid);
+      this.button.classList.remove(this.buttonClass.onInvalid);
+    });
+  }
+
+  // Call destroy() to switch back to the original password box
+  destroy() {
+    this.button.removeEventListener("click");
+    this.input.removeEventListener("change");
+    this.form.removeEventListener("submit");
+    const input = this.input.detach();
+    this.inputGroup.replaceWith(input);
+  }
+
+  createControls() {
+    this.createButton();
+    this.createStatusText();
+    this.addInputGroupWrapperAsParent();
+  }
+
+  createButton() {
+    const button = document.createElement("button");
+    button.classList.add(this.buttonClass.onValid);
+    button.setAttribute("type", "button");
+    button.setAttribute("aria-controls", this.input.getAttribute("id"));
+    button.setAttribute("aria-label", this.texts.showPassword);
+    button.innerHTML = this.icons.show;
+    this.button = button;
+  }
+
+  createStatusText() {
+    const statusText = document.createElement("span");
+    statusText.classList.add("sr-only");
+    statusText.setAttribute("aria-live", "polite");
+    statusText.textContent = this.texts.hiddenPassword;
+    this.statusText = statusText;
+  }
+
+  addInputGroupWrapperAsParent() {
+    const inputGroupWrapper = document.createElement("div");
+    inputGroupWrapper.classList.add("filter-search", "filter-container");
+
+    this.input.parentNode.replaceChild(inputGroupWrapper, this.input);
+    inputGroupWrapper.appendChild(this.input);
+    this.input.after(this.button);
+
+    const formError = this.password.querySelector(".form-error");
+    if (formError) {
+      this.input.after(formError);
+    }
+  }
+
+  toggleVisibiliy(evt) {
+    evt.preventDefault();
+    if (this.isText()) {
+      this.hidePassword();
+    } else {
+      this.showPassword();
+    }
+  }
+
+  showPassword() {
+    this.statusText.textContent = this.texts.shownPassword;
+    this.button.setAttribute("aria-label", this.texts.hidePassword);
+    this.button.innerHTML = this.icons.hide;
+    this.input.setAttribute("type", "text");
+  }
+
+  hidePassword() {
+    this.statusText.textContent = this.texts.hiddenPassword;
+    this.button.setAttribute("aria-label", this.texts.showPassword);
+    this.button.innerHTML = this.icons.show;
+    this.input.setAttribute("type", "password");
+  }
+
+  isText() {
+    return this.input.getAttribute("type") === "text"
+  }
+}
diff --git a/decidim-core/app/packs/src/decidim/user_registrations.js b/decidim-core/app/packs/src/decidim/user_registrations.js
@@ -1,6 +1,9 @@
+import PasswordToggler from "./password_toggler";
+
 $(() => {
   const $userRegistrationForm = $("#register-form");
   const $userGroupFields      = $userRegistrationForm.find(".user-group-fields");
+  const userPassword         =  document.querySelector(".user-password");
   const inputSelector         = 'input[name="user[sign_up_as]"]';
   const newsletterSelector    = 'input[type="checkbox"][name="user[newsletter]"]';
   const $newsletterModal      = $("#sign-up-newsletter-modal");
@@ -42,4 +45,8 @@ $(() => {
   $newsletterModal.find("[data-check]").on("click", (event) => {
     checkNewsletter($(event.target).data("check"));
   });
+
+  if (userPassword) {
+    new PasswordToggler(userPassword).init();
+  }
 });
diff --git a/decidim-core/app/views/decidim/account/_password_fields.html.erb b/decidim-core/app/views/decidim/account/_password_fields.html.erb
@@ -1,2 +1,13 @@
-<%= form.password_field :password, password_field_options_for(current_user) %>
-<%= form.password_field :password_confirmation, password_field_options_for(current_user).except(:help_text) %>
+<div class="user-password"
+  data-show-password="<%= t "show_password", scope: "decidim.devise.shared.password_fields" %>"
+  data-hide-password="<%= t "hide_password", scope: "decidim.devise.shared.password_fields" %>"
+  data-hidden-password="<%= t "hidden_password", scope: "decidim.devise.shared.password_fields" %>"
+  data-shown-password="<%= t "shown_password", scope: "decidim.devise.shared.password_fields" %>">
+  <%= form.send(:custom_label, :password, t("activemodel.attributes.user.password"), { required: true }) %>
+
+  <% if local_assigns.has_key?(:show_help_text) ||  local_assigns.has_key?(:user) %>
+    <%= form.password_field :password, password_field_options_for(user).merge(value: form.object.password) %>
+  <% else %>
+    <%= form.password_field :password, autocomplete: "current-password", required: true, label: false %>
+  <% end %>
+</div>
diff --git a/decidim-core/app/views/decidim/account/show.html.erb b/decidim-core/app/views/decidim/account/show.html.erb
@@ -42,16 +42,16 @@
         <%= t("available_locales_helper", scope:"decidim.account.show") %>
       </p>
 
-      <% if @account.errors[:password].any? || @account.errors[:password_confirmation].any? %>
-        <%= render partial: "password_fields", locals: { form: f } %>
+      <% if @account.errors[:password].any? %>
+        <%= render partial: "password_fields", locals: { form: f, user: current_user } %>
       <% else %>
         <% if current_organization.sign_in_enabled? %>
           <div data-component="accordion" id="accordion-password">
             <span id="accordion-trigger-panel-password" class="text-lg font-semibold text-secondary underline block" data-controls="panel-password">
               <%= t("change_password", scope:"decidim.account.show") %>
             </span>
             <div id="panel-password" class="form__wrapper">
-              <%= render partial: "password_fields", locals: { form: f } %>
+              <%= render partial: "password_fields", locals: { form: f, user: current_user } %>
             </div>
           </div>
         <% end %>

diff --git a/decidim-core/app/views/decidim/devise/invitations/edit.html.erb b/decidim-core/app/views/decidim/devise/invitations/edit.html.erb
@@ -17,8 +17,7 @@
       <%= f.text_field :nickname, help_text: t("devise.invitations.edit.nickname_help", organization: current_organization.name), required: "required", autocomplete: "nickname" %>
 
       <% if f.object.class.require_password_on_accepting %>
-        <%= f.password_field :password, password_field_options_for(:user).except(:help_text) %>
-        <%= f.password_field :password_confirmation, password_field_options_for(:user).except(:help_text) %>
+        <%= render partial: "decidim/account/password_fields", locals: { form: f, user: :user } %>
       <% end %>
     </div>
 

diff --git a/decidim-core/app/views/decidim/devise/passwords/edit.html.erb b/decidim-core/app/views/decidim/devise/passwords/edit.html.erb
@@ -10,9 +10,7 @@
     <div class="form__wrapper">
       <%= f.hidden_field :reset_password_token %>
 
-      <%= f.password_field :password, password_field_options_for(current_user || params["reset_password_token"]) %>
-
-      <%= f.password_field :password_confirmation, password_field_options_for(current_user || params["reset_password_token"]).except(:help_text) %>
+      <%= render partial: "decidim/account/password_fields", locals: { form: f, user: current_user || params["reset_password_token"], show_help_text: true } %>
     </div>
 
     <div class="form__wrapper-block">

diff --git a/decidim-core/app/views/decidim/devise/registrations/new.html.erb b/decidim-core/app/views/decidim/devise/registrations/new.html.erb
@@ -39,9 +39,7 @@
 
       <%= f.email_field :email, autocomplete: "email", placeholder: "hi@email.com" %>
 
-      <%= f.password_field :password, password_field_options_for(:user) %>
-
-      <%= f.password_field :password_confirmation, password_field_options_for(:user).except(:help_text) %>
+      <%= render partial: "decidim/account/password_fields", locals: { form: f, user: :user } %>
     </div>
 
     <div id="card__tos" class="form__wrapper-block">

diff --git a/decidim-core/app/views/decidim/devise/sessions/new.html.erb b/decidim-core/app/views/decidim/devise/sessions/new.html.erb
@@ -32,9 +32,9 @@
   <% if current_organization.sign_in_enabled? %>
     <%= decidim_form_for(resource, namespace: "session", as: resource_name, url: session_path(resource_name) ) do |f| %>
       <div class="form__wrapper">
-        <%= f.email_field :email, autocomplete: "email", placeholder: "hi@email.com" %>
+        <%= f.email_field :email, autocomplete: "email", placeholder: "hi@email.com", required: true %>
 
-        <%= f.password_field :password, autocomplete: "current-password", placeholder: "••••••" %>
+        <%= render partial: "decidim/account/password_fields", locals: { form: f } %>
 
         <% if devise_mapping.rememberable? %>
           <%= f.check_box :remember_me, label_options: { class: "form__wrapper-checkbox-label" } %>

diff --git a/decidim-core/app/views/decidim/shared/_login_modal.html.erb b/decidim-core/app/views/decidim/shared/_login_modal.html.erb
@@ -21,7 +21,7 @@
           <div class="form__wrapper">
             <%= f.email_field :email, autocomplete: "email", placeholder: "hi@email.com" %>
 
-            <%= f.password_field :password, autocomplete: "current-password", placeholder: "••••••" %>
+            <%= render partial: "decidim/account/password_fields", locals: { form: f } %>
           </div>
 
           <div class="login__modal-links">

diff --git a/decidim-core/config/locales/en.yml b/decidim-core/config/locales/en.yml
@@ -629,6 +629,11 @@ en:
           title: Newsletter notifications
         omniauth_buttons:
           or: Or
+        password_fields:
+          hidden_password: Your password is hidden
+          hide_password: Hide password
+          show_password: Show password
+          shown_password: Your password is shown
     doorkeeper:
       authorizations:
         new: