From 1312addfe7fa2d686eae9a6f33288abb5fb8ebc7 Mon Sep 17 00:00:00 2001 From: Syphax bouazzouni Date: Tue, 6 Feb 2024 18:35:57 +0100 Subject: [PATCH] Fix: user with space reset password (#492) * extract find_user method the user controller with correct id escaping * add user reset password test --- app/controllers/users_controller.rb | 42 ++++++++++++++++------------ app/views/users/index.html.haml | 4 +-- test/fixtures/users.yml | 24 ++++++++-------- test/integration/login_flows_test.rb | 8 +++--- test/system/login_flows_test.rb | 28 ++++++++++++++++--- 5 files changed, 66 insertions(+), 40 deletions(-) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 5135af9a2..8ec961f0b 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -24,16 +24,13 @@ def index # GET /users/1.xml def show @user = if session[:user].admin? && params.has_key?(:id) - LinkedData::Client::Models::User.find_by_username(params[:id]).first + find_user(params[:id]) else - LinkedData::Client::Models::User.find(session[:user].id) + find_user(session[:user].id) end @all_ontologies = LinkedData::Client::Models::Ontology.all(ignore_custom_ontologies: true) - logger.info 'user show' - logger.info @user.bring_remaining - logger.info @user @user_ontologies = @user.customOntology ## Copied from home controller , account action @@ -51,10 +48,9 @@ def new # GET /users/1;edit def edit - @user = LinkedData::Client::Models::User.find(helpers.escape(params[:id]), include: 'all') - @user ||= LinkedData::Client::Models::User.find_by_username(helpers.escape(params[:id]), include: 'all').first + @user = find_user - if (params[:password].eql?("true")) + if params[:password].eql?("true") @user.validate_password = true end end @@ -89,8 +85,7 @@ def create # PUT /users/1 # PUT /users/1.xml def update - @user = LinkedData::Client::Models::User.find(helpers.escape(params[:id]), include: 'all') - @user = LinkedData::Client::Models::User.find_by_username(helpers.escape(params[:id]), include: 'all').first if @user.nil? + @user = find_user @errors = validate_update(user_params) if @errors.size < 1 @@ -127,12 +122,11 @@ def update # DELETE /users/1 def destroy response = {errors: nil, success: ''} - @user = LinkedData::Client::Models::User.find(params[:id]) - @user = LinkedData::Client::Models::User.find_by_username(params[:id]).first if @user.nil? - if(session[:user].admin?) + @user = find_user + + if session[:user].admin? @user.delete response[:success] << 'User deleted successfully ' - else response[:errors] << 'Not permitted ' end @@ -152,8 +146,7 @@ def destroy end def custom_ontologies - @user = LinkedData::Client::Models::User.find(params[:id], include: 'all') - @user = LinkedData::Client::Models::User.find_by_username(params[:id], include: 'all').first if @user.nil? + @user = find_user custom_ontologies = params[:ontology] ? params[:ontology][:ontologyId] : [] custom_ontologies.reject!(&:blank?) @@ -176,10 +169,11 @@ def custom_ontologies def subscribe - @user = LinkedData::Client::Models::User.find_by_username(params[:username]).first + @user = find_user deliver "subscribe", SubscribeMailer.register_for_announce_list(@user.email,@user.firstName,@user.lastName) end + def un_subscribe @email = params[:email] deliver "unsubscribe", SubscribeMailer.unregister_for_announce_list(@email) @@ -188,6 +182,16 @@ def un_subscribe private + def find_user(id = params[:id]) + id = helpers.unescape(id) + @user = LinkedData::Client::Models::User.find(helpers.escape(id), include: 'all') + @user ||= LinkedData::Client::Models::User.find_by_username(helpers.escape(id), include: 'all').first + + not_found("User with id #{id} not found") if @user.nil? + + @user + end + def deliver(action,job) begin job.deliver @@ -251,7 +255,9 @@ def validate(params) errors << "Please fill in the proper text from the supplied image" end end - if ((!params[:orcidId].match(/^\d{4}+(-\d{4})+$/)) || (params[:orcidId].length != 19)) && !(params[:orcidId].nil? || params[:orcidId].length < 1) + + + if ((!params[:orcidId]&.match(/^\d{4}+(-\d{4})+$/)) || (params[:orcidId].length != 19)) && !(params[:orcidId].nil? || params[:orcidId].length < 1) errors << "Please enter a valid ORCID." end diff --git a/app/views/users/index.html.haml b/app/views/users/index.html.haml index a6f555ecc..f612ff68a 100644 --- a/app/views/users/index.html.haml +++ b/app/views/users/index.html.haml @@ -26,10 +26,10 @@ - count = (user.ontologies&.size || 0) + (user.projects&.size || 0) %div.d-flex.align-items-center{style: 'width: 250px'} %span.mx-1 - = link_to 'Detail', user_path(CGI.escape(user.id.split('/').last)), {data: {turbo_frame: '_top'}} + = link_to 'Detail', user_path(user.id.split('/').last), {data: {turbo_frame: '_top'}} %span.mx-1 - if count.zero? - = button_to "Delete", user_path(CGI.escape(user.id.split('/').last)), method: :delete, class: 'btn btn-link', form: {data: { turbo: true, turbo_confirm: "Are you sure?", turbo_frame: '_top'}} + = button_to "Delete", user_path(user.id.split('/').last), method: :delete, class: 'btn btn-link', form: {data: { turbo: true, turbo_confirm: "Are you sure?", turbo_frame: '_top'}} - else %span{data: { controller: 'tooltip' }, title: "Can't delete this user because still used"} = link_to "Delete", "", class: 'btn btn-link disabled' diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml index c4a633c02..eaaf3596e 100644 --- a/test/fixtures/users.yml +++ b/test/fixtures/users.yml @@ -1,26 +1,26 @@ john: - first_name: John - last_name: Doe + firstName: John + lastName: Doe username: johndoe - orcid_id: '0000-0001-2345-6789' - github_id: 'johndoe_github' + orcidId: '0000-0001-2345-6789' + githubId: 'johndoe_github' email: john.doe@example.com password: password jane: - first_name: Jane - last_name: Smith + firstName: Jane + lastName: Smith username: janesmith - orcid_id: '0000-0002-3456-7890' - github_id: 'janesmith_github' + orcidId: '0000-0002-3456-7890' + githubId: 'janesmith_github' email: jane.smith@example.com password: password bob: - first_name: Bob - last_name: Jones + firstName: Bob + lastName: Jones username: bobjones - orcid_id: '0000-0003-4567-8901' - github_id: 'bobjones_github' + orcidId: '0000-0003-4567-8901' + githubId: 'bobjones_github' email: bob.jones@example.com password: password diff --git a/test/integration/login_flows_test.rb b/test/integration/login_flows_test.rb index b19ef95f1..6772e31ca 100644 --- a/test/integration/login_flows_test.rb +++ b/test/integration/login_flows_test.rb @@ -33,11 +33,11 @@ class LoginFlowsTest < ActionDispatch::IntegrationTest post users_path, params: { user: { - firstName: new_user.first_name, - lastName: new_user.last_name, + firstName: new_user.firstName, + lastName: new_user.lastName, username: new_user.username, - orcidId: new_user.orcid_id, - githubId: new_user.github_id, + orcidId: new_user.orcidId, + githubId: new_user.githubId, email: new_user.email, password: new_user.password, password_confirmation: new_user.password diff --git a/test/system/login_flows_test.rb b/test/system/login_flows_test.rb index d7c7caf4e..1ac94324d 100644 --- a/test/system/login_flows_test.rb +++ b/test/system/login_flows_test.rb @@ -23,11 +23,11 @@ class LoginFlowsTest < ApplicationSystemTestCase LinkedData::Client::Models::User.find_by_username(new_user.username).first&.delete - fill_in 'user_firstName', with: new_user.first_name - fill_in 'user_lastName', with: new_user.last_name + fill_in 'user_firstName', with: new_user.firstName + fill_in 'user_lastName', with: new_user.lastName fill_in 'user_username', with: new_user.username - fill_in 'user_orcidId', with: new_user.orcid_id - fill_in 'user_githubId', with: new_user.github_id + fill_in 'user_orcidId', with: new_user.orcidId + fill_in 'user_githubId', with: new_user.githubId fill_in 'user_email', with: new_user.email fill_in 'user_password', with: new_user.password fill_in 'user_password_confirmation', with: new_user.password @@ -72,4 +72,24 @@ class LoginFlowsTest < ApplicationSystemTestCase assert_selector '.notification', text: "Welcome #{@user_bob.username}!", wait: 10 end + + test "login and reset password" do + login_in_as(@user_bob) + + visit root_url + '/account' + + find("a[href=\"#{edit_user_path(@user_bob.username)}\"]").click + + click_on 'Change password' + + fill_in 'user_password', with: "new password" + fill_in 'user_password_confirmation', with: "new password" + + click_on 'Save' + + logged_in_user = LinkedData::Client::Models::User.authenticate(@user_bob.username, "new password") + + assert logged_in_user && !logged_in_user.errors + end + end