diff --git a/app/controllers/settings_controller.rb b/app/controllers/settings_controller.rb index d9f5979..622b623 100644 --- a/app/controllers/settings_controller.rb +++ b/app/controllers/settings_controller.rb @@ -2,35 +2,41 @@ class SettingsController < ApplicationController before_action :authenticate_user! before_action :set_main_nav_section before_action :set_settings_section, only: [:show, :update, :update_email] + before_action :set_user, only: [:show, :update, :update_email] def index redirect_to setting_path(:profile) end def show - @user = current_user end def update - @user = current_user - @user.preferences.merge! user_params[:preferences] - @user.save! + @user.preferences.merge!(user_params[:preferences] || {}) + @user.display_name = user_params[:display_name] - redirect_to setting_path(@settings_section), flash: { - success: 'Settings saved.' - } + if @user.save + if @user.display_name && (@user.display_name != @user.ldap_entry[:display_name]) + LdapManager::UpdateDisplayName.call(@user.dn, user_params[:display_name]) + end + + redirect_to setting_path(@settings_section), flash: { + success: 'Settings saved.' + } + else + @validation_errors = @user.errors + render :show, status: :unprocessable_entity + end 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] + if @user.valid_ldap_authentication?(email_params[:current_password]) + if @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 + @validation_errors = @user.errors render :show, status: :unprocessable_entity end else @@ -49,27 +55,31 @@ class SettingsController < ApplicationController private - def set_main_nav_section - @current_section = :settings - end - - def set_settings_section - @settings_section = params[:section] - allowed_sections = [:profile, :account, :lightning, :xmpp] - - unless allowed_sections.include?(@settings_section.to_sym) - redirect_to setting_path(:profile) + def set_main_nav_section + @current_section = :settings end - end - def user_params - params.require(:user).permit(preferences: [ - :lightning_notify_sats_received, - :xmpp_exchange_contacts_with_invitees - ]) - end + def set_settings_section + @settings_section = params[:section] + allowed_sections = [:profile, :account, :lightning, :xmpp] - def email_params - params.require(:user).permit(:email, :current_password) - end + unless allowed_sections.include?(@settings_section.to_sym) + redirect_to setting_path(:profile) + end + end + + def set_user + @user = current_user + end + + def user_params + params.require(:user).permit(:display_name, preferences: [ + :lightning_notify_sats_received, + :xmpp_exchange_contacts_with_invitees + ]) + end + + def email_params + params.require(:user).permit(:email, :current_password) + end end diff --git a/app/models/user.rb b/app/models/user.rb index 93b7a5d..855fd0d 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,6 +1,8 @@ class User < ApplicationRecord include EmailValidatable + attr_accessor :display_name + serialize :preferences, UserPreferences # Relations @@ -17,7 +19,7 @@ class User < ApplicationRecord has_many :accounts, through: :lndhub_user validates_uniqueness_of :cn - validates_length_of :cn, :minimum => 3 + validates_length_of :cn, minimum: 3 validates_format_of :cn, with: /\A([a-z0-9\-])*\z/, if: Proc.new{ |u| u.cn.present? }, message: "is invalid. Please use only letters, numbers and -" @@ -31,6 +33,9 @@ class User < ApplicationRecord validates_uniqueness_of :email validates :email, email: true + validates_length_of :display_name, minimum: 3, maximum: 35, allow_blank: true, + if: -> { defined?(@display_name) } + scope :confirmed, -> { where.not(confirmed_at: nil) } scope :pending, -> { where(confirmed_at: nil) } @@ -115,8 +120,13 @@ class User < ApplicationRecord @dn = Devise::LDAP::Adapter.get_dn(self.cn) end - def ldap_entry - ldap.fetch_users(uid: self.cn, ou: self.ou).first + def ldap_entry(reload: false) + return @ldap_entry if defined?(@ldap_entry) && !reload + @ldap_entry = ldap.fetch_users(uid: self.cn, ou: self.ou).first + end + + def display_name + @display_name ||= ldap_entry[:display_name] end def services_enabled diff --git a/app/services/ldap_manager/update_display_name.rb b/app/services/ldap_manager/update_display_name.rb new file mode 100644 index 0000000..2d3e90f --- /dev/null +++ b/app/services/ldap_manager/update_display_name.rb @@ -0,0 +1,12 @@ +module LdapManager + class UpdateDisplayName < LdapManagerService + def initialize(dn, display_name) + @dn = dn + @display_name = display_name + end + + def call + replace_attribute @dn, :displayName, @display_name + end + end +end diff --git a/app/services/ldap_manager/update_email.rb b/app/services/ldap_manager/update_email.rb index 5acd77f..80f40e4 100644 --- a/app/services/ldap_manager/update_email.rb +++ b/app/services/ldap_manager/update_email.rb @@ -6,7 +6,7 @@ module LdapManager end def call - replace_attribute @dn, :mail, [ @address ] + replace_attribute @dn, :mail, @address end end end diff --git a/app/services/ldap_service.rb b/app/services/ldap_service.rb index 5a572c5..eac64c2 100644 --- a/app/services/ldap_service.rb +++ b/app/services/ldap_service.rb @@ -50,7 +50,7 @@ class LdapService < ApplicationService treebase = ldap_config["base"] end - attributes = %w{dn cn uid mail admin service} + attributes = %w{dn cn uid mail displayName admin service} filter = Net::LDAP::Filter.eq("uid", args[:uid] || "*") entries = ldap_client.search(base: treebase, filter: filter, attributes: attributes) @@ -59,6 +59,7 @@ class LdapService < ApplicationService { uid: e.uid.first, mail: e.try(:mail) ? e.mail.first : nil, + display_name: e.try(:displayName) ? e.displayName.first : nil, admin: e.try(:admin) ? 'admin' : nil, service: e.try(:service) } diff --git a/app/views/settings/_account.html.erb b/app/views/settings/_account.html.erb index 426bfcc..5726230 100644 --- a/app/views/settings/_account.html.erb +++ b/app/views/settings/_account.html.erb @@ -3,7 +3,7 @@ "settings--account--email-validation-failed-value": @validation_errors.present? } do %>

E-Mail

- <%= form_for(current_user, url: update_email_settings_path, method: "post") do |f| %> + <%= form_for(@user, url: update_email_settings_path, method: "post") do |f| %> <%= hidden_field_tag :section, "account" %>

<%= f.label :email, 'Address', class: 'font-bold' %> @@ -34,7 +34,7 @@ <%= f.password_field :current_password, class: "w-full", required: true %>

- <%= f.submit "Update", class: "btn-md btn-blue" %> + <%= f.submit "Update", class: "btn-md btn-blue w-full md:w-auto" %>

<% end %> diff --git a/app/views/settings/_profile.html.erb b/app/views/settings/_profile.html.erb index f1d14ae..e72430c 100644 --- a/app/views/settings/_profile.html.erb +++ b/app/views/settings/_profile.html.erb @@ -21,10 +21,15 @@

Your user address for Chat and Lightning Network.

- - <%# <%= form_for(@user, as: "profile", url: settings_profile_path) do |f| %> - <%#

- <%# <%= f.submit "Save changes", class: 'btn-md btn-blue w-full sm:w-auto' %> - <%#

- <%# <% end %> + <%= form_for(@user, url: setting_path(:profile), html: { :method => :put }) do |f| %> + <%= render FormElements::FieldsetComponent.new(tag: "div", title: "Display name") do %> + <%= f.text_field :display_name, class: "w-full sm:w-3/5 mb-2" %> + <% if @validation_errors.present? && @validation_errors[:display_name].present? %> +

<%= @validation_errors[:display_name].first %>

+ <% end %> + <% end %> +

+ <%= f.submit 'Save', class: "btn-md btn-blue w-full md:w-auto" %> +

+ <% end %> diff --git a/app/views/shared/_sidenav_settings.html.erb b/app/views/shared/_sidenav_settings.html.erb index 3ebf8e1..ce2a527 100644 --- a/app/views/shared/_sidenav_settings.html.erb +++ b/app/views/shared/_sidenav_settings.html.erb @@ -1,20 +1,20 @@ <%= render SidenavLinkComponent.new( name: "Profile", path: setting_path(:profile), icon: "user", - active: current_page?(setting_path(:profile)) + active: @settings_section.to_s == "profile" ) %> <%= render SidenavLinkComponent.new( name: "Account", path: setting_path(:account), icon: "key", - active: current_page?(setting_path(:account)) + active: @settings_section.to_s == "account" ) %> <% if Setting.ejabberd_enabled %> <%= render SidenavLinkComponent.new( name: "Chat", path: setting_path(:xmpp), icon: "message-circle", - active: current_page?(setting_path(:xmpp)) + active: @settings_section.to_s == "xmpp" ) %> <% end %> <% if Setting.lndhub_enabled %> <%= render SidenavLinkComponent.new( name: "Lightning", path: setting_path(:lightning), icon: "zap", - active: current_page?(setting_path(:lightning)) + active: @settings_section.to_s == "lightning" ) %> <% end %> diff --git a/spec/features/settings/account_spec.rb b/spec/features/settings/account_spec.rb index b90777f..51f1c83 100644 --- a/spec/features/settings/account_spec.rb +++ b/spec/features/settings/account_spec.rb @@ -2,54 +2,57 @@ 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! + feature "Update email address" do + let(:geraint) { create :user, id: 2, cn: 'geraint', email: "lamagliarosa@example.com" } - 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 + before do + login_as user, :scope => :user + geraint.save! - 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") + 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 - 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" + scenario '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(:update_email)) - expect(user.reload.unconfirmed_email).to be_nil - within ".error-msg" do - expect(page).to have_content("has already been taken") + 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 - 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" + scenario '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(: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") + 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 'works with valid password and address' 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 end diff --git a/spec/features/settings/profile_spec.rb b/spec/features/settings/profile_spec.rb new file mode 100644 index 0000000..c54cffe --- /dev/null +++ b/spec/features/settings/profile_spec.rb @@ -0,0 +1,45 @@ +require 'rails_helper' + +RSpec.describe 'Profile settings', type: :feature do + let(:user) { create :user, cn: "mwahlberg" } + + before do + login_as user, :scope => :user + end + + feature "Update display name" do + before do + allow(user).to receive(:display_name).and_return("Mark") + allow_any_instance_of(User).to receive(:dn).and_return("cn=mwahlberg,ou=kosmos.org,cn=users,dc=kosmos,dc=org") + allow_any_instance_of(User).to receive(:ldap_entry).and_return({ + uid: user.cn, ou: user.ou, display_name: "Mark" + }) + end + + scenario 'fails with validation error' do + visit setting_path(:profile) + fill_in 'Display name', with: "M" + click_button "Save" + + expect(current_url).to eq(setting_url(:profile)) + expect(page).to have_field('Display name', with: 'M') + within ".error-msg" do + expect(page).to have_content("is too short") + end + end + + scenario 'works with valid input' do + expect(LdapManager::UpdateDisplayName).to receive(:call) + .with(user.dn, "Marky Mark").and_return(true) + + visit setting_path(:profile) + fill_in 'Display name', with: "Marky Mark" + click_button "Save" + + expect(current_url).to eq(setting_url(:profile)) + within ".flash-msg" do + expect(page).to have_content("Settings saved") + end + end + end +end