diff --git a/app/assets/stylesheets/components/buttons.css b/app/assets/stylesheets/components/buttons.css index 5da9116..c50d071 100644 --- a/app/assets/stylesheets/components/buttons.css +++ b/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; - } } diff --git a/app/assets/stylesheets/components/forms.css b/app/assets/stylesheets/components/forms.css index 633c293..46f1df9 100644 --- a/app/assets/stylesheets/components/forms.css +++ b/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 { diff --git a/app/controllers/settings_controller.rb b/app/controllers/settings_controller.rb index c5bd8f8..d9f5979 100644 --- a/app/controllers/settings_controller.rb +++ b/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 diff --git a/app/javascript/controllers/settings/account/email_controller.js b/app/javascript/controllers/settings/account/email_controller.js new file mode 100644 index 0000000..5967cfd --- /dev/null +++ b/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"); + }) + } +} diff --git a/app/mailers/devise/mailer.rb b/app/mailers/devise/mailer.rb new file mode 100644 index 0000000..3ff2bb3 --- /dev/null +++ b/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 diff --git a/app/models/user.rb b/app/models/user.rb index 4945d6e..93b7a5d 100644 --- a/app/models/user.rb +++ b/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) diff --git a/app/services/ldap_manager/update_email.rb b/app/services/ldap_manager/update_email.rb new file mode 100644 index 0000000..5acd77f --- /dev/null +++ b/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 diff --git a/app/services/ldap_manager_service.rb b/app/services/ldap_manager_service.rb new file mode 100644 index 0000000..0f43e32 --- /dev/null +++ b/app/services/ldap_manager_service.rb @@ -0,0 +1,2 @@ +class LdapManagerService < LdapService +end diff --git a/app/views/devise/mailer/confirmation_instructions.html.erb b/app/views/devise/mailer/confirmation_instructions.html.erb index dc55f64..7cfe815 100644 --- a/app/views/devise/mailer/confirmation_instructions.html.erb +++ b/app/views/devise/mailer/confirmation_instructions.html.erb @@ -1,5 +1,5 @@ -
Welcome <%= @email %>!
+Welcome <%= @resource.cn %>!
-You can confirm your account email through the link below:
+Please confirm your email address through the link below:
<%= link_to 'Confirm my account', confirmation_url(@resource, confirmation_token: @token) %>
diff --git a/app/views/devise/mailer/email_changed.html.erb b/app/views/devise/mailer/email_changed.html.erb index 32f4ba8..26ac013 100644 --- a/app/views/devise/mailer/email_changed.html.erb +++ b/app/views/devise/mailer/email_changed.html.erb @@ -1,4 +1,4 @@ -Hello <%= @email %>!
+Hello <%= @resource.cn %>!
<% if @resource.try(:unconfirmed_email?) %>We're contacting you to notify you that your email is being changed to <%= @resource.unconfirmed_email %>.
diff --git a/app/views/devise/mailer/password_change.html.erb b/app/views/devise/mailer/password_change.html.erb index b41daf4..4793cbe 100644 --- a/app/views/devise/mailer/password_change.html.erb +++ b/app/views/devise/mailer/password_change.html.erb @@ -1,3 +1,3 @@ -Hello <%= @resource.email %>!
+Hello <%= @resource.cn %>!
We're contacting you to notify you that your password has been changed.
diff --git a/app/views/devise/mailer/reconfirmation_instructions.html.erb b/app/views/devise/mailer/reconfirmation_instructions.html.erb new file mode 100644 index 0000000..2897428 --- /dev/null +++ b/app/views/devise/mailer/reconfirmation_instructions.html.erb @@ -0,0 +1,5 @@ +Hello <%= @resource.cn %>,
+ +Please confirm your new email address through the link below:
+ +<%= link_to 'Confirm my address', confirmation_url(@resource, confirmation_token: @token) %>
diff --git a/app/views/devise/mailer/reset_password_instructions.html.erb b/app/views/devise/mailer/reset_password_instructions.html.erb index f667dc1..d10b9dc 100644 --- a/app/views/devise/mailer/reset_password_instructions.html.erb +++ b/app/views/devise/mailer/reset_password_instructions.html.erb @@ -1,4 +1,4 @@ -Hello <%= @resource.email %>!
+Hello <%= @resource.cn %>!
Someone has requested a link to change your password. You can do this through the link below.
diff --git a/app/views/devise/mailer/unlock_instructions.html.erb b/app/views/devise/mailer/unlock_instructions.html.erb index 41e148b..c216377 100644 --- a/app/views/devise/mailer/unlock_instructions.html.erb +++ b/app/views/devise/mailer/unlock_instructions.html.erb @@ -1,4 +1,4 @@ -Hello <%= @resource.email %>!
+Hello <%= @resource.cn %>!
Your account has been locked due to an excessive number of unsuccessful sign in attempts.
diff --git a/app/views/icons/_edit-2.html.erb b/app/views/icons/_edit-2.html.erb index 06830c9..736b6c8 100644 --- a/app/views/icons/_edit-2.html.erb +++ b/app/views/icons/_edit-2.html.erb @@ -1 +1 @@ - \ No newline at end of file + diff --git a/app/views/icons/_edit-3.html.erb b/app/views/icons/_edit-3.html.erb index d728efc..0473c82 100644 --- a/app/views/icons/_edit-3.html.erb +++ b/app/views/icons/_edit-3.html.erb @@ -1 +1 @@ - \ No newline at end of file + diff --git a/app/views/icons/_edit.html.erb b/app/views/icons/_edit.html.erb index ec7b4ca..62c65ee 100644 --- a/app/views/icons/_edit.html.erb +++ b/app/views/icons/_edit.html.erb @@ -1 +1 @@ - \ No newline at end of file + diff --git a/app/views/settings/_account.html.erb b/app/views/settings/_account.html.erb index df0a2ad..426bfcc 100644 --- a/app/views/settings/_account.html.erb +++ b/app/views/settings/_account.html.erb @@ -1,13 +1,44 @@ -- <%= label :email, 'Address', class: 'font-bold' %> -
-- disabled="disabled" /> -
-+ <%= f.label :email, 'Address', class: 'font-bold' %> +
++ <%= f.email_field :email, class: "grow", data: { + 'settings--account--email-target': 'emailField' + }, required: true %> + +
+ <% if @validation_errors.present? && @validation_errors[:email].present? %> +<%= @validation_errors[:email].first %>
+ <% end %> ++ <%= f.label :current_password, 'Current password', class: 'font-bold' %> +
++ <%= f.password_field :current_password, class: "w-full", required: true %> +
++ <%= f.submit "Update", class: "btn-md btn-blue" %> +
+Use the following button to request an email with a password reset link:
diff --git a/app/views/shared/_sidenav_settings.html.erb b/app/views/shared/_sidenav_settings.html.erb index 3d6c17e..3ebf8e1 100644 --- a/app/views/shared/_sidenav_settings.html.erb +++ b/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 %> diff --git a/config/initializers/field_with_errors.rb b/config/initializers/field_with_errors.rb new file mode 100644 index 0000000..3652aa9 --- /dev/null +++ b/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 diff --git a/config/locales/devise.en.yml b/config/locales/devise.en.yml index 9003184..f696f4d 100644 --- a/config/locales/devise.en.yml +++ b/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: diff --git a/config/routes.rb b/config/routes.rb index e08f097..aa7fac3 100644 --- a/config/routes.rb +++ b/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 diff --git a/spec/features/settings/account_spec.rb b/spec/features/settings/account_spec.rb new file mode 100644 index 0000000..b90777f --- /dev/null +++ b/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 diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 68677b7..8bd2382 100644 --- a/spec/models/user_spec.rb +++ b/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