Users who have made no more than one post can delete their own accounts from their user preferences page.

This commit is contained in:
Neil Lalonde 2014-02-13 11:42:35 -05:00
parent 200b1c40bc
commit 8711762143
16 changed files with 239 additions and 83 deletions

View file

@ -42,7 +42,7 @@ Discourse.AdminUser = Discourse.User.extend({
var message = I18n.t('admin.user.delete_all_posts_confirm', {posts: user.get('post_count'), topics: user.get('topic_count')}); var message = I18n.t('admin.user.delete_all_posts_confirm', {posts: user.get('post_count'), topics: user.get('topic_count')});
var buttons = [{ var buttons = [{
"label": I18n.t("composer.cancel"), "label": I18n.t("composer.cancel"),
"class": "cancel", "class": "cancel-inline",
"link": true, "link": true,
"callback": function() { "callback": function() {
user.set('can_delete_all_posts', true); user.set('can_delete_all_posts', true);
@ -308,7 +308,7 @@ Discourse.AdminUser = Discourse.User.extend({
var message = I18n.t('flagging.delete_confirm', {posts: user.get('post_count'), topics: user.get('topic_count'), email: user.get('email'), ip_address: user.get('ip_address')}); var message = I18n.t('flagging.delete_confirm', {posts: user.get('post_count'), topics: user.get('topic_count'), email: user.get('email'), ip_address: user.get('ip_address')});
var buttons = [{ var buttons = [{
"label": I18n.t("composer.cancel"), "label": I18n.t("composer.cancel"),
"class": "cancel", "class": "cancel-inline",
"link": true "link": true
}, { }, {
"label": '<i class="fa fa-exclamation-triangle"></i> ' + I18n.t("flagging.yes_delete_spammer"), "label": '<i class="fa fa-exclamation-triangle"></i> ' + I18n.t("flagging.yes_delete_spammer"),

View file

@ -21,6 +21,12 @@ Discourse.PreferencesController = Discourse.ObjectController.extend({
return false; return false;
}.property('saving', 'name', 'email'), }.property('saving', 'name', 'email'),
deleteDisabled: function() {
if (!this.get('can_delete_account')) return true;
if (this.get('saving')) return true;
if (this.get('deleting')) return true;
}.property('deleting', 'saving', 'can_delete_account'),
canEditName: function() { canEditName: function() {
return Discourse.SiteSettings.enable_names; return Discourse.SiteSettings.enable_names;
}.property(), }.property(),
@ -90,6 +96,35 @@ Discourse.PreferencesController = Discourse.ObjectController.extend({
}); });
}); });
} }
},
delete: function() {
this.set('deleting', true);
var self = this,
message = I18n.t('user.delete_account_confirm'),
model = this.get('model'),
buttons = [{
"label": I18n.t("cancel"),
"class": "cancel-inline",
"link": true,
"callback": function() {
self.set('deleting', false);
}
}, {
"label": '<i class="fa fa-exclamation-triangle"></i> ' + I18n.t("user.delete_account"),
"class": "btn btn-danger",
"callback": function() {
model.delete().then(function() {
bootbox.alert(I18n.t('user.deleted_yourself'), function() {
window.location.pathname = Discourse.getURL('/');
});
}, function() {
bootbox.alert(I18n.t('user.delete_yourself_not_allowed'));
self.set('deleting', false);
});
}
}];
bootbox.dialog(message, buttons, {"classes": "delete-account"});
} }
} }

View file

@ -375,7 +375,22 @@ Discourse.User = Discourse.Model.extend({
updateWatchedCategories: function() { updateWatchedCategories: function() {
this.set("watchedCategories", Discourse.Category.findByIds(this.watched_category_ids)); this.set("watchedCategories", Discourse.Category.findByIds(this.watched_category_ids));
}.observes("watched_category_ids") }.observes("watched_category_ids"),
canDeleteAccount: function() {
return this.get('can_delete_account') && ((this.get('reply_count')||0) + (this.get('topic_count')||0)) <= 1;
}.property('can_delete_account', 'reply_count', 'topic_count'),
delete: function() {
if (this.get('can_delete_account')) {
return Discourse.ajax("/users/" + this.get('username'), {
type: 'DELETE',
data: {context: window.location.pathname}
});
} else {
return Ember.RSVP.reject(I18n.t('user.delete_yourself_not_allowed'));
}
}
}); });

View file

@ -165,5 +165,14 @@
</div> </div>
</div> </div>
{{#if canDeleteAccount}}
<div class="control-group delete-account">
<hr/>
<div class="controls">
<button {{action delete}} {{bind-attr disabled="deleteDisabled"}} class="btn btn-danger"><i class="fa fa-trash-o"></i> {{i18n user.delete_account}}</button>
</div>
</div>
{{/if}}
</form> </form>
</section> </section>

View file

@ -354,11 +354,9 @@
} }
} }
.flagging-delete-spammer, .delete-all-posts { .modal-footer .cancel-inline {
.modal-footer .cancel {
text-decoration: underline; text-decoration: underline;
margin-left: 10px; margin-left: 10px;
}
} }
.permission-list{ .permission-list{

View file

@ -7,7 +7,7 @@ class UsersController < ApplicationController
skip_before_filter :authorize_mini_profiler, only: [:avatar] skip_before_filter :authorize_mini_profiler, only: [:avatar]
skip_before_filter :check_xhr, only: [:show, :password_reset, :update, :activate_account, :authorize_email, :user_preferences_redirect, :avatar] skip_before_filter :check_xhr, only: [:show, :password_reset, :update, :activate_account, :authorize_email, :user_preferences_redirect, :avatar]
before_filter :ensure_logged_in, only: [:username, :update, :change_email, :user_preferences_redirect, :upload_avatar, :toggle_avatar] before_filter :ensure_logged_in, only: [:username, :update, :change_email, :user_preferences_redirect, :upload_avatar, :toggle_avatar, :destroy]
before_filter :respond_to_suspicious_request, only: [:create] before_filter :respond_to_suspicious_request, only: [:create]
# we need to allow account creation with bad CSRF tokens, if people are caching, the CSRF token on the # we need to allow account creation with bad CSRF tokens, if people are caching, the CSRF token on the
@ -341,6 +341,13 @@ class UsersController < ApplicationController
render nothing: true render nothing: true
end end
def destroy
@user = fetch_user_from_params
guardian.ensure_can_delete_user!(@user)
UserDestroyer.new(current_user).destroy(@user, {delete_posts: true, context: params[:context]})
render json: success_json
end
private private
def honeypot_value def honeypot_value

View file

@ -16,7 +16,8 @@ class CurrentUserSerializer < BasicUserSerializer
:trust_level, :trust_level,
:can_edit, :can_edit,
:can_invite_to_forum, :can_invite_to_forum,
:no_password :no_password,
:can_delete_account
def include_site_flagged_posts_count? def include_site_flagged_posts_count?
object.staff? object.staff?
@ -54,4 +55,12 @@ class CurrentUserSerializer < BasicUserSerializer
!object.has_password? !object.has_password?
end end
def include_can_delete_account?
can_delete_account
end
def can_delete_account
scope.can_delete_user?(object)
end
end end

View file

@ -3,16 +3,17 @@ class UserDestroyer
class PostsExistError < RuntimeError; end class PostsExistError < RuntimeError; end
def initialize(staff) def initialize(actor)
@staff = staff @actor = actor
raise Discourse::InvalidParameters.new('staff user is nil') unless @staff and @staff.is_a?(User) raise Discourse::InvalidParameters.new('acting user is nil') unless @actor and @actor.is_a?(User)
raise Discourse::InvalidAccess unless @staff.staff? @guardian = Guardian.new(actor)
end end
# Returns false if the user failed to be deleted. # Returns false if the user failed to be deleted.
# Returns a frozen instance of the User if the delete succeeded. # Returns a frozen instance of the User if the delete succeeded.
def destroy(user, opts={}) def destroy(user, opts={})
raise Discourse::InvalidParameters.new('user is nil') unless user and user.is_a?(User) raise Discourse::InvalidParameters.new('user is nil') unless user and user.is_a?(User)
@guardian.ensure_can_delete_user!(user)
raise PostsExistError if !opts[:delete_posts] && user.post_count != 0 raise PostsExistError if !opts[:delete_posts] && user.post_count != 0
User.transaction do User.transaction do
if opts[:delete_posts] if opts[:delete_posts]
@ -24,12 +25,11 @@ class UserDestroyer
end end
end end
end end
PostDestroyer.new(@staff, post).destroy x = PostDestroyer.new(@actor.staff? ? @actor : Discourse.system_user, post).destroy
if post.topic and post.post_number == 1 if post.topic and post.post_number == 1
Topic.unscoped.where(id: post.topic.id).update_all(user_id: nil) Topic.unscoped.where(id: post.topic.id).update_all(user_id: nil)
end end
end end
raise PostsExistError if user.reload.post_count != 0
end end
user.destroy.tap do |u| user.destroy.tap do |u|
if u if u
@ -55,7 +55,7 @@ class UserDestroyer
end end
end end
StaffActionLogger.new(@staff).log_user_deletion(user, opts.slice(:context)) StaffActionLogger.new(@actor == user ? Discourse.system_user : @actor).log_user_deletion(user, opts.slice(:context))
DiscourseHub.unregister_nickname(user.username) if SiteSetting.call_discourse_hub? DiscourseHub.unregister_nickname(user.username) if SiteSetting.call_discourse_hub?
MessageBus.publish "/file-change", ["refresh"], user_ids: [user.id] MessageBus.publish "/file-change", ["refresh"], user_ids: [user.id]
end end

View file

@ -252,6 +252,10 @@ en:
tracked_categories_instructions: "You will automatically track all new and existing topics providing you a count of unread and new posts" tracked_categories_instructions: "You will automatically track all new and existing topics providing you a count of unread and new posts"
muted_categories: "Muted" muted_categories: "Muted"
muted_categories_instructions: "You will not be notified of anything about this topic, and it will not appear on your unread tab." muted_categories_instructions: "You will not be notified of anything about this topic, and it will not appear on your unread tab."
delete_account: "Delete My Account"
delete_account_confirm: "Are you sure you want to permanently delete your account? This action cannot be undone!"
deleted_yourself: "Your account has been deleted successfully."
delete_yourself_not_allowed: "You cannot delete your account right now. Contact an admin to do delete your account for you."
messages: messages:
all: "All" all: "All"

View file

@ -118,7 +118,7 @@ Discourse::Application.routes.draw do
get "session/csrf" => "session#csrf" get "session/csrf" => "session#csrf"
get "composer-messages" => "composer_messages#index" get "composer-messages" => "composer_messages#index"
resources :users, except: [:show, :update] do resources :users, except: [:show, :update, :destroy] do
collection do collection do
get "check_username" get "check_username"
get "is_local_username" get "is_local_username"
@ -157,6 +157,7 @@ Discourse::Application.routes.draw do
post "users/:username/send_activation_email" => "users#send_activation_email", constraints: {username: USERNAME_ROUTE_FORMAT} post "users/:username/send_activation_email" => "users#send_activation_email", constraints: {username: USERNAME_ROUTE_FORMAT}
get "users/:username/activity" => "users#show", constraints: {username: USERNAME_ROUTE_FORMAT} get "users/:username/activity" => "users#show", constraints: {username: USERNAME_ROUTE_FORMAT}
get "users/:username/activity/:filter" => "users#show", constraints: {username: USERNAME_ROUTE_FORMAT} get "users/:username/activity/:filter" => "users#show", constraints: {username: USERNAME_ROUTE_FORMAT}
delete "users/:username" => "users#destroy", constraints: {username: USERNAME_ROUTE_FORMAT}
get "uploads/:site/:id/:sha.:extension" => "uploads#show", constraints: {site: /\w+/, id: /\d+/, sha: /[a-z0-9]{15,16}/i, extension: /\w{2,}/} get "uploads/:site/:id/:sha.:extension" => "uploads#show", constraints: {site: /\w+/, id: /\d+/, sha: /[a-z0-9]{15,16}/i, extension: /\w{2,}/}
post "uploads" => "uploads#create" post "uploads" => "uploads#create"

View file

@ -2,12 +2,15 @@ require_dependency 'guardian/category_guardian'
require_dependency 'guardian/ensure_magic' require_dependency 'guardian/ensure_magic'
require_dependency 'guardian/post_guardian' require_dependency 'guardian/post_guardian'
require_dependency 'guardian/topic_guardian' require_dependency 'guardian/topic_guardian'
require_dependency 'guardian/user_guardian'
# The guardian is responsible for confirming access to various site resources and operations # The guardian is responsible for confirming access to various site resources and operations
class Guardian class Guardian
include EnsureMagic include EnsureMagic
include CategoryGuardian include CategoryGuardian
include PostGuardain include PostGuardain
include TopicGuardian include TopicGuardian
include UserGuardian
class AnonymousUser class AnonymousUser
def blank?; true; end def blank?; true; end
@ -141,18 +144,6 @@ class Guardian
user && is_staff? user && is_staff?
end end
def can_block_user?(user)
user && is_staff? && not(user.staff?)
end
def can_unblock_user?(user)
user && is_staff?
end
def can_delete_user?(user)
user && is_staff? && !user.admin? && user.created_at > SiteSetting.delete_user_max_age.to_i.days.ago
end
# Support sites that have to approve users # Support sites that have to approve users
def can_access_forum? def can_access_forum?
return true unless SiteSetting.must_approve_users? return true unless SiteSetting.must_approve_users?
@ -184,22 +175,6 @@ class Guardian
is_admin? || (authenticated? && @user.id == user_id) is_admin? || (authenticated? && @user.id == user_id)
end end
def can_edit_user?(user)
is_me?(user) || is_staff?
end
def can_edit_username?(user)
return true if is_staff?
return false if SiteSetting.username_change_period <= 0
is_me?(user) && (user.post_count == 0 || user.created_at > SiteSetting.username_change_period.days.ago)
end
def can_edit_email?(user)
return true if is_staff?
return false unless SiteSetting.email_editable?
can_edit?(user)
end
def can_send_private_message?(target) def can_send_private_message?(target)
(User === target || Group === target) && (User === target || Group === target) &&
authenticated? && authenticated? &&

View file

@ -1,4 +1,4 @@
#mixin for all guardian methods dealing with post permisions #mixin for all guardian methods dealing with post permissions
module PostGuardain module PostGuardain
# Can the user act on the post in a particular way. # Can the user act on the post in a particular way.
# taken_actions = the list of actions the user has already taken # taken_actions = the list of actions the user has already taken

View file

@ -0,0 +1,38 @@
# mixin for all Guardian methods dealing with user permissions
module UserGuardian
def can_edit_user?(user)
is_me?(user) || is_staff?
end
def can_edit_username?(user)
return true if is_staff?
return false if SiteSetting.username_change_period <= 0
is_me?(user) && (user.post_count == 0 || user.created_at > SiteSetting.username_change_period.days.ago)
end
def can_edit_email?(user)
return true if is_staff?
return false unless SiteSetting.email_editable?
can_edit?(user)
end
def can_block_user?(user)
user && is_staff? && not(user.staff?)
end
def can_unblock_user?(user)
user && is_staff?
end
def can_delete_user?(user)
return false if user.nil?
return false if user.admin?
if is_me?(user)
user.post_count <= 1
else
is_staff? && user.created_at > SiteSetting.delete_user_max_age.to_i.days.ago
end
end
end

View file

@ -1118,31 +1118,50 @@ describe Guardian do
describe "can_delete_user?" do describe "can_delete_user?" do
it "is false without a logged in user" do it "is false without a logged in user" do
Guardian.new(nil).can_delete_user?(user).should be_false Guardian.new(nil).can_delete_user?(user).should == false
end end
it "is false without a user to look at" do it "is false without a user to look at" do
Guardian.new(admin).can_delete_user?(nil).should be_false Guardian.new(admin).can_delete_user?(nil).should == false
end end
it "is false for regular users" do it "is false for regular users" do
Guardian.new(user).can_delete_user?(coding_horror).should be_false Guardian.new(user).can_delete_user?(coding_horror).should == false
end
context "delete myself" do
let(:myself) { Fabricate.build(:user, created_at: 6.months.ago) }
subject { Guardian.new(myself).can_delete_user?(myself) }
it "is true to delete myself and I have never made a post" do
subject.should == true
end
it "is true to delete myself and I have only made 1 post" do
myself.stubs(:post_count).returns(1)
subject.should == true
end
it "is false to delete myself and I have made 2 posts" do
myself.stubs(:post_count).returns(2)
subject.should == false
end
end end
shared_examples "can_delete_user examples" do shared_examples "can_delete_user examples" do
let(:deletable_user) { Fabricate.build(:user, created_at: 5.minutes.ago) } let(:deletable_user) { Fabricate.build(:user, created_at: 5.minutes.ago) }
it "is true if user is not an admin and is not too old" do it "is true if user is not an admin and is not too old" do
Guardian.new(actor).can_delete_user?(deletable_user).should be_true Guardian.new(actor).can_delete_user?(deletable_user).should == true
end end
it "is false if user is an admin" do it "is false if user is an admin" do
Guardian.new(actor).can_delete_user?(another_admin).should be_false Guardian.new(actor).can_delete_user?(another_admin).should == false
end end
it "is false if user is too old" do it "is false if user is too old" do
SiteSetting.stubs(:delete_user_max_age).returns(7) SiteSetting.stubs(:delete_user_max_age).returns(7)
Guardian.new(actor).can_delete_user?(Fabricate(:user, created_at: 8.days.ago)).should be_false Guardian.new(actor).can_delete_user?(Fabricate(:user, created_at: 8.days.ago)).should == false
end end
end end

View file

@ -1233,4 +1233,35 @@ describe UsersController do
end end
end end
describe '.destroy' do
it 'raises an error when not logged in' do
lambda { xhr :delete, :destroy, username: 'nobody' }.should raise_error(Discourse::NotLoggedIn)
end
context 'while logged in' do
let!(:user) { log_in }
it 'raises an error when you cannot delete your account' do
Guardian.any_instance.stubs(:can_delete_user?).returns(false)
UserDestroyer.any_instance.expects(:destroy).never
xhr :delete, :destroy, username: user.username
response.should be_forbidden
end
it "raises an error when you try to delete someone else's account" do
UserDestroyer.any_instance.expects(:destroy).never
xhr :delete, :destroy, username: Fabricate(:user).username
response.should be_forbidden
end
it "deletes your account when you're allowed to" do
Guardian.any_instance.stubs(:can_delete_user?).returns(true)
UserDestroyer.any_instance.expects(:destroy).with(user, anything).returns(user)
xhr :delete, :destroy, username: user.username
response.should be_success
end
end
end
end end

View file

@ -15,18 +15,6 @@ describe UserDestroyer do
it 'raises an error when user is not a User' do it 'raises an error when user is not a User' do
expect { UserDestroyer.new(5) }.to raise_error(Discourse::InvalidParameters) expect { UserDestroyer.new(5) }.to raise_error(Discourse::InvalidParameters)
end end
it 'raises an error when user is a regular user' do
expect { UserDestroyer.new( Fabricate(:user) ) }.to raise_error(Discourse::InvalidAccess)
end
it 'returns an instance of UserDestroyer when user is a moderator' do
UserDestroyer.new( Fabricate(:moderator) ).should be_a(UserDestroyer)
end
it 'returns an instance of UserDestroyer when user is an admin' do
UserDestroyer.new( Fabricate(:admin) ).should be_a(UserDestroyer)
end
end end
describe 'destroy' do describe 'destroy' do
@ -43,6 +31,10 @@ describe UserDestroyer do
expect { UserDestroyer.new(@admin).destroy('nothing') }.to raise_error(Discourse::InvalidParameters) expect { UserDestroyer.new(@admin).destroy('nothing') }.to raise_error(Discourse::InvalidParameters)
end end
it 'raises an error when regular user tries to delete another user' do
expect { UserDestroyer.new(@user).destroy(Fabricate(:user)) }.to raise_error(Discourse::InvalidAccess)
end
shared_examples "successfully destroy a user" do shared_examples "successfully destroy a user" do
it 'should delete the user' do it 'should delete the user' do
expect { destroy }.to change { User.count }.by(-1) expect { destroy }.to change { User.count }.by(-1)
@ -86,6 +78,13 @@ describe UserDestroyer do
end end
end end
context 'user deletes self' do
let(:destroy_opts) { {delete_posts: true} }
subject(:destroy) { UserDestroyer.new(@user).destroy(@user, destroy_opts) }
include_examples "successfully destroy a user"
end
context 'user has posts' do context 'user has posts' do
let!(:topic_starter) { Fabricate(:user) } let!(:topic_starter) { Fabricate(:user) }
let!(:topic) { Fabricate(:topic, user: topic_starter) } let!(:topic) { Fabricate(:topic, user: topic_starter) }
@ -116,6 +115,8 @@ describe UserDestroyer do
context "delete_posts is true" do context "delete_posts is true" do
let(:destroy_opts) { {delete_posts: true} } let(:destroy_opts) { {delete_posts: true} }
context "staff deletes user" do
subject(:destroy) { UserDestroyer.new(@admin).destroy(@user, destroy_opts) } subject(:destroy) { UserDestroyer.new(@admin).destroy(@user, destroy_opts) }
include_examples "successfully destroy a user" include_examples "successfully destroy a user"
@ -141,6 +142,20 @@ describe UserDestroyer do
spammer_topic.user_id.should be_nil spammer_topic.user_id.should be_nil
end end
end end
context "users deletes self" do
subject(:destroy) { UserDestroyer.new(@user).destroy(@user, destroy_opts) }
include_examples "successfully destroy a user"
include_examples "email block list"
it "deletes the posts" do
destroy
post.reload.deleted_at.should_not be_nil
post.user_id.should be_nil
end
end
end
end end
context 'user has deleted posts' do context 'user has deleted posts' do