FIX: hide sso payload behind a button click and log views (#12110)

app/assets/javascripts/admin/addon/controllers/admin-user-index.js

@@ -22,6 +22,7 @@ export default Controller.extend(CanCheckEmails, {
   availableGroups: null,
   userTitleValue: null,
   ssoExternalEmail: null,
+  ssoLastPayload: null,
 
   showBadges: setting("enable_badges"),
   hasLockedTrustLevel: notEmpty("model.manual_locked_trust_level"),
@@ -137,7 +138,7 @@ export default Controller.extend(CanCheckEmails, {
       .catch(() => bootbox.alert(I18n.t("generic_error")));
   },
 
-  @discourseComputed("model.single_sign_on_record.last_payload")
+  @discourseComputed("ssoLastPayload")
   ssoPayload(lastPayload) {
     return lastPayload.split("&");
   },
@@ -610,5 +611,15 @@ export default Controller.extend(CanCheckEmails, {
         }
       });
     },
+
+    checkSsoPayload() {
+      return ajax(userPath(`${this.model.username_lower}/sso-payload.json`), {
+        data: { context: window.location.pathname },
+      }).then((result) => {
+        if (result) {
+          this.set("ssoLastPayload", result.payload);
+        }
+      });
+    },
   },
 });

app/assets/javascripts/admin/addon/templates/user-index.hbs

@@ -690,14 +690,23 @@
         <div class="field">{{i18n "admin.user.discourse_connect.external_avatar_url"}}</div>
         <div class="value">{{sso.external_avatar_url}}</div>
       </div>
-      {{#if sso.last_payload}}
+      {{#if canAdminCheckEmails}}
         <div class="display-row">
           <div class="field">{{i18n "admin.user.discourse_connect.last_payload"}}</div>
-          <div class="value">
-            {{#each ssoPayload as |line|}}
-              {{line}}<br>
-            {{/each}}
-          </div>
+          {{#if ssoLastPayload}}
+            <div class="value">
+              {{#each ssoPayload as |line|}}
+                {{line}}<br>
+              {{/each}}
+            </div>
+          {{else}}
+            {{d-button
+              class="btn-default"
+              action=(action "checkSsoPayload")
+              actionParam=model icon="far-list-alt"
+              label="admin.users.check_sso.text"
+              title="admin.users.check_sso.title"}}
+          {{/if}}
         </div>
       {{/if}}
     {{/with}}

app/controllers/users_controller.rb

@@ -11,7 +11,7 @@ class UsersController < ApplicationController
     :update_second_factor, :create_second_factor_backup, :select_avatar,
     :notification_level, :revoke_auth_token, :register_second_factor_security_key,
     :create_second_factor_security_key, :feature_topic, :clear_featured_topic,
-    :bookmarks, :invited, :invite_links, :check_sso_email
+    :bookmarks, :invited, :invite_links, :check_sso_email, :check_sso_payload
   ]
 
   skip_before_action :check_xhr, only: [
@@ -210,7 +210,7 @@ class UsersController < ApplicationController
     user = fetch_user_from_params(include_inactive: true)
 
     unless user == current_user
-      guardian.ensure_can_check_sso_email!(user)
+      guardian.ensure_can_check_sso_details!(user)
       StaffActionLogger.new(current_user).log_check_email(user, context: params[:context])
     end
 
@@ -222,6 +222,22 @@ class UsersController < ApplicationController
     render json: failed_json, status: 403
   end
 
+  def check_sso_payload
+    user = fetch_user_from_params(include_inactive: true)
+
+    guardian.ensure_can_check_sso_details!(user)
+    unless user == current_user
+      StaffActionLogger.new(current_user).log_check_email(user, context: params[:context])
+    end
+
+    payload = user&.single_sign_on_record&.last_payload
+    payload = I18n.t("user.email.does_not_exist") if payload.blank?
+
+    render json: { payload: payload }
+  rescue Discourse::InvalidAccess
+    render json: failed_json, status: 403
+  end
+
   def update_primary_email
     if !SiteSetting.enable_secondary_emails
       return render json: failed_json, status: 410

app/serializers/single_sign_on_record_serializer.rb

@@ -2,13 +2,9 @@
 
 class SingleSignOnRecordSerializer < ApplicationSerializer
   attributes :user_id, :external_id,
-             :last_payload, :created_at,
-             :updated_at, :external_username,
-             :external_name, :external_avatar_url,
+             :created_at, :updated_at,
+             :external_username, :external_name,
+             :external_avatar_url,
              :external_profile_background_url,
              :external_card_background_url
-
-  def include_last_payload?
-    scope.is_admin?
-  end
 end

config/locales/client.en.yml

@@ -4677,6 +4677,9 @@ en:
         check_email:
           title: "Reveal this user's email address"
           text: "Show"
+        check_sso:
+          title: "Reveal SSO payload"
+          text: "Show"
 
       user:
         suspend_failed: "Something went wrong suspending this user %{error}"

config/routes.rb

@@ -446,6 +446,7 @@ Discourse::Application.routes.draw do
       put "#{root_path}/:username" => "users#update", constraints: { username: RouteFormat.username }, defaults: { format: :json }
       get "#{root_path}/:username/emails" => "users#check_emails", constraints: { username: RouteFormat.username }
       get "#{root_path}/:username/sso-email" => "users#check_sso_email", constraints: { username: RouteFormat.username }
+      get "#{root_path}/:username/sso-payload" => "users#check_sso_payload", constraints: { username: RouteFormat.username }
       get "#{root_path}/:username/preferences" => "users#preferences", constraints: { username: RouteFormat.username }
       get "#{root_path}/:username/preferences/email" => "users_email#index", constraints: { username: RouteFormat.username }
       get "#{root_path}/:username/preferences/account" => "users#preferences", constraints: { username: RouteFormat.username }

lib/guardian/user_guardian.rb

@@ -92,7 +92,7 @@ module UserGuardian
     is_admin? || (is_staff? && SiteSetting.moderators_view_emails)
   end
 
-  def can_check_sso_email?(user)
+  def can_check_sso_details?(user)
     user && is_admin?
   end
 

spec/requests/users_controller_spec.rb

@@ -2782,6 +2782,34 @@ describe UsersController do
     end
   end
 
+  describe '#check_sso_payload' do
+    it 'raises an error when not logged in' do
+      get "/u/zogstrip/sso-payload.json"
+      expect(response.status).to eq(403)
+    end
+
+    context 'while logged in' do
+      let(:sign_in_admin) { sign_in(Fabricate(:admin)) }
+      let(:user) { Fabricate(:user) }
+
+      it "raises an error when you aren't allowed to check sso payload" do
+        sign_in(Fabricate(:user))
+        get "/u/#{user.username}/sso-payload.json"
+        expect(response).to be_forbidden
+      end
+
+      it "returns SSO payload when you're allowed to see" do
+        user.single_sign_on_record = SingleSignOnRecord.create(user_id: user.id, external_email: "foobar@example.com", external_id: "example", last_payload: "foobar")
+        sign_in_admin
+
+        get "/u/#{user.username}/sso-payload.json"
+
+        expect(response.status).to eq(200)
+        expect(response.parsed_body["payload"]).to eq("foobar")
+      end
+    end
+  end
+
   describe '#update_primary_email' do
     fab!(:user) { Fabricate(:user) }
     fab!(:user_email) { user.primary_email }