From e7e16318d056dbd9ec272085487cce6039627b09 Mon Sep 17 00:00:00 2001 From: elijah Date: Sun, 31 Jan 2016 14:43:19 -0800 Subject: remove cert fingerprints for disabled users, so that they cannot send email anymore. closes #7690 --- app/controllers/users_controller.rb | 3 +-- app/controllers/v1/certs_controller.rb | 7 +++++++ app/controllers/v1/smtp_certs_controller.rb | 5 +++++ app/models/account.rb | 11 +++++++++++ app/models/anonymous_user.rb | 5 ++++- app/models/identity.rb | 18 ++++++++++++++++++ test/functional/users_controller_test.rb | 2 +- test/functional/v1/certs_controller_test.rb | 8 +++++++- test/functional/v1/smtp_certs_controller_test.rb | 6 ++++++ test/unit/account_test.rb | 20 +++++++++++++++----- test/unit/identity_test.rb | 10 +++++----- 11 files changed, 80 insertions(+), 15 deletions(-) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 446b726..ec52cff 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -47,8 +47,7 @@ class UsersController < ApplicationController end def deactivate - @user.enabled = false - @user.save + @user.account.disable flash[:notice] = I18n.t("actions.user_disabled_message", username: @user.username) redirect_to :back end diff --git a/app/controllers/v1/certs_controller.rb b/app/controllers/v1/certs_controller.rb index 99aec16..ffa6e35 100644 --- a/app/controllers/v1/certs_controller.rb +++ b/app/controllers/v1/certs_controller.rb @@ -1,6 +1,7 @@ class V1::CertsController < ApiController before_filter :require_login, :unless => :anonymous_access_allowed? + before_filter :require_enabled # GET /cert # deprecated - we actually create a new cert and that can @@ -18,6 +19,12 @@ class V1::CertsController < ApiController protected + def require_enabled + if !current_user.is_anonymous? && !current_user.enabled? + access_denied + end + end + def service_level current_user.effective_service_level end diff --git a/app/controllers/v1/smtp_certs_controller.rb b/app/controllers/v1/smtp_certs_controller.rb index 75f524c..5760645 100644 --- a/app/controllers/v1/smtp_certs_controller.rb +++ b/app/controllers/v1/smtp_certs_controller.rb @@ -3,6 +3,7 @@ class V1::SmtpCertsController < ApiController before_filter :require_login before_filter :require_email_account before_filter :fetch_identity + before_filter :require_enabled # POST /1/smtp_cert def create @@ -22,6 +23,10 @@ class V1::SmtpCertsController < ApiController access_denied unless service_level.provides? 'email' end + def require_enabled + access_denied unless current_user.enabled? + end + def fetch_identity @identity = current_user.identity end diff --git a/app/models/account.rb b/app/models/account.rb index a5cd833..46e5446 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -69,6 +69,17 @@ class Account @user.destroy end + # when a user is disable, all their data and associations remain + # in place, but the user should not be able to send email or + # create new authentication certificates. + def disable + if @user && !@user.tmp? + @user.enabled = false + @user.save + Identity.remove_cert_fingerprints_for(@user) + end + end + protected def update_login(login) diff --git a/app/models/anonymous_user.rb b/app/models/anonymous_user.rb index 73e95e5..5745316 100644 --- a/app/models/anonymous_user.rb +++ b/app/models/anonymous_user.rb @@ -12,7 +12,7 @@ class AnonymousUser < Object def id nil end - + def has_payment_info? false end @@ -37,4 +37,7 @@ class AnonymousUser < Object true end + def enabled? + false + end end diff --git a/app/models/identity.rb b/app/models/identity.rb index 9dc9c7a..e4162c8 100644 --- a/app/models/identity.rb +++ b/app/models/identity.rb @@ -1,3 +1,11 @@ +# +# NOTE: there is some confusing terminology between User and Identity: +# If a user is disabled, the user still exists but has been marked as disabled +# and this condition can be easily reversed. If an identity is disabled, then +# it loses any association with the user and exists only to reserve that username +# and prevent anyone else from registering it. +# + class Identity < CouchRest::Model::Base include LoginFormatValidation @@ -59,6 +67,16 @@ class Identity < CouchRest::Model::Base end end + # if an identity is disabled, it loses contact + # with its former user. but sometimes we want to keep the association + # and remove the fingerprints that allow the user to send email. + def self.remove_cert_fingerprints_for(user) + Identity.by_user_id.key(user.id).each do |identity| + identity.write_attribute(:cert_fingerprints, {}) + identity.save + end + end + def self.destroy_all_for(user) Identity.by_user_id.key(user.id).each do |identity| identity.destroy diff --git a/test/functional/users_controller_test.rb b/test/functional/users_controller_test.rb index 70f483e..261f201 100644 --- a/test/functional/users_controller_test.rb +++ b/test/functional/users_controller_test.rb @@ -1,4 +1,4 @@ -require 'test_helper' +require_relative '../test_helper' class UsersControllerTest < ActionController::TestCase diff --git a/test/functional/v1/certs_controller_test.rb b/test/functional/v1/certs_controller_test.rb index ec34b01..04c1c86 100644 --- a/test/functional/v1/certs_controller_test.rb +++ b/test/functional/v1/certs_controller_test.rb @@ -1,4 +1,4 @@ -require 'test_helper' +require_relative '../../test_helper' class V1::CertsControllerTest < ActionController::TestCase @@ -21,6 +21,12 @@ class V1::CertsControllerTest < ActionController::TestCase end end + test "fail to create cert when disabled" do + login :enabled? => false + post :create + assert_access_denied + end + test "create unlimited cert" do login effective_service_level: ServiceLevel.new(id: 2) cert = expect_cert('UNLIMITED') diff --git a/test/functional/v1/smtp_certs_controller_test.rb b/test/functional/v1/smtp_certs_controller_test.rb index ba70410..1b03995 100644 --- a/test/functional/v1/smtp_certs_controller_test.rb +++ b/test/functional/v1/smtp_certs_controller_test.rb @@ -24,6 +24,12 @@ class V1::SmtpCertsControllerTest < ActionController::TestCase assert_equal cert.to_s, @response.body end + test "fail to create cert when disabled" do + login :enabled? => false + post :create + assert_access_denied + end + protected def expect_cert(email) diff --git a/test/unit/account_test.rb b/test/unit/account_test.rb index 6b814b6..7c26d5c 100644 --- a/test/unit/account_test.rb +++ b/test/unit/account_test.rb @@ -1,4 +1,4 @@ -require 'test_helper' +require_relative '../test_helper' class AccountTest < ActiveSupport::TestCase @@ -23,10 +23,10 @@ class AccountTest < ActiveSupport::TestCase test "create a new account" do with_config invite_required: false do - user = Account.create(FactoryGirl.attributes_for(:user)) - assert user.valid?, "unexpected errors: #{user.errors.inspect}" - assert user.persisted? - user.account.destroy + user = Account.create(FactoryGirl.attributes_for(:user)) + assert user.valid?, "unexpected errors: #{user.errors.inspect}" + assert user.persisted? + user.account.destroy end end @@ -80,4 +80,14 @@ class AccountTest < ActiveSupport::TestCase assert_equal 0, user_code.invite_count end + + test "disabled accounts have no cert fingerprints" do + user = Account.create(FactoryGirl.attributes_for(:user)) + cert = stub(expiry: 1.month.from_now, fingerprint: SecureRandom.hex) + user.identity.register_cert cert + user.identity.save + assert_equal cert.fingerprint, Identity.for(user).cert_fingerprints.keys.first + user.account.disable + assert_equal({}, Identity.for(user).cert_fingerprints) + end end diff --git a/test/unit/identity_test.rb b/test/unit/identity_test.rb index f5c95f8..1f93109 100644 --- a/test/unit/identity_test.rb +++ b/test/unit/identity_test.rb @@ -1,4 +1,4 @@ -require 'test_helper' +require_relative '../test_helper' class IdentityTest < ActiveSupport::TestCase include StubRecordHelper @@ -177,9 +177,9 @@ class IdentityTest < ActiveSupport::TestCase end def cert_stub - # make this expire later than the others so it's on top - # when sorting by expiry descending. - @cert_stub ||= stub expiry: 2.month.from_now, - fingerprint: SecureRandom.hex + # make this expire later than the other test identities + # so that the query that returns certs sorted by expiry will + # return cert_stub first. + @cert_stub ||= stub(expiry: 2.month.from_now, fingerprint: SecureRandom.hex) end end -- cgit v1.2.3