summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/models/account.rb8
-rw-r--r--app/models/identity.rb55
-rw-r--r--app/models/local_email.rb10
-rw-r--r--app/models/pgp_key.rb3
-rw-r--r--app/models/user.rb16
-rw-r--r--test/integration/api/smtp_cert_test.rb2
-rw-r--r--test/integration/browser/account_test.rb10
-rw-r--r--test/unit/identity_test.rb21
8 files changed, 85 insertions, 40 deletions
diff --git a/app/models/account.rb b/app/models/account.rb
index cf998e4..32ed445 100644
--- a/app/models/account.rb
+++ b/app/models/account.rb
@@ -16,9 +16,13 @@ class Account
# Returns the user record so it can be used in views.
def self.create(attrs)
- @user = User.create(attrs).tap do |user|
- Identity.create_for user
+ @user = User.create(attrs)
+ if @user.persisted?
+ identity = @user.identity
+ identity.user_id = @user.id
+ identity.save
end
+ return @user
end
def update(attrs)
diff --git a/app/models/identity.rb b/app/models/identity.rb
index a4225e7..2f6241c 100644
--- a/app/models/identity.rb
+++ b/app/models/identity.rb
@@ -10,8 +10,10 @@ class Identity < CouchRest::Model::Base
property :keys, HashWithIndifferentAccess
property :cert_fingerprints, Hash
- validate :unique_forward
- validate :alias_available
+ validates :address, presence: true
+ validate :address_available
+ validates :destination, presence: true, if: :enabled?
+ validates :destination, uniqueness: {scope: :address}
validate :address_local_email
validate :destination_email
@@ -50,7 +52,8 @@ class Identity < CouchRest::Model::Base
def self.find_for(user, attributes = {})
attributes.reverse_merge! attributes_from_user(user)
- find_by_address_and_destination [attributes[:address], attributes[:destination]]
+ id = find_by_address_and_destination attributes.values_at(:address, :destination)
+ return id if id && id.user == user
end
def self.build_for(user, attributes = {})
@@ -67,7 +70,9 @@ class Identity < CouchRest::Model::Base
def self.disable_all_for(user)
Identity.by_user_id.key(user.id).each do |identity|
identity.disable
- identity.save
+ # if the identity is not unique anymore because the destination
+ # was reset to nil we destroy it.
+ identity.save || identity.destroy
end
end
@@ -91,7 +96,11 @@ class Identity < CouchRest::Model::Base
end
def enabled?
- self.destination && self.user_id
+ self.user_id
+ end
+
+ def disabled?
+ !enabled?
end
def disable
@@ -120,34 +129,38 @@ class Identity < CouchRest::Model::Base
# for LoginFormatValidation
def login
- self.address.handle
+ address.handle if address.present?
end
protected
- def unique_forward
- same = Identity.find_by_address_and_destination([address, destination])
- if same && same != self
- errors.add :base, "This alias already exists"
+ def address_available
+ blocking_identities = Identity.by_address.key(address).all
+ blocking_identities.delete self
+ if self.user
+ blocking_identities.reject! { |other| other.user == self.user }
end
- end
-
- def alias_available
- same = Identity.find_by_address(address)
- if same && same.user != self.user
- errors.add :base, "This email has already been taken"
+ if blocking_identities.any?
+ errors.add :address, :taken
end
end
def address_local_email
- return if address.valid? #this ensures it is LocalEmail
- self.errors.add(:address, address.errors.messages[:email].first) #assumes only one error
+ # caught by presence validation
+ return if address.blank?
+ return if address.valid?
+ address.errors.each do |attribute, error|
+ self.errors.add(:address, error)
+ end
end
def destination_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
+ # caught by presence validation or this identity is disabled
+ return if destination.blank?
+ return if destination.valid?
+ destination.errors.each do |attribute, error|
+ self.errors.add(:destination, error)
+ end
end
end
diff --git a/app/models/local_email.rb b/app/models/local_email.rb
index 2b4c65e..ded7baf 100644
--- a/app/models/local_email.rb
+++ b/app/models/local_email.rb
@@ -58,11 +58,9 @@ class LocalEmail < Email
end
def handle_in_passwd?
- begin
- !!Etc.getpwnam(handle)
- rescue ArgumentError
- # handle was not found
- return false
- end
+ Etc.getpwnam(handle).present?
+ rescue ArgumentError
+ # handle was not found
+ return false
end
end
diff --git a/app/models/pgp_key.rb b/app/models/pgp_key.rb
index 66f8660..3384f4c 100644
--- a/app/models/pgp_key.rb
+++ b/app/models/pgp_key.rb
@@ -25,9 +25,10 @@ class PgpKey
# allow comparison with plain keyblock strings.
def ==(other)
+ return false if (self.present? != other.present?)
self.equal?(other) or
# relax the comparison on line ends.
- self.to_s.tr_s("\n\r", '') == other.tr_s("\r\n", '')
+ self.to_s.tr_s("\n\r", '') == other.tr_s("\n\r", '')
end
protected
diff --git a/app/models/user.rb b/app/models/user.rb
index 6678de6..f8b9ddc 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -24,7 +24,7 @@ class User < CouchRest::Model::Base
:uniqueness => true,
:if => :serverside?
- validate :login_is_unique_alias
+ validate :identity_is_valid
validates :password_salt, :password_verifier,
:format => { :with => /\A[\dA-Fa-f]+\z/, :message => "Only hex numbers allowed" }
@@ -42,6 +42,11 @@ class User < CouchRest::Model::Base
view :by_created_at
end # end of design
+ def reload
+ @identity = nil
+ super
+ end
+
def to_json(options={})
{
:login => login,
@@ -161,11 +166,10 @@ class User < CouchRest::Model::Base
# Validation Functions
##
- def login_is_unique_alias
- alias_identity = Identity.find_by_address(self.email_address)
- return if alias_identity.blank?
- if alias_identity.user != self
- errors.add(:login, "has already been taken")
+ def identity_is_valid
+ return if identity.valid?
+ identity.errors.each do |attribute, error|
+ self.errors.add(:login, error)
end
end
diff --git a/test/integration/api/smtp_cert_test.rb b/test/integration/api/smtp_cert_test.rb
index 04e6f31..f72362d 100644
--- a/test/integration/api/smtp_cert_test.rb
+++ b/test/integration/api/smtp_cert_test.rb
@@ -34,7 +34,7 @@ class SmtpCertTest < ApiIntegrationTest
cert = OpenSSL::X509::Certificate.new(get_response.body)
fingerprint = OpenSSL::Digest::SHA1.hexdigest(cert.to_der).scan(/../).join(':')
today = DateTime.now.to_date.to_s
- assert_equal({fingerprint => today}, @user.identity.cert_fingerprints)
+ assert_equal({fingerprint => today}, @user.reload.identity.cert_fingerprints)
end
test "fetching smtp certs requires email account" do
diff --git a/test/integration/browser/account_test.rb b/test/integration/browser/account_test.rb
index 491a9e1..8e6d433 100644
--- a/test/integration/browser/account_test.rb
+++ b/test/integration/browser/account_test.rb
@@ -22,6 +22,12 @@ class AccountTest < BrowserIntegrationTest
assert page.has_content?("Welcome #{username}")
end
+ test "signup with reserved username" do
+ username = 'certmaster'
+ submit_signup username
+ assert page.has_content?("is reserved.")
+ end
+
test "successful login" do
username, password = submit_signup
click_on 'Logout'
@@ -44,6 +50,7 @@ class AccountTest < BrowserIntegrationTest
click_on I18n.t('account_settings')
click_on I18n.t('destroy_my_account')
assert page.has_content?(I18n.t('account_destroyed'))
+ assert_equal 1, Identity.by_address.key("#{username}@test.me").count
attempt_login(username, password)
assert_invalid_login(page)
end
@@ -102,7 +109,8 @@ class AccountTest < BrowserIntegrationTest
# at some point we're done:
page.assert_no_selector 'input[value="Saving..."]'
assert page.has_field? 'Public key', with: pgp_key.to_s
- assert_equal pgp_key, @user.reload.public_key
+ @user.reload
+ assert_equal pgp_key, @user.public_key
end
end
diff --git a/test/unit/identity_test.rb b/test/unit/identity_test.rb
index eca104f..49b2075 100644
--- a/test/unit/identity_test.rb
+++ b/test/unit/identity_test.rb
@@ -7,6 +7,22 @@ class IdentityTest < ActiveSupport::TestCase
@user = find_record :user
end
+ test "blank identity does not crash on valid?" do
+ id = Identity.new
+ assert !id.valid?
+ end
+
+ test "enabled identity requires destination" do
+ id = Identity.new user: @user, address: @user.email_address
+ assert !id.valid?
+ assert_equal ["can't be blank"], id.errors[:destination]
+ end
+
+ test "disabled identity requires no destination" do
+ id = Identity.new address: @user.email_address
+ assert id.valid?
+ end
+
test "initial identity for a user" do
id = Identity.for(@user)
assert_equal @user.email_address, id.address
@@ -39,7 +55,7 @@ class IdentityTest < ActiveSupport::TestCase
id = Identity.create_for @user, address: alias_name, destination: forward_address
dup = Identity.build_for @user, address: alias_name, destination: forward_address
assert !dup.valid?
- assert_equal ["This alias already exists"], dup.errors[:base]
+ assert_equal ["has already been taken"], dup.errors[:destination]
id.destroy
end
@@ -48,7 +64,7 @@ class IdentityTest < ActiveSupport::TestCase
id = Identity.create_for @user, address: alias_name, destination: forward_address
taken = Identity.build_for other_user, address: alias_name
assert !taken.valid?
- assert_equal ["This email has already been taken"], taken.errors[:base]
+ assert_equal ["has already been taken"], taken.errors[:address]
id.destroy
end
@@ -107,6 +123,7 @@ class IdentityTest < ActiveSupport::TestCase
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]
Identity.destroy_all_disabled
end
generated by cgit v1.2.3 (git 2.25.1) at 2024-11-04 21:06:07 +0000