FIX: prevent anonymous users from changing their email/username/name (#7311)

FIX: prevent anonymous users from changing their email/username/name (#7311)

diff --git a/app/assets/javascripts/discourse/controllers/preferences/account.js.es6 b/app/assets/javascripts/discourse/controllers/preferences/account.js.es6
index b3c09ef..d97c720 100644
--- a/app/assets/javascripts/discourse/controllers/preferences/account.js.es6
+++ b/app/assets/javascripts/discourse/controllers/preferences/account.js.es6
@@ -55,11 +55,15 @@ export default Ember.Controller.extend(
       return availableTitles.length > 0;
     },
 
-    @computed()
-    canChangePassword() {
-      return (
-        !this.siteSettings.enable_sso && this.siteSettings.enable_local_logins
-      );
+    @computed("model.is_anonymous")
+    canChangePassword(isAnonymous) {
+      if (isAnonymous) {
+        return false;
+      } else {
+        return (
+          !this.siteSettings.enable_sso && this.siteSettings.enable_local_logins
+        );
+      }
     },
 
     @computed("model.associated_accounts")
@@ -92,9 +96,17 @@ export default Ember.Controller.extend(
       return userId !== this.get("currentUser.id");
     },
 
-    @computed("model.second_factor_enabled", "canCheckEmails")
-    canUpdateAssociatedAccounts(secondFactorEnabled, canCheckEmails) {
-      if (secondFactorEnabled || !canCheckEmails) {
+    @computed(
+      "model.second_factor_enabled",
+      "canCheckEmails",
+      "model.is_anonymous"
+    )
+    canUpdateAssociatedAccounts(
+      secondFactorEnabled,
+      canCheckEmails,
+      isAnonymous
+    ) {
+      if (secondFactorEnabled || !canCheckEmails || isAnonymous) {
         return false;
       }
 
diff --git a/lib/guardian.rb b/lib/guardian.rb
index fa24d84..7bbb3bd 100644
--- a/lib/guardian.rb
+++ b/lib/guardian.rb
@@ -31,6 +31,9 @@ class Guardian
     def moderator?
       false
     end
+    def anonymous?
+      true
+    end
     def approved?
       false
     end
@@ -107,6 +110,10 @@ class Guardian
     @user.staged?
   end
 
+  def is_anonymous?
+    @user.anonymous?
+  end
+
   # Can the user see the object?
   def can_see?(obj)
     if obj
diff --git a/lib/guardian/user_guardian.rb b/lib/guardian/user_guardian.rb
index 4bfbc09..75f130e 100644
--- a/lib/guardian/user_guardian.rb
+++ b/lib/guardian/user_guardian.rb
@@ -17,23 +17,26 @@ module UserGuardian
   end
 
   def can_edit_username?(user)
-    return false if (SiteSetting.sso_overrides_username? && SiteSetting.enable_sso?)
+    return false if SiteSetting.sso_overrides_username? && SiteSetting.enable_sso?
     return true if is_staff?
     return false if SiteSetting.username_change_period <= 0
+    return false if is_anonymous?
     is_me?(user) && ((user.post_count + user.topic_count) == 0 || user.created_at > SiteSetting.username_change_period.days.ago)
   end
 
   def can_edit_email?(user)
-    return false if (SiteSetting.sso_overrides_email? && SiteSetting.enable_sso?)
+    return false if SiteSetting.sso_overrides_email? && SiteSetting.enable_sso?
     return false unless SiteSetting.email_editable?
     return true if is_staff?
+    return false if is_anonymous?
     can_edit?(user)
   end
 
   def can_edit_name?(user)
-    return false if not(SiteSetting.enable_names?)
-    return false if (SiteSetting.sso_overrides_name? && SiteSetting.enable_sso?)
+    return false unless SiteSetting.enable_names?
+    return false if SiteSetting.sso_overrides_name? && SiteSetting.enable_sso?
     return true if is_staff?
+    return false if is_anonymous?
     can_edit?(user)
   end
 
diff --git a/spec/components/guardian_spec.rb b/spec/components/guardian_spec.rb
index 1fff913..7324501 100644
--- a/spec/components/guardian_spec.rb
+++ b/spec/components/guardian_spec.rb
@@ -9,6 +9,7 @@ describe Guardian do
   let(:user) { Fabricate(:user) }
   let(:moderator) { Fabricate(:moderator) }
   let(:admin) { Fabricate(:admin) }
+  let(:anonymous_user) { Fabricate(:anonymous) }
   let(:trust_level_1) { build(:user, trust_level: 1) }
   let(:trust_level_2) { build(:user, trust_level: 2) }
   let(:trust_level_3) { build(:user, trust_level: 3) }
@@ -2403,6 +2404,20 @@ describe Guardian do
       it "is true for admins" do
         expect(Guardian.new(admin).can_edit_username?(user)).to be_truthy
       end
+
+      it "is true for admins when changing anonymous username" do
+        expect(Guardian.new(admin).can_edit_username?(anonymous_user)).to be_truthy
+      end
+    end
+
+    context "for anonymous user" do
+      before do
+        SiteSetting.allow_anonymous_posting = true
+      end
+
+      it "is false" do
+        expect(Guardian.new(anonymous_user).can_edit_username?(anonymous_user)).to be_falsey
+      end
     end
 
     context 'for a new user' do
@@ -2476,6 +2491,16 @@ describe Guardian do
         SiteSetting.email_editable = true
       end
 
+      context "for anonymous user" do
+        before do
+          SiteSetting.allow_anonymous_posting = true
+        end
+
+        it "is false" do
+          expect(Guardian.new(anonymous_user).can_edit_email?(anonymous_user)).to be_falsey
+        end
+      end
+
       it "is false when not logged in" do
         expect(Guardian.new(nil).can_edit_email?(build(:user, created_at: 1.minute.ago))).to be_falsey
       end
@@ -2554,6 +2579,16 @@ describe Guardian do
       expect(Guardian.new(build(:user)).can_edit_name?(build(:user, created_at: 1.minute.ago))).to be_falsey
     end
 
+    context "for anonymous user" do
+      before do
+        SiteSetting.allow_anonymous_posting = true
+      end
+
+      it "is false" do
+        expect(Guardian.new(anonymous_user).can_edit_name?(anonymous_user)).to be_falsey
+      end
+    end
+
     context 'for a new user' do
       let(:target_user) { build(:user, created_at: 1.minute.ago) }
 
diff --git a/test/javascripts/controllers/preferences-account-test.js.es6 b/test/javascripts/controllers/preferences-account-test.js.es6
index c0f6914..2d40ac2 100644
--- a/test/javascripts/controllers/preferences-account-test.js.es6
+++ b/test/javascripts/controllers/preferences-account-test.js.es6
@@ -1,12 +1,13 @@
 moduleFor("controller:preferences/account");
 
-QUnit.skip("updating of associated accounts", function(assert) {
+QUnit.test("updating of associated accounts", function(assert) {
   const controller = this.subject({
     siteSettings: {
       enable_google_oauth2_logins: true
     },
     model: Ember.Object.create({
-      second_factor_enabled: true
+      second_factor_enabled: true,
+      is_anonymous: true
     }),
     site: Ember.Object.create({
       isMobileDevice: false
@@ -21,6 +22,10 @@ QUnit.skip("updating of associated accounts", function(assert) {
 
   assert.equal(controller.get("canUpdateAssociatedAccounts"), false);
 
+  controller.set("model.is_anonymous", false);
+
+  assert.equal(controller.get("canUpdateAssociatedAccounts"), false);
+
   controller.set("canCheckEmails", true);
 
   assert.equal(controller.get("canUpdateAssociatedAccounts"), true);

GitHub sha: 5c795bc4

This commit has been mentioned on Discourse Meta. There might be relevant details there: