Hello!
- -You have requested deleting your mooc.fi account.
- -Please proceed to the following link to continue the deletion process.
- - <%= @url %> - -If you didn't request deleting your account, just ignore this message.
- - diff --git a/app/views/user_mailer/destroy_confirmation.text.erb b/app/views/user_mailer/destroy_confirmation.text.erb deleted file mode 100644 index db40e259f..000000000 --- a/app/views/user_mailer/destroy_confirmation.text.erb +++ /dev/null @@ -1,9 +0,0 @@ -Hello! - -You have requested deleting your mooc.fi account. - -Please proceed to the following link to continue the deletion process. - -<%= @url %> - -If you didn't request deleting your account, just ignore this message. diff --git a/app/views/users/_account_deletion.html.erb b/app/views/users/_account_deletion.html.erb new file mode 100644 index 000000000..e84d3d7c8 --- /dev/null +++ b/app/views/users/_account_deletion.html.erb @@ -0,0 +1,7 @@ +<% if user == current_user %> + <% if (deletion_url = User.courses_mooc_fi_account_deletion_url) %> + <%= link_to 'Delete account on courses.mooc.fi', deletion_url, class: 'btn btn-danger', target: '_top' %> + <% else %> +To delete your account, please contact support.
+ <% end %> +<% end %> diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index 24be96465..86d0866be 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -2,6 +2,4 @@ <%= render :partial => 'users/form', :user => @user, locals: { hide_login: true } %> -<% if can? :destroy, @user %> - <%= button_to 'Request deleting account', send_destroy_email_path(@user.id), method: :post, class: 'btn btn-danger' %> -<% end %> +<%= render 'users/account_deletion', user: @user %> diff --git a/app/views/users/verify_destroying_user.html.erb b/app/views/users/verify_destroying_user.html.erb deleted file mode 100644 index 675cfcfd3..000000000 --- a/app/views/users/verify_destroying_user.html.erb +++ /dev/null @@ -1,32 +0,0 @@ -Pressing the button below will permanently destroy your mooc.fi account.
- <%= submit_tag("Destroy my account permanently", class: "btn btn-danger", data: { confirm: "Are you sure you want to delete the account #{@user.login}?"}, disabled: "disabled", id: :destroy_button) %> -<% end %> - - - diff --git a/config/brakeman.ignore b/config/brakeman.ignore index d23d5f4d2..1694a47f2 100644 --- a/config/brakeman.ignore +++ b/config/brakeman.ignore @@ -436,6 +436,29 @@ 77 ], "note": "sh! Shellwords-escapes every word unless escape: false, which no caller passes." + }, + { + "warning_type": "Redirect", + "warning_code": 18, + "fingerprint": "6359af9828749190072deea63d61f76cd4ebf9c9987ef93c154d7da9e902aeaa", + "check_name": "Redirect", + "message": "Possible unprotected redirect", + "file": "app/controllers/users_controller.rb", + "line": 136, + "link": "https://brakemanscanner.org/docs/warning_types/redirect/", + "code": "redirect_to(User.courses_mooc_fi_account_deletion_url, :allow_other_host => true)", + "render_path": null, + "location": { + "type": "method", + "class": "UsersController", + "method": "redirect_to_account_deletion" + }, + "user_input": "User.courses_mooc_fi_account_deletion_url", + "confidence": "Weak", + "cwe_id": [ + 601 + ], + "note": "The URL comes from the courses_mooc_fi_base_url site setting, not from request input." } ], "brakeman_version": "8.1.0" diff --git a/config/routes.rb b/config/routes.rb index 12e706d69..dedcc90a2 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -61,6 +61,7 @@ resources :assistantships, module: :users, only: :index resources :teacherships, module: :users, only: :index post :set_password_managed_by_courses_mooc_fi, on: :member + get :course_progress, on: :member get :get_user_with_email, on: :collection end @@ -290,9 +291,9 @@ resources :users, only: [:index] - post '/users/:user_id/send_destroy_email', to: 'users#send_destroy_email', as: 'send_destroy_email' - get '/users/:user_id/destroy/:token', to: 'users#verify_destroying_user', as: 'verify_destroying_user' - delete '/users/:user_id/destroy/:token', to: 'users#destroy_user', as: 'destroy_user' + post '/users/:user_id/send_destroy_email', to: 'users#redirect_to_account_deletion' + # Deletion confirmation links already in users' inboxes land here too. + match '/users/:user_id/destroy/:id', to: 'users#redirect_to_account_deletion', via: %i[get delete] resources :certificates, only: %i[show create] diff --git a/sorbet/rbi/dsl/generated_path_helpers_module.rbi b/sorbet/rbi/dsl/generated_path_helpers_module.rbi index 21d7316b0..d95fdeb9f 100644 --- a/sorbet/rbi/dsl/generated_path_helpers_module.rbi +++ b/sorbet/rbi/dsl/generated_path_helpers_module.rbi @@ -195,6 +195,9 @@ module GeneratedPathHelpersModule sig { params(args: T.untyped).returns(String) } def course_path(*args); end + sig { params(args: T.untyped).returns(String) } + def course_progress_api_v8_user_path(*args); end + sig { params(args: T.untyped).returns(String) } def course_template_path(*args); end @@ -213,9 +216,6 @@ module GeneratedPathHelpersModule sig { params(args: T.untyped).returns(String) } def dangerously_destroy_user_participant_settings_path(*args); end - sig { params(args: T.untyped).returns(String) } - def destroy_user_path(*args); end - sig { params(args: T.untyped).returns(String) } def disable_organization_course_path(*args); end @@ -654,9 +654,6 @@ module GeneratedPathHelpersModule sig { params(args: T.untyped).returns(String) } def save_unlocks_organization_course_path(*args); end - sig { params(args: T.untyped).returns(String) } - def send_destroy_email_path(*args); end - sig { params(args: T.untyped).returns(String) } def send_verification_email_path(*args); end @@ -777,9 +774,6 @@ module GeneratedPathHelpersModule sig { params(args: T.untyped).returns(String) } def verify_dangerously_destroying_user_participant_settings_path(*args); end - sig { params(args: T.untyped).returns(String) } - def verify_destroying_user_path(*args); end - sig { params(args: T.untyped).returns(String) } def verify_organization_path(*args); end end diff --git a/sorbet/rbi/dsl/generated_url_helpers_module.rbi b/sorbet/rbi/dsl/generated_url_helpers_module.rbi index f0166195e..503625b8d 100644 --- a/sorbet/rbi/dsl/generated_url_helpers_module.rbi +++ b/sorbet/rbi/dsl/generated_url_helpers_module.rbi @@ -192,6 +192,9 @@ module GeneratedUrlHelpersModule sig { params(args: T.untyped).returns(String) } def course_migrate_to_other_course_url(*args); end + sig { params(args: T.untyped).returns(String) } + def course_progress_api_v8_user_url(*args); end + sig { params(args: T.untyped).returns(String) } def course_template_url(*args); end @@ -213,9 +216,6 @@ module GeneratedUrlHelpersModule sig { params(args: T.untyped).returns(String) } def dangerously_destroy_user_participant_settings_url(*args); end - sig { params(args: T.untyped).returns(String) } - def destroy_user_url(*args); end - sig { params(args: T.untyped).returns(String) } def disable_organization_course_url(*args); end @@ -654,9 +654,6 @@ module GeneratedUrlHelpersModule sig { params(args: T.untyped).returns(String) } def save_unlocks_organization_course_url(*args); end - sig { params(args: T.untyped).returns(String) } - def send_destroy_email_url(*args); end - sig { params(args: T.untyped).returns(String) } def send_verification_email_url(*args); end @@ -777,9 +774,6 @@ module GeneratedUrlHelpersModule sig { params(args: T.untyped).returns(String) } def verify_dangerously_destroying_user_participant_settings_url(*args); end - sig { params(args: T.untyped).returns(String) } - def verify_destroying_user_url(*args); end - sig { params(args: T.untyped).returns(String) } def verify_organization_url(*args); end end diff --git a/sorbet/rbi/dsl/user_mailer.rbi b/sorbet/rbi/dsl/user_mailer.rbi index a4e2256af..b0447e2d6 100644 --- a/sorbet/rbi/dsl/user_mailer.rbi +++ b/sorbet/rbi/dsl/user_mailer.rbi @@ -10,9 +10,6 @@ class UserMailer include GeneratedMountedHelpers class << self - sig { params(user: ::User).returns(::ActionMailer::MessageDelivery) } - def destroy_confirmation(user); end - sig do params( user: ::User, diff --git a/spec/controllers/api/v8/users/request_deletion_controller_spec.rb b/spec/controllers/api/v8/users/request_deletion_controller_spec.rb index e98cd5754..78a1fbae8 100644 --- a/spec/controllers/api/v8/users/request_deletion_controller_spec.rb +++ b/spec/controllers/api/v8/users/request_deletion_controller_spec.rb @@ -5,6 +5,12 @@ describe Api::V8::Users::RequestDeletionController, type: :controller do let(:user) { FactoryBot.create(:user, courses_mooc_fi_user_id: SecureRandom.uuid) } + let(:application) { Doorkeeper::Application.create!(name: 'client', redirect_uri: 'urn:ietf:wg:oauth:2.0:oob') } + + def authenticate(scopes: 'public') + token = Doorkeeper::AccessToken.create!(application: application, resource_owner_id: user.id, scopes: scopes) + request.headers['Authorization'] = "Bearer #{token.token}" + end around do |example| original = Rails.configuration.x.accept_courses_mooc_fi_tokens @@ -13,13 +19,42 @@ Rails.configuration.x.accept_courses_mooc_fi_tokens = original end + before do + SiteSetting.all_settings['courses_mooc_fi_base_url'] = 'https://courses.mooc.fi' + end + + it 'points the caller to courses.mooc.fi without emailing anything' do + authenticate + + expect { post :create, params: { user_id: 'current' } }.not_to(change { ActionMailer::Base.deliveries.count }) + + expect(response).to have_http_status(410) + expect(response.parsed_body['account_deletion_url']).to eq('https://courses.mooc.fi/user-settings/permissions#delete-account') + expect(User.find_by(id: user.id)).to eq(user) + end + + it "says to contact support when courses.mooc.fi isn't configured" do + SiteSetting.all_settings['courses_mooc_fi_base_url'] = nil + authenticate + + post :create, params: { user_id: 'current' } + + expect(response).to have_http_status(410) + expect(response.parsed_body['errors']).to eq(['Please contact support to delete your account.']) + expect(response.parsed_body['account_deletion_url']).to be_nil + end + + it 'requires authentication' do + post :create, params: { user_id: 'current' } + + expect(response).to have_http_status(401) + end + it 'refuses a deletion request made with a courses.mooc.fi access token' do - allow(controller).to receive(:doorkeeper_token).and_return(nil) request.headers['Authorization'] = 'Bearer sp331-access-token' allow(CoursesMoocFiTokenIntrospector).to receive(:introspect).and_return( CoursesMoocFiTokenIntrospector::Result.new(sub: user.courses_mooc_fi_user_id) ) - expect(UserMailer).not_to receive(:destroy_confirmation) post :create, params: { user_id: 'current' } @@ -27,10 +62,7 @@ end it 'refuses a deletion request made with a handoff token' do - application = Doorkeeper::Application.create!(name: 'mooc.fi frontend', redirect_uri: 'urn:ietf:wg:oauth:2.0:oob') - token = Doorkeeper::AccessToken.create!(application: application, resource_owner_id: user.id, scopes: 'public handoff') - request.headers['Authorization'] = "Bearer #{token.token}" - expect(UserMailer).not_to receive(:destroy_confirmation) + authenticate(scopes: 'public handoff') post :create, params: { user_id: 'current' } diff --git a/spec/controllers/api/v8/users_controller_spec.rb b/spec/controllers/api/v8/users_controller_spec.rb index 9d79aa298..c3be1acf6 100644 --- a/spec/controllers/api/v8/users_controller_spec.rb +++ b/spec/controllers/api/v8/users_controller_spec.rb @@ -289,6 +289,77 @@ def do_update(old_password) end end + describe 'GET course_progress' do + let!(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: admin.id, scopes: 'public') } + + it 'lists each course the user has submissions or points in, latest submission first' do + submitted_course = FactoryBot.create(:course, name: 'submitted', moocfi_id: SecureRandom.uuid, hidden: true) + FactoryBot.create(:submission, user: user, course: submitted_course, created_at: Time.zone.parse('2026-10-01T12:00:00Z')) + FactoryBot.create(:submission, user: user, course: submitted_course, created_at: Time.zone.parse('2026-09-01T12:00:00Z')) + older_course = FactoryBot.create(:course, name: 'older', disabled_status: 1) + FactoryBot.create(:submission, user: user, course: older_course, created_at: Time.zone.parse('2025-01-01T00:00:00Z')) + points_only_course = FactoryBot.create(:course, name: 'points-only') + AwardedPoint.create!(user: user, course: points_only_course, name: 'point1') + AwardedPoint.create!(user: user, course: points_only_course, name: 'point2') + FactoryBot.create(:submission, user: other_user, course: FactoryBot.create(:course)) + + get :course_progress, params: { id: user.id } + + expect(response).to have_http_status(200) + expect(response.parsed_body['courses']).to eq([ + { + 'id' => submitted_course.id, 'name' => 'submitted', 'title' => submitted_course.title, + 'moocfi_id' => submitted_course.moocfi_id, 'awarded_points' => 0, 'submissions' => 2, + 'last_submission_at' => '2026-10-01T12:00:00Z' + }, + { + 'id' => older_course.id, 'name' => 'older', 'title' => older_course.title, 'moocfi_id' => nil, + 'awarded_points' => 0, 'submissions' => 1, 'last_submission_at' => '2025-01-01T00:00:00Z' + }, + { + 'id' => points_only_course.id, 'name' => 'points-only', 'title' => points_only_course.title, + 'moocfi_id' => nil, 'awarded_points' => 2, 'submissions' => 0, 'last_submission_at' => nil + } + ]) + end + + it 'returns an empty list for a user without progress' do + get :course_progress, params: { id: user.id } + + expect(response).to have_http_status(200) + expect(response.parsed_body).to eq('courses' => []) + end + + it 'returns 404 for a missing user' do + get :course_progress, params: { id: User.maximum(:id) + 1 } + + expect(response).to have_http_status(404) + expect(response.parsed_body['errors']).to be_present + end + + context 'as a non-admin' do + let!(:token) { Doorkeeper::AccessToken.create!(resource_owner_id: user.id, scopes: 'public') } + + it 'is forbidden, even for their own progress' do + get :course_progress, params: { id: user.id } + + expect(response).to have_http_status(403) + end + end + + context 'as a guest' do + let!(:token) { nil } + + it 'requires authentication' do + controller.current_user = Guest.new + + get :course_progress, params: { id: user.id } + + expect(response).to have_http_status(401) + end + end + end + describe 'with a courses.mooc.fi access token' do let(:token) { nil } @@ -356,10 +427,15 @@ def authenticate_with_token_as(token_user) before :each do allow(ExtraField).to receive(:by_kind).with(:user).and_return([UserField.new(name: 'first_name'), UserField.new(name: 'last_name')]) + UserField.instance_variable_set(:@all, nil) user.password = 'oldpassword' user.save! end + after :each do + UserField.instance_variable_set(:@all, nil) + end + it 'updates the name when the rest of the profile is sent back unchanged' do put :update, params: { id: user.id, user: { email: user.email.upcase }, user_field: { first_name: 'Ada', last_name: 'Lovelace' } } expect(response).to have_http_status(200) diff --git a/spec/controllers/settings_controller_spec.rb b/spec/controllers/settings_controller_spec.rb new file mode 100644 index 000000000..624c76730 --- /dev/null +++ b/spec/controllers/settings_controller_spec.rb @@ -0,0 +1,32 @@ +# typed: false +# frozen_string_literal: true + +require 'spec_helper' + +describe SettingsController, type: :controller do + render_views + + let!(:user) { FactoryBot.create(:user) } + let(:deletion_url) { 'https://courses.mooc.fi/user-settings/permissions#delete-account' } + + before :each do + SiteSetting.all_settings['courses_mooc_fi_base_url'] = 'https://courses.mooc.fi' + end + + it 'links users to account deletion on courses.mooc.fi' do + controller.current_user = user + + get :show, params: { participant_id: user.id } + + expect(response.body).to include(%(href="#{deletion_url}")) + end + + it "does not offer an admin the courses.mooc.fi link on another user's settings" do + controller.current_user = FactoryBot.create(:admin) + + get :show, params: { participant_id: user.id } + + expect(response.body).not_to include(deletion_url) + expect(response.body).to include('Dangerously destroy account') + end +end diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 8e0831575..15aba1b75 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -244,61 +244,30 @@ end end - describe 'DELETE destroy_user' do - let!(:user) { FactoryBot.create(:user, password: 'secret123') } - let!(:verification_token) { VerificationToken.create!(user: user, type: :delete_user) } + describe 'GET show account deletion' do + render_views + + let!(:user) { FactoryBot.create(:user) } + let(:deletion_url) { 'https://courses.mooc.fi/user-settings/permissions#delete-account' } before :each do controller.current_user = user + SiteSetting.all_settings['courses_mooc_fi_base_url'] = 'https://courses.mooc.fi' end - def do_destroy_user(password) - delete :destroy_user, params: { user_id: user.id, token: verification_token.token, im_sure: '1', - user: { password: password } } - end - - it 'should destroy the account when the password is correct' do - allow_any_instance_of(User).to receive(:post_new_user_to_courses_mooc_fi).and_return(true) - - do_destroy_user('secret123') - - expect(User.find_by(id: user.id)).to be_nil - end - - it 'should blame the password when it was actually rejected' do - do_destroy_user('wrongpassword') - - expect(flash[:alert]).to eq('The password was incorrect.') - expect(User.find_by(id: user.id)).not_to be_nil - end - - it 'should not blame the password when courses.mooc.fi could not be reached' do - user.update!(password_managed_by_courses_mooc_fi: true, courses_mooc_fi_user_id: SecureRandom.uuid) - allow_any_instance_of(User).to receive(:courses_mooc_fi_authentication_status).and_return(:unavailable) - - do_destroy_user('secret123') - - expect(flash[:alert]).to include('could not reach') - expect(User.find_by(id: user.id)).not_to be_nil - end - - it 'should tell the user to contact support when the courses.mooc.fi id is missing' do - user.update_columns(password_managed_by_courses_mooc_fi: true, courses_mooc_fi_user_id: nil) - - do_destroy_user('secret123') + it 'links to account deletion on courses.mooc.fi' do + get :show - expect(flash[:alert]).to include('contact support') - expect(flash[:alert]).not_to include('try again') - expect(User.find_by(id: user.id)).not_to be_nil + expect(response.body).to include(%(href="#{deletion_url}")) end - it 'should not destroy the account on a status this controller does not know' do - allow(User).to receive(:authenticate_with_status).and_return([user, :some_future_status]) + it 'tells the user to contact support when courses.mooc.fi is not configured' do + SiteSetting.all_settings['courses_mooc_fi_base_url'] = nil - do_destroy_user('secret123') + get :show - expect(User.find_by(id: user.id)).not_to be_nil - expect(flash[:alert]).to eq('The password was incorrect.') + expect(response.body).to include('contact support') + expect(response.body).not_to include('user-settings/permissions') end end end diff --git a/spec/integration/destroying_user_spec.rb b/spec/integration/destroying_user_spec.rb deleted file mode 100644 index 6dfc16505..000000000 --- a/spec/integration/destroying_user_spec.rb +++ /dev/null @@ -1,130 +0,0 @@ -# typed: false -# frozen_string_literal: true - -require 'spec_helper' - -describe 'Deleting own user', type: :request, integration: true do - include IntegrationTestActions - - it 'sending email should generate token' do - user = User.create!(login: 'user', password: 'password', email: 'theuser@example.com') - - visit '/' - log_in_as('user', 'password') - - visit '/user' - - click_button 'Request deleting account' - - # Yay, you got mail! - - token = user.verification_tokens.delete_user[0] - - expect(token).not_to be_nil - expect(User.find_by(login: user.login)).to eq(user) - end - - it 'verify destroy page should have a verification button and password field' do - user = User.create!(login: 'user', password: 'password', email: 'theuser@example.com') - - visit '/' - log_in_as('user', 'password') - - visit '/user' - - click_button 'Request deleting account' - - # Yay, you got mail! - - token = user.verification_tokens.delete_user[0] - - visit "/users/#{user.id}/destroy/#{token.token}" - - # expect(page).to have_content("Deleting the account #{user.login}") - expect(page).to have_content('Deleting your account will have the following consequences for all the courses where you have used this user account') - - expect(page).to have_button('Destroy my account permanently', disabled: true) - - check "I've read the above and i'm sure i understand the consequences" - - expect(page).to have_button('Destroy my account permanently', disabled: false) - expect(User.find_by(login: user.login)).to eq(user) - expect(page).to have_content('Your password') - end - - it 'pressing verification button destroys user' do - user = User.create!(login: 'user', password: 'password', email: 'theuser@example.com') - username = user.login - - visit '/' - log_in_as('user', 'password') - - visit '/user' - - click_button 'Request deleting account' - - # Yay, you got mail! - - token = user.verification_tokens.delete_user[0] - - visit "/users/#{user.id}/destroy/#{token.token}" - - check "I've read the above and i'm sure i understand the consequences" - - fill_in 'user[password]', with: user.password - - accept_confirm { click_button 'Destroy my account permanently' } - - expect { User.find_by!(login: username) }.to raise_error ActiveRecord::RecordNotFound - expect(page).to have_content('Your account has been permanently destroyed.') - expect(page).to have_content('Log in') - end - - it 'will not destroy user if password incorrect' do - user = User.create!(login: 'user', password: 'password', email: 'theuser@example.com') - - visit '/' - log_in_as('user', 'password') - - visit '/user' - - click_button 'Request deleting account' - - # Yay, you got mail! - - token = user.verification_tokens.delete_user[0] - - visit "/users/#{user.id}/destroy/#{token.token}" - - check "I've read the above and i'm sure i understand the consequences" - - fill_in 'user[password]', with: 'thisiswrongpassword' - - accept_confirm { click_button 'Destroy my account permanently' } - - expect(User.find_by(login: user.login)).to eq(user) - expect(page).to have_content('The password was incorrect') - end - - it 'cannot be verified by another user' do - user1 = User.create!(login: 'user1', password: 'password1', email: 'user1@example.com') - User.create!(login: 'user2', password: 'password2', email: 'user2@example.com') - - visit '/' - log_in_as('user1', 'password1') - - visit '/user' - - click_button 'Request deleting account' - - log_out - - log_in_as('user2', 'password2') - - token = user1.verification_tokens.delete_user[0] - - visit "/users/#{user1.id}/destroy/#{token.token}" - - expect(page).to have_content('Forbidden') - end -end diff --git a/spec/requests/account_deletion_redirect_spec.rb b/spec/requests/account_deletion_redirect_spec.rb new file mode 100644 index 000000000..b9f03b5f5 --- /dev/null +++ b/spec/requests/account_deletion_redirect_spec.rb @@ -0,0 +1,52 @@ +# typed: false +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Self-service account deletion', type: :request do + let!(:user) { FactoryBot.create(:user) } + let!(:token) { VerificationToken.create!(user: user, type: :delete_user) } + let(:deletion_url) { 'https://courses.mooc.fi/user-settings/permissions#delete-account' } + + before :each do + SiteSetting.all_settings['courses_mooc_fi_base_url'] = 'https://courses.mooc.fi' + end + + it 'redirects an emailed deletion link to courses.mooc.fi' do + get "/users/#{user.id}/destroy/#{token.token}" + + expect(response).to redirect_to(deletion_url) + end + + it 'redirects the old deletion form without deleting the user' do + delete "/users/#{user.id}/destroy/#{token.token}", params: { im_sure: '1', user: { password: user.password } } + + expect(response).to redirect_to(deletion_url) + expect(User.find_by(id: user.id)).to eq(user) + end + + it 'redirects a deletion email request without sending email' do + expect { post "/users/#{user.id}/send_destroy_email" }.not_to(change { ActionMailer::Base.deliveries.count }) + + expect(response).to redirect_to(deletion_url) + end + + it 'redirects a stale deletion form when forgery protection is on' do + original = ActionController::Base.allow_forgery_protection + ActionController::Base.allow_forgery_protection = true + + post "/users/#{user.id}/send_destroy_email", params: { authenticity_token: 'stale' } + + expect(response).to redirect_to(deletion_url) + ensure + ActionController::Base.allow_forgery_protection = original + end + + it "sends the user to their account page, which says to contact support, when courses.mooc.fi isn't configured" do + SiteSetting.all_settings['courses_mooc_fi_base_url'] = nil + + get "/users/#{user.id}/destroy/#{token.token}" + + expect(response).to redirect_to(user_path) + end +end diff --git a/spec/requests/secret_path_logging_spec.rb b/spec/requests/secret_path_logging_spec.rb index b3279cd16..9f1bc1f93 100644 --- a/spec/requests/secret_path_logging_spec.rb +++ b/spec/requests/secret_path_logging_spec.rb @@ -39,18 +39,6 @@ def logged_path_and_params(&block) expect(response.body).not_to include("key=#{key.token}") end - it 'keeps the account deletion token out of redirect logs' do - token = VerificationToken.create!(user: user, type: :delete_user) - post '/sessions', params: { session: { login: user.login, password: 'secret123' } } - - payloads = instrumented('redirect_to.action_controller') do - delete "/users/#{user.id}/destroy/#{token.token}", params: { im_sure: '1', user: { password: 'wrong' } } - end - - expect(response).to redirect_to("http://www.example.com/users/#{user.id}/destroy/#{token.token}") - expect(payloads.pluck(:location)).to eq(['[FILTERED]']) - end - it 'filters the token from Rack::Attack log lines' do expect(SecretPathFilter.filter('//users/1/destroy/abc.json')).to eq('//users/1/destroy/[FILTERED]') expect(SecretPathFilter.filter('/reset_password')).to eq('/reset_password')