summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/controllers/application_controller.rb6
-rw-r--r--help/app/controllers/tickets_controller.rb11
-rw-r--r--help/app/models/account_extension/tickets.rb13
-rw-r--r--help/app/models/ticket.rb7
-rw-r--r--help/config/initializers/account_lifecycle.rb3
-rw-r--r--help/test/factories.rb12
-rw-r--r--help/test/unit/account_extension_test.rb12
-rw-r--r--help/test/unit/ticket_test.rb42
-rw-r--r--users/app/controllers/users_controller.rb2
-rw-r--r--users/app/controllers/v1/users_controller.rb8
-rw-r--r--users/app/models/account.rb14
-rw-r--r--users/app/models/identity.rb36
-rw-r--r--users/app/models/token.rb36
-rw-r--r--users/test/factories.rb4
-rw-r--r--users/test/functional/users_controller_test.rb8
-rw-r--r--users/test/integration/browser/account_test.rb47
-rw-r--r--users/test/support/integration_test_helper.rb6
-rw-r--r--users/test/unit/account_test.rb7
-rw-r--r--users/test/unit/identity_test.rb29
-rw-r--r--users/test/unit/token_test.rb23
20 files changed, 245 insertions, 81 deletions
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index b808e1c..de8d06b 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -10,12 +10,14 @@ class ApplicationController < ActionController::Base
rescue_from StandardError do |e|
respond_to do |format|
- format.json { render_json_error }
+ format.json { render_json_error(e) }
format.all { raise e } # reraise the exception so the normal thing happens.
end
end
- def render_json_error
+ def render_json_error(e)
+ Rails.logger.error e
+ Rails.logger.error e.backtrace.join("\n")
render status: 500,
json: {error: "The server failed to process your request. We'll look into it."}
end
diff --git a/help/app/controllers/tickets_controller.rb b/help/app/controllers/tickets_controller.rb
index a669e19..c193ff4 100644
--- a/help/app/controllers/tickets_controller.rb
+++ b/help/app/controllers/tickets_controller.rb
@@ -62,14 +62,11 @@ class TicketsController < ApplicationController
@ticket.comments.last.private = false unless admin?
end
- if @ticket.changed?
- if @ticket.save
- flash[:notice] = t(:changes_saved)
- redirect_to_tickets
- else
- respond_with @ticket
- end
+ if @ticket.changed? and @ticket.save
+ flash[:notice] = t(:changes_saved)
+ redirect_to_tickets
else
+ flash[:error] = @ticket.errors.full_messages.join(". ") if @ticket.changed?
redirect_to auto_ticket_path(@ticket)
end
end
diff --git a/help/app/models/account_extension/tickets.rb b/help/app/models/account_extension/tickets.rb
new file mode 100644
index 0000000..f898b56
--- /dev/null
+++ b/help/app/models/account_extension/tickets.rb
@@ -0,0 +1,13 @@
+module AccountExtension::Tickets
+ extend ActiveSupport::Concern
+
+ def destroy_with_tickets
+ Ticket.destroy_all_from(self.user)
+ destroy_without_tickets
+ end
+
+ included do
+ alias_method_chain :destroy, :tickets
+ end
+
+end
diff --git a/help/app/models/ticket.rb b/help/app/models/ticket.rb
index 8066d0d..74f2050 100644
--- a/help/app/models/ticket.rb
+++ b/help/app/models/ticket.rb
@@ -24,6 +24,7 @@ class Ticket < CouchRest::Model::Base
design do
view :by_updated_at
view :by_created_at
+ view :by_created_by
view :by_is_open_and_created_at
view :by_is_open_and_updated_at
@@ -40,6 +41,12 @@ class Ticket < CouchRest::Model::Base
@selection.tickets
end
+ def self.destroy_all_from(user)
+ self.by_created_by.key(user.id).each do |ticket|
+ ticket.destroy
+ end
+ end
+
def is_creator_validated?
!!created_by
end
diff --git a/help/config/initializers/account_lifecycle.rb b/help/config/initializers/account_lifecycle.rb
new file mode 100644
index 0000000..d9f04c1
--- /dev/null
+++ b/help/config/initializers/account_lifecycle.rb
@@ -0,0 +1,3 @@
+ActiveSupport.on_load(:account) do
+ include AccountExtension::Tickets
+end
diff --git a/help/test/factories.rb b/help/test/factories.rb
index 5b38952..bce3af1 100644
--- a/help/test/factories.rb
+++ b/help/test/factories.rb
@@ -2,8 +2,16 @@ FactoryGirl.define do
factory :ticket do
title { Faker::Lorem.sentence }
- comments_attributes do
- { "0" => { "body" => Faker::Lorem.sentences.join(" ") } }
+ email { Faker::Internet.email }
+
+ factory :ticket_with_comment do
+ comments_attributes do
+ { "0" => { "body" => Faker::Lorem.sentences.join(" ") } }
+ end
+ end
+
+ factory :ticket_with_creator do
+ created_by { FactoryGirl.create(:user).id }
end
end
diff --git a/help/test/unit/account_extension_test.rb b/help/test/unit/account_extension_test.rb
new file mode 100644
index 0000000..aba162c
--- /dev/null
+++ b/help/test/unit/account_extension_test.rb
@@ -0,0 +1,12 @@
+require 'test_helper'
+
+class AccountExtensionTest < ActiveSupport::TestCase
+
+ test "destroying an account triggers ticket destruction" do
+ t = FactoryGirl.create :ticket_with_creator
+ u = t.created_by_user
+ Account.new(u).destroy
+ assert_equal nil, Ticket.find(t.id)
+ end
+
+end
diff --git a/help/test/unit/ticket_test.rb b/help/test/unit/ticket_test.rb
index ce35e1d..751fe70 100644
--- a/help/test/unit/ticket_test.rb
+++ b/help/test/unit/ticket_test.rb
@@ -1,47 +1,45 @@
require 'test_helper'
class TicketTest < ActiveSupport::TestCase
- #test "the truth" do
- # assert true
- #end
- setup do
- @sample = Ticket.new
+ test "ticket with default attribs is valid" do
+ t = FactoryGirl.build :ticket
+ assert t.valid?
end
- test "validity" do
- t = Ticket.create :title => 'test title', :email => 'blah@blah.com'
+ test "ticket without email is valid" do
+ t = FactoryGirl.build :ticket, email: ""
assert t.valid?
- assert_equal t.title, 'test title'
+ end
+ test "ticket validates email format" do
+ t = FactoryGirl.build :ticket, email: "aswerssfd"
+ assert !t.valid?
+ end
+
+ test "ticket open states" do
+ t = FactoryGirl.build :ticket
assert t.is_open
t.close
assert !t.is_open
t.reopen
assert t.is_open
- #user = LeapWebHelp::User.new(User.valid_attributes_hash)
- #user = LeapWebUsers::User.create
-
- #t.user = user
-
- #t.email = '' #invalid
- #assert !t.valid?
- #t.email = 'blah@blah.com, bb@jjj.org'
- #assert t.valid?
- t.email = 'bdlfjlkasfjklasjf' #invalid
- #p t.email_address
- #p t.email_address.strip =~ RFC822::EmailAddress
- assert !t.valid?
- t.reload.destroy
end
test "creation validated" do
+ @sample = Ticket.new
assert !@sample.is_creator_validated?
#p current_user
@sample.created_by = 22 #current_user
assert @sample.is_creator_validated?
end
+ test "destroy all tickets from a user" do
+ t = FactoryGirl.create :ticket_with_creator
+ u = t.created_by_user
+ Ticket.destroy_all_from(u)
+ assert_equal nil, Ticket.find(t.id)
+ end
=begin
# TODO: do once have current_user stuff in order
test "code if & only if not creator-validated" do
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/app/models/account.rb b/users/app/models/account.rb
index 5368a1b..5c943bb 100644
--- a/users/app/models/account.rb
+++ b/users/app/models/account.rb
@@ -1,5 +1,10 @@
#
-# A Composition of a User record and it's identity records.
+# The Account model takes care of the livecycle of a user.
+# It composes a User record and it's identity records.
+# It also allows for other engines to hook into the livecycle by
+# monkeypatching the create, update and destroy methods.
+# There's an ActiveSupport load_hook at the end of this file to
+# make this more easy.
#
class Account
@@ -29,9 +34,7 @@ class Account
def destroy
return unless @user
- Identity.by_user_id.key(@user.id).each do |identity|
- identity.destroy
- end
+ Identity.disable_all_for(@user)
@user.destroy
end
@@ -54,4 +57,7 @@ class Account
@new_identity.try(:save) && @old_identity.try(:save)
end
+ # You can hook into the account lifecycle from different engines using
+ # ActiveSupport.on_load(:account) do ...
+ ActiveSupport.run_load_hooks(:account, self)
end
diff --git a/users/app/models/identity.rb b/users/app/models/identity.rb
index e0a24e9..97966d0 100644
--- a/users/app/models/identity.rb
+++ b/users/app/models/identity.rb
@@ -27,6 +27,17 @@ class Identity < CouchRest::Model::Base
emit(doc.address, doc.keys["pgp"]);
}
EOJS
+ view :disabled,
+ map: <<-EOJS
+ function(doc) {
+ if (doc.type != 'Identity') {
+ return;
+ }
+ if (typeof doc.user_id === "undefined") {
+ emit(doc._id, 1);
+ }
+ }
+ EOJS
end
@@ -50,6 +61,19 @@ class Identity < CouchRest::Model::Base
identity
end
+ def self.disable_all_for(user)
+ Identity.by_user_id.key(user.id).each do |identity|
+ identity.disable
+ identity.save
+ end
+ end
+
+ def self.destroy_all_disabled
+ Identity.disabled.each do |identity|
+ identity.destroy
+ end
+ end
+
def self.attributes_from_user(user)
{ user_id: user.id,
address: user.email_address,
@@ -57,6 +81,15 @@ class Identity < CouchRest::Model::Base
}
end
+ def enabled?
+ self.destination && self.user_id
+ end
+
+ def disable
+ self.destination = nil
+ self.user_id = nil
+ end
+
def keys
read_attribute('keys') || HashWithIndifferentAccess.new
end
@@ -93,7 +126,8 @@ class Identity < CouchRest::Model::Base
end
def destination_email
- return if destination.valid? #this ensures it is Email
+ return if destination.nil? # this identity is disabled
+ return if destination.valid? # this ensures it is Email
self.errors.add(:destination, destination.errors.messages[:email].first) #assumes only one error #TODO
end
diff --git a/users/app/models/token.rb b/users/app/models/token.rb
index dd87344..001eb40 100644
--- a/users/app/models/token.rb
+++ b/users/app/models/token.rb
@@ -11,6 +11,25 @@ class Token < CouchRest::Model::Base
validates :user_id, presence: true
+ design do
+ view :by_last_seen_at
+ end
+
+ def self.expires_after
+ APP_CONFIG[:auth] && APP_CONFIG[:auth][:token_expires_after]
+ end
+
+ def self.expired
+ return [] unless expires_after
+ by_last_seen_at.endkey(expires_after.minutes.ago)
+ end
+
+ def self.destroy_all_expired
+ self.expired.each do |token|
+ token.destroy
+ end
+ end
+
def authenticate
if expired?
destroy
@@ -27,21 +46,16 @@ class Token < CouchRest::Model::Base
end
def expired?
- expires_after and
- last_seen_at + expires_after.minutes < Time.now
- end
-
- def expires_after
- APP_CONFIG[:auth] && APP_CONFIG[:auth][:token_expires_after]
+ Token.expires_after and
+ last_seen_at < Token.expires_after.minutes.ago
end
def initialize(*args)
super
- self.id = SecureRandom.urlsafe_base64(32).gsub(/^_*/, '')
- self.last_seen_at = Time.now
- end
-
- design do
+ if new_record?
+ self.id = SecureRandom.urlsafe_base64(32).gsub(/^_*/, '')
+ self.last_seen_at = Time.now
+ end
end
end
diff --git a/users/test/factories.rb b/users/test/factories.rb
index c87e290..f5fb77d 100644
--- a/users/test/factories.rb
+++ b/users/test/factories.rb
@@ -19,6 +19,8 @@ FactoryGirl.define do
end
end
- factory :token
+ factory :token do
+ user
+ end
end
diff --git a/users/test/functional/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
diff --git a/users/test/integration/browser/account_test.rb b/users/test/integration/browser/account_test.rb
index b712c95..b349489 100644
--- a/users/test/integration/browser/account_test.rb
+++ b/users/test/integration/browser/account_test.rb
@@ -6,6 +6,10 @@ class AccountTest < BrowserIntegrationTest
Capybara.current_driver = Capybara.javascript_driver
end
+ teardown do
+ Identity.destroy_all_disabled
+ end
+
test "normal account workflow" do
username, password = submit_signup
assert page.has_content?("Welcome #{username}")
@@ -19,31 +23,32 @@ class AccountTest < BrowserIntegrationTest
test "successful login" do
username, password = submit_signup
click_on 'Logout'
- click_on 'Log In'
- fill_in 'Username', with: username
- fill_in 'Password', with: password
- click_on 'Log In'
+ attempt_login(username, password)
assert page.has_content?("Welcome #{username}")
User.find_by_login(username).account.destroy
end
test "failed login" do
visit '/'
- click_on 'Log In'
- fill_in 'Username', with: "username"
- fill_in 'Password', with: "wrong password"
- click_on 'Log In'
- assert page.has_selector? 'input.btn-primary.disabled'
- assert page.has_content? I18n.t(:invalid_user_pass)
- assert page.has_no_selector? 'input.btn-primary.disabled'
+ attempt_login("username", "wrong password")
+ assert_invalid_login(page)
end
test "account destruction" do
username, password = submit_signup
click_on I18n.t('account_settings')
click_on I18n.t('destroy_my_account')
- page.save_screenshot('/tmp/destroy.png')
assert page.has_content?(I18n.t('account_destroyed'))
+ attempt_login(username, password)
+ assert_invalid_login(page)
+ end
+
+ test "handle blocked after account destruction" do
+ username, password = submit_signup
+ click_on I18n.t('account_settings')
+ click_on I18n.t('destroy_my_account')
+ submit_signup(username)
+ assert page.has_content?('has already been taken')
end
test "change password" do
@@ -55,10 +60,7 @@ class AccountTest < BrowserIntegrationTest
click_on 'Save'
end
click_on 'Logout'
- click_on 'Log In'
- fill_in 'Username', with: username
- fill_in 'Password', with: "other password"
- click_on 'Log In'
+ attempt_login(username, "other password")
assert page.has_content?("Welcome #{username}")
User.find_by_login(username).account.destroy
end
@@ -100,6 +102,19 @@ class AccountTest < BrowserIntegrationTest
assert page.has_content?("server failed")
end
+ def attempt_login(username, password)
+ click_on 'Log In'
+ fill_in 'Username', with: username
+ fill_in 'Password', with: password
+ click_on 'Log In'
+ end
+
+ def assert_invalid_login(page)
+ assert page.has_selector? 'input.btn-primary.disabled'
+ assert page.has_content? I18n.t(:invalid_user_pass)
+ assert page.has_no_selector? 'input.btn-primary.disabled'
+ end
+
def inject_malicious_js
page.execute_script <<-EOJS
var calc = new srp.Calculate();
diff --git a/users/test/support/integration_test_helper.rb b/users/test/support/integration_test_helper.rb
index cfe72cf..51e47c6 100644
--- a/users/test/support/integration_test_helper.rb
+++ b/users/test/support/integration_test_helper.rb
@@ -1,7 +1,7 @@
module IntegrationTestHelper
- def submit_signup
- username = "test_#{SecureRandom.urlsafe_base64}".downcase
- password = SecureRandom.base64
+ def submit_signup(username = nil, password = nil)
+ username ||= "test_#{SecureRandom.urlsafe_base64}".downcase
+ password ||= SecureRandom.base64
visit '/users/new'
fill_in 'Username', with: username
fill_in 'Password', with: password
diff --git a/users/test/unit/account_test.rb b/users/test/unit/account_test.rb
index 94a9980..4fb3c3d 100644
--- a/users/test/unit/account_test.rb
+++ b/users/test/unit/account_test.rb
@@ -2,6 +2,10 @@ require 'test_helper'
class AccountTest < ActiveSupport::TestCase
+ teardown do
+ Identity.destroy_all_disabled
+ end
+
test "create a new account" do
user = Account.create(FactoryGirl.attributes_for(:user))
assert user.valid?
@@ -13,7 +17,8 @@ class AccountTest < ActiveSupport::TestCase
end
test "create and remove a user account" do
- assert_no_difference "Identity.count" do
+ # We keep an identity that will block the handle from being reused.
+ assert_difference "Identity.count" do
assert_no_difference "User.count" do
user = Account.create(FactoryGirl.attributes_for(:user))
user.account.destroy
diff --git a/users/test/unit/identity_test.rb b/users/test/unit/identity_test.rb
index 0842a77..eca104f 100644
--- a/users/test/unit/identity_test.rb
+++ b/users/test/unit/identity_test.rb
@@ -90,6 +90,35 @@ class IdentityTest < ActiveSupport::TestCase
assert id.errors.messages[:destination].include? "needs to be a valid email address"
end
+ test "disabled identity" do
+ id = Identity.for(@user)
+ id.disable
+ assert_equal @user.email_address, id.address
+ assert_equal nil, id.destination
+ assert_equal nil, id.user
+ assert !id.enabled?
+ assert id.valid?
+ end
+
+ test "disabled identity blocks handle" do
+ id = Identity.for(@user)
+ id.disable
+ id.save
+ other_user = find_record :user
+ taken = Identity.build_for other_user, address: id.address
+ assert !taken.valid?
+ Identity.destroy_all_disabled
+ end
+
+ test "destroy all disabled identities" do
+ id = Identity.for(@user)
+ id.disable
+ id.save
+ assert Identity.count > 0
+ Identity.destroy_all_disabled
+ assert_equal 0, Identity.disabled.count
+ end
+
def alias_name
@alias_name ||= Faker::Internet.user_name
end
diff --git a/users/test/unit/token_test.rb b/users/test/unit/token_test.rb
index f56c576..6c9f209 100644
--- a/users/test/unit/token_test.rb
+++ b/users/test/unit/token_test.rb
@@ -7,9 +7,6 @@ class ClientCertificateTest < ActiveSupport::TestCase
@user = find_record :user
end
- teardown do
- end
-
test "new token for user" do
sample = Token.new(:user_id => @user.id)
assert sample.valid?
@@ -61,6 +58,26 @@ class ClientCertificateTest < ActiveSupport::TestCase
end
end
+ test "Token.destroy_all_expired is noop if no expiry is set" do
+ expired = FactoryGirl.create :token, last_seen_at: 2.hours.ago
+ with_config auth: {} do
+ Token.destroy_all_expired
+ end
+ assert_equal expired, Token.find(expired.id)
+ end
+
+ test "Token.destroy_all_expired cleans up expired tokens only" do
+ expired = FactoryGirl.create :token, last_seen_at: 2.hours.ago
+ fresh = FactoryGirl.create :token
+ with_config auth: {token_expires_after: 60} do
+ Token.destroy_all_expired
+ end
+ assert_nil Token.find(expired.id)
+ assert_equal fresh, Token.find(fresh.id)
+ fresh.destroy
+ end
+
+
end
generated by cgit v1.2.3 (git 2.25.1) at 2024-11-10 04:41:54 +0000