From dd88c7f84cb3c497c6327c364b3c08993c51a08f Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Mon, 28 Oct 2013 12:47:46 +0100
Subject: notify user their account was successfully deleted (refs #4216)

Also fixes a cornercase when admins deleted their own account. So far they would be redirected to the users list - which then refused access. Now they'll be redirected to the home landing page as well.
---
 users/app/controllers/users_controller.rb      | 10 +++++++++-
 users/config/locales/en.yml                    |  1 +
 users/test/integration/browser/account_test.rb |  8 ++++++++
 3 files changed, 18 insertions(+), 1 deletion(-)

(limited to 'users')

diff --git a/users/app/controllers/users_controller.rb b/users/app/controllers/users_controller.rb
index f66277d..de21983 100644
--- a/users/app/controllers/users_controller.rb
+++ b/users/app/controllers/users_controller.rb
@@ -48,7 +48,15 @@ class UsersController < UsersBaseController
 
   def destroy
     @user.destroy
-    redirect_to admin? ? users_url : root_url
+    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/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/integration/browser/account_test.rb b/users/test/integration/browser/account_test.rb
index 8e03856..b712c95 100644
--- a/users/test/integration/browser/account_test.rb
+++ b/users/test/integration/browser/account_test.rb
@@ -38,6 +38,14 @@ class AccountTest < BrowserIntegrationTest
     assert page.has_no_selector? 'input.btn-primary.disabled'
   end
 
+  test "account destruction" do
+    username, password = submit_signup
+    click_on I18n.t('account_settings')
+    click_on I18n.t('destroy_my_account')
+    page.save_screenshot('/tmp/destroy.png')
+    assert page.has_content?(I18n.t('account_destroyed'))
+  end
+
   test "change password" do
     username, password = submit_signup
     click_on "Account Settings"
-- 
cgit v1.2.3


From f6924bfe3b540c384fa53e55db9db3a64a34ced3 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Wed, 30 Oct 2013 20:13:16 +0100
Subject: test helper to expect_logout.

Currently it expects both the session and the token to be cleared. This might change. But we'll always have a definition of what it means to logout we can test this way.
---
 users/test/functional/sessions_controller_test.rb    | 14 +++-----------
 users/test/functional/users_controller_test.rb       |  1 +
 users/test/functional/v1/sessions_controller_test.rb | 19 ++-----------------
 users/test/support/auth_test_helper.rb               | 14 ++++++++++++++
 4 files changed, 20 insertions(+), 28 deletions(-)

(limited to 'users')

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..75d900f 100644
--- a/users/test/functional/users_controller_test.rb
+++ b/users/test/functional/users_controller_test.rb
@@ -91,6 +91,7 @@ class UsersControllerTest < ActionController::TestCase
     user.expects(:destroy)
 
     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/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
-- 
cgit v1.2.3


From b4ca13257341792f5e6496264c421af1888bcdb8 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Tue, 5 Nov 2013 11:40:25 +0100
Subject: refactor: Identity.disable_all_for(user) on user destruction

This way the identity model defines how identities should be disabled. We currently still destroy them. But it will be easy and nicely isolated to change this next.
---
 users/app/models/account.rb  |  4 +---
 users/app/models/identity.rb | 12 ++++++++++++
 2 files changed, 13 insertions(+), 3 deletions(-)

(limited to 'users')

diff --git a/users/app/models/account.rb b/users/app/models/account.rb
index 5368a1b..726f642 100644
--- a/users/app/models/account.rb
+++ b/users/app/models/account.rb
@@ -29,9 +29,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
 
diff --git a/users/app/models/identity.rb b/users/app/models/identity.rb
index e0a24e9..c24af73 100644
--- a/users/app/models/identity.rb
+++ b/users/app/models/identity.rb
@@ -50,6 +50,12 @@ 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
+    end
+  end
+
   def self.attributes_from_user(user)
     { user_id: user.id,
       address: user.email_address,
@@ -57,6 +63,12 @@ class Identity < CouchRest::Model::Base
     }
   end
 
+  #
+  # about to change towards actually disabling the identity instead of
+  # destroying it.
+  #
+  alias_method :disable, :destroy
+
   def keys
     read_attribute('keys') || HashWithIndifferentAccess.new
   end
-- 
cgit v1.2.3


From 99ecdbf71632970d4c83f99beea325e5d213e4c6 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Tue, 5 Nov 2013 12:12:13 +0100
Subject: disabled identities to block handles after a user was deleted

---
 users/app/models/identity.rb     | 17 +++++++++++------
 users/test/unit/account_test.rb  |  3 ++-
 users/test/unit/identity_test.rb | 20 ++++++++++++++++++++
 3 files changed, 33 insertions(+), 7 deletions(-)

(limited to 'users')

diff --git a/users/app/models/identity.rb b/users/app/models/identity.rb
index c24af73..40ce4ae 100644
--- a/users/app/models/identity.rb
+++ b/users/app/models/identity.rb
@@ -53,6 +53,7 @@ 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
     end
   end
 
@@ -63,11 +64,14 @@ class Identity < CouchRest::Model::Base
     }
   end
 
-  #
-  # about to change towards actually disabling the identity instead of
-  # destroying it.
-  #
-  alias_method :disable, :destroy
+  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
@@ -105,7 +109,8 @@ class Identity < CouchRest::Model::Base
   end
 
   def destination_email
-    return if destination.valid? #this ensures it is Email
+    return if destination.nil?   # this identity is disabled
+    return if destination.valid? # this ensures it is Email
     self.errors.add(:destination, destination.errors.messages[:email].first) #assumes only one error #TODO
   end
 
diff --git a/users/test/unit/account_test.rb b/users/test/unit/account_test.rb
index 94a9980..a8c6efd 100644
--- a/users/test/unit/account_test.rb
+++ b/users/test/unit/account_test.rb
@@ -13,7 +13,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..8270689 100644
--- a/users/test/unit/identity_test.rb
+++ b/users/test/unit/identity_test.rb
@@ -90,6 +90,26 @@ 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?
+    id.destroy
+  end
+
   def alias_name
     @alias_name ||= Faker::Internet.user_name
   end
-- 
cgit v1.2.3


From 4a2490cc5eac1803be80fade65bbe9d32fa0bd9b Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Tue, 5 Nov 2013 17:03:55 +0100
Subject: Identity.destroy_all_disabled will clean up disabled identities

This is mostly for cleaning up after tests so far. But we might expand this to destroy all identities disabled before a certain date.
---
 users/app/models/identity.rb     | 17 +++++++++++++++++
 users/test/unit/account_test.rb  |  4 ++++
 users/test/unit/identity_test.rb | 11 ++++++++++-
 3 files changed, 31 insertions(+), 1 deletion(-)

(limited to 'users')

diff --git a/users/app/models/identity.rb b/users/app/models/identity.rb
index 40ce4ae..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
 
@@ -57,6 +68,12 @@ class Identity < CouchRest::Model::Base
     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,
diff --git a/users/test/unit/account_test.rb b/users/test/unit/account_test.rb
index a8c6efd..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?
diff --git a/users/test/unit/identity_test.rb b/users/test/unit/identity_test.rb
index 8270689..78ef52c 100644
--- a/users/test/unit/identity_test.rb
+++ b/users/test/unit/identity_test.rb
@@ -107,7 +107,16 @@ class IdentityTest < ActiveSupport::TestCase
     other_user = find_record :user
     taken = Identity.build_for other_user, address: id.address
     assert !taken.valid?
-    id.destroy
+    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.count
   end
 
   def alias_name
-- 
cgit v1.2.3


From 40f24e2887672957acf7ecedce58e692cc9505ca Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Wed, 6 Nov 2013 10:10:44 +0100
Subject: refactor: extract method on account test

also test one can't login anymore after destroying the account.
---
 users/test/integration/browser/account_test.rb | 35 ++++++++++++++------------
 1 file changed, 19 insertions(+), 16 deletions(-)

(limited to 'users')

diff --git a/users/test/integration/browser/account_test.rb b/users/test/integration/browser/account_test.rb
index b712c95..6e9aab5 100644
--- a/users/test/integration/browser/account_test.rb
+++ b/users/test/integration/browser/account_test.rb
@@ -19,31 +19,24 @@ class AccountTest < BrowserIntegrationTest
   test "successful login" do
     username, password = submit_signup
     click_on 'Logout'
-    click_on 'Log In'
-    fill_in 'Username', with: username
-    fill_in 'Password', with: password
-    click_on 'Log In'
+    attempt_login(username, password)
     assert page.has_content?("Welcome #{username}")
     User.find_by_login(username).account.destroy
   end
 
   test "failed login" do
     visit '/'
-    click_on 'Log In'
-    fill_in 'Username', with: "username"
-    fill_in 'Password', with: "wrong password"
-    click_on 'Log In'
-    assert page.has_selector? 'input.btn-primary.disabled'
-    assert page.has_content? I18n.t(:invalid_user_pass)
-    assert page.has_no_selector? 'input.btn-primary.disabled'
+    attempt_login("username", "wrong password")
+    assert_invalid_login(page)
   end
 
   test "account destruction" do
     username, password = submit_signup
     click_on I18n.t('account_settings')
     click_on I18n.t('destroy_my_account')
-    page.save_screenshot('/tmp/destroy.png')
     assert page.has_content?(I18n.t('account_destroyed'))
+    attempt_login(username, password)
+    assert_invalid_login(page)
   end
 
   test "change password" do
@@ -55,10 +48,7 @@ class AccountTest < BrowserIntegrationTest
       click_on 'Save'
     end
     click_on 'Logout'
-    click_on 'Log In'
-    fill_in 'Username', with: username
-    fill_in 'Password', with: "other password"
-    click_on 'Log In'
+    attempt_login(username, "other password")
     assert page.has_content?("Welcome #{username}")
     User.find_by_login(username).account.destroy
   end
@@ -100,6 +90,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();
-- 
cgit v1.2.3


From 44d273fd03645af5e546133adf4e9906800d3d5f Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Wed, 6 Nov 2013 10:20:33 +0100
Subject: integration test for blocking handles after account destroyed

has not been run yet.
---
 users/test/integration/browser/account_test.rb | 12 ++++++++++++
 users/test/support/integration_test_helper.rb  |  6 +++---
 2 files changed, 15 insertions(+), 3 deletions(-)

(limited to 'users')

diff --git a/users/test/integration/browser/account_test.rb b/users/test/integration/browser/account_test.rb
index 6e9aab5..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}")
@@ -39,6 +43,14 @@ class AccountTest < BrowserIntegrationTest
     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
     username, password = submit_signup
     click_on "Account Settings"
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
-- 
cgit v1.2.3


From 24598b5c5e4df20c423ec74ea8e9df1592483c6b Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Wed, 6 Nov 2013 11:26:46 +0100
Subject: destroy all tickets created by a user when account is destroyed

In order to keep the users engine independent of the tickets engine i added a generic load hook to the account model. The tickets engine then monkeypatches the account destruction and destroys all tickets before the user is destroyed.

The tickets are destroyed first so that even if things break there should never be tickets with an outdated user id.

I would have prefered to use super over using an alias_method_chain but I have not been able to figure out a way to make account a superclass of the account extension and still refer to Account from the users engine.
---
 users/app/models/account.rb | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

(limited to 'users')

diff --git a/users/app/models/account.rb b/users/app/models/account.rb
index 726f642..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
 
@@ -52,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
-- 
cgit v1.2.3


From ded302ebc6a9e145775f7847c5e89f91d683c777 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Wed, 6 Nov 2013 11:55:43 +0100
Subject: use the account lifecycle from UsersController#destroy

---
 users/app/controllers/users_controller.rb      | 2 +-
 users/app/controllers/v1/users_controller.rb   | 8 +-------
 users/test/functional/users_controller_test.rb | 8 ++++++++
 3 files changed, 10 insertions(+), 8 deletions(-)

(limited to 'users')

diff --git a/users/app/controllers/users_controller.rb b/users/app/controllers/users_controller.rb
index de21983..3cbb6dc 100644
--- a/users/app/controllers/users_controller.rb
+++ b/users/app/controllers/users_controller.rb
@@ -47,7 +47,7 @@ class UsersController < UsersBaseController
   end
 
   def destroy
-    @user.destroy
+    @user.account.destroy
     flash[:notice] = I18n.t(:account_destroyed)
     # admins can destroy other users
     if @user != current_user
diff --git a/users/app/controllers/v1/users_controller.rb b/users/app/controllers/v1/users_controller.rb
index 03a5a62..0903888 100644
--- a/users/app/controllers/v1/users_controller.rb
+++ b/users/app/controllers/v1/users_controller.rb
@@ -24,15 +24,9 @@ module V1
     end
 
     def update
-      account.update params[:user]
+      @user.account.update params[:user]
       respond_with @user
     end
 
-    protected
-
-    def account
-      @user.account
-    end
-
   end
 end
diff --git a/users/test/functional/users_controller_test.rb b/users/test/functional/users_controller_test.rb
index 75d900f..9c5f8d9 100644
--- a/users/test/functional/users_controller_test.rb
+++ b/users/test/functional/users_controller_test.rb
@@ -77,7 +77,11 @@ class UsersControllerTest < ActionController::TestCase
 
   test "admin can destroy user" do
     user = find_record :user
+
+    # we destroy the user record and the associated data...
     user.expects(:destroy)
+    Identity.expects(:disable_all_for).with(user)
+    Ticket.expects(:destroy_all_from).with(user)
 
     login :is_admin? => true
     delete :destroy, :id => user.id
@@ -88,7 +92,11 @@ class UsersControllerTest < ActionController::TestCase
 
   test "user can cancel account" do
     user = find_record :user
+
+    # we destroy the user record and the associated data...
     user.expects(:destroy)
+    Identity.expects(:disable_all_for).with(user)
+    Ticket.expects(:destroy_all_from).with(user)
 
     login user
     expect_logout
-- 
cgit v1.2.3


From a0e72e6ee7786e3b1fd7276f1c64912c606f4559 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Thu, 7 Nov 2013 12:46:28 +0100
Subject: only check number of disabled identities to make test more robust

---
 users/test/unit/identity_test.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'users')

diff --git a/users/test/unit/identity_test.rb b/users/test/unit/identity_test.rb
index 78ef52c..eca104f 100644
--- a/users/test/unit/identity_test.rb
+++ b/users/test/unit/identity_test.rb
@@ -116,7 +116,7 @@ class IdentityTest < ActiveSupport::TestCase
     id.save
     assert Identity.count > 0
     Identity.destroy_all_disabled
-    assert_equal 0, Identity.count
+    assert_equal 0, Identity.disabled.count
   end
 
   def alias_name
-- 
cgit v1.2.3


From e2c0962077cf759b23639276cca42606ea2135ec Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Thu, 7 Nov 2013 23:27:27 +0100
Subject: Token.destroy_all_expired to cleanup expired tokens (#4411)

---
 users/app/models/token.rb     | 35 ++++++++++++++++++++++++-----------
 users/test/unit/token_test.rb | 15 +++++++++++++++
 2 files changed, 39 insertions(+), 11 deletions(-)

(limited to 'users')

diff --git a/users/app/models/token.rb b/users/app/models/token.rb
index dd87344..bf9b0d0 100644
--- a/users/app/models/token.rb
+++ b/users/app/models/token.rb
@@ -11,6 +11,24 @@ 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
+    self.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 +45,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/test/unit/token_test.rb b/users/test/unit/token_test.rb
index f56c576..445a20c 100644
--- a/users/test/unit/token_test.rb
+++ b/users/test/unit/token_test.rb
@@ -61,6 +61,21 @@ class ClientCertificateTest < ActiveSupport::TestCase
     end
   end
 
+  test "Token.destroy_all_expired cleans up expired tokens only" do
+    expired = Token.new(user_id: @user.id)
+    expired.last_seen_at = 2.hours.ago
+    expired.save
+    fresh = Token.new(user_id: @user.id)
+    fresh.save
+    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
-- 
cgit v1.2.3


From a7cd2ef0877e79302f27fb175384a0cf4ded52d9 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Thu, 7 Nov 2013 23:36:37 +0100
Subject: fix cornercase of non expiring tokens

---
 users/app/models/token.rb     |  3 ++-
 users/test/factories.rb       |  4 +++-
 users/test/unit/token_test.rb | 18 ++++++++++--------
 3 files changed, 15 insertions(+), 10 deletions(-)

(limited to 'users')

diff --git a/users/app/models/token.rb b/users/app/models/token.rb
index bf9b0d0..001eb40 100644
--- a/users/app/models/token.rb
+++ b/users/app/models/token.rb
@@ -20,7 +20,8 @@ class Token < CouchRest::Model::Base
   end
 
   def self.expired
-    self.by_last_seen_at.endkey(expires_after.minutes.ago)
+    return [] unless expires_after
+    by_last_seen_at.endkey(expires_after.minutes.ago)
   end
 
   def self.destroy_all_expired
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/unit/token_test.rb b/users/test/unit/token_test.rb
index 445a20c..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,12 +58,17 @@ 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 = Token.new(user_id: @user.id)
-    expired.last_seen_at = 2.hours.ago
-    expired.save
-    fresh = Token.new(user_id: @user.id)
-    fresh.save
+    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
-- 
cgit v1.2.3