From 38e7ba3a7c7a414c5b087f7f32df1a09403fff89 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Tue, 16 Jul 2013 19:38:37 +0200
Subject: first take on identity model - still broken

---
 users/app/models/identity.rb     | 20 ++++++++++++++++++++
 users/test/unit/identity_test.rb | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+)
 create mode 100644 users/app/models/identity.rb
 create mode 100644 users/test/unit/identity_test.rb

diff --git a/users/app/models/identity.rb b/users/app/models/identity.rb
new file mode 100644
index 0000000..a081394
--- /dev/null
+++ b/users/app/models/identity.rb
@@ -0,0 +1,20 @@
+class Identity < CouchRest::Model::Base
+
+  use_database :identities
+
+  belongs_to :user
+
+  property :address
+  property :destination
+
+  def initialize(attribs = {}, &block):q
+    attribs.reverse_merge! user_id: user.id,
+      address: user.main_email_address,
+      destination: user.main_email_address
+    Identity.new attribs
+  end
+
+  design do
+    view :by_user_id
+  end
+end
diff --git a/users/test/unit/identity_test.rb b/users/test/unit/identity_test.rb
new file mode 100644
index 0000000..389ef89
--- /dev/null
+++ b/users/test/unit/identity_test.rb
@@ -0,0 +1,34 @@
+require 'test_helper'
+
+class IdentityTest < ActiveSupport::TestCase
+
+  setup do
+    @user = FactoryGirl.create(:user)
+  end
+
+  test "user has identity to start with" do
+    id = Identity.new user_id: @user.id
+    id.save
+    assert_equal 1, Identity.by_user_id.key(@user.id).count
+    identity = Identity.find_by_user_id(@user.id)
+    assert_equal @user.email_address, identity.address
+    assert_equal @user.email_address, identity.destination
+    assert_equal @user, identity.user
+  end
+
+  test "add alias" do
+    skip
+    @user.create_identity address: @alias
+  end
+
+  test "add forward" do
+    skip
+    @user.create_identity destination: @external
+  end
+
+  test "forward alias" do
+    skip
+    @user.create_identity address: @alias, destination: @external
+  end
+
+end
-- 
cgit v1.2.3


From 4a071ef1b33525fa2d1052aa7f21b549447fe767 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Thu, 18 Jul 2013 11:31:25 +0200
Subject: move identity creation into user class

It's always based on a user and most default values are based on user properties.
---
 users/app/models/identity.rb     |  7 -------
 users/app/models/user.rb         | 13 +++++++++++++
 users/test/unit/identity_test.rb | 11 ++++-------
 3 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/users/app/models/identity.rb b/users/app/models/identity.rb
index a081394..9fd0cad 100644
--- a/users/app/models/identity.rb
+++ b/users/app/models/identity.rb
@@ -7,13 +7,6 @@ class Identity < CouchRest::Model::Base
   property :address
   property :destination
 
-  def initialize(attribs = {}, &block):q
-    attribs.reverse_merge! user_id: user.id,
-      address: user.main_email_address,
-      destination: user.main_email_address
-    Identity.new attribs
-  end
-
   design do
     view :by_user_id
   end
diff --git a/users/app/models/user.rb b/users/app/models/user.rb
index 413b4ac..dda5a41 100644
--- a/users/app/models/user.rb
+++ b/users/app/models/user.rb
@@ -73,6 +73,19 @@ class User < CouchRest::Model::Base
     alias_method :find_by_param, :find
   end
 
+  def create_identity(attribs = {}, &block)
+    build_identity(attribs, &block)
+    identity.save
+    identity
+  end
+
+  def build_identity(attribs = {}, &block)
+    attribs.reverse_merge! user_id: self.id,
+      address: self.email_address,
+      destination: self.email_address
+    Identity.new(attribs, &block)
+  end
+
   def to_param
     self.id
   end
diff --git a/users/test/unit/identity_test.rb b/users/test/unit/identity_test.rb
index 389ef89..3130ddc 100644
--- a/users/test/unit/identity_test.rb
+++ b/users/test/unit/identity_test.rb
@@ -7,13 +7,10 @@ class IdentityTest < ActiveSupport::TestCase
   end
 
   test "user has identity to start with" do
-    id = Identity.new user_id: @user.id
-    id.save
-    assert_equal 1, Identity.by_user_id.key(@user.id).count
-    identity = Identity.find_by_user_id(@user.id)
-    assert_equal @user.email_address, identity.address
-    assert_equal @user.email_address, identity.destination
-    assert_equal @user, identity.user
+    id = @user.build_identity
+    assert_equal @user.email_address, id.address
+    assert_equal @user.email_address, id.destination
+    assert_equal @user, id.user
   end
 
   test "add alias" do
-- 
cgit v1.2.3


From cc96c60c4617c09379d5e1ddbefa42407329c19a Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Thu, 18 Jul 2013 12:12:07 +0200
Subject: testing all versions of emial identities, emails are now strings

---
 users/app/models/email.rb        | 21 ++++-----------------
 users/app/models/identity.rb     |  4 ++--
 users/app/models/local_email.rb  | 20 +++-----------------
 users/test/unit/identity_test.rb | 32 +++++++++++++++++++++++++-------
 4 files changed, 34 insertions(+), 43 deletions(-)

diff --git a/users/app/models/email.rb b/users/app/models/email.rb
index 6d82f2a..90fc645 100644
--- a/users/app/models/email.rb
+++ b/users/app/models/email.rb
@@ -1,6 +1,5 @@
-module Email
-  extend ActiveSupport::Concern
-
+class Email < String
+=begin
   included do
     validates :email,
       :format => {
@@ -8,26 +7,14 @@ module Email
         :message => "needs to be a valid email address"
       }
   end
-
-  def initialize(attributes = nil, &block)
-    attributes = {:email => attributes} if attributes.is_a? String
-    super(attributes, &block)
-  end
-
-  def to_s
-    email
-  end
-
-  def ==(other)
-    other.is_a?(Email) ? self.email == other.email : self.email == other
-  end
+=end
 
   def to_partial_path
     "emails/email"
   end
 
   def to_param
-    email
+    to_s
   end
 
 end
diff --git a/users/app/models/identity.rb b/users/app/models/identity.rb
index 9fd0cad..c9c8b73 100644
--- a/users/app/models/identity.rb
+++ b/users/app/models/identity.rb
@@ -4,8 +4,8 @@ class Identity < CouchRest::Model::Base
 
   belongs_to :user
 
-  property :address
-  property :destination
+  property :address, LocalEmail
+  property :destination, Email
 
   design do
     view :by_user_id
diff --git a/users/app/models/local_email.rb b/users/app/models/local_email.rb
index 69cba01..1cadc71 100644
--- a/users/app/models/local_email.rb
+++ b/users/app/models/local_email.rb
@@ -1,25 +1,11 @@
-class LocalEmail
-  include CouchRest::Model::Embeddable
-  include Email
-
-  property :username, String
-
-  before_validation :strip_domain_if_needed
-
-  validates :username,
-    :presence => true,
-    :format => { :with => /\A([^@\s]+)(@#{APP_CONFIG[:domain]})?\Z/i, :message => "needs to be a valid login or email address @#{APP_CONFIG[:domain]}"}
+class LocalEmail < Email
 
+=begin
   validate :unique_on_server
   validate :unique_alias_for_user
   validate :differs_from_login
+=end
 
-  validates :casted_by, :presence => true
-
-  def email
-    return '' if username.nil?
-    username + '@' + APP_CONFIG[:domain]
-  end
 
   def email=(value)
     return if value.blank?
diff --git a/users/test/unit/identity_test.rb b/users/test/unit/identity_test.rb
index 3130ddc..a5d30b0 100644
--- a/users/test/unit/identity_test.rb
+++ b/users/test/unit/identity_test.rb
@@ -6,7 +6,11 @@ class IdentityTest < ActiveSupport::TestCase
     @user = FactoryGirl.create(:user)
   end
 
-  test "user has identity to start with" do
+  teardown do
+    @user.destroy
+  end
+
+  test "initial identity for a user" do
     id = @user.build_identity
     assert_equal @user.email_address, id.address
     assert_equal @user.email_address, id.destination
@@ -14,18 +18,32 @@ class IdentityTest < ActiveSupport::TestCase
   end
 
   test "add alias" do
-    skip
-    @user.create_identity address: @alias
+    id = @user.build_identity address: alias_name
+    assert_equal LocalEmail.new(alias_name), id.address
+    assert_equal @user.email_address, id.destination
+    assert_equal @user, id.user
   end
 
   test "add forward" do
-    skip
-    @user.create_identity destination: @external
+    id = @user.build_identity destination: forward_address
+    assert_equal @user.email_address, id.address
+    assert_equal Email.new(forward_address), id.destination
+    assert_equal @user, id.user
   end
 
   test "forward alias" do
-    skip
-    @user.create_identity address: @alias, destination: @external
+    id = @user.build_identity address: alias_name, destination: forward_address
+    assert_equal LocalEmail.new(alias_name), id.address
+    assert_equal Email.new(forward_address), id.destination
+    assert_equal @user, id.user
+    id.save
   end
 
+  def alias_name
+    @alias_name ||= Faker::Internet.user_name
+  end
+
+  def forward_address
+    @forward_address ||= Faker::Internet.email
+  end
 end
-- 
cgit v1.2.3


From a2e49d1b946fa34dd41ce1f07920515df13e09db Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Thu, 18 Jul 2013 12:28:51 +0200
Subject: local email adds domain if needed

---
 users/app/models/local_email.rb     | 20 ++++++++++++--------
 users/test/unit/local_email_test.rb | 27 +++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 8 deletions(-)
 create mode 100644 users/test/unit/local_email_test.rb

diff --git a/users/app/models/local_email.rb b/users/app/models/local_email.rb
index 1cadc71..d919102 100644
--- a/users/app/models/local_email.rb
+++ b/users/app/models/local_email.rb
@@ -6,15 +6,17 @@ class LocalEmail < Email
   validate :differs_from_login
 =end
 
-
-  def email=(value)
-    return if value.blank?
-    self.username = value
-    strip_domain_if_needed
+  def initialize(s)
+    super
+    append_domain_if_needed
   end
 
   def to_key
-    [username]
+    [handle]
+  end
+
+  def handle
+    gsub(/@#{APP_CONFIG[:domain]}/i, '')
   end
 
   protected
@@ -42,8 +44,10 @@ class LocalEmail < Email
     end
   end
 
-  def strip_domain_if_needed
-    self.username.gsub! /@#{APP_CONFIG[:domain]}/i, ''
+  def append_domain_if_needed
+    unless self.index('@')
+      self << "@#{APP_CONFIG[:domain]}"
+    end
   end
 
 end
diff --git a/users/test/unit/local_email_test.rb b/users/test/unit/local_email_test.rb
new file mode 100644
index 0000000..9031a98
--- /dev/null
+++ b/users/test/unit/local_email_test.rb
@@ -0,0 +1,27 @@
+require 'test_helper'
+
+class LocalEmailTest < ActiveSupport::TestCase
+
+  test "appends domain" do
+    local = LocalEmail.new(handle)
+    assert_equal LocalEmail.new(email), local
+  end
+
+  test "returns handle" do
+    local = LocalEmail.new(email)
+    assert_equal handle, local.handle
+  end
+
+  test "prints full email" do
+    local = LocalEmail.new(handle)
+    assert_equal email, "#{local}"
+  end
+
+  def handle
+    "asdf"
+  end
+
+  def email
+    "asdf@" + APP_CONFIG[:domain]
+  end
+end
-- 
cgit v1.2.3


From d96fac2de074bbe3a44d888af5ceaff45b1b9b27 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Thu, 18 Jul 2013 12:52:02 +0200
Subject: validations of email format and local domain moved over

---
 test/unit/email_test.rb             | 18 ++++++++++++++++++
 users/app/models/email.rb           | 20 +++++++++++---------
 users/app/models/local_email.rb     | 20 ++++++++++++++++++--
 users/test/unit/email_test.rb       | 19 +++++++++++++++++++
 users/test/unit/local_email_test.rb | 11 +++++++++--
 5 files changed, 75 insertions(+), 13 deletions(-)
 create mode 100644 test/unit/email_test.rb
 create mode 100644 users/test/unit/email_test.rb

diff --git a/test/unit/email_test.rb b/test/unit/email_test.rb
new file mode 100644
index 0000000..e858bd5
--- /dev/null
+++ b/test/unit/email_test.rb
@@ -0,0 +1,18 @@
+require 'test_helper'
+
+class EmailTest < ActiveSupport::TestCase
+
+  test "valid format" do
+    email = Email.new(email_string)
+    assert email.valid?
+  end
+
+  test "validates format" do
+    email = Email.new("email")
+    assert !email.valid?
+  end
+
+  def email_string
+    @email_string ||= Faker::Internet.email
+  end
+end
diff --git a/users/app/models/email.rb b/users/app/models/email.rb
index 90fc645..1bcff1c 100644
--- a/users/app/models/email.rb
+++ b/users/app/models/email.rb
@@ -1,13 +1,11 @@
 class Email < String
-=begin
-  included do
-    validates :email,
-      :format => {
-        :with => /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\Z/,
-        :message => "needs to be a valid email address"
-      }
-  end
-=end
+  include ActiveModel::Validations
+
+  validates :email,
+    :format => {
+      :with => /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\Z/,
+      :message => "needs to be a valid email address"
+    }
 
   def to_partial_path
     "emails/email"
@@ -17,4 +15,8 @@ class Email < String
     to_s
   end
 
+  def email
+    self
+  end
+
 end
diff --git a/users/app/models/local_email.rb b/users/app/models/local_email.rb
index d919102..e71d494 100644
--- a/users/app/models/local_email.rb
+++ b/users/app/models/local_email.rb
@@ -6,6 +6,18 @@ class LocalEmail < Email
   validate :differs_from_login
 =end
 
+  def self.domain
+    APP_CONFIG[:domain]
+  end
+
+  validates :email,
+    :format => {
+      :with => /@#{domain}\Z/i,
+      :message => "needs to end in @#{domain}"
+    }
+
+
+
   def initialize(s)
     super
     append_domain_if_needed
@@ -16,7 +28,11 @@ class LocalEmail < Email
   end
 
   def handle
-    gsub(/@#{APP_CONFIG[:domain]}/i, '')
+    gsub(/@#{domain}/i, '')
+  end
+
+  def domain
+    LocalEmail.domain
   end
 
   protected
@@ -46,7 +62,7 @@ class LocalEmail < Email
 
   def append_domain_if_needed
     unless self.index('@')
-      self << "@#{APP_CONFIG[:domain]}"
+      self << '@' + domain
     end
   end
 
diff --git a/users/test/unit/email_test.rb b/users/test/unit/email_test.rb
new file mode 100644
index 0000000..7cfbc84
--- /dev/null
+++ b/users/test/unit/email_test.rb
@@ -0,0 +1,19 @@
+require 'test_helper'
+
+class EmailTest < ActiveSupport::TestCase
+
+  test "valid format" do
+    email = Email.new(email_string)
+    assert email.valid?
+  end
+
+  test "validates format" do
+    email = Email.new("email")
+    assert !email.valid?
+    assert_equal ["needs to be a valid email address"], email.errors[:email]
+  end
+
+  def email_string
+    @email_string ||= Faker::Internet.email
+  end
+end
diff --git a/users/test/unit/local_email_test.rb b/users/test/unit/local_email_test.rb
index 9031a98..b25f46f 100644
--- a/users/test/unit/local_email_test.rb
+++ b/users/test/unit/local_email_test.rb
@@ -5,6 +5,7 @@ class LocalEmailTest < ActiveSupport::TestCase
   test "appends domain" do
     local = LocalEmail.new(handle)
     assert_equal LocalEmail.new(email), local
+    assert local.valid?
   end
 
   test "returns handle" do
@@ -17,11 +18,17 @@ class LocalEmailTest < ActiveSupport::TestCase
     assert_equal email, "#{local}"
   end
 
+  test "validates domain" do
+    local = LocalEmail.new(Faker::Internet.email)
+    assert !local.valid?
+    assert_equal ["needs to end in @#{LocalEmail.domain}"], local.errors[:email]
+  end
+
   def handle
-    "asdf"
+    @handle ||= Faker::Internet.user_name
   end
 
   def email
-    "asdf@" + APP_CONFIG[:domain]
+    handle + "@" + APP_CONFIG[:domain]
   end
 end
-- 
cgit v1.2.3


From 0acace58c31c96fc1f8836167aeb4f204f72617f Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Thu, 18 Jul 2013 17:17:36 +0200
Subject: allow available and unique forwards only

---
 users/app/models/identity.rb     | 22 ++++++++++++++++++++++
 users/app/models/local_email.rb  | 30 ------------------------------
 users/app/models/user.rb         |  8 +++-----
 users/test/unit/identity_test.rb | 18 ++++++++++++++++++
 4 files changed, 43 insertions(+), 35 deletions(-)

diff --git a/users/app/models/identity.rb b/users/app/models/identity.rb
index c9c8b73..4dff93a 100644
--- a/users/app/models/identity.rb
+++ b/users/app/models/identity.rb
@@ -7,7 +7,29 @@ class Identity < CouchRest::Model::Base
   property :address, LocalEmail
   property :destination, Email
 
+  validate :unique_forward
+  validate :alias_available
+
   design do
     view :by_user_id
+    view :by_address_and_destination
+    view :by_address
+  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"
+    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"
+    end
+  end
+
 end
diff --git a/users/app/models/local_email.rb b/users/app/models/local_email.rb
index e71d494..c1f7c11 100644
--- a/users/app/models/local_email.rb
+++ b/users/app/models/local_email.rb
@@ -1,10 +1,5 @@
 class LocalEmail < Email
 
-=begin
-  validate :unique_on_server
-  validate :unique_alias_for_user
-  validate :differs_from_login
-=end
 
   def self.domain
     APP_CONFIG[:domain]
@@ -16,8 +11,6 @@ class LocalEmail < Email
       :message => "needs to end in @#{domain}"
     }
 
-
-
   def initialize(s)
     super
     append_domain_if_needed
@@ -37,29 +30,6 @@ class LocalEmail < Email
 
   protected
 
-  def unique_on_server
-    has_email = User.find_by_login_or_alias(username)
-    if has_email && has_email != self.casted_by
-      errors.add :username, "has already been taken"
-    end
-  end
-
-  def unique_alias_for_user
-    aliases = self.casted_by.email_aliases
-    if aliases.select{|a|a.username == self.username}.count > 1
-      errors.add :username, "is already your alias"
-    end
-  end
-
-  def differs_from_login
-    # If this has not changed but the email let's mark the email invalid instead.
-    return if self.persisted?
-    user = self.casted_by
-    if user.login == self.username
-      errors.add :username, "may not be the same as your email address"
-    end
-  end
-
   def append_domain_if_needed
     unless self.index('@')
       self << '@' + domain
diff --git a/users/app/models/user.rb b/users/app/models/user.rb
index dda5a41..5707f24 100644
--- a/users/app/models/user.rb
+++ b/users/app/models/user.rb
@@ -74,7 +74,7 @@ class User < CouchRest::Model::Base
   end
 
   def create_identity(attribs = {}, &block)
-    build_identity(attribs, &block)
+    identity = build_identity(attribs, &block)
     identity.save
     identity
   end
@@ -135,12 +135,10 @@ class User < CouchRest::Model::Base
   ##
 
   def login_is_unique_alias
-    has_alias = User.find_by_login_or_alias(username)
+    alias_identity = Identity.find_by_address(self.email_address)
     return if has_alias.nil?
-    if has_alias != self
+    if alias_identity.user != self
       errors.add(:login, "has already been taken")
-    elsif has_alias.login != self.login
-      errors.add(:login, "may not be the same as one of your aliases")
     end
   end
 
diff --git a/users/test/unit/identity_test.rb b/users/test/unit/identity_test.rb
index a5d30b0..4ebd72e 100644
--- a/users/test/unit/identity_test.rb
+++ b/users/test/unit/identity_test.rb
@@ -39,6 +39,24 @@ class IdentityTest < ActiveSupport::TestCase
     id.save
   end
 
+  test "prevents duplicates" do
+    id = @user.create_identity address: alias_name, destination: forward_address
+    dup = @user.build_identity address: alias_name, destination: forward_address
+    assert !dup.valid?
+    assert_equal ["This alias already exists"], dup.errors[:base]
+  end
+
+  test "validates availability" do
+    other_user = FactoryGirl.create(:user)
+    id = @user.create_identity address: alias_name, destination: forward_address
+    taken = other_user.build_identity address: alias_name
+    assert !taken.valid?
+    assert_equal ["This email has already been taken"], taken.errors[:base]
+    other_user.destroy
+  end
+
+
+
   def alias_name
     @alias_name ||= Faker::Internet.user_name
   end
-- 
cgit v1.2.3


From 495c97fbf400ed44a1e64692f2c04d2155a326e7 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Thu, 18 Jul 2013 18:09:12 +0200
Subject: remove the remainders of email aliases and forward from user

---
 users/app/models/user.rb | 28 +---------------------------
 1 file changed, 1 insertion(+), 27 deletions(-)

diff --git a/users/app/models/user.rb b/users/app/models/user.rb
index 5707f24..62fb7ec 100644
--- a/users/app/models/user.rb
+++ b/users/app/models/user.rb
@@ -6,9 +6,6 @@ class User < CouchRest::Model::Base
   property :password_verifier, String, :accessible => true
   property :password_salt, String, :accessible => true
 
-  property :email_forward, String, :accessible => true
-  property :email_aliases, [LocalEmail]
-
   property :public_key, :accessible => true
 
   property :enabled, TrueClass, :default => true
@@ -43,10 +40,6 @@ class User < CouchRest::Model::Base
     :confirmation => true,
     :format => { :with => /.{8}.*/, :message => "needs to be at least 8 characters long" }
 
-  validates :email_forward,
-    :allow_blank => true,
-    :format => { :with => /\A(([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,}))?\Z/, :message => "needs to be a valid email address"}
-
   timestamps!
 
   design do
@@ -54,19 +47,6 @@ class User < CouchRest::Model::Base
     load_views(own_path.join('..', 'designs', 'user'))
     view :by_login
     view :by_created_at
-    view :pgp_key_by_handle,
-      map: <<-EOJS
-      function(doc) {
-        if (doc.type != 'User') {
-          return;
-        }
-        emit(doc.login, doc.public_key);
-        doc.email_aliases.forEach(function(alias){
-          emit(alias.username, doc.public_key);
-        });
-      }
-    EOJS
-
   end # end of design
 
   class << self
@@ -118,12 +98,6 @@ class User < CouchRest::Model::Base
     APP_CONFIG['admins'].include? self.login
   end
 
-  # this currently only adds the first email address submitted.
-  # All the ui needs for now.
-  def email_aliases_attributes=(attrs)
-    email_aliases.build(attrs.values.first) if attrs
-  end
-
   def most_recent_tickets(count=3)
     Ticket.for_user(self).limit(count).all #defaults to having most recent updated first
   end
@@ -136,7 +110,7 @@ class User < CouchRest::Model::Base
 
   def login_is_unique_alias
     alias_identity = Identity.find_by_address(self.email_address)
-    return if has_alias.nil?
+    return if alias_identity.blank?
     if alias_identity.user != self
       errors.add(:login, "has already been taken")
     end
-- 
cgit v1.2.3


From dcaf6232dbd1131cdbd12ac5c2ef997979fd01ff Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Fri, 19 Jul 2013 09:50:02 +0200
Subject: add keys to identity

---
 users/app/models/identity.rb     | 11 +++++++++++
 users/app/models/user.rb         |  3 ++-
 users/test/unit/identity_test.rb | 19 +++++++++++++++++++
 3 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/users/app/models/identity.rb b/users/app/models/identity.rb
index 4dff93a..b862590 100644
--- a/users/app/models/identity.rb
+++ b/users/app/models/identity.rb
@@ -6,6 +6,7 @@ class Identity < CouchRest::Model::Base
 
   property :address, LocalEmail
   property :destination, Email
+  property :keys, Hash
 
   validate :unique_forward
   validate :alias_available
@@ -14,6 +15,16 @@ class Identity < CouchRest::Model::Base
     view :by_user_id
     view :by_address_and_destination
     view :by_address
+    view :pgp_key_by_email,
+      map: <<-EOJS
+      function(doc) {
+        if (doc.type != 'Identity') {
+          return;
+        }
+        emit(doc.address, doc.keys["pgp"]);
+      }
+    EOJS
+
   end
 
   protected
diff --git a/users/app/models/user.rb b/users/app/models/user.rb
index 62fb7ec..fb7f0aa 100644
--- a/users/app/models/user.rb
+++ b/users/app/models/user.rb
@@ -62,7 +62,8 @@ class User < CouchRest::Model::Base
   def build_identity(attribs = {}, &block)
     attribs.reverse_merge! user_id: self.id,
       address: self.email_address,
-      destination: self.email_address
+      destination: self.email_address,
+      keys: {}
     Identity.new(attribs, &block)
   end
 
diff --git a/users/test/unit/identity_test.rb b/users/test/unit/identity_test.rb
index 4ebd72e..6b0a6b1 100644
--- a/users/test/unit/identity_test.rb
+++ b/users/test/unit/identity_test.rb
@@ -55,7 +55,22 @@ class IdentityTest < ActiveSupport::TestCase
     other_user.destroy
   end
 
+  test "setting and getting pgp key" do
+    id = @user.build_identity
+    id.keys[:pgp] = pgp_key_string
+    assert_equal pgp_key_string, id.keys[:pgp]
+  end
 
+  test "querying pgp key via couch" do
+    id = @user.build_identity
+    id.keys[:pgp] = pgp_key_string
+    id.save
+    view = Identity.pgp_key_by_email.key(id.address)
+    assert_equal 1, view.rows.count
+    assert result = view.rows.first
+    assert_equal id.address, result["key"]
+    assert_equal id.keys[:pgp], result["value"]
+  end
 
   def alias_name
     @alias_name ||= Faker::Internet.user_name
@@ -64,4 +79,8 @@ class IdentityTest < ActiveSupport::TestCase
   def forward_address
     @forward_address ||= Faker::Internet.email
   end
+
+  def pgp_key_string
+    @pgp_key ||= "DUMMY PGP KEY ... "+SecureRandom.base64(4096)
+  end
 end
-- 
cgit v1.2.3


From b6242bbefc1e9fe193bbf3479e8fa822829c6d1a Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Fri, 19 Jul 2013 11:07:53 +0200
Subject: remove email aliases test - we'll move them to identities

---
 users/test/unit/email_aliases_test.rb | 66 -----------------------------------
 1 file changed, 66 deletions(-)
 delete mode 100644 users/test/unit/email_aliases_test.rb

diff --git a/users/test/unit/email_aliases_test.rb b/users/test/unit/email_aliases_test.rb
deleted file mode 100644
index 86d14aa..0000000
--- a/users/test/unit/email_aliases_test.rb
+++ /dev/null
@@ -1,66 +0,0 @@
-require 'test_helper'
-
-class EmailAliasTest < ActiveSupport::TestCase
-
-  setup do
-    @user = FactoryGirl.build :user
-    @alias = "valid_alias"
-    # make sure no existing records are in the way...
-    User.find_by_login_or_alias(@alias).try(:destroy)
-  end
-
-  test "no email aliases set in the beginning" do
-    assert_equal [], @user.email_aliases
-  end
-
-  test "adding email alias through params" do
-    @user.attributes = {:email_aliases_attributes => {"0" => {:email => @alias}}}
-    assert @user.changed?
-    assert @user.save
-    assert_equal @alias, @user.email_aliases.first.username
-  end
-
-  test "adding email alias directly" do
-    @user.email_aliases.build :email => @alias
-    assert @user.save
-    assert_equal @alias, @user.email_aliases.first.username
-  end
-
-  test "duplicated email aliases are invalid" do
-    @user.email_aliases.build :email => @alias
-    @user.save
-    assert_invalid_alias @alias
-  end
-
-  test "email alias needs to be different from other peoples login" do
-    other_user = FactoryGirl.create :user, :login => @alias
-    assert_invalid_alias @alias
-    other_user.destroy
-  end
-
-  test "email needs to be different from other peoples email aliases" do
-    other_user = FactoryGirl.create :user, :email_aliases_attributes => {'1' => @alias}
-    assert_invalid_alias @alias
-    other_user.destroy
-  end
-
-  test "login is invalid as email alias" do
-    @user.login = @alias
-    assert_invalid_alias @alias
-  end
-
-  test "find user by email alias" do
-    @user.email_aliases.build :email => @alias
-    assert @user.save
-    assert_equal @user, User.find_by_login_or_alias(@alias)
-    assert_equal @user, User.find_by_alias(@alias)
-    assert_nil User.find_by_login(@alias)
-  end
-
-  def assert_invalid_alias(string)
-    email_alias = @user.email_aliases.build :email => string
-    assert !email_alias.valid?
-    assert !@user.valid?
-  end
-
-end
-- 
cgit v1.2.3


From 8ddbaa6184e4dbcc6ef7e81cf555cc18d3822dce Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Fri, 19 Jul 2013 11:09:25 +0200
Subject: support deprecated API to set users main identity pgp key

We'll want to get rid of the #public_key and #public_key= functions but they are still used from the users controller. We'll probably have an identity controller instead at some point.
---
 users/app/models/user.rb         | 44 +++++++++++++++++++++++++++++++++++-----
 users/test/unit/identity_test.rb |  6 +++---
 users/test/unit/user_test.rb     | 14 ++++++-------
 3 files changed, 48 insertions(+), 16 deletions(-)

diff --git a/users/app/models/user.rb b/users/app/models/user.rb
index fb7f0aa..546d571 100644
--- a/users/app/models/user.rb
+++ b/users/app/models/user.rb
@@ -6,8 +6,6 @@ class User < CouchRest::Model::Base
   property :password_verifier, String, :accessible => true
   property :password_salt, String, :accessible => true
 
-  property :public_key, :accessible => true
-
   property :enabled, TrueClass, :default => true
 
   validates :login, :password_salt, :password_verifier,
@@ -49,14 +47,50 @@ class User < CouchRest::Model::Base
     view :by_created_at
   end # end of design
 
+  # We proxy access to the pgp_key. So we need to make sure
+  # the updated identity actually gets saved.
+  def save(*args)
+    super
+    identity.user_id ||= self.id
+    identity.save if identity.changed?
+  end
+
+  # So far this only works for creating a new user.
+  # TODO: Create an alias for the old login when changing the login
+  def login=(value)
+    write_attribute 'login', value
+    @identity = build_identity
+  end
+
+  # DEPRECATED
+  #
+  # Please access identity.keys[:pgp] directly
+  def public_key=(value)
+    identity.keys[:pgp] = value
+  end
+
+  # DEPRECATED
+  #
+  # Please access identity.keys[:pgp] directly
+  def public_key
+    identity.keys[:pgp]
+  end
+
   class << self
     alias_method :find_by_param, :find
   end
 
+  # this is the main identity. login@domain.tld
+  # aliases and forwards are represented in other identities.
+  def identity
+    @identity ||=
+      Identity.find_by_address_and_destination([email_address, email_address])
+  end
+
   def create_identity(attribs = {}, &block)
-    identity = build_identity(attribs, &block)
-    identity.save
-    identity
+    new_identity = build_identity(attribs, &block)
+    new_identity.save
+    new_identity
   end
 
   def build_identity(attribs = {}, &block)
diff --git a/users/test/unit/identity_test.rb b/users/test/unit/identity_test.rb
index 6b0a6b1..d20ad93 100644
--- a/users/test/unit/identity_test.rb
+++ b/users/test/unit/identity_test.rb
@@ -11,7 +11,7 @@ class IdentityTest < ActiveSupport::TestCase
   end
 
   test "initial identity for a user" do
-    id = @user.build_identity
+    id = @user.identity
     assert_equal @user.email_address, id.address
     assert_equal @user.email_address, id.destination
     assert_equal @user, id.user
@@ -56,13 +56,13 @@ class IdentityTest < ActiveSupport::TestCase
   end
 
   test "setting and getting pgp key" do
-    id = @user.build_identity
+    id = @user.identity
     id.keys[:pgp] = pgp_key_string
     assert_equal pgp_key_string, id.keys[:pgp]
   end
 
   test "querying pgp key via couch" do
-    id = @user.build_identity
+    id = @user.identity
     id.keys[:pgp] = pgp_key_string
     id.save
     view = Identity.pgp_key_by_email.key(id.address)
diff --git a/users/test/unit/user_test.rb b/users/test/unit/user_test.rb
index c8c837b..5d1fe06 100644
--- a/users/test/unit/user_test.rb
+++ b/users/test/unit/user_test.rb
@@ -56,23 +56,21 @@ class UserTest < ActiveSupport::TestCase
     other_user.destroy
   end
 
-  test "login needs to be different from other peoples email aliases" do
-    other_user = FactoryGirl.create :user
-    other_user.email_aliases.build :email => @user.login
-    other_user.save
-    assert !@user.valid?
-    other_user.destroy
+  test "deprecated public key api still works" do
+    key = SecureRandom.base64(4096)
+    @user.public_key = key
+    assert_equal key, @user.public_key
   end
 
   test "pgp key view" do
     @user.public_key = SecureRandom.base64(4096)
     @user.save
 
-    view = User.pgp_key_by_handle.key(@user.login)
+    view = Identity.pgp_key_by_email.key(@user.email_address)
 
     assert_equal 1, view.rows.count
     assert result = view.rows.first
-    assert_equal @user.login, result["key"]
+    assert_equal @user.email_address, result["key"]
     assert_equal @user.public_key, result["value"]
   end
 end
-- 
cgit v1.2.3


From 34c06b49f26bbffe2e29477f0b55a56a38aa0ce7 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Fri, 19 Jul 2013 11:14:43 +0200
Subject: no need for a remote email class

---
 users/app/models/remote_email.rb | 14 --------------
 1 file changed, 14 deletions(-)
 delete mode 100644 users/app/models/remote_email.rb

diff --git a/users/app/models/remote_email.rb b/users/app/models/remote_email.rb
deleted file mode 100644
index 4fe7425..0000000
--- a/users/app/models/remote_email.rb
+++ /dev/null
@@ -1,14 +0,0 @@
-class RemoteEmail
-  include CouchRest::Model::Embeddable
-  include Email
-
-  property :email, String
-
-  def username
-    email.spilt('@').first
-  end
-
-  def domain
-    email.split('@').last
-  end
-end
-- 
cgit v1.2.3


From 51582a668b04d2c1322ad1babe8599ae8797cd3b Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Fri, 19 Jul 2013 11:20:01 +0200
Subject: test user validates uniqueness of login amongst aliases

---
 users/test/unit/user_test.rb | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/users/test/unit/user_test.rb b/users/test/unit/user_test.rb
index 5d1fe06..f303c8d 100644
--- a/users/test/unit/user_test.rb
+++ b/users/test/unit/user_test.rb
@@ -56,6 +56,13 @@ class UserTest < ActiveSupport::TestCase
     other_user.destroy
   end
 
+  test "login needs to be unique amongst aliases" do
+    other_user = FactoryGirl.create :user
+    other_user.create_identity address: @user.login
+    assert !@user.valid?
+    other_user.destroy
+  end
+
   test "deprecated public key api still works" do
     key = SecureRandom.base64(4096)
     @user.public_key = key
-- 
cgit v1.2.3


From c0b88d9e8fe574d6164f48211db50f3b8a4c4d93 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Fri, 19 Jul 2013 12:21:40 +0200
Subject: setter for keys for dirty tracking, more robust tests

Just altering identity.keys did not mark identities as changed. Also we now have a sane default for keys.
---
 users/app/models/identity.rb                    | 11 ++++++++++-
 users/app/models/user.rb                        | 21 ++++++++++++++-------
 users/test/integration/api/account_flow_test.rb |  5 +++++
 users/test/unit/identity_test.rb                |  4 ++--
 4 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/users/app/models/identity.rb b/users/app/models/identity.rb
index b862590..73531ec 100644
--- a/users/app/models/identity.rb
+++ b/users/app/models/identity.rb
@@ -6,7 +6,7 @@ class Identity < CouchRest::Model::Base
 
   property :address, LocalEmail
   property :destination, Email
-  property :keys, Hash
+  property :keys, HashWithIndifferentAccess
 
   validate :unique_forward
   validate :alias_available
@@ -27,6 +27,15 @@ class Identity < CouchRest::Model::Base
 
   end
 
+  def keys
+    read_attribute('keys') || HashWithIndifferentAccess.new
+  end
+
+  def set_key(type, value)
+    return if keys[type] == value
+    write_attribute('keys', keys.merge(type => value))
+  end
+
   protected
 
   def unique_forward
diff --git a/users/app/models/user.rb b/users/app/models/user.rb
index 546d571..c791069 100644
--- a/users/app/models/user.rb
+++ b/users/app/models/user.rb
@@ -59,14 +59,19 @@ class User < CouchRest::Model::Base
   # TODO: Create an alias for the old login when changing the login
   def login=(value)
     write_attribute 'login', value
-    @identity = build_identity
+    if @identity
+      @identity.address = email_address
+      @identity.destination = email_address
+    else
+      build_identity
+    end
   end
 
   # DEPRECATED
   #
-  # Please access identity.keys[:pgp] directly
+  # Please set the key on the identity directly
   def public_key=(value)
-    identity.keys[:pgp] = value
+    identity.set_key(:pgp, value)
   end
 
   # DEPRECATED
@@ -83,8 +88,7 @@ class User < CouchRest::Model::Base
   # this is the main identity. login@domain.tld
   # aliases and forwards are represented in other identities.
   def identity
-    @identity ||=
-      Identity.find_by_address_and_destination([email_address, email_address])
+    @identity ||= find_identity || build_identity
   end
 
   def create_identity(attribs = {}, &block)
@@ -96,8 +100,7 @@ class User < CouchRest::Model::Base
   def build_identity(attribs = {}, &block)
     attribs.reverse_merge! user_id: self.id,
       address: self.email_address,
-      destination: self.email_address,
-      keys: {}
+      destination: self.email_address
     Identity.new(attribs, &block)
   end
 
@@ -139,6 +142,10 @@ class User < CouchRest::Model::Base
 
   protected
 
+  def find_identity
+    Identity.find_by_address_and_destination([email_address, email_address])
+  end
+
   ##
   #  Validation Functions
   ##
diff --git a/users/test/integration/api/account_flow_test.rb b/users/test/integration/api/account_flow_test.rb
index 4c94389..93b6507 100644
--- a/users/test/integration/api/account_flow_test.rb
+++ b/users/test/integration/api/account_flow_test.rb
@@ -79,8 +79,13 @@ class AccountFlowTest < RackTest
     test_public_key = 'asdlfkjslfdkjasd'
     original_login = @user.login
     new_login = 'zaph'
+    User.find_by_login(new_login).try(:destroy)
+    Identity.by_address.key(new_login + '@' + APP_CONFIG[:domain]).each do |identity|
+      identity.destroy
+    end
     put "http://api.lvh.me:3000/1/users/" + @user.id + '.json', :user => {:public_key => test_public_key, :login => new_login}, :format => :json
     @user.reload
+    assert last_response.successful?
     assert_equal test_public_key, @user.public_key
     assert_equal new_login, @user.login
     # eventually probably want to remove most of this into a non-integration functional test
diff --git a/users/test/unit/identity_test.rb b/users/test/unit/identity_test.rb
index d20ad93..40b1296 100644
--- a/users/test/unit/identity_test.rb
+++ b/users/test/unit/identity_test.rb
@@ -57,13 +57,13 @@ class IdentityTest < ActiveSupport::TestCase
 
   test "setting and getting pgp key" do
     id = @user.identity
-    id.keys[:pgp] = pgp_key_string
+    id.set_key(:pgp, pgp_key_string)
     assert_equal pgp_key_string, id.keys[:pgp]
   end
 
   test "querying pgp key via couch" do
     id = @user.identity
-    id.keys[:pgp] = pgp_key_string
+    id.set_key(:pgp, pgp_key_string)
     id.save
     view = Identity.pgp_key_by_email.key(id.address)
     assert_equal 1, view.rows.count
-- 
cgit v1.2.3


From c0527cc18788fc60180d9293a546c93e6a77b788 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Fri, 19 Jul 2013 13:06:57 +0200
Subject: removed email settings controller and views

PGP setting has been moved into account settings. It's using the API now issueing an Ajax request without any visual feedback.

This obviously is not what we want but it hopefully suffices for uploading gpg keys for testing purposes before the Identity UI is in place.
---
 app/views/layouts/_navigation.html.haml            |  1 -
 users/app/controllers/email_settings_controller.rb | 41 ----------------------
 users/app/controllers/v1/users_controller.rb       |  3 --
 users/app/views/email_settings/edit.html.haml      | 38 --------------------
 users/app/views/users/_edit.html.haml              | 16 ++++++++-
 5 files changed, 15 insertions(+), 84 deletions(-)
 delete mode 100644 users/app/controllers/email_settings_controller.rb
 delete mode 100644 users/app/views/email_settings/edit.html.haml

diff --git a/app/views/layouts/_navigation.html.haml b/app/views/layouts/_navigation.html.haml
index 2f79a22..3d094a6 100644
--- a/app/views/layouts/_navigation.html.haml
+++ b/app/views/layouts/_navigation.html.haml
@@ -1,6 +1,5 @@
 %ul.nav.sidenav
   = link_to_navigation t(:overview), user_overview_path(@user), :active => controller?(:overviews)
   = link_to_navigation t(:account_settings), edit_user_path(@user), :active => controller?(:users)
-  = link_to_navigation t(:email_settings), edit_user_email_settings_path(@user), :active => controller?(:email_settings)
   = link_to_navigation t(:support_tickets), auto_tickets_path, :active => controller?(:tickets)
   = link_to_navigation t(:logout), logout_path, :method => :delete
diff --git a/users/app/controllers/email_settings_controller.rb b/users/app/controllers/email_settings_controller.rb
deleted file mode 100644
index f7d85be..0000000
--- a/users/app/controllers/email_settings_controller.rb
+++ /dev/null
@@ -1,41 +0,0 @@
-class EmailSettingsController < UsersBaseController
-
-  before_filter :authorize
-  before_filter :fetch_user
-
-  def edit
-    @email_alias = LocalEmail.new
-  end
-
-  def update
-    @user.attributes = cleanup_params(params[:user])
-    if @user.changed?
-      if @user.save
-        flash[:notice] = t(:changes_saved)
-        redirect
-      else
-        if @user.email_aliases.last && !@user.email_aliases.last.valid?
-          # display bad alias in text field:
-          @email_alias = @user.email_aliases.pop
-        end
-        render 'email_settings/edit'
-      end
-    else
-      redirect
-    end
-  end
-
-  private
-
-  def redirect
-    redirect_to edit_user_email_settings_url(@user)
-  end
-
-  def cleanup_params(user)
-    if !user['email_forward'].nil? && user['email_forward'].empty?
-      user.delete('email_forward') # don't allow "" as an email forward
-    end
-    user
-  end
-
-end
diff --git a/users/app/controllers/v1/users_controller.rb b/users/app/controllers/v1/users_controller.rb
index fda56f2..467c9b4 100644
--- a/users/app/controllers/v1/users_controller.rb
+++ b/users/app/controllers/v1/users_controller.rb
@@ -24,9 +24,6 @@ module V1
 
     def update
       @user.update_attributes params[:user]
-      if @user.valid?
-        flash[:notice] = t(:user_updated_successfully)
-      end
       respond_with @user
     end
 
diff --git a/users/app/views/email_settings/edit.html.haml b/users/app/views/email_settings/edit.html.haml
deleted file mode 100644
index 7757a31..0000000
--- a/users/app/views/email_settings/edit.html.haml
+++ /dev/null
@@ -1,38 +0,0 @@
-- form_options = {:url => user_email_settings_path(@user), :html => {:class => 'form-horizontal'}, :validate => true}
-- alias_error_class = @email_alias.username && !@email_alias.valid? ? 'error' : ''
-
-- content_for :head do
-  :css
-    table.aliases tr:first-child td {
-      border-top: none;
-    }
-
-= simple_form_for @user, form_options.dup do |f|
-  %legend= t(:email_aliases)
-  .control-group
-    %label.control-label= t(:current_aliases)
-    .controls
-      %table.table.table-condensed.no-header.slim.aliases
-        - if @user.email_aliases.any?
-          - @user.email_aliases.each do |email|
-            %tr
-              %td= email
-              %td= link_to(icon(:remove) + t(:remove), user_email_alias_path(@user, email), :method => :delete)
-        - else
-          %tr
-            %td{:colspan=>2}= t(:none)
-  .control-group{:class => alias_error_class}
-    %label.control-label= t(:add_email_alias)
-    .controls
-      = f.simple_fields_for :email_aliases, @email_alias do |e|
-        .input-append
-          = e.input_field :username
-          = e.submit t(:add), :class => 'btn'
-        = e.error :username
-
-= simple_form_for @user, form_options do |f|
-  %legend= t(:advanced_options)
-  = f.input :email_forward
-  = f.input :public_key, :as => :text, :hint => t(:use_ascii_key), :input_html => {:class => "full-width", :rows => 4}
-  .form-actions
-    = f.submit t(:save), :class => 'btn btn-primary'
diff --git a/users/app/views/users/_edit.html.haml b/users/app/views/users/_edit.html.haml
index 0402f37..5f74d32 100644
--- a/users/app/views/users/_edit.html.haml
+++ b/users/app/views/users/_edit.html.haml
@@ -22,6 +22,20 @@
     .controls
       = f.submit t(:save), :class => 'btn btn-primary'
 
+-#
+-# CHANGE PGP KEY
+-#
+-# this will be replaced by a identities controller/view at some point
+-#
+
+- form_options = {:html => {:class => user_form_class('form-horizontal'), :id => 'update_pgp_key'}, :validate => true}
+= simple_form_for [:api, @user], form_options do |f|
+  %legend= t(:advanced_options)
+  = f.input :public_key, :as => :text, :hint => t(:use_ascii_key), :input_html => {:class => "full-width", :rows => 4}
+  .control-group
+    .controls
+      = f.submit t(:save), :class => 'btn'
+
 -#
 -# DESTROY ACCOUNT
 -#
@@ -48,4 +62,4 @@
   %p= t(:enable_description)
   = link_to enable_user_path(@user), :method => :post, :class => "btn btn-warning"  do
     %i.icon-ok.icon-white
-    = t(:enable)
\ No newline at end of file
+      = t(:enable)
-- 
cgit v1.2.3


From d70c5796a2989b7b43f7e287899fb1394ae28280 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Fri, 19 Jul 2013 16:02:02 +0200
Subject: separate signup and settings service objects for user

---
 users/app/controllers/v1/users_controller.rb      | 13 +++++-
 users/app/models/account_settings.rb              | 36 ++++++++++++++
 users/app/models/identity.rb                      | 27 +++++++++++
 users/app/models/signup_service.rb                |  9 ++++
 users/app/models/user.rb                          | 57 -----------------------
 users/test/functional/v1/users_controller_test.rb |  8 +++-
 users/test/integration/api/account_flow_test.rb   | 13 ++----
 users/test/unit/identity_test.rb                  | 20 ++++----
 users/test/unit/user_test.rb                      |  2 +-
 9 files changed, 105 insertions(+), 80 deletions(-)
 create mode 100644 users/app/models/account_settings.rb
 create mode 100644 users/app/models/signup_service.rb

diff --git a/users/app/controllers/v1/users_controller.rb b/users/app/controllers/v1/users_controller.rb
index 467c9b4..f380c19 100644
--- a/users/app/controllers/v1/users_controller.rb
+++ b/users/app/controllers/v1/users_controller.rb
@@ -18,14 +18,23 @@ module V1
     end
 
     def create
-      @user = User.create(params[:user])
+      @user = signup_service.register(params[:user])
       respond_with @user # return ID instead?
     end
 
     def update
-      @user.update_attributes params[:user]
+      account_settings.update params[:user]
       respond_with @user
     end
 
+    protected
+
+    def account_settings
+      AccountSettings.new(@user)
+    end
+
+    def signup_service
+      SignupService.new
+    end
   end
 end
diff --git a/users/app/models/account_settings.rb b/users/app/models/account_settings.rb
new file mode 100644
index 0000000..a73a95a
--- /dev/null
+++ b/users/app/models/account_settings.rb
@@ -0,0 +1,36 @@
+class AccountSettings
+
+  def initialize(user)
+    @user = user
+  end
+
+  def update(attrs)
+    if attrs[:password_verifier].present?
+      update_login(attrs[:login])
+      @user.update_attributes attrs.slice(:password_verifier, :password_salt)
+    end
+    # TODO: move into identity controller
+    update_pgp_key(attrs[:public_key]) if attrs.has_key? :public_key
+    @user.save && save_identities
+  end
+
+  protected
+
+  def update_login(login, verifier)
+    return unless login.present?
+    @old_identity = Identity.for(@user)
+    @user.login = login
+    @new_identity = Identity.for(@user) # based on the new login
+    @old_identity.destination = @user.email_address # alias old -> new
+  end
+
+  def update_pgp_key(key)
+    @new_identity ||= Identity.for(@user)
+    @new_identity.set_key(:pgp, key)
+  end
+
+  def save_identities
+    @new_identity.try(:save) && @old_identity.try(:save)
+  end
+
+end
diff --git a/users/app/models/identity.rb b/users/app/models/identity.rb
index 73531ec..355f67a 100644
--- a/users/app/models/identity.rb
+++ b/users/app/models/identity.rb
@@ -27,6 +27,33 @@ class Identity < CouchRest::Model::Base
 
   end
 
+  def self.for(user, attributes = {})
+    find_for(user, attributes) || build_for(user, attributes)
+  end
+
+  def self.find_for(user, attributes = {})
+    attributes.reverse_merge! attributes_from_user(user)
+    find_by_address_and_destination [attributes[:address], attributes[:destination]]
+  end
+
+  def self.build_for(user, attributes = {})
+    attributes.reverse_merge! attributes_from_user(user)
+    Identity.new(attributes)
+  end
+
+  def self.create_for(user, attributes = {})
+    identity = build_for(user, attributes)
+    identity.save
+    identity
+  end
+
+  def self.attributes_from_user(user)
+    { user_id: user.id,
+      address: user.email_address,
+      destination: user.email_address
+    }
+  end
+
   def keys
     read_attribute('keys') || HashWithIndifferentAccess.new
   end
diff --git a/users/app/models/signup_service.rb b/users/app/models/signup_service.rb
new file mode 100644
index 0000000..f316ca9
--- /dev/null
+++ b/users/app/models/signup_service.rb
@@ -0,0 +1,9 @@
+class SignupService
+
+  def register(attrs)
+    User.create(attrs).tap do |user|
+      Identity.create_for user
+    end
+  end
+
+end
diff --git a/users/app/models/user.rb b/users/app/models/user.rb
index c791069..f78f290 100644
--- a/users/app/models/user.rb
+++ b/users/app/models/user.rb
@@ -47,63 +47,10 @@ class User < CouchRest::Model::Base
     view :by_created_at
   end # end of design
 
-  # We proxy access to the pgp_key. So we need to make sure
-  # the updated identity actually gets saved.
-  def save(*args)
-    super
-    identity.user_id ||= self.id
-    identity.save if identity.changed?
-  end
-
-  # So far this only works for creating a new user.
-  # TODO: Create an alias for the old login when changing the login
-  def login=(value)
-    write_attribute 'login', value
-    if @identity
-      @identity.address = email_address
-      @identity.destination = email_address
-    else
-      build_identity
-    end
-  end
-
-  # DEPRECATED
-  #
-  # Please set the key on the identity directly
-  def public_key=(value)
-    identity.set_key(:pgp, value)
-  end
-
-  # DEPRECATED
-  #
-  # Please access identity.keys[:pgp] directly
-  def public_key
-    identity.keys[:pgp]
-  end
-
   class << self
     alias_method :find_by_param, :find
   end
 
-  # this is the main identity. login@domain.tld
-  # aliases and forwards are represented in other identities.
-  def identity
-    @identity ||= find_identity || build_identity
-  end
-
-  def create_identity(attribs = {}, &block)
-    new_identity = build_identity(attribs, &block)
-    new_identity.save
-    new_identity
-  end
-
-  def build_identity(attribs = {}, &block)
-    attribs.reverse_merge! user_id: self.id,
-      address: self.email_address,
-      destination: self.email_address
-    Identity.new(attribs, &block)
-  end
-
   def to_param
     self.id
   end
@@ -142,10 +89,6 @@ class User < CouchRest::Model::Base
 
   protected
 
-  def find_identity
-    Identity.find_by_address_and_destination([email_address, email_address])
-  end
-
   ##
   #  Validation Functions
   ##
diff --git a/users/test/functional/v1/users_controller_test.rb b/users/test/functional/v1/users_controller_test.rb
index 0d44e50..a330bf3 100644
--- a/users/test/functional/v1/users_controller_test.rb
+++ b/users/test/functional/v1/users_controller_test.rb
@@ -5,7 +5,9 @@ class V1::UsersControllerTest < ActionController::TestCase
   test "user can change settings" do
     user = find_record :user
     changed_attribs = record_attributes_for :user_with_settings
-    user.expects(:update_attributes).with(changed_attribs)
+    account_settings = stub
+    account_settings.expects(:update).with(changed_attribs)
+    AccountSettings.expects(:new).with(user).returns(account_settings)
 
     login user
     put :update, :user => changed_attribs, :id => user.id, :format => :json
@@ -18,7 +20,9 @@ class V1::UsersControllerTest < ActionController::TestCase
   test "admin can update user" do
     user = find_record :user
     changed_attribs = record_attributes_for :user_with_settings
-    user.expects(:update_attributes).with(changed_attribs)
+    account_settings = stub
+    account_settings.expects(:update).with(changed_attribs)
+    AccountSettings.expects(:new).with(user).returns(account_settings)
 
     login :is_admin? => true
     put :update, :user => changed_attribs, :id => user.id, :format => :json
diff --git a/users/test/integration/api/account_flow_test.rb b/users/test/integration/api/account_flow_test.rb
index 93b6507..c09dcb6 100644
--- a/users/test/integration/api/account_flow_test.rb
+++ b/users/test/integration/api/account_flow_test.rb
@@ -84,20 +84,17 @@ class AccountFlowTest < RackTest
       identity.destroy
     end
     put "http://api.lvh.me:3000/1/users/" + @user.id + '.json', :user => {:public_key => test_public_key, :login => new_login}, :format => :json
-    @user.reload
     assert last_response.successful?
-    assert_equal test_public_key, @user.public_key
-    assert_equal new_login, @user.login
+    assert_equal test_public_key, Identity.for(@user).keys[:pgp]
+    # does not change login if no password_verifier is present
+    assert_equal original_login, @user.login
     # eventually probably want to remove most of this into a non-integration functional test
     # should not overwrite public key:
     put "http://api.lvh.me:3000/1/users/" + @user.id + '.json', :user => {:blee => :blah}, :format => :json
-    @user.reload
-    assert_equal test_public_key, @user.public_key
+    assert_equal test_public_key, Identity.for(@user).keys[:pgp]
     # should overwrite public key:
     put "http://api.lvh.me:3000/1/users/" + @user.id + '.json', :user => {:public_key => nil}, :format => :json
-    # TODO: not sure why i need this, but when public key is removed, the DB is updated but @user.reload doesn't seem to actually reload.
-    @user = User.find(@user.id) # @user.reload
-    assert_nil @user.public_key
+    assert_nil Identity.for(@user).keys[:pgp]
   end
 
 end
diff --git a/users/test/unit/identity_test.rb b/users/test/unit/identity_test.rb
index 40b1296..bf24f02 100644
--- a/users/test/unit/identity_test.rb
+++ b/users/test/unit/identity_test.rb
@@ -11,28 +11,28 @@ class IdentityTest < ActiveSupport::TestCase
   end
 
   test "initial identity for a user" do
-    id = @user.identity
+    id = Identity.for(@user)
     assert_equal @user.email_address, id.address
     assert_equal @user.email_address, id.destination
     assert_equal @user, id.user
   end
 
   test "add alias" do
-    id = @user.build_identity address: alias_name
+    id = Identity.for @user, address: alias_name
     assert_equal LocalEmail.new(alias_name), id.address
     assert_equal @user.email_address, id.destination
     assert_equal @user, id.user
   end
 
   test "add forward" do
-    id = @user.build_identity destination: forward_address
+    id = Identity.for @user, destination: forward_address
     assert_equal @user.email_address, id.address
     assert_equal Email.new(forward_address), id.destination
     assert_equal @user, id.user
   end
 
   test "forward alias" do
-    id = @user.build_identity address: alias_name, destination: forward_address
+    id = Identity.for @user, address: alias_name, destination: forward_address
     assert_equal LocalEmail.new(alias_name), id.address
     assert_equal Email.new(forward_address), id.destination
     assert_equal @user, id.user
@@ -40,29 +40,29 @@ class IdentityTest < ActiveSupport::TestCase
   end
 
   test "prevents duplicates" do
-    id = @user.create_identity address: alias_name, destination: forward_address
-    dup = @user.build_identity address: alias_name, destination: forward_address
+    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]
   end
 
   test "validates availability" do
     other_user = FactoryGirl.create(:user)
-    id = @user.create_identity address: alias_name, destination: forward_address
-    taken = other_user.build_identity address: alias_name
+    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]
     other_user.destroy
   end
 
   test "setting and getting pgp key" do
-    id = @user.identity
+    id = Identity.for(@user)
     id.set_key(:pgp, pgp_key_string)
     assert_equal pgp_key_string, id.keys[:pgp]
   end
 
   test "querying pgp key via couch" do
-    id = @user.identity
+    id = Identity.for(@user)
     id.set_key(:pgp, pgp_key_string)
     id.save
     view = Identity.pgp_key_by_email.key(id.address)
diff --git a/users/test/unit/user_test.rb b/users/test/unit/user_test.rb
index f303c8d..477c727 100644
--- a/users/test/unit/user_test.rb
+++ b/users/test/unit/user_test.rb
@@ -58,7 +58,7 @@ class UserTest < ActiveSupport::TestCase
 
   test "login needs to be unique amongst aliases" do
     other_user = FactoryGirl.create :user
-    other_user.create_identity address: @user.login
+    Identity.create_for other_user, address: @user.login
     assert !@user.valid?
     other_user.destroy
   end
-- 
cgit v1.2.3


From 370be6caa8a366774fba15d09fb03ee4d923b861 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Wed, 24 Jul 2013 11:09:04 +0200
Subject: keeping the pgp_key accessors for User so views still work

---
 users/app/models/user.rb     | 20 ++++++++++++++++++++
 users/test/unit/user_test.rb |  8 +++++---
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/users/app/models/user.rb b/users/app/models/user.rb
index f78f290..0a89f7c 100644
--- a/users/app/models/user.rb
+++ b/users/app/models/user.rb
@@ -87,6 +87,26 @@ class User < CouchRest::Model::Base
     Ticket.for_user(self).limit(count).all #defaults to having most recent updated first
   end
 
+  # DEPRECATED
+  #
+  # Please set the key on the identity directly
+  # WARNING: This will not be serialized with the user record!
+  # It is only a workaround for the key form.
+  def public_key=(value)
+    identity.set_key(:pgp, value)
+  end
+
+  # DEPRECATED
+  #
+  # Please access identity.keys[:pgp] directly
+  def public_key
+    identity.keys[:pgp]
+  end
+
+  def identity
+    @identity ||= Identity.for(self)
+  end
+
   protected
 
   ##
diff --git a/users/test/unit/user_test.rb b/users/test/unit/user_test.rb
index 477c727..89ee749 100644
--- a/users/test/unit/user_test.rb
+++ b/users/test/unit/user_test.rb
@@ -70,14 +70,16 @@ class UserTest < ActiveSupport::TestCase
   end
 
   test "pgp key view" do
-    @user.public_key = SecureRandom.base64(4096)
-    @user.save
+    key = SecureRandom.base64(4096)
+    identity = Identity.create_for @user
+    identity.set_key('pgp', key)
+    identity.save
 
     view = Identity.pgp_key_by_email.key(@user.email_address)
 
     assert_equal 1, view.rows.count
     assert result = view.rows.first
     assert_equal @user.email_address, result["key"]
-    assert_equal @user.public_key, result["value"]
+    assert_equal key, result["value"]
   end
 end
-- 
cgit v1.2.3


From b1065710102193ba11e2a18b5f6506ba1d5b31f9 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Wed, 24 Jul 2013 12:30:59 +0200
Subject: also destroy the identity for a test user during teardown

---
 users/test/integration/api/account_flow_test.rb | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/users/test/integration/api/account_flow_test.rb b/users/test/integration/api/account_flow_test.rb
index c09dcb6..ec7753c 100644
--- a/users/test/integration/api/account_flow_test.rb
+++ b/users/test/integration/api/account_flow_test.rb
@@ -18,7 +18,10 @@ class AccountFlowTest < RackTest
   end
 
   teardown do
-    @user.destroy if @user
+    if @user
+      @user.identity.destroy
+      @user.destroy
+    end
     Warden.test_reset!
   end
 
-- 
cgit v1.2.3