summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/controllers/users_controller.rb3
-rw-r--r--app/models/account.rb22
-rw-r--r--app/models/identity.rb104
-rw-r--r--app/models/user.rb14
-rw-r--r--test/functional/identities_controller_test.rb2
-rw-r--r--test/functional/users_controller_test.rb4
-rw-r--r--test/integration/browser/account_test.rb2
-rw-r--r--test/support/api_integration_test.rb2
-rw-r--r--test/support/browser_integration_test.rb2
-rw-r--r--test/unit/account_test.rb7
-rw-r--r--test/unit/identity_test.rb29
11 files changed, 116 insertions, 75 deletions
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index ec52cff..1404b0e 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -53,8 +53,7 @@ class UsersController < ApplicationController
end
def enable
- @user.enabled = true
- @user.save
+ @user.account.enable
flash[:notice] = I18n.t("actions.user_enabled_message", username: @user.username)
redirect_to :back
end
diff --git a/app/models/account.rb b/app/models/account.rb
index 46e5446..abde89d 100644
--- a/app/models/account.rb
+++ b/app/models/account.rb
@@ -59,11 +59,15 @@ class Account
def destroy(destroy_identity=false)
return unless @user
- if !user.tmp?
+ if !@user.tmp?
if destroy_identity == false
- Identity.disable_all_for(@user)
+ @user.identities.each do |id|
+ id.orphan!
+ end
else
- Identity.destroy_all_for(@user)
+ @user.identities.each do |id|
+ id.destroy
+ end
end
end
@user.destroy
@@ -76,7 +80,17 @@ class Account
if @user && !@user.tmp?
@user.enabled = false
@user.save
- Identity.remove_cert_fingerprints_for(@user)
+ @user.identities.each do |id|
+ id.disable!
+ end
+ end
+ end
+
+ def enable
+ @user.enabled = true
+ @user.save
+ @user.identities.each do |id|
+ id.enable!
end
end
diff --git a/app/models/identity.rb b/app/models/identity.rb
index e4162c8..f987e4e 100644
--- a/app/models/identity.rb
+++ b/app/models/identity.rb
@@ -1,9 +1,13 @@
#
-# NOTE: there is some confusing terminology between User and Identity:
-# If a user is disabled, the user still exists but has been marked as disabled
-# and this condition can be easily reversed. If an identity is disabled, then
-# it loses any association with the user and exists only to reserve that username
-# and prevent anyone else from registering it.
+# Identity states:
+#
+# DISABLED -- An identity is disabled if and only if its associated user
+# is also disabled. In the disabled state, incoming email
+# should bounce and outgoing email should not be relayed.
+#
+# ORPHANED -- An identity is orphaned if it has lost its association
+# with a user account. This is in order to keep the name
+# reserved to prevent anyone else from using it.
#
class Identity < CouchRest::Model::Base
@@ -17,10 +21,12 @@ class Identity < CouchRest::Model::Base
property :destination, Email
property :keys, HashWithIndifferentAccess
property :cert_fingerprints, Hash
+ property :disabled_cert_fingerprints, Hash
+ property :enabled, TrueClass, :default => true
validates :address, presence: true
validate :address_available
- validates :destination, presence: true, if: :enabled?
+ validates :destination, presence: true, if: :user_id
validates :destination, uniqueness: {scope: :address}
validate :address_local_email
validate :destination_email
@@ -58,33 +64,33 @@ 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
- # if the identity is not unique anymore because the destination
- # was reset to nil we destroy it.
- identity.save || identity.destroy
- end
+ # currently leap_mx ignores enabled property, so we
+ # also disable the fingerprints instead of just marking
+ # identity as disabled.
+
+ def disable!
+ self.disabled_cert_fingerprints = self.cert_fingerprints
+ self.cert_fingerprints = {}
+ self.write_attribute(:enabled, false)
+ self.save
end
- # if an identity is disabled, it loses contact
- # with its former user. but sometimes we want to keep the association
- # and remove the fingerprints that allow the user to send email.
- def self.remove_cert_fingerprints_for(user)
- Identity.by_user_id.key(user.id).each do |identity|
- identity.write_attribute(:cert_fingerprints, {})
- identity.save
- end
+ def enable!
+ self.cert_fingerprints = self.disabled_cert_fingerprints
+ self.disabled_cert_fingerprints = nil
+ self.write_attribute(:enabled, true)
+ self.save
end
- def self.destroy_all_for(user)
- Identity.by_user_id.key(user.id).each do |identity|
- identity.destroy
- end
+ # removes the association between this identity and the user.
+ def orphan!
+ self.destination = nil
+ self.user_id = nil
+ self.disable!
end
- def self.destroy_all_disabled
- Identity.disabled.each do |identity|
+ def self.destroy_all_orphaned
+ Identity.orphaned.each do |identity|
identity.destroy
end
end
@@ -97,38 +103,28 @@ class Identity < CouchRest::Model::Base
end
def status
- return :blocked if disabled?
- case destination
- when address
- :main_email
- when /@#{APP_CONFIG[:domain]}\Z/i,
- :alias
+ if !enabled? || orphaned?
+ return :blocked
else
- :forward
+ case destination
+ when address
+ :main_email
+ when /@#{APP_CONFIG[:domain]}\Z/i,
+ :alias
+ else
+ :forward
+ end
end
end
- def enabled?
- self.user_id
- end
-
- def disabled?
- !enabled?
- end
-
def actions
- if enabled?
+ if !orphaned?
[] # [:show, :edit]
else
[:destroy]
end
end
- def disable
- self.destination = nil
- self.user_id = nil
- end
-
def keys
read_attribute('keys') || HashWithIndifferentAccess.new
end
@@ -153,6 +149,18 @@ class Identity < CouchRest::Model::Base
address.handle if address.present?
end
+ def orphaned?
+ self.user_id.nil?
+ end
+
+ def self.orphaned
+ # the "disabled" view is a misnomer. it returns
+ # identities that have been orphaned, not identities that
+ # have been disabled.
+ # TODO: fix the view name
+ Identity.disabled
+ end
+
protected
def address_available
diff --git a/app/models/user.rb b/app/models/user.rb
index 4bb1e79..61793be 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -136,14 +136,27 @@ class User < CouchRest::Model::Base
Account.new(self)
end
+ # deprecated
def identity
@identity ||= Identity.for(self)
end
+ # deprecated
def refresh_identity
@identity = Identity.for(self)
end
+ def identities
+ Identity.by_user_id.key(self.id)
+ end
+
+ def destroy_identities
+ identities.each do |id|
+ id.destroy
+ end
+ end
+
+
def desired_service_level
code = self.desired_service_level_code || APP_CONFIG[:default_service_level]
ServiceLevel.new({id: code})
@@ -154,7 +167,6 @@ class User < CouchRest::Model::Base
ServiceLevel.new({id: code})
end
-
def self.send_one_month_warnings
# To determine warnings to send, need to get all users where one_month_warning_sent is not set, and where it was created greater than or equal to 1 month ago.
diff --git a/test/functional/identities_controller_test.rb b/test/functional/identities_controller_test.rb
index e491c52..5af2e88 100644
--- a/test/functional/identities_controller_test.rb
+++ b/test/functional/identities_controller_test.rb
@@ -1,4 +1,4 @@
-require 'test_helper'
+require_relative '../test_helper'
class IdentitiesControllerTest < ActionController::TestCase
diff --git a/test/functional/users_controller_test.rb b/test/functional/users_controller_test.rb
index 261f201..7b24098 100644
--- a/test/functional/users_controller_test.rb
+++ b/test/functional/users_controller_test.rb
@@ -86,7 +86,7 @@ class UsersControllerTest < ActionController::TestCase
# we destroy the user record and the associated data...
user.expects(:destroy)
- Identity.expects(:disable_all_for).with(user)
+ #user.identity.expects(:orphan!) << factory girl user doesn't have identities
Ticket.expects(:destroy_all_from).with(user)
login :is_admin? => true
@@ -101,7 +101,7 @@ class UsersControllerTest < ActionController::TestCase
# we destroy the user record and the associated data...
user.expects(:destroy)
- Identity.expects(:disable_all_for).with(user)
+ #user.identity.expects(:orphan!) << factory girl user doesn't have identities
Ticket.expects(:destroy_all_from).with(user)
login user
diff --git a/test/integration/browser/account_test.rb b/test/integration/browser/account_test.rb
index cbe7ba9..50adb23 100644
--- a/test/integration/browser/account_test.rb
+++ b/test/integration/browser/account_test.rb
@@ -3,7 +3,7 @@ require 'test_helper'
class AccountTest < BrowserIntegrationTest
teardown do
- Identity.destroy_all_disabled
+ Identity.destroy_all_orphaned
end
test "signup successfully when invited" do
diff --git a/test/support/api_integration_test.rb b/test/support/api_integration_test.rb
index 4077920..3b3481b 100644
--- a/test/support/api_integration_test.rb
+++ b/test/support/api_integration_test.rb
@@ -21,7 +21,7 @@ class ApiIntegrationTest < ActionDispatch::IntegrationTest
teardown do
if @user && @user.persisted?
- Identity.destroy_all_for @user
+ @user.destroy_identities
@user.reload.destroy
end
if @token && @token.persisted?
diff --git a/test/support/browser_integration_test.rb b/test/support/browser_integration_test.rb
index 950a395..1deb8fa 100644
--- a/test/support/browser_integration_test.rb
+++ b/test/support/browser_integration_test.rb
@@ -86,7 +86,7 @@ class BrowserIntegrationTest < ActionDispatch::IntegrationTest
teardown do
if @user && @user.reload
- Identity.destroy_all_for @user
+ @user.destroy_identities
@user.destroy
end
end
diff --git a/test/unit/account_test.rb b/test/unit/account_test.rb
index 7c26d5c..9680b33 100644
--- a/test/unit/account_test.rb
+++ b/test/unit/account_test.rb
@@ -8,7 +8,7 @@ class AccountTest < ActiveSupport::TestCase
end
teardown do
- Identity.destroy_all_disabled
+ Identity.destroy_all_orphaned
end
test "create a new account when invited" do
@@ -86,8 +86,11 @@ class AccountTest < ActiveSupport::TestCase
cert = stub(expiry: 1.month.from_now, fingerprint: SecureRandom.hex)
user.identity.register_cert cert
user.identity.save
- assert_equal cert.fingerprint, Identity.for(user).cert_fingerprints.keys.first
+ assert_equal(cert.fingerprint, Identity.for(user).cert_fingerprints.keys.first)
user.account.disable
assert_equal({}, Identity.for(user).cert_fingerprints)
+ assert_equal(cert.fingerprint, Identity.for(user).read_attribute(:disabled_cert_fingerprints).keys.first)
+ user.account.enable
+ assert_equal(cert.fingerprint, Identity.for(user).cert_fingerprints.keys.first)
end
end
diff --git a/test/unit/identity_test.rb b/test/unit/identity_test.rb
index 1f93109..9d4bc90 100644
--- a/test/unit/identity_test.rb
+++ b/test/unit/identity_test.rb
@@ -110,33 +110,38 @@ class IdentityTest < ActiveSupport::TestCase
assert @id.errors.messages[:destination].include? "needs to be a valid email address"
end
- test "disabled identity" do
+ test "disable identity" do
@id = Identity.for(@user)
- @id.disable
+ @id.disable!
+ assert !@id.enabled?
+ assert @id.valid?
+ end
+
+ test "orphan identity" do
+ @id = Identity.for(@user)
+ @id.orphan!
assert_equal @user.email_address, @id.address
assert_equal nil, @id.destination
assert_equal nil, @id.user
- assert !@id.enabled?
+ assert @id.orphaned?
assert @id.valid?
end
- test "disabled identity blocks handle" do
+ test "orphaned identity blocks handle" do
@id = Identity.for(@user)
- @id.disable
- @id.save
+ @id.orphan!
other_user = find_record :user
taken = Identity.build_for other_user, address: @id.address
assert !taken.valid?
assert_equal ["has already been taken"], taken.errors[:address]
end
- test "destroy all disabled identities" do
+ test "destroy all orphaned identities" do
@id = Identity.for(@user)
- @id.disable
- @id.save
- assert Identity.disabled.count > 0
- Identity.destroy_all_disabled
- assert_equal 0, Identity.disabled.count
+ @id.orphan!
+ assert Identity.orphaned.count > 0
+ Identity.destroy_all_orphaned
+ assert_equal 0, Identity.orphaned.count
end
test "store cert fingerprint" do