diff options
Diffstat (limited to 'users')
| -rw-r--r-- | users/app/controllers/users_controller.rb | 2 | ||||
| -rw-r--r-- | users/app/controllers/v1/users_controller.rb | 8 | ||||
| -rw-r--r-- | users/app/models/account.rb | 14 | ||||
| -rw-r--r-- | users/app/models/identity.rb | 36 | ||||
| -rw-r--r-- | users/test/functional/users_controller_test.rb | 8 | ||||
| -rw-r--r-- | users/test/integration/browser/account_test.rb | 47 | ||||
| -rw-r--r-- | users/test/support/integration_test_helper.rb | 6 | ||||
| -rw-r--r-- | users/test/unit/account_test.rb | 7 | ||||
| -rw-r--r-- | users/test/unit/identity_test.rb | 29 |
9 files changed, 124 insertions, 33 deletions
diff --git a/users/app/controllers/users_controller.rb b/users/app/controllers/users_controller.rb index de21983..3cbb6dc 100644 --- a/users/app/controllers/users_controller.rb +++ b/users/app/controllers/users_controller.rb @@ -47,7 +47,7 @@ class UsersController < UsersBaseController end def destroy - @user.destroy + @user.account.destroy flash[:notice] = I18n.t(:account_destroyed) # admins can destroy other users if @user != current_user diff --git a/users/app/controllers/v1/users_controller.rb b/users/app/controllers/v1/users_controller.rb index 03a5a62..0903888 100644 --- a/users/app/controllers/v1/users_controller.rb +++ b/users/app/controllers/v1/users_controller.rb @@ -24,15 +24,9 @@ module V1 end def update - account.update params[:user] + @user.account.update params[:user] respond_with @user end - protected - - def account - @user.account - end - end end diff --git a/users/app/models/account.rb b/users/app/models/account.rb index 5368a1b..5c943bb 100644 --- a/users/app/models/account.rb +++ b/users/app/models/account.rb @@ -1,5 +1,10 @@ # -# A Composition of a User record and it's identity records. +# The Account model takes care of the livecycle of a user. +# It composes a User record and it's identity records. +# It also allows for other engines to hook into the livecycle by +# monkeypatching the create, update and destroy methods. +# There's an ActiveSupport load_hook at the end of this file to +# make this more easy. # class Account @@ -29,9 +34,7 @@ class Account def destroy return unless @user - Identity.by_user_id.key(@user.id).each do |identity| - identity.destroy - end + Identity.disable_all_for(@user) @user.destroy end @@ -54,4 +57,7 @@ class Account @new_identity.try(:save) && @old_identity.try(:save) end + # You can hook into the account lifecycle from different engines using + # ActiveSupport.on_load(:account) do ... + ActiveSupport.run_load_hooks(:account, self) end diff --git a/users/app/models/identity.rb b/users/app/models/identity.rb index e0a24e9..97966d0 100644 --- a/users/app/models/identity.rb +++ b/users/app/models/identity.rb @@ -27,6 +27,17 @@ class Identity < CouchRest::Model::Base emit(doc.address, doc.keys["pgp"]); } EOJS + view :disabled, + map: <<-EOJS + function(doc) { + if (doc.type != 'Identity') { + return; + } + if (typeof doc.user_id === "undefined") { + emit(doc._id, 1); + } + } + EOJS end @@ -50,6 +61,19 @@ class Identity < CouchRest::Model::Base identity end + def self.disable_all_for(user) + Identity.by_user_id.key(user.id).each do |identity| + identity.disable + identity.save + end + end + + def self.destroy_all_disabled + Identity.disabled.each do |identity| + identity.destroy + end + end + def self.attributes_from_user(user) { user_id: user.id, address: user.email_address, @@ -57,6 +81,15 @@ class Identity < CouchRest::Model::Base } end + def enabled? + self.destination && self.user_id + end + + def disable + self.destination = nil + self.user_id = nil + end + def keys read_attribute('keys') || HashWithIndifferentAccess.new end @@ -93,7 +126,8 @@ class Identity < CouchRest::Model::Base end def destination_email - return if destination.valid? #this ensures it is Email + return if destination.nil? # this identity is disabled + return if destination.valid? # this ensures it is Email self.errors.add(:destination, destination.errors.messages[:email].first) #assumes only one error #TODO end 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 diff --git a/users/test/integration/browser/account_test.rb b/users/test/integration/browser/account_test.rb index b712c95..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}") @@ -19,31 +23,32 @@ 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 "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 @@ -55,10 +60,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 +102,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(); 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 diff --git a/users/test/unit/account_test.rb b/users/test/unit/account_test.rb index 94a9980..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? @@ -13,7 +17,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..eca104f 100644 --- a/users/test/unit/identity_test.rb +++ b/users/test/unit/identity_test.rb @@ -90,6 +90,35 @@ 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? + 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.disabled.count + end + def alias_name @alias_name ||= Faker::Internet.user_name end |