diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 90bfaa8f9..345eed674 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -40,6 +40,7 @@ Lint/AssignmentInCondition: Exclude: - 'app/controllers/api/traces_controller.rb' - 'app/controllers/api/user_preferences_controller.rb' + - 'app/controllers/accounts_controller.rb' - 'app/controllers/application_controller.rb' - 'app/controllers/geocoder_controller.rb' - 'app/controllers/notes_controller.rb' diff --git a/app/abilities/ability.rb b/app/abilities/ability.rb index 769fbca47..a45bf9a57 100644 --- a/app/abilities/ability.rb +++ b/app/abilities/ability.rb @@ -42,6 +42,7 @@ class Ability can [:index, :new, :create, :show, :edit, :update, :destroy], :oauth2_application can [:index, :destroy], :oauth2_authorized_application can [:new, :show, :create, :destroy], :oauth2_authorization + can [:edit, :update], :account can [:show], :dashboard can [:new, :create, :edit, :update, :comment, :subscribe, :unsubscribe], DiaryEntry can [:make_friend, :remove_friend], Friendship diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb new file mode 100644 index 000000000..3b540234b --- /dev/null +++ b/app/controllers/accounts_controller.rb @@ -0,0 +1,52 @@ +class AccountsController < ApplicationController + include SessionMethods + include UserMethods + + layout "site" + + before_action :authorize_web + before_action :set_locale + + authorize_resource :class => false + + before_action :check_database_readable + before_action :check_database_writable, :only => [:update] + before_action :allow_thirdparty_images, :only => [:edit, :update] + + def edit + @tokens = current_user.oauth_tokens.authorized + + append_content_security_policy_directives( + :form_action => %w[accounts.google.com *.facebook.com login.live.com github.com meta.wikimedia.org] + ) + + if errors = session.delete(:user_errors) + errors.each do |attribute, error| + current_user.errors.add(attribute, error) + end + end + @title = t ".title" + end + + def update + @tokens = current_user.oauth_tokens.authorized + + append_content_security_policy_directives( + :form_action => %w[accounts.google.com *.facebook.com login.live.com github.com meta.wikimedia.org] + ) + + if params[:user][:auth_provider].blank? || + (params[:user][:auth_provider] == current_user.auth_provider && + params[:user][:auth_uid] == current_user.auth_uid) + update_user(current_user, params) + if current_user.errors.count.zero? + redirect_to edit_account_path + else + render :edit + end + else + session[:new_user_settings] = params + redirect_to auth_url(params[:user][:auth_provider], params[:user][:auth_uid]), :status => :temporary_redirect + end + end +end diff --git a/app/controllers/concerns/user_methods.rb b/app/controllers/concerns/user_methods.rb new file mode 100644 index 000000000..9099b37c9 --- /dev/null +++ b/app/controllers/concerns/user_methods.rb @@ -0,0 +1,47 @@ +module UserMethods + extend ActiveSupport::Concern + + private + + ## + # update a user's details + def update_user(user, params) + user.display_name = params[:user][:display_name] + user.new_email = params[:user][:new_email] + + unless params[:user][:pass_crypt].empty? && params[:user][:pass_crypt_confirmation].empty? + user.pass_crypt = params[:user][:pass_crypt] + user.pass_crypt_confirmation = params[:user][:pass_crypt_confirmation] + end + + if params[:user][:auth_provider].nil? || params[:user][:auth_provider].blank? + user.auth_provider = nil + user.auth_uid = nil + end + + if user.save + session[:fingerprint] = user.fingerprint + + if user.new_email.blank? || user.new_email == user.email + flash[:notice] = t "accounts.update.success" + else + user.email = user.new_email + + if user.valid? + flash[:notice] = t "accounts.update.success_confirm_needed" + + begin + UserMailer.email_confirm(user, user.tokens.create).deliver_later + rescue StandardError + # Ignore errors sending email + end + else + current_user.errors.add(:new_email, current_user.errors[:email]) + current_user.errors.add(:email, []) + end + + user.restore_email! + end + end + end +end diff --git a/app/controllers/confirmations_controller.rb b/app/controllers/confirmations_controller.rb index 2a00a49b0..bcb4c1617 100644 --- a/app/controllers/confirmations_controller.rb +++ b/app/controllers/confirmations_controller.rb @@ -93,10 +93,10 @@ class ConfirmationsController < ApplicationController current_user.tokens.delete_all session[:user] = current_user.id session[:fingerprint] = current_user.fingerprint - redirect_to user_account_path(current_user) + redirect_to edit_account_path elsif token flash[:error] = t "confirmations.confirm_email.failure" - redirect_to user_account_path(token.user) + redirect_to edit_account_path else flash[:error] = t "confirmations.confirm_email.unknown_token" end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 23263ebba..4f05ece74 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -1,5 +1,6 @@ class UsersController < ApplicationController include SessionMethods + include UserMethods layout "site" @@ -11,11 +12,10 @@ class UsersController < ApplicationController authorize_resource - before_action :require_self, :only => [:account] - before_action :check_database_writable, :only => [:new, :account, :go_public] + before_action :check_database_writable, :only => [:new, :go_public] before_action :require_cookies, :only => [:new] before_action :lookup_user_by_name, :only => [:set_status, :destroy] - before_action :allow_thirdparty_images, :only => [:show, :account] + before_action :allow_thirdparty_images, :only => [:show] def terms @legale = params[:legale] || OSM.ip_to_country(request.remote_ip) || Settings.default_legale @@ -28,7 +28,7 @@ class UsersController < ApplicationController if current_user&.terms_agreed? # Already agreed to terms, so just show settings - redirect_to user_account_path(current_user) + redirect_to edit_account_path elsif current_user.nil? && session[:new_user].nil? redirect_to login_path(:referer => request.fullpath) end @@ -46,7 +46,7 @@ class UsersController < ApplicationController referer = safe_referer(params[:referer]) if params[:referer] - redirect_to referer || user_account_path(current_user) + redirect_to referer || edit_account_path elsif params[:decline] redirect_to t("users.terms.declined") else @@ -64,7 +64,7 @@ class UsersController < ApplicationController referer = safe_referer(params[:referer]) if params[:referer] - redirect_to referer || user_account_path(current_user) + redirect_to referer || edit_account_path else self.current_user = session.delete(:new_user) @@ -114,36 +114,11 @@ class UsersController < ApplicationController end end - def account - @tokens = current_user.oauth_tokens.authorized - - append_content_security_policy_directives( - :form_action => %w[accounts.google.com *.facebook.com login.live.com github.com meta.wikimedia.org] - ) - - if request.post? - if params[:user][:auth_provider].blank? || - (params[:user][:auth_provider] == current_user.auth_provider && - params[:user][:auth_uid] == current_user.auth_uid) - update_user(current_user, params) - redirect_to user_account_url(current_user) if current_user.errors.count.zero? - else - session[:new_user_settings] = params - redirect_to auth_url(params[:user][:auth_provider], params[:user][:auth_uid]), :status => :temporary_redirect - end - elsif errors = session.delete(:user_errors) - errors.each do |attribute, error| - current_user.errors.add(attribute, error) - end - end - @title = t "users.account.title" - end - def go_public current_user.data_public = true current_user.save flash[:notice] = t "users.go_public.flash success" - redirect_to user_account_path(current_user) + redirect_to edit_account_path end def new @@ -293,7 +268,7 @@ class UsersController < ApplicationController session[:user_errors] = current_user.errors.as_json - redirect_to user_account_path(current_user) + redirect_to edit_account_path elsif session[:new_user] session[:new_user].auth_provider = provider session[:new_user].auth_uid = uid @@ -340,54 +315,6 @@ class UsersController < ApplicationController private - ## - # update a user's details - def update_user(user, params) - user.display_name = params[:user][:display_name] - user.new_email = params[:user][:new_email] - - unless params[:user][:pass_crypt].empty? && params[:user][:pass_crypt_confirmation].empty? - user.pass_crypt = params[:user][:pass_crypt] - user.pass_crypt_confirmation = params[:user][:pass_crypt_confirmation] - end - - if params[:user][:auth_provider].nil? || params[:user][:auth_provider].blank? - user.auth_provider = nil - user.auth_uid = nil - end - - if user.save - session[:fingerprint] = user.fingerprint - - if user.new_email.blank? || user.new_email == user.email - flash[:notice] = t "users.account.flash update success" - else - user.email = user.new_email - - if user.valid? - flash[:notice] = t "users.account.flash update success confirm needed" - - begin - UserMailer.email_confirm(user, user.tokens.create).deliver_later - rescue StandardError - # Ignore errors sending email - end - else - current_user.errors.add(:new_email, current_user.errors[:email]) - current_user.errors.add(:email, []) - end - - user.restore_email! - end - end - end - - ## - # require that the user in the URL is the logged in user - def require_self - head :forbidden if params[:display_name] != current_user.display_name - end - ## # ensure that there is a "user" instance variable def lookup_user_by_name diff --git a/app/views/users/account.html.erb b/app/views/accounts/edit.html.erb similarity index 90% rename from app/views/users/account.html.erb rename to app/views/accounts/edit.html.erb index f647cc84f..ba809d16a 100644 --- a/app/views/users/account.html.erb +++ b/app/views/accounts/edit.html.erb @@ -6,9 +6,9 @@

<%= t ".my settings" %>

<% end %> -<%= render :partial => "settings_menu", :locals => { :selected => "account" } %> +<%= render :partial => "settings_menu" %> -<%= bootstrap_form_for current_user, :url => { :action => :account }, :method => :post, :html => { :multipart => true, :id => "accountForm", :autocomplete => :off } do |f| %> +<%= bootstrap_form_for current_user, :url => { :action => :update }, :html => { :multipart => true, :id => "accountForm", :autocomplete => :off } do |f| %> <%= f.text_field :display_name %> <%= f.email_field :email, :disabled => true, :label => t(".current email address") %> diff --git a/app/views/application/_settings_menu.html.erb b/app/views/application/_settings_menu.html.erb index 03d8c74c2..05cee9185 100644 --- a/app/views/application/_settings_menu.html.erb +++ b/app/views/application/_settings_menu.html.erb @@ -3,7 +3,7 @@ <% content_for :heading do %>