FIX: hide sso email behind a button click and log views (#11186)

This commit is contained in:
Arpit Jalan 2020-11-11 00:42:44 +05:30 committed by GitHub
parent cf4be109e2
commit 00b41437b0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 74 additions and 23 deletions

View File

@ -19,6 +19,7 @@ export default Controller.extend(CanCheckEmails, {
customGroupIdsBuffer: null, customGroupIdsBuffer: null,
availableGroups: null, availableGroups: null,
userTitleValue: null, userTitleValue: null,
ssoExternalEmail: null,
showBadges: setting("enable_badges"), showBadges: setting("enable_badges"),
hasLockedTrustLevel: notEmpty("model.manual_locked_trust_level"), hasLockedTrustLevel: notEmpty("model.manual_locked_trust_level"),
@ -339,5 +340,15 @@ export default Controller.extend(CanCheckEmails, {
} }
); );
}, },
checkSsoEmail() {
return ajax(userPath(`${this.model.username_lower}/sso-email.json`), {
data: { context: window.location.pathname },
}).then((result) => {
if (result) {
this.set("ssoExternalEmail", result.email);
}
});
},
}, },
}); });

View File

@ -671,10 +671,19 @@
<div class="field">{{i18n "admin.user.sso.external_name"}}</div> <div class="field">{{i18n "admin.user.sso.external_name"}}</div>
<div class="value">{{sso.external_name}}</div> <div class="value">{{sso.external_name}}</div>
</div> </div>
{{#if sso.external_email}} {{#if canAdminCheckEmails}}
<div class="display-row"> <div class="display-row">
<div class="field">{{i18n "admin.user.sso.external_email"}}</div> <div class="field">{{i18n "admin.user.sso.external_email"}}</div>
<div class="value">{{sso.external_email}}</div> {{#if ssoExternalEmail}}
<div class="value">{{ssoExternalEmail}}</div>
{{else}}
{{d-button
class="btn-default"
action=(action "checkSsoEmail")
actionParam=model icon="far-envelope"
label="admin.users.check_email.text"
title="admin.users.check_email.title"}}
{{/if}}
</div> </div>
{{/if}} {{/if}}
<div class="display-row"> <div class="display-row">

View File

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

View File

@ -4,12 +4,7 @@ class SingleSignOnRecordSerializer < ApplicationSerializer
attributes :user_id, :external_id, attributes :user_id, :external_id,
:last_payload, :created_at, :last_payload, :created_at,
:updated_at, :external_username, :updated_at, :external_username,
:external_email, :external_name, :external_name, :external_avatar_url,
:external_avatar_url,
:external_profile_background_url, :external_profile_background_url,
:external_card_background_url :external_card_background_url
def include_external_email?
scope.is_admin?
end
end end

View File

@ -2501,6 +2501,7 @@ en:
not_allowed: "is not allowed from that email provider. Please use another email address." not_allowed: "is not allowed from that email provider. Please use another email address."
blocked: "is not allowed." blocked: "is not allowed."
revoked: "Won't be sending emails to '%{email}' until %{date}." revoked: "Won't be sending emails to '%{email}' until %{date}."
does_not_exist: "N/A"
ip_address: ip_address:
blocked: "New registrations are not allowed from your IP address." blocked: "New registrations are not allowed from your IP address."
max_new_accounts_per_registration_ip: "New registrations are not allowed from your IP address (maximum limit reached). Contact a staff member." max_new_accounts_per_registration_ip: "New registrations are not allowed from your IP address (maximum limit reached). Contact a staff member."

View File

@ -446,6 +446,7 @@ Discourse::Application.routes.draw do
get({ "#{root_path}/:username" => "users#show", constraints: { username: RouteFormat.username } }.merge(index == 1 ? { as: 'user' } : {})) get({ "#{root_path}/:username" => "users#show", constraints: { username: RouteFormat.username } }.merge(index == 1 ? { as: 'user' } : {}))
put "#{root_path}/:username" => "users#update", constraints: { username: RouteFormat.username }, defaults: { format: :json } 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/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/preferences" => "users#preferences", 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/email" => "users_email#index", constraints: { username: RouteFormat.username }
get "#{root_path}/:username/preferences/account" => "users#preferences", constraints: { username: RouteFormat.username } get "#{root_path}/:username/preferences/account" => "users#preferences", constraints: { username: RouteFormat.username }

View File

@ -92,6 +92,10 @@ module UserGuardian
is_admin? || (is_staff? && SiteSetting.moderators_view_emails) is_admin? || (is_staff? && SiteSetting.moderators_view_emails)
end end
def can_check_sso_email?(user)
user && is_admin?
end
def restrict_user_fields?(user) def restrict_user_fields?(user)
user.trust_level == TrustLevel[0] && anonymous? user.trust_level == TrustLevel[0] && anonymous?
end end

View File

@ -2667,6 +2667,34 @@ describe UsersController do
end end
end end
describe '#check_sso_email' do
it 'raises an error when not logged in' do
get "/u/zogstrip/sso-email.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 email" do
sign_in(Fabricate(:user))
get "/u/#{user.username}/sso-email.json"
expect(response).to be_forbidden
end
it "returns emails and associated_accounts when you're allowed to see them" do
user.single_sign_on_record = SingleSignOnRecord.create(user_id: user.id, external_email: "foobar@example.com", external_id: "example", last_payload: "looks good")
sign_in_admin
get "/u/#{user.username}/sso-email.json"
expect(response.status).to eq(200)
expect(response.parsed_body["email"]).to eq("foobar@example.com")
end
end
end
describe '#update_primary_email' do describe '#update_primary_email' do
fab!(:user) { Fabricate(:user) } fab!(:user) { Fabricate(:user) }
fab!(:user_email) { user.primary_email } fab!(:user_email) { user.primary_email }

View File

@ -14,20 +14,6 @@ RSpec.describe SingleSignOnRecordSerializer do
SingleSignOnRecordSerializer.new(sso, scope: Guardian.new(admin), root: false) SingleSignOnRecordSerializer.new(sso, scope: Guardian.new(admin), root: false)
end end
it "should include user sso info" do
payload = serializer.as_json
expect(payload[:user_id]).to eq(user.id)
expect(payload[:external_id]).to eq('12345')
expect(payload[:external_email]).to eq(user.email)
end
end
context "moderator" do
fab!(:moderator) { Fabricate(:moderator) }
let :serializer do
SingleSignOnRecordSerializer.new(sso, scope: Guardian.new(moderator), root: false)
end
it "should include user sso info" do it "should include user sso info" do
payload = serializer.as_json payload = serializer.as_json
expect(payload[:user_id]).to eq(user.id) expect(payload[:user_id]).to eq(user.id)