FEATURE: add option to hide IP addresses from moderators (#33682)

# Hide IP Addresses from Moderators When `moderators_view_ips` is Disabled ## Summary Feature Request Link - https://meta.discourse.org/t/option-to-hide-ip-addresses-from-moderators/207715/51 This PR implements a feature to **hide IP addresses from moderators** when the `moderators_view_ips` site setting is disabled. Previously, moderators could view IPs in multiple locations across the admin UI. This update ensures that IP addresses are visible to moderators when the setting allows it. ## Changes Implemented ### Backend Updates - **Added `moderators_view_ips` site setting** in `site_settings.yml` - **Updated `CurrentUserSerializer`** to include `can_see_ip` field based on the user’s role and site setting. - **Modified `AdminUserSerializer`** to restrict IP address visibility. - **Updated `UsersController`** to prevent IP addresses from being included in API responses. - **Restricted IPs in `ScreenedIpAddressesController`** by throwing `Discourse::InvalidAccess` if the user lacks permission. ### Frontend Updates - **Hid "Screened IPs" tab** in `/admin/logs` when `moderators_view_ips` is disabled. - **Blocked direct access to `/admin/logs/screened_ip_addresses`** for unauthorized users. - **Updated `user-index.hbs` and `logs.hbs`** to conditionally hide IP fields. ### UI Screenshots New option for Admins in the Admin Security settings dashboard: ![Screenshot 2025-02-21 at 5 32 00 PM](https://github.com/user-attachments/assets/5b315434-7724-4cb9-a3dc-d88750df00a6) Moderator's view before: ![Screenshot 2025-02-21 at 5 25 41 PM](https://github.com/user-attachments/assets/0fb269e2-db40-488b-b11d-8bdfbe2a5245) Moderator's view after: ![Screenshot 2025-02-21 at 5 26 59 PM](https://github.com/user-attachments/assets/efb848b0-1d7f-4ec9-8238-d8ee4eddbbe1) Moderator's view before: ![Screenshot 2025-02-21 at 5 23 52 PM](https://github.com/user-attachments/assets/226e6d63-df3e-45d0-833f-de52593a086e) Moderator's view after: ![Screenshot 2025-02-21 at 5 23 15 PM](https://github.com/user-attachments/assets/af313af2-2329-46d1-827d-290243c320e5) --------- Co-authored-by: Bennett Dungan <bennettdungan@gmail.com>
2025-08-17 18:04:11 +08:00 · 2025-08-05 10:09:02 -05:00 · 2025-08-05 10:09:02 -05:00 · 72e4e53fda
commit 72e4e53fda
parent c2dfc495f6
14 changed files with 151 additions and 31 deletions
--- a/app/assets/javascripts/admin/addon/routes/admin-logs-screened-ip-addresses.js
+++ b/app/assets/javascripts/admin/addon/routes/admin-logs-screened-ip-addresses.js
@ -1,6 +1,15 @@
+import { service } from "@ember/service";
 import DiscourseRoute from "discourse/routes/discourse";

 export default class AdminLogsScreenedIpAddressesRoute extends DiscourseRoute {
+  @service currentUser;
+
+  beforeModel() {
+    if (!this.currentUser.can_see_ip) {
+      this.transitionTo("adminLogs.staffActionLogs");
+    }
+  }
+
  setupController() {
    return this.controllerFor("adminLogsScreenedIpAddresses").show();
  }
--- a/app/assets/javascripts/admin/addon/templates/logs.gjs
+++ b/app/assets/javascripts/admin/addon/templates/logs.gjs
@ -31,10 +31,12 @@ export default RouteTemplate(
            @label="admin.config.staff_action_logs.sub_pages.screened_emails.title"
          />
        {{/if}}
-        <NavItem
-          @route="adminLogs.screenedIpAddresses"
-          @label="admin.config.staff_action_logs.sub_pages.screened_ips.title"
-        />
+        {{#if @controller.currentUser.can_see_ip}}
+          <NavItem
+            @route="adminLogs.screenedIpAddresses"
+            @label="admin.config.staff_action_logs.sub_pages.screened_ips.title"
+          />
+        {{/if}}
        <NavItem
          @route="adminLogs.screenedUrls"
          @label="admin.config.staff_action_logs.sub_pages.screened_urls.title"
--- a/app/assets/javascripts/admin/addon/templates/user-index.gjs
+++ b/app/assets/javascripts/admin/addon/templates/user-index.gjs
@ -228,35 +228,39 @@ export default RouteTemplate(
        />
      </div>

-      <div class="display-row last-ip">
-        <div class="field">{{i18n "user.ip_address.title"}}</div>
-        <div class="value">{{@controller.model.ip_address}}</div>
-        <div class="controls">
-          {{#if @controller.currentUser.staff}}
-            {{#if @controller.model.ip_address}}
-              <IpLookup
-                @ip={{@controller.model.ip_address}}
-                @userId={{@controller.model.id}}
-              />
+      {{#if @controller.model.include_ip}}
+        <div class="display-row last-ip">
+          <div class="field">{{i18n "user.ip_address.title"}}</div>
+          <div class="value">{{@controller.model.ip_address}}</div>
+          <div class="controls">
+            {{#if @controller.currentUser.can_see_ip}}
+              {{#if @controller.model.ip_address}}
+                <IpLookup
+                  @ip={{@controller.model.ip_address}}
+                  @userId={{@controller.model.id}}
+                />
+              {{/if}}
            {{/if}}
-          {{/if}}
+          </div>
        </div>
-      </div>
+      {{/if}}

-      <div class="display-row registration-ip">
-        <div class="field">{{i18n "user.registration_ip_address.title"}}</div>
-        <div class="value">{{@controller.model.registration_ip_address}}</div>
-        <div class="controls">
-          {{#if @controller.currentUser.staff}}
-            {{#if @controller.model.registration_ip_address}}
-              <IpLookup
-                @ip={{@controller.model.registration_ip_address}}
-                @userId={{@controller.model.id}}
-              />
+      {{#if @controller.model.include_ip}}
+        <div class="display-row registration-ip">
+          <div class="field">{{i18n "user.registration_ip_address.title"}}</div>
+          <div class="value">{{@controller.model.registration_ip_address}}</div>
+          <div class="controls">
+            {{#if @controller.currentUser.can_see_ip}}
+              {{#if @controller.model.registration_ip_address}}
+                <IpLookup
+                  @ip={{@controller.model.registration_ip_address}}
+                  @userId={{@controller.model.id}}
+                />
+              {{/if}}
            {{/if}}
-          {{/if}}
+          </div>
        </div>
-      </div>
+      {{/if}}

      {{#if @controller.showBadges}}
        <div class="display-row">
--- a/app/controllers/admin/screened_ip_addresses_controller.rb
+++ b/app/controllers/admin/screened_ip_addresses_controller.rb
@ -1,6 +1,7 @@
 # frozen_string_literal: true

 class Admin::ScreenedIpAddressesController < Admin::StaffController
+  before_action :can_see_ip
  before_action :fetch_screened_ip_address, only: %i[update destroy]

  def index
@ -49,6 +50,10 @@ class Admin::ScreenedIpAddressesController < Admin::StaffController

  private

+  def can_see_ip
+    raise Discourse::InvalidAccess.new if !guardian.can_see_ip?
+  end
+
  def allowed_params
    params.require(:ip_address)
    params.permit(:ip_address, :action_name)
--- a/app/controllers/admin/users_controller.rb
+++ b/app/controllers/admin/users_controller.rb
@ -51,6 +51,7 @@ class Admin::UsersController < Admin::StaffController
      root: false,
      similar_users_count: @user.similar_users.count,
      include_silence_reason: true,
+      include_ip: guardian.can_see_ip?,
    )
  end

@ -438,6 +439,7 @@ class Admin::UsersController < Admin::StaffController

  def ip_info
    params.require(:ip)
+    raise Discourse::InvalidAccess.new unless guardian.can_see_ip?

    render json: DiscourseIpInfo.get(params[:ip], resolve_hostname: true)
  end
--- a/app/serializers/admin_user_serializer.rb
+++ b/app/serializers/admin_user_serializer.rb
@ -8,7 +8,8 @@ class AdminUserSerializer < AdminUserListSerializer
             :can_deactivate,
             :can_approve,
             :ip_address,
-             :registration_ip_address
+             :registration_ip_address,
+             :include_ip

  has_one :single_sign_on_record, serializer: SingleSignOnRecordSerializer, embed: :objects

@ -40,7 +41,19 @@ class AdminUserSerializer < AdminUserListSerializer
    object.registration_ip_address.try(:to_s)
  end

+  def include_ip_address?
+    scope.can_see_ip?
+  end
+
+  def include_registration_ip_address?
+    scope.can_see_ip?
+  end
+
  def include_can_be_deleted?
    true
  end
+
+  def include_ip
+    @options[:include_ip]
+  end
 end
--- a/app/serializers/current_user_serializer.rb
+++ b/app/serializers/current_user_serializer.rb
@ -81,7 +81,8 @@ class CurrentUserSerializer < BasicUserSerializer
             :can_localize_content?,
             :effective_locale,
             :use_reviewable_ui_refresh,
-             :use_experimental_sidebar_messages_count
+             :use_experimental_sidebar_messages_count,
+             :can_see_ip

  delegate :user_stat, to: :object, private: true
  delegate :any_posts, :draft_count, :pending_posts_count, :read_faq?, to: :user_stat
@ -361,4 +362,12 @@ class CurrentUserSerializer < BasicUserSerializer
  def include_use_reviewable_ui_refresh?
    scope.can_see_review_queue?
  end
+
+  def can_see_ip
+    scope.can_see_ip?
+  end
+
+  def include_can_see_ip?
+    object.admin? || (object.moderator? && SiteSetting.moderators_view_ips)
+  end
 end
--- a/app/serializers/user_history_serializer.rb
+++ b/app/serializers/user_history_serializer.rb
@ -40,4 +40,9 @@ class UserHistorySerializer < ApplicationSerializer
      nil
    end
  end
+
+  def ip_address
+    return nil unless scope.can_see_ip?
+    object.ip_address.try(:to_s)
+  end
 end
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@ -1921,6 +1921,7 @@ en:
    redirect_users_to_top_page: "Automatically redirect new and long absent users to the top page. Only applies when 'top' is present in the 'top menu' site setting."
    top_page_default_timeframe: "Default top page time period for anonymous users (automatically adjusts for logged in users based on their last visit)."
    moderators_view_emails: "Allow moderators to view user email addresses."
+    moderators_view_ips: "Allow moderators to view user ip addresses."
    prioritize_username_in_ux: "Show username first on user page, user card and posts (when disabled name is shown first)"
    enable_rich_text_paste: "Enable automatic HTML to Markdown conversion when pasting text into the composer."
    send_old_credential_reminder_days: "Remind about old credentials after days"
--- a/config/site_settings.yml
+++ b/config/site_settings.yml
@ -2585,6 +2585,9 @@ security:
  moderators_view_emails:
    client: true
    default: false
+  moderators_view_ips:
+    default: true
+    client: true
  non_crawler_user_agents:
    hidden: true
    default: "trident|webkit|gecko|chrome|safari|msie|opera|goanna|discourse"
--- a/lib/guardian.rb
+++ b/lib/guardian.rb
@ -500,6 +500,13 @@ class Guardian
    SiteSetting.moderators_view_emails && is_moderator?
  end

+  def can_see_ip?
+    return true if is_admin?
+    return true if is_moderator? && SiteSetting.moderators_view_ips
+
+    false
+  end
+
  def can_mute_user?(target_user)
    can_mute_users? && @user.id != target_user.id && !target_user.staff?
  end
--- a/spec/lib/guardian_spec.rb
+++ b/spec/lib/guardian_spec.rb
@ -3286,6 +3286,30 @@ RSpec.describe Guardian do
    end
  end

+  describe "#can_see_ip?" do
+    let(:guardian_admin) { Guardian.new(admin) }
+    let(:guardian_moderator) { Guardian.new(moderator) }
+
+    context "when user is an admin" do
+      it "returns true" do
+        expect(guardian_admin.can_see_ip?).to eq(true)
+      end
+    end
+
+    context "when user is a moderator" do
+      before { SiteSetting.moderators_view_ips = true }
+
+      it "returns true if moderators_view_ips is enabled" do
+        expect(guardian_moderator.can_see_ip?).to eq(true)
+      end
+
+      it "returns false if moderators_view_ips is disabled" do
+        SiteSetting.moderators_view_ips = false
+        expect(guardian_moderator.can_see_ip?).to eq(false)
+      end
+    end
+  end
+
  describe "#is_developer?" do
    after { Developer.rebuild_cache }

--- a/spec/requests/api/schemas/json/admin_user_response.json
+++ b/spec/requests/api/schemas/json/admin_user_response.json
@ -494,6 +494,9 @@
    },
    "external_ids": {
      "type": "object"
+    },
+    "include_ip": {
+      "type": "boolean"
    }
  },
  "required": [
@ -553,6 +556,7 @@
    "suspended_by",
    "silenced_by",
    "groups",
-    "external_ids"
+    "external_ids",
+    "include_ip"
  ]
 }
--- a/spec/serializers/current_user_serializer_spec.rb
+++ b/spec/serializers/current_user_serializer_spec.rb
@ -331,6 +331,38 @@ RSpec.describe CurrentUserSerializer do
    end
  end

+  describe "#can_see_ip" do
+    let(:payload) { serializer.as_json }
+
+    context "when user is an admin" do
+      let(:user) { Fabricate(:admin) }
+
+      it "includes can_see_ip as true" do
+        expect(payload[:can_see_ip]).to eq(true)
+      end
+    end
+
+    context "when user is a moderator and moderators_view_ips is enabled" do
+      let(:user) { Fabricate(:moderator) }
+
+      before { SiteSetting.moderators_view_ips = true }
+
+      it "includes can_see_ip as true" do
+        expect(payload[:can_see_ip]).to eq(true)
+      end
+    end
+
+    context "when user is a moderator and moderators_view_ips is disabled" do
+      let(:user) { Fabricate(:moderator) }
+
+      before { SiteSetting.moderators_view_ips = false }
+
+      it "does not include can_see_ip" do
+        expect(payload).not_to have_key(:can_see_ip)
+      end
+    end
+  end
+
  describe "#featured_topic" do
    fab!(:featured_topic, :topic)