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..fc5763d
--- /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 (evt) {
+ 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/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" />
- E-Mail
-
+ <%= 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/config/routes.rb b/config/routes.rb index c8dd8a4..a369bd3 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -28,6 +28,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