From 99ecdbf71632970d4c83f99beea325e5d213e4c6 Mon Sep 17 00:00:00 2001 From: Azul Date: Tue, 5 Nov 2013 12:12:13 +0100 Subject: disabled identities to block handles after a user was deleted --- users/test/unit/account_test.rb | 3 ++- users/test/unit/identity_test.rb | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) (limited to 'users/test') diff --git a/users/test/unit/account_test.rb b/users/test/unit/account_test.rb index 94a9980..a8c6efd 100644 --- a/users/test/unit/account_test.rb +++ b/users/test/unit/account_test.rb @@ -13,7 +13,8 @@ class AccountTest < ActiveSupport::TestCase end test "create and remove a user account" do - assert_no_difference "Identity.count" do + # We keep an identity that will block the handle from being reused. + assert_difference "Identity.count" do assert_no_difference "User.count" do user = Account.create(FactoryGirl.attributes_for(:user)) user.account.destroy diff --git a/users/test/unit/identity_test.rb b/users/test/unit/identity_test.rb index 0842a77..8270689 100644 --- a/users/test/unit/identity_test.rb +++ b/users/test/unit/identity_test.rb @@ -90,6 +90,26 @@ class IdentityTest < ActiveSupport::TestCase assert id.errors.messages[:destination].include? "needs to be a valid email address" end + test "disabled identity" do + id = Identity.for(@user) + id.disable + assert_equal @user.email_address, id.address + assert_equal nil, id.destination + assert_equal nil, id.user + assert !id.enabled? + assert id.valid? + end + + test "disabled identity blocks handle" do + id = Identity.for(@user) + id.disable + id.save + other_user = find_record :user + taken = Identity.build_for other_user, address: id.address + assert !taken.valid? + id.destroy + end + def alias_name @alias_name ||= Faker::Internet.user_name end -- cgit v1.2.3 From 4a2490cc5eac1803be80fade65bbe9d32fa0bd9b Mon Sep 17 00:00:00 2001 From: Azul Date: Tue, 5 Nov 2013 17:03:55 +0100 Subject: Identity.destroy_all_disabled will clean up disabled identities This is mostly for cleaning up after tests so far. But we might expand this to destroy all identities disabled before a certain date. --- users/test/unit/account_test.rb | 4 ++++ users/test/unit/identity_test.rb | 11 ++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) (limited to 'users/test') diff --git a/users/test/unit/account_test.rb b/users/test/unit/account_test.rb index a8c6efd..4fb3c3d 100644 --- a/users/test/unit/account_test.rb +++ b/users/test/unit/account_test.rb @@ -2,6 +2,10 @@ require 'test_helper' class AccountTest < ActiveSupport::TestCase + teardown do + Identity.destroy_all_disabled + end + test "create a new account" do user = Account.create(FactoryGirl.attributes_for(:user)) assert user.valid? diff --git a/users/test/unit/identity_test.rb b/users/test/unit/identity_test.rb index 8270689..78ef52c 100644 --- a/users/test/unit/identity_test.rb +++ b/users/test/unit/identity_test.rb @@ -107,7 +107,16 @@ class IdentityTest < ActiveSupport::TestCase other_user = find_record :user taken = Identity.build_for other_user, address: id.address assert !taken.valid? - id.destroy + Identity.destroy_all_disabled + end + + test "destroy all disabled identities" do + id = Identity.for(@user) + id.disable + id.save + assert Identity.count > 0 + Identity.destroy_all_disabled + assert_equal 0, Identity.count end def alias_name -- cgit v1.2.3 From 40f24e2887672957acf7ecedce58e692cc9505ca Mon Sep 17 00:00:00 2001 From: Azul Date: Wed, 6 Nov 2013 10:10:44 +0100 Subject: refactor: extract method on account test also test one can't login anymore after destroying the account. --- users/test/integration/browser/account_test.rb | 35 ++++++++++++++------------ 1 file changed, 19 insertions(+), 16 deletions(-) (limited to 'users/test') diff --git a/users/test/integration/browser/account_test.rb b/users/test/integration/browser/account_test.rb index b712c95..6e9aab5 100644 --- a/users/test/integration/browser/account_test.rb +++ b/users/test/integration/browser/account_test.rb @@ -19,31 +19,24 @@ class AccountTest < BrowserIntegrationTest test "successful login" do username, password = submit_signup click_on 'Logout' - click_on 'Log In' - fill_in 'Username', with: username - fill_in 'Password', with: password - click_on 'Log In' + attempt_login(username, password) assert page.has_content?("Welcome #{username}") User.find_by_login(username).account.destroy end test "failed login" do visit '/' - click_on 'Log In' - fill_in 'Username', with: "username" - fill_in 'Password', with: "wrong password" - click_on 'Log In' - assert page.has_selector? 'input.btn-primary.disabled' - assert page.has_content? I18n.t(:invalid_user_pass) - assert page.has_no_selector? 'input.btn-primary.disabled' + attempt_login("username", "wrong password") + assert_invalid_login(page) end test "account destruction" do username, password = submit_signup click_on I18n.t('account_settings') click_on I18n.t('destroy_my_account') - page.save_screenshot('/tmp/destroy.png') assert page.has_content?(I18n.t('account_destroyed')) + attempt_login(username, password) + assert_invalid_login(page) end test "change password" do @@ -55,10 +48,7 @@ class AccountTest < BrowserIntegrationTest click_on 'Save' end click_on 'Logout' - click_on 'Log In' - fill_in 'Username', with: username - fill_in 'Password', with: "other password" - click_on 'Log In' + attempt_login(username, "other password") assert page.has_content?("Welcome #{username}") User.find_by_login(username).account.destroy end @@ -100,6 +90,19 @@ class AccountTest < BrowserIntegrationTest assert page.has_content?("server failed") end + def attempt_login(username, password) + click_on 'Log In' + fill_in 'Username', with: username + fill_in 'Password', with: password + click_on 'Log In' + end + + def assert_invalid_login(page) + assert page.has_selector? 'input.btn-primary.disabled' + assert page.has_content? I18n.t(:invalid_user_pass) + assert page.has_no_selector? 'input.btn-primary.disabled' + end + def inject_malicious_js page.execute_script <<-EOJS var calc = new srp.Calculate(); -- cgit v1.2.3 From 44d273fd03645af5e546133adf4e9906800d3d5f Mon Sep 17 00:00:00 2001 From: Azul Date: Wed, 6 Nov 2013 10:20:33 +0100 Subject: integration test for blocking handles after account destroyed has not been run yet. --- users/test/integration/browser/account_test.rb | 12 ++++++++++++ users/test/support/integration_test_helper.rb | 6 +++--- 2 files changed, 15 insertions(+), 3 deletions(-) (limited to 'users/test') diff --git a/users/test/integration/browser/account_test.rb b/users/test/integration/browser/account_test.rb index 6e9aab5..b349489 100644 --- a/users/test/integration/browser/account_test.rb +++ b/users/test/integration/browser/account_test.rb @@ -6,6 +6,10 @@ class AccountTest < BrowserIntegrationTest Capybara.current_driver = Capybara.javascript_driver end + teardown do + Identity.destroy_all_disabled + end + test "normal account workflow" do username, password = submit_signup assert page.has_content?("Welcome #{username}") @@ -39,6 +43,14 @@ class AccountTest < BrowserIntegrationTest assert_invalid_login(page) end + test "handle blocked after account destruction" do + username, password = submit_signup + click_on I18n.t('account_settings') + click_on I18n.t('destroy_my_account') + submit_signup(username) + assert page.has_content?('has already been taken') + end + test "change password" do username, password = submit_signup click_on "Account Settings" diff --git a/users/test/support/integration_test_helper.rb b/users/test/support/integration_test_helper.rb index cfe72cf..51e47c6 100644 --- a/users/test/support/integration_test_helper.rb +++ b/users/test/support/integration_test_helper.rb @@ -1,7 +1,7 @@ module IntegrationTestHelper - def submit_signup - username = "test_#{SecureRandom.urlsafe_base64}".downcase - password = SecureRandom.base64 + def submit_signup(username = nil, password = nil) + username ||= "test_#{SecureRandom.urlsafe_base64}".downcase + password ||= SecureRandom.base64 visit '/users/new' fill_in 'Username', with: username fill_in 'Password', with: password -- cgit v1.2.3 From ded302ebc6a9e145775f7847c5e89f91d683c777 Mon Sep 17 00:00:00 2001 From: Azul Date: Wed, 6 Nov 2013 11:55:43 +0100 Subject: use the account lifecycle from UsersController#destroy --- users/test/functional/users_controller_test.rb | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'users/test') diff --git a/users/test/functional/users_controller_test.rb b/users/test/functional/users_controller_test.rb index 75d900f..9c5f8d9 100644 --- a/users/test/functional/users_controller_test.rb +++ b/users/test/functional/users_controller_test.rb @@ -77,7 +77,11 @@ class UsersControllerTest < ActionController::TestCase test "admin can destroy user" do user = find_record :user + + # we destroy the user record and the associated data... user.expects(:destroy) + Identity.expects(:disable_all_for).with(user) + Ticket.expects(:destroy_all_from).with(user) login :is_admin? => true delete :destroy, :id => user.id @@ -88,7 +92,11 @@ class UsersControllerTest < ActionController::TestCase test "user can cancel account" do user = find_record :user + + # we destroy the user record and the associated data... user.expects(:destroy) + Identity.expects(:disable_all_for).with(user) + Ticket.expects(:destroy_all_from).with(user) login user expect_logout -- cgit v1.2.3 From a0e72e6ee7786e3b1fd7276f1c64912c606f4559 Mon Sep 17 00:00:00 2001 From: Azul Date: Thu, 7 Nov 2013 12:46:28 +0100 Subject: only check number of disabled identities to make test more robust --- users/test/unit/identity_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'users/test') diff --git a/users/test/unit/identity_test.rb b/users/test/unit/identity_test.rb index 78ef52c..eca104f 100644 --- a/users/test/unit/identity_test.rb +++ b/users/test/unit/identity_test.rb @@ -116,7 +116,7 @@ class IdentityTest < ActiveSupport::TestCase id.save assert Identity.count > 0 Identity.destroy_all_disabled - assert_equal 0, Identity.count + assert_equal 0, Identity.disabled.count end def alias_name -- cgit v1.2.3