summaryrefslogtreecommitdiff
path: root/users
diff options
context:
space:
mode:
authorAzul <azul@leap.se>2013-11-15 11:53:21 +0100
committerAzul <azul@leap.se>2013-11-15 11:53:21 +0100
commit8b49ee15466b728213ec7f8bd4c8462876625acf (patch)
treec301a8033650a1067c9caf012669d1a47d6f2a0c /users
parent7e93258f552d6fd1114626561e6393aa483228fe (diff)
parent7a107e0d38271e7103d3494e06d52f3434022f22 (diff)
Merge branch 'develop'
Diffstat (limited to 'users')
-rw-r--r--users/app/controllers/users_controller.rb12
-rw-r--r--users/app/controllers/v1/users_controller.rb8
-rw-r--r--users/app/models/account.rb14
-rw-r--r--users/app/models/identity.rb36
-rw-r--r--users/app/models/token.rb36
-rw-r--r--users/config/locales/en.yml1
-rw-r--r--users/test/factories.rb4
-rw-r--r--users/test/functional/sessions_controller_test.rb14
-rw-r--r--users/test/functional/users_controller_test.rb9
-rw-r--r--users/test/functional/v1/sessions_controller_test.rb19
-rw-r--r--users/test/integration/browser/account_test.rb53
-rw-r--r--users/test/support/auth_test_helper.rb14
-rw-r--r--users/test/support/integration_test_helper.rb6
-rw-r--r--users/test/unit/account_test.rb7
-rw-r--r--users/test/unit/identity_test.rb29
-rw-r--r--users/test/unit/token_test.rb23
16 files changed, 209 insertions, 76 deletions
diff --git a/users/app/controllers/users_controller.rb b/users/app/controllers/users_controller.rb
index f66277d..3cbb6dc 100644
--- a/users/app/controllers/users_controller.rb
+++ b/users/app/controllers/users_controller.rb
@@ -47,8 +47,16 @@ class UsersController < UsersBaseController
end
def destroy
- @user.destroy
- redirect_to admin? ? users_url : root_url
+ @user.account.destroy
+ flash[:notice] = I18n.t(:account_destroyed)
+ # admins can destroy other users
+ if @user != current_user
+ redirect_to users_url
+ else
+ # let's remove the invalid session
+ logout
+ redirect_to root_url
+ end
end
end
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/app/models/token.rb b/users/app/models/token.rb
index dd87344..001eb40 100644
--- a/users/app/models/token.rb
+++ b/users/app/models/token.rb
@@ -11,6 +11,25 @@ class Token < CouchRest::Model::Base
validates :user_id, presence: true
+ design do
+ view :by_last_seen_at
+ end
+
+ def self.expires_after
+ APP_CONFIG[:auth] && APP_CONFIG[:auth][:token_expires_after]
+ end
+
+ def self.expired
+ return [] unless expires_after
+ by_last_seen_at.endkey(expires_after.minutes.ago)
+ end
+
+ def self.destroy_all_expired
+ self.expired.each do |token|
+ token.destroy
+ end
+ end
+
def authenticate
if expired?
destroy
@@ -27,21 +46,16 @@ class Token < CouchRest::Model::Base
end
def expired?
- expires_after and
- last_seen_at + expires_after.minutes < Time.now
- end
-
- def expires_after
- APP_CONFIG[:auth] && APP_CONFIG[:auth][:token_expires_after]
+ Token.expires_after and
+ last_seen_at < Token.expires_after.minutes.ago
end
def initialize(*args)
super
- self.id = SecureRandom.urlsafe_base64(32).gsub(/^_*/, '')
- self.last_seen_at = Time.now
- end
-
- design do
+ if new_record?
+ self.id = SecureRandom.urlsafe_base64(32).gsub(/^_*/, '')
+ self.last_seen_at = Time.now
+ end
end
end
diff --git a/users/config/locales/en.yml b/users/config/locales/en.yml
index b69f7f4..1b5dd5e 100644
--- a/users/config/locales/en.yml
+++ b/users/config/locales/en.yml
@@ -17,6 +17,7 @@ en:
destroy_my_account: "Destroy my account"
destroy_account_info: "This will permanently destroy your account and all the data associated with it. Proceed with caution!"
admin_destroy_account: "Destroy the account %{username}"
+ account_destroyed: "The account has been destroyed successfully."
set_email_address: "Set email address"
forward_email: "Forward Email"
email_aliases: "Email Aliases"
diff --git a/users/test/factories.rb b/users/test/factories.rb
index c87e290..f5fb77d 100644
--- a/users/test/factories.rb
+++ b/users/test/factories.rb
@@ -19,6 +19,8 @@ FactoryGirl.define do
end
end
- factory :token
+ factory :token do
+ user
+ end
end
diff --git a/users/test/functional/sessions_controller_test.rb b/users/test/functional/sessions_controller_test.rb
index a630e6e..28143da 100644
--- a/users/test/functional/sessions_controller_test.rb
+++ b/users/test/functional/sessions_controller_test.rb
@@ -41,20 +41,12 @@ class SessionsControllerTest < ActionController::TestCase
assert_json_error :login => I18n.t(:all_strategies_failed)
end
- test "logout should reset warden user" do
- expect_warden_logout
+ test "destory should logout" do
+ login
+ expect_logout
delete :destroy
assert_response :redirect
assert_redirected_to root_url
end
- def expect_warden_logout
- raw = mock('raw session') do
- expects(:inspect)
- end
- request.env['warden'].expects(:raw_session).returns(raw)
- request.env['warden'].expects(:logout)
- end
-
-
end
diff --git a/users/test/functional/users_controller_test.rb b/users/test/functional/users_controller_test.rb
index 052de04..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,9 +92,14 @@ 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
delete :destroy, :id => @current_user.id
assert_response :redirect
diff --git a/users/test/functional/v1/sessions_controller_test.rb b/users/test/functional/v1/sessions_controller_test.rb
index ff9fca1..4200e8f 100644
--- a/users/test/functional/v1/sessions_controller_test.rb
+++ b/users/test/functional/v1/sessions_controller_test.rb
@@ -52,26 +52,11 @@ class V1::SessionsControllerTest < ActionController::TestCase
assert_equal @user.id, token.user_id
end
- test "logout should reset session" do
- expect_warden_logout
- delete :destroy
- assert_response 204
- end
-
- test "logout should destroy token" do
+ test "destroy should logout" do
login
- expect_warden_logout
- @token.expects(:destroy)
+ expect_logout
delete :destroy
assert_response 204
end
- def expect_warden_logout
- raw = mock('raw session') do
- expects(:inspect)
- end
- request.env['warden'].expects(:raw_session).returns(raw)
- request.env['warden'].expects(:logout)
- end
-
end
diff --git a/users/test/integration/browser/account_test.rb b/users/test/integration/browser/account_test.rb
index 8e03856..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,23 +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')
+ 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
@@ -47,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
@@ -92,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/auth_test_helper.rb b/users/test/support/auth_test_helper.rb
index 609f115..50e9453 100644
--- a/users/test/support/auth_test_helper.rb
+++ b/users/test/support/auth_test_helper.rb
@@ -38,12 +38,26 @@ module AuthTestHelper
end
end
+ def expect_logout
+ expect_warden_logout
+ @token.expects(:destroy) if @token
+ end
+
protected
def header_for_token_auth
@token = find_record(:token, :authenticate => @current_user)
ActionController::HttpAuthentication::Token.encode_credentials @token.id
end
+
+ def expect_warden_logout
+ raw = mock('raw session') do
+ expects(:inspect)
+ end
+ request.env['warden'].expects(:raw_session).returns(raw)
+ request.env['warden'].expects(:logout)
+ end
+
end
class ActionController::TestCase
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
diff --git a/users/test/unit/token_test.rb b/users/test/unit/token_test.rb
index f56c576..6c9f209 100644
--- a/users/test/unit/token_test.rb
+++ b/users/test/unit/token_test.rb
@@ -7,9 +7,6 @@ class ClientCertificateTest < ActiveSupport::TestCase
@user = find_record :user
end
- teardown do
- end
-
test "new token for user" do
sample = Token.new(:user_id => @user.id)
assert sample.valid?
@@ -61,6 +58,26 @@ class ClientCertificateTest < ActiveSupport::TestCase
end
end
+ test "Token.destroy_all_expired is noop if no expiry is set" do
+ expired = FactoryGirl.create :token, last_seen_at: 2.hours.ago
+ with_config auth: {} do
+ Token.destroy_all_expired
+ end
+ assert_equal expired, Token.find(expired.id)
+ end
+
+ test "Token.destroy_all_expired cleans up expired tokens only" do
+ expired = FactoryGirl.create :token, last_seen_at: 2.hours.ago
+ fresh = FactoryGirl.create :token
+ with_config auth: {token_expires_after: 60} do
+ Token.destroy_all_expired
+ end
+ assert_nil Token.find(expired.id)
+ assert_equal fresh, Token.find(fresh.id)
+ fresh.destroy
+ end
+
+
end