From dd88c7f84cb3c497c6327c364b3c08993c51a08f Mon Sep 17 00:00:00 2001 From: Azul 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. --- app/views/home/index.html.haml | 4 ++++ users/app/controllers/users_controller.rb | 10 +++++++++- users/config/locales/en.yml | 1 + users/test/integration/browser/account_test.rb | 8 ++++++++ 4 files changed, 22 insertions(+), 1 deletion(-) diff --git a/app/views/home/index.html.haml b/app/views/home/index.html.haml index 728b5b8..5a54354 100644 --- a/app/views/home/index.html.haml +++ b/app/views/home/index.html.haml @@ -6,6 +6,10 @@ %p We provide secure communication services, including encrypted internet, email (coming soon), and chat (coming later). + .row-fluid + .span6.offset3 + = render 'layouts/messages' + .row-fluid = home_page_buttons - if Rails.env == 'development' 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 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(-) 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 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(-) 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 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(-) 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 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(-) 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 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(-) 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 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(-) 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 d620fd42925f1d3d29a66bb61a75bed8c2bdaf8f Mon Sep 17 00:00:00 2001 From: Azul Date: Wed, 6 Nov 2013 10:40:32 +0100 Subject: refactor: split up and cleaned up ticket validation tests --- help/test/factories.rb | 8 ++++++-- help/test/unit/ticket_test.rb | 41 +++++++++++++++++++---------------------- 2 files changed, 25 insertions(+), 24 deletions(-) diff --git a/help/test/factories.rb b/help/test/factories.rb index 5b38952..5368ce6 100644 --- a/help/test/factories.rb +++ b/help/test/factories.rb @@ -2,8 +2,12 @@ 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 end diff --git a/help/test/unit/ticket_test.rb b/help/test/unit/ticket_test.rb index ce35e1d..04832d9 100644 --- a/help/test/unit/ticket_test.rb +++ b/help/test/unit/ticket_test.rb @@ -1,41 +1,38 @@ 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 allows for multiple email addresses" do + t = FactoryGirl.build :ticket, email: 'blah@blah.com, bb@jjj.org' + 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 -- cgit v1.2.3 From ebc60f3aba1ca08e454ba5e91f49905df2e5fa13 Mon Sep 17 00:00:00 2001 From: Azul Date: Wed, 6 Nov 2013 10:55:25 +0100 Subject: Ticket.destroy_all_from(user) - remove all tickets created by a user We'll use this to clean up after user destruction --- help/app/models/ticket.rb | 7 +++++++ help/test/factories.rb | 4 ++++ help/test/unit/ticket_test.rb | 6 ++++++ 3 files changed, 17 insertions(+) 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/test/factories.rb b/help/test/factories.rb index 5368ce6..bce3af1 100644 --- a/help/test/factories.rb +++ b/help/test/factories.rb @@ -9,6 +9,10 @@ FactoryGirl.define do { "0" => { "body" => Faker::Lorem.sentences.join(" ") } } end end + + factory :ticket_with_creator do + created_by { FactoryGirl.create(:user).id } + end end end diff --git a/help/test/unit/ticket_test.rb b/help/test/unit/ticket_test.rb index 04832d9..6c29d11 100644 --- a/help/test/unit/ticket_test.rb +++ b/help/test/unit/ticket_test.rb @@ -39,6 +39,12 @@ class TicketTest < ActiveSupport::TestCase 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 -- cgit v1.2.3 From 24598b5c5e4df20c423ec74ea8e9df1592483c6b Mon Sep 17 00:00:00 2001 From: Azul 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. --- help/app/models/account_extension/tickets.rb | 13 +++++++++++++ help/config/initializers/account_lifecycle.rb | 3 +++ help/test/unit/account_extension_test.rb | 12 ++++++++++++ users/app/models/account.rb | 10 +++++++++- 4 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 help/app/models/account_extension/tickets.rb create mode 100644 help/config/initializers/account_lifecycle.rb create mode 100644 help/test/unit/account_extension_test.rb 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/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/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/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 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(-) 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 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(-) 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 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(-) 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 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(-) 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 From 69a41bee2548fa8743dd3188b0ebfc84dac17062 Mon Sep 17 00:00:00 2001 From: Azul Date: Fri, 8 Nov 2013 09:48:57 +0100 Subject: removed outdated test. --- help/test/unit/ticket_test.rb | 5 ----- 1 file changed, 5 deletions(-) diff --git a/help/test/unit/ticket_test.rb b/help/test/unit/ticket_test.rb index 6c29d11..751fe70 100644 --- a/help/test/unit/ticket_test.rb +++ b/help/test/unit/ticket_test.rb @@ -17,11 +17,6 @@ class TicketTest < ActiveSupport::TestCase assert !t.valid? end - test "ticket allows for multiple email addresses" do - t = FactoryGirl.build :ticket, email: 'blah@blah.com, bb@jjj.org' - assert !t.valid? - end - test "ticket open states" do t = FactoryGirl.build :ticket assert t.is_open -- cgit v1.2.3 From 8c19b447dec3982107f93ea1ae2626f844045249 Mon Sep 17 00:00:00 2001 From: Azul Date: Sun, 10 Nov 2013 21:13:31 +0100 Subject: update readme to require ruby 1.9.3 instead of 1.8 --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index cfdad33..1d6016c 100644 --- a/README.md +++ b/README.md @@ -41,7 +41,7 @@ Typically, this application is installed automatically as part of the LEAP Platf ### Install system requirements - sudo apt-get install git ruby1.8 rubygems1.8 couchdb + sudo apt-get install git ruby1.9.3 rubygems couchdb sudo gem install bundler On Debian Wheezy or later, there is a Debian package for bundler, so you can alternately run ``sudo apt-get install bundler``. -- cgit v1.2.3 From d70161b55e37e0d9e7a23ed7dbac4ea6d323971a Mon Sep 17 00:00:00 2001 From: jessib Date: Mon, 11 Nov 2013 14:16:16 -0800 Subject: Maybe not ideal fix, but since there is no edit view, we want to show the show view with the appropriate error messages. --- help/app/controllers/tickets_controller.rb | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) 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 -- cgit v1.2.3 From 11e80906b49bea120ae398c7d6524127eaa9363a Mon Sep 17 00:00:00 2001 From: Azul Date: Tue, 12 Nov 2013 14:50:14 +0100 Subject: make sure we log json request errors and their backtraces --- app/controllers/application_controller.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 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 -- cgit v1.2.3 From 10be8c0073b67dcfb7925996e81c2e717f8b499e Mon Sep 17 00:00:00 2001 From: elijah Date: Thu, 14 Nov 2013 02:19:03 -0800 Subject: added support for easier customizations via "config/customization" directory --- CUSTOM.md | 13 ++++++++----- config/application.rb | 6 ++++++ config/customization/README.md | 27 +++++++++++++++++++++++++++ config/initializers/customization.rb | 31 +++++++++++++++++++++++++++++++ 4 files changed, 72 insertions(+), 5 deletions(-) create mode 100644 config/customization/README.md create mode 100644 config/initializers/customization.rb diff --git a/CUSTOM.md b/CUSTOM.md index 67fdac0..8671323 100644 --- a/CUSTOM.md +++ b/CUSTOM.md @@ -1,11 +1,14 @@ -# Customization # +Customization +============================== -Leap Web is based on Engines. All things in `app` will overwrite the default behaviour. You can either create a new rails app and include the leap_web gem or clone the leap web repository and add your customizations to the `app` directory. +Customization directory +--------------------------------------- -## CSS Customization ## +See config/customization/README.md -We use scss. It's a superset of css3. Add your customizations to `app/assets/stylesheets`. +Engines +--------------------- -## Disabling an Engine ## +Leap Web is based on Engines. All things in `app` will overwrite the default behaviour. You can either create a new rails app and include the leap_web gem or clone the leap web repository and add your customizations to the `app` directory. If you have no use for one of the engines you can remove it from the Gemfile. Not however that your app might still need to provide some functionality for the other engines to work. For example the users engine provides `current_user` and other methods. diff --git a/config/application.rb b/config/application.rb index 8587ffc..8cf7e30 100644 --- a/config/application.rb +++ b/config/application.rb @@ -85,5 +85,11 @@ module LeapWeb # Set to false in order to see asset requests in the log config.quiet_assets = true + + ## + ## CUSTOMIZATION + ## see initializers/customization.rb + ## + config.paths['app/views'].unshift "config/customization/views" end end diff --git a/config/customization/README.md b/config/customization/README.md new file mode 100644 index 0000000..9c3e434 --- /dev/null +++ b/config/customization/README.md @@ -0,0 +1,27 @@ +Customizing LEAP Webapp +============================================ + +By default, this directory is empty. Any file you place here will override the default files for the application. + +For example: + + stylesheets/ -- overrides files Rails.root/app/assets/stylesheets + tail.scss -- included before all others + head.scss -- included after all others + + public/ -- overrides files in Rails.root/public + favicon.ico -- custom favicon + img/ -- customary directory to put images in + + views/ -- overrides files Rails.root/app/views + home/ + index.html.haml -- this file is what shows up on the home page + + locales/ -- overrides files in Rails.root/config/locales + en.yml -- overrides for English + de.yml -- overrides for German + and so on... + +For most changes, the web application must be restarted after any changes are made to the customization directory. + +Sometimes a `rake tmp:clear` and a rails restart is required to pick up a new stylesheet. diff --git a/config/initializers/customization.rb b/config/initializers/customization.rb new file mode 100644 index 0000000..a2f6f88 --- /dev/null +++ b/config/initializers/customization.rb @@ -0,0 +1,31 @@ +# +# When deploying, common customizations can be dropped in config/customizations. This initializer makes this work. +# +customization_directory = "#{Rails.root}/config/customization" + +# +# Set customization views as the first view path +# +# Rails.application.config.paths['app/views'].unshift "config/customization/views" +# (For some reason, this does not work here. See application.rb for where this is actually called.) + +# +# Set customization stylesheets as the first asset path +# +# (This cannot go in application.rb, because the default paths +# haven't been loaded yet, as far as I can tell) +# +Rails.application.config.assets.paths.unshift "#{customization_directory}/stylesheets" + +# +# Copy files to public +# +if Dir.exists?("#{customization_directory}/public") + require 'fileutils' + FileUtils.cp_r("#{customization_directory}/public/.", "#{Rails.root}/public") +end + +# +# Add I18n path +# +Rails.application.config.i18n.load_path += Dir["#{customization_directory}/locales/*.{rb,yml,yaml}"] -- cgit v1.2.3 From 108938615ff7490080f80ea2d6bd1cd8037cdd84 Mon Sep 17 00:00:00 2001 From: elijah Date: Thu, 14 Nov 2013 02:19:57 -0800 Subject: minor improvements to the download button (proper localization, better image, better hooks for customization) --- core/app/helpers/download_helper.rb | 2 +- core/app/views/common/_download_for_os.html.haml | 4 ++-- core/app/views/common/_home_page_buttons.html.haml | 10 +++++++--- core/config/locales/en.yml | 4 ++-- public/leap-img/128/mask.png | Bin 10080 -> 3654 bytes 5 files changed, 12 insertions(+), 8 deletions(-) diff --git a/core/app/helpers/download_helper.rb b/core/app/helpers/download_helper.rb index f9c6c40..ee0fe73 100644 --- a/core/app/helpers/download_helper.rb +++ b/core/app/helpers/download_helper.rb @@ -2,7 +2,7 @@ module DownloadHelper def alternative_client_links(os = nil) alternative_clients(os).map do |client| - link_to(client.capitalize, client_download_url(client)) + link_to(I18n.t("os."+client), client_download_url(client)) end end diff --git a/core/app/views/common/_download_for_os.html.haml b/core/app/views/common/_download_for_os.html.haml index b7c88ba..4c096ce 100644 --- a/core/app/views/common/_download_for_os.html.haml +++ b/core/app/views/common/_download_for_os.html.haml @@ -8,8 +8,8 @@ %br/ %small= I18n.t("os.#{os}") %span.info - = t(:client_info, :provider => content_tag(:b,APP_CONFIG[:domain])).html_safe - %br/ + %div= t(:client_info, :provider => content_tag(:b,APP_CONFIG[:domain])).html_safe + %div - if os == "other" = t(:all_downloads_info, :clients => alternative_client_links(os).to_sentence).html_safe - else diff --git a/core/app/views/common/_home_page_buttons.html.haml b/core/app/views/common/_home_page_buttons.html.haml index e10fd38..3be12e2 100644 --- a/core/app/views/common/_home_page_buttons.html.haml +++ b/core/app/views/common/_home_page_buttons.html.haml @@ -2,10 +2,14 @@ .home-buttons .row-fluid.first - .span3 - .download.span6 + .span2 + .download.span8 = render partial: 'common/download_for_os', collection: available_clients + ['other'] - .span3 + .span2 + - if local_assigns[:divider] + .row-fluid + .span12 + = render local_assigns[:divider] .row-fluid.second .login.span4 %span.link= link_to(icon('ok-sign', icon_color) + t(:login), login_path, :class => 'btn') diff --git a/core/config/locales/en.yml b/core/config/locales/en.yml index 4710a16..4abf4e8 100644 --- a/core/config/locales/en.yml +++ b/core/config/locales/en.yml @@ -23,7 +23,7 @@ en: download_client: "Download Bitmask" client_info: "The Bitmask application allows you to use %{provider} services." all_downloads_info: "It is available for %{clients}." - other_downloads_info: "It is also available for %{clients}." + other_downloads_info: "Bitmask is also available for %{clients}." login_info: "Log in to change your account settings, create support tickets, and manage payments." signup_info: "Sign up for a new user account via this website (it is better if you use the Bitmask application to sign up, but this website works too)." welcome: "Welcome to %{provider}." @@ -35,6 +35,6 @@ en: linux64: "Linux (64 bit)" windows: "Windows" android: "Android" - osx: "Mac OSX" + osx: "Mac OS" other: "(not available for your OS.)" diff --git a/public/leap-img/128/mask.png b/public/leap-img/128/mask.png index c7390eb..444a62c 100644 Binary files a/public/leap-img/128/mask.png and b/public/leap-img/128/mask.png differ -- cgit v1.2.3 From 84682ee6261967935d16fbeae1190af26420563e Mon Sep 17 00:00:00 2001 From: elijah Date: Thu, 14 Nov 2013 15:50:22 -0800 Subject: ensure that we only copy files when running restarting the app, not every time a rake task is run (especially since some rake tasks get run as root!) --- Rakefile | 2 ++ config/initializers/customization.rb | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Rakefile b/Rakefile index 8b58316..47b6c3f 100644 --- a/Rakefile +++ b/Rakefile @@ -2,6 +2,8 @@ # Add your own tasks in files placed in lib/tasks ending in .rake, # for example lib/tasks/capistrano.rake, and they will automatically be available to Rake. +RAKE=true # let environment initialization code know if we are running via rake or not. + require 'rake/packagetask' require 'rubygems/package_task' diff --git a/config/initializers/customization.rb b/config/initializers/customization.rb index a2f6f88..08da518 100644 --- a/config/initializers/customization.rb +++ b/config/initializers/customization.rb @@ -20,7 +20,7 @@ Rails.application.config.assets.paths.unshift "#{customization_directory}/styles # # Copy files to public # -if Dir.exists?("#{customization_directory}/public") +if !defined?(RAKE) && Dir.exists?("#{customization_directory}/public") require 'fileutils' FileUtils.cp_r("#{customization_directory}/public/.", "#{Rails.root}/public") end -- cgit v1.2.3 From 4193a94b4cc5b5cabbace8311562c0ca88a79f74 Mon Sep 17 00:00:00 2001 From: elijah Date: Fri, 15 Nov 2013 00:25:40 -0800 Subject: fix problem with custom scss files and precompiling assets in production mode. --- config/application.rb | 2 +- config/initializers/customization.rb | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/config/application.rb b/config/application.rb index 8cf7e30..2c9c55a 100644 --- a/config/application.rb +++ b/config/application.rb @@ -78,7 +78,7 @@ module LeapWeb # Enable the asset pipeline config.assets.enabled = true - config.assets.initialize_on_precompile = false + config.assets.initialize_on_precompile = true # don't change this (see customization.rb) # Version of your assets, change this if you want to expire all your assets config.assets.version = '1.0' diff --git a/config/initializers/customization.rb b/config/initializers/customization.rb index 08da518..bc9c834 100644 --- a/config/initializers/customization.rb +++ b/config/initializers/customization.rb @@ -12,8 +12,13 @@ customization_directory = "#{Rails.root}/config/customization" # # Set customization stylesheets as the first asset path # -# (This cannot go in application.rb, because the default paths -# haven't been loaded yet, as far as I can tell) +# Some notes: +# +# * This cannot go in application.rb, as far as I can tell. In application.rb, the default paths +# haven't been loaded yet, so the path we add will always end up at the end unless we add it here. +# +# * For this to work, config.assets.initialize_on_precompile MUST be set to true, otherwise +# this initializer will never get called in production mode when the assets are precompiled. # Rails.application.config.assets.paths.unshift "#{customization_directory}/stylesheets" -- cgit v1.2.3