Merge pull request 'Allow updating one's email address on the account settings page' (#127) from feature/103-update_email into master

Reviewed-on: #127
Reviewed-by: greg <greg@noreply.kosmos.org>

app/assets/stylesheets/components/buttons.css

@@ -32,8 +32,4 @@
     @apply bg-red-600 hover:bg-red-700 text-white
            focus:ring-red-500 focus:ring-opacity-75;
   }
-
-  input[type=text]:disabled {
-    @apply text-gray-700;
-  }
 }

app/assets/stylesheets/components/forms.css

@@ -6,12 +6,13 @@
            focus:ring-blue-600 focus:ring-opacity-75;
   }
 
-  .field_with_errors {
-    @apply inline-block;
+  input[type=text]:disabled,
+  input[type=email]:disabled {
+    @apply text-gray-700;
   }
 
-  .field_with_errors input {
-    @apply w-full bg-red-100;
+  input.field_with_errors {
+    @apply border-b-red-600;
   }
 
   .error-msg {

app/controllers/settings_controller.rb

@@ -1,7 +1,7 @@
 class SettingsController < ApplicationController
   before_action :authenticate_user!
   before_action :set_main_nav_section
-  before_action :set_settings_section, only: ['show', 'update']
+  before_action :set_settings_section, only: [:show, :update, :update_email]
 
   def index
     redirect_to setting_path(:profile)
@@ -21,6 +21,25 @@ class SettingsController < ApplicationController
     }
   end
 
+  def update_email
+    if current_user.valid_ldap_authentication?(email_params[:current_password])
+      current_user.email = email_params[:email]
+
+      if current_user.update email: email_params[:email]
+        redirect_to setting_path(:account), flash: {
+          notice: 'Please confirm your new address using the confirmation link we just sent you.'
+        }
+      else
+        @validation_errors = current_user.errors
+        render :show, status: :unprocessable_entity
+      end
+    else
+      redirect_to setting_path(:account), flash: {
+        error: 'Password did not match your current password. Try again.'
+      }
+    end
+  end
+
   def reset_password
     current_user.send_reset_password_instructions
     sign_out current_user
@@ -49,4 +68,8 @@ class SettingsController < ApplicationController
       :xmpp_exchange_contacts_with_invitees
     ])
   end
+
+  def email_params
+    params.require(:user).permit(:email, :current_password)
+  end
 end

app/javascript/controllers/settings/account/email_controller.js

@@ -0,0 +1,27 @@
+import { Controller } from "@hotwired/stimulus"
+
+export default class extends Controller {
+  static targets = [ "emailField", "editEmailButton" ]
+  static values = { validationFailed: Boolean }
+
+  connect () {
+    if (this.validationFailedValue) return;
+
+    this.emailFieldTarget.disabled = true;
+    this.element.querySelectorAll(".initial-hidden").forEach(el => {
+      el.classList.add("hidden");
+    })
+    this.element.querySelectorAll(".initial-visible").forEach(el => {
+      el.classList.remove("hidden");
+    })
+  }
+
+  editEmail () {
+    this.emailFieldTarget.disabled = false;
+    this.emailFieldTarget.select();
+    this.editEmailButtonTarget.classList.add("hidden");
+    this.element.querySelectorAll(".initial-hidden").forEach(el => {
+      el.classList.remove("hidden");
+    })
+  }
+}

app/mailers/devise/mailer.rb

@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+if defined?(ActionMailer)
+  class Devise::Mailer < Devise.parent_mailer.constantize
+    include Devise::Mailers::Helpers
+
+    def confirmation_instructions(record, token, opts = {})
+      @token = token
+      if record.pending_reconfirmation?
+        devise_mail(record, :reconfirmation_instructions, opts)
+      else
+        devise_mail(record, :confirmation_instructions, opts)
+      end
+    end
+
+    def reset_password_instructions(record, token, opts = {})
+      @token = token
+      devise_mail(record, :reset_password_instructions, opts)
+    end
+
+    def unlock_instructions(record, token, opts = {})
+      @token = token
+      devise_mail(record, :unlock_instructions, opts)
+    end
+
+    def email_changed(record, opts = {})
+      devise_mail(record, :email_changed, opts)
+    end
+
+    def password_change(record, opts = {})
+      devise_mail(record, :password_change, opts)
+    end
+  end
+end

app/models/user.rb

@@ -58,13 +58,19 @@ class User < ApplicationRecord
   end
 
   def devise_after_confirmation
-    enable_service %w[ discourse gitea mediawiki xmpp ]
+    if ldap_entry[:mail] != self.email
+      # E-Mail update confirmed
+      LdapManager::UpdateEmail.call(self.dn, self.email)
+    else
+      # E-Mail from signup confirmed (i.e. account activation)
+      enable_service %w[ discourse gitea mediawiki xmpp ]
 
-    #TODO enable in development when we have easy setup of ejabberd etc.
-    return if Rails.env.development? || !Setting.ejabberd_enabled?
+      #TODO enable in development when we have easy setup of ejabberd etc.
+      return if Rails.env.development? || !Setting.ejabberd_enabled?
 
-    XmppExchangeContactsJob.perform_later(inviter, self) if inviter.present?
-    XmppSetDefaultBookmarksJob.perform_later(self)
+      XmppExchangeContactsJob.perform_later(inviter, self) if inviter.present?
+      XmppSetDefaultBookmarksJob.perform_later(self)
+    end
   end
 
   def send_devise_notification(notification, *args)

app/services/ldap_manager/update_email.rb

@@ -0,0 +1,12 @@
+module LdapManager
+  class UpdateEmail < LdapManagerService
+    def initialize(dn, address)
+      @dn = dn
+      @address = address
+    end
+
+    def call
+      replace_attribute @dn, :mail, [ @address ]
+    end
+  end
+end

app/services/ldap_manager_service.rb

@@ -0,0 +1,2 @@
+class LdapManagerService < LdapService
+end

app/views/devise/mailer/confirmation_instructions.html.erb

@@ -1,5 +1,5 @@
-<p>Welcome <%= @email %>!</p>
+<p>Welcome <%= @resource.cn %>!</p>
 
-<p>You can confirm your account email through the link below:</p>
+<p>Please confirm your email address through the link below:</p>
 
 <p><%= link_to 'Confirm my account', confirmation_url(@resource, confirmation_token: @token) %></p>

app/views/devise/mailer/email_changed.html.erb

@@ -1,4 +1,4 @@
-<p>Hello <%= @email %>!</p>
+<p>Hello <%= @resource.cn %>!</p>
 
 <% if @resource.try(:unconfirmed_email?) %>
   <p>We're contacting you to notify you that your email is being changed to <%= @resource.unconfirmed_email %>.</p>

app/views/devise/mailer/password_change.html.erb

@@ -1,3 +1,3 @@
-<p>Hello <%= @resource.email %>!</p>
+<p>Hello <%= @resource.cn %>!</p>
 
 <p>We're contacting you to notify you that your password has been changed.</p>

app/views/devise/mailer/reconfirmation_instructions.html.erb

@@ -0,0 +1,5 @@
+<p>Hello <%= @resource.cn %>,</p>
+
+<p>Please confirm your new email address through the link below:</p>
+
+<p><%= link_to 'Confirm my address', confirmation_url(@resource, confirmation_token: @token) %></p>

app/views/devise/mailer/reset_password_instructions.html.erb

@@ -1,4 +1,4 @@
-<p>Hello <%= @resource.email %>!</p>
+<p>Hello <%= @resource.cn %>!</p>
 
 <p>Someone has requested a link to change your password. You can do this through the link below.</p>
 

app/views/devise/mailer/unlock_instructions.html.erb

@@ -1,4 +1,4 @@
-<p>Hello <%= @resource.email %>!</p>
+<p>Hello <%= @resource.cn %>!</p>
 
 <p>Your account has been locked due to an excessive number of unsuccessful sign in attempts.</p>
 

app/views/icons/_edit-2.html.erb

@@ -1 +1 @@
-<svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" class="feather feather-edit-2"><path d="M17 3a2.828 2.828 0 1 1 4 4L7.5 20.5 2 22l1.5-5.5L17 3z"></path></svg>
+<svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" class="feather feather-edit-2 <%= custom_class %>"><path d="M17 3a2.828 2.828 0 1 1 4 4L7.5 20.5 2 22l1.5-5.5L17 3z"></path></svg>

app/views/icons/_edit-3.html.erb

@@ -1 +1 @@
-<svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" class="feather feather-edit-3"><path d="M12 20h9"></path><path d="M16.5 3.5a2.121 2.121 0 0 1 3 3L7 19l-4 1 1-4L16.5 3.5z"></path></svg>
+<svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" class="feather feather-edit-3 <%= custom_class %>"><path d="M12 20h9"></path><path d="M16.5 3.5a2.121 2.121 0 0 1 3 3L7 19l-4 1 1-4L16.5 3.5z"></path></svg>

app/views/icons/_edit.html.erb

@@ -1 +1 @@
-<svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" class="feather feather-edit"><path d="M11 4H4a2 2 0 0 0-2 2v14a2 2 0 0 0 2 2h14a2 2 0 0 0 2-2v-7"></path><path d="M18.5 2.5a2.121 2.121 0 0 1 3 3L12 15l-4 1 1-4 9.5-9.5z"></path></svg>
+<svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" class="feather feather-edit <%= custom_class %>"><path d="M11 4H4a2 2 0 0 0-2 2v14a2 2 0 0 0 2 2h14a2 2 0 0 0 2-2v-7"></path><path d="M18.5 2.5a2.121 2.121 0 0 1 3 3L12 15l-4 1 1-4 9.5-9.5z"></path></svg>

app/views/settings/_account.html.erb

@@ -1,13 +1,44 @@
-<section>
+<%= tag.section data: {
+      controller: "settings--account--email",
+      "settings--account--email-validation-failed-value": @validation_errors.present?
+    } do %>
   <h3>E-Mail</h3>
-  <p class="mb-2">
-    <%= label :email, 'Address', class: 'font-bold' %>
-  </p>
-  <p class="flex gap-1 mb-2 sm:w-3/5">
-    <input type="text" id="email" class="grow"
-           value=<%= current_user.email %> disabled="disabled" />
-  </p>
-</section>
+  <%= form_for(current_user, url: update_email_settings_path, method: "post") do |f| %>
+    <%= hidden_field_tag :section, "account" %>
+    <p class="mb-2">
+      <%= f.label :email, 'Address', class: 'font-bold' %>
+    </p>
+    <p class="mb-2 flex gap-1 sm:w-3/5">
+      <%= f.email_field :email, class: "grow", data: {
+            'settings--account--email-target': 'emailField'
+          }, required: true %>
+      <button type="button" id="edit-email"
+              class="btn-md btn-icon btn-blue shrink-0 hidden initial-visible"
+              data-settings--account--email-target="editEmailButton"
+              data-action="settings--account--email#editEmail"
+              title="Edit email address">
+        <span class="">
+          <%= render partial: "icons/edit-3", locals: {
+                custom_class: "text-white h-4 w-4 inline" } %>
+        </span>
+      </button>
+    </p>
+    <% if @validation_errors.present? && @validation_errors[:email].present? %>
+      <p class="error-msg"><%= @validation_errors[:email].first %></p>
+    <% end %>
+    <div class="initial-hidden">
+      <p class="mt-4 mb-2">
+        <%= f.label :current_password, 'Current password', class: 'font-bold' %>
+      </p>
+      <p class="sm:w-3/5">
+        <%= f.password_field :current_password, class: "w-full", required: true %>
+      </p>
+      <p class="mt-6">
+        <%= f.submit "Update", class: "btn-md btn-blue" %>
+      </p>
+    </div>
+  <% end %>
+<% end %>
 <section>
   <h3>Password</h3>
   <p class="mb-8">Use the following button to request an email with a password reset link:</p>

app/views/shared/_sidenav_settings.html.erb

@@ -14,7 +14,7 @@
 <% end %>
 <% if Setting.lndhub_enabled %>
 <%= render SidenavLinkComponent.new(
-  name: "Wallet", path: setting_path(:lightning), icon: "zap",
+  name: "Lightning", path: setting_path(:lightning), icon: "zap",
   active: current_page?(setting_path(:lightning))
 ) %>
 <% end %>

config/initializers/field_with_errors.rb

@@ -0,0 +1,9 @@
+ActionView::Base.field_error_proc = Proc.new do |html_tag, instance|
+  if html_tag.match('class')
+    html_tag.gsub(/class="(.*?)"/, 'class="\1 field_with_errors"').html_safe
+  else
+    parts = html_tag.split('>', 2)
+    parts[0] += ' class="field_with_errors">'
+    (parts[0] + parts[1]).html_safe
+  end
+end

config/locales/devise.en.yml

@@ -3,7 +3,7 @@
 en:
   devise:
     confirmations:
-      confirmed: "Thanks for confirming your email address! Your account has been activated."
+      confirmed: "Thanks for confirming your email address."
       send_instructions: "You will receive an email with instructions for how to confirm your email address in a moment."
       send_paranoid_instructions: "If your email address exists in our database, you will receive an email with instructions for how to confirm your email address in a few minutes."
     failure:

config/routes.rb

@@ -30,6 +30,7 @@ Rails.application.routes.draw do
 
   resources :settings, param: 'section', only: ['index', 'show', 'update'] do
     collection do
+      post 'update_email'
       post 'reset_password'
     end
   end

spec/features/settings/account_spec.rb

@@ -0,0 +1,55 @@
+require 'rails_helper'
+
+RSpec.describe 'Account settings', type: :feature do
+  let(:user) { create :user }
+  let(:geraint) { create :user, id: 2, cn: 'geraint', email: "lamagliarosa@example.com" }
+
+  before do
+    login_as user, :scope => :user
+    geraint.save!
+
+    allow_any_instance_of(User).to receive(:valid_ldap_authentication?)
+      .with("invalid password").and_return(false)
+    allow_any_instance_of(User).to receive(:valid_ldap_authentication?)
+      .with("valid password").and_return(true)
+  end
+
+  scenario 'Update email address fails with invalid password' do
+    visit setting_path(:account)
+    fill_in 'Address', with: "lamagliarosa@example.com"
+    fill_in 'Current password', with: "invalid password"
+    click_button "Update"
+
+    expect(current_url).to eq(setting_url(:account))
+    expect(user.reload.unconfirmed_email).to be_nil
+    within ".flash-msg" do
+      expect(page).to have_content("did not match your current password")
+    end
+  end
+
+  scenario 'Update email address fails when new address already taken' do
+    visit setting_path(:account)
+    fill_in 'Address', with: "lamagliarosa@example.com"
+    fill_in 'Current password', with: "valid password"
+    click_button "Update"
+
+    expect(current_url).to eq(setting_url(:update_email))
+    expect(user.reload.unconfirmed_email).to be_nil
+    within ".error-msg" do
+      expect(page).to have_content("has already been taken")
+    end
+  end
+
+  scenario 'Update email address works' do
+    visit setting_path(:account)
+    fill_in 'Address', with: "lamagliabianca@example.com"
+    fill_in 'Current password', with: "valid password"
+    click_button "Update"
+
+    expect(current_url).to eq(setting_url(:account))
+    expect(user.reload.unconfirmed_email).to eq("lamagliabianca@example.com")
+    within ".flash-msg" do
+      expect(page).to have_content("Please confirm your new address")
+    end
+  end
+end

spec/models/user_spec.rb

@@ -103,9 +103,16 @@ RSpec.describe User, type: :model do
 
   describe "#devise_after_confirmation" do
     include ActiveJob::TestHelper
-    after { clear_enqueued_jobs }
 
-    let(:user) { create :user, cn: "willherschel", ou: "kosmos.org" }
+    let(:user) { create :user, cn: "willherschel", ou: "kosmos.org", email: "will@hrsch.el" }
+
+    before do
+      allow(user).to receive(:ldap_entry).and_return({
+        uid: "willherschel", ou: "kosmos.org", mail: "will@hrsch.el"
+      })
+    end
+
+    after { clear_enqueued_jobs }
 
     it "enables default services" do
       expect(user).to receive(:enable_service).with(%w[ discourse gitea mediawiki xmpp ])
@@ -124,10 +131,11 @@ RSpec.describe User, type: :model do
       let(:guest) { create :user, id: 2, cn: "isaacnewton", ou: "kosmos.org", email: "newt@example.com" }
 
       before do
-        # TODO remove when defaults are implemented
-        user.update! preferences: { xmpp_exchange_contacts_with_invitees: true }
         Invitation.create! user: user, invited_user_id: guest.id, used_at: DateTime.now
         allow_any_instance_of(User).to receive(:enable_service)
+        allow(guest).to receive(:ldap_entry).and_return({
+          uid: "isaacnewton", ou: "kosmos.org", mail: "newt@example.com"
+        })
       end
 
       it "enqueues jobs to exchange XMPP contacts between inviter and invitee" do
@@ -138,5 +146,31 @@ RSpec.describe User, type: :model do
         expect(job["arguments"][1]['_aj_globalid']).to eq('gid://akkounts/User/2')
       end
     end
+
+    context "for email address update of existing account" do
+      before do
+        allow(user).to receive(:ldap_entry)
+          .and_return({ uid: "willherschel", ou: "kosmos.org", mail: "willyboy@aol.com" })
+        allow(user).to receive(:dn)
+          .and_return("cn=willherschel,ou=kosmos.org,cn=users,dc=kosmos,dc=org")
+        allow(LdapManager::UpdateEmail).to receive(:call)
+      end
+
+      it "updates the LDAP 'mail' attribute" do
+        expect(LdapManager::UpdateEmail).to receive(:call)
+          .with("cn=willherschel,ou=kosmos.org,cn=users,dc=kosmos,dc=org", "will@hrsch.el")
+        user.send :devise_after_confirmation
+      end
+
+      it "does not re-enable default services" do
+        expect(user).not_to receive(:enable_service)
+        user.send :devise_after_confirmation
+      end
+
+      it "does not enqueue any delayed jobs" do
+        user.send :devise_after_confirmation
+        expect(enqueued_jobs).to be_empty
+      end
+    end
   end
 end