Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

FEATURE: hide emails behind a button for staff members

This commit is contained in:
Régis Hanol 2014-09-29 22:31:05 +02:00
parent ff795a267d
commit 7e309a21cf
25 changed files with 181 additions and 86 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
app/assets/javascripts/admin/controllers/admin-user-index.js.es6
View File

@ -1,4 +1,5 @@
import ObjectController from 'discourse/controllers/object';
import CanCheckEmails from 'discourse/mixins/can-check-emails';
/**
A controller related to viewing a user in the admin section
@ -8,7 +9,7 @@ import ObjectController from 'discourse/controllers/object';
@namespace Discourse
@module Discourse
**/
export default ObjectController.extend({
export default ObjectController.extend(CanCheckEmails, {
editingTitle: false,
originalPrimaryGroupId: null,
availableGroups: null,

36
app/assets/javascripts/admin/templates/user_index.hbs
View File

@ -32,18 +32,32 @@
</div>
</div>
<div class='display-row email'>
<div class='field'>{{i18n user.email.title}}</div>
<div class='value'><a href="mailto:{{unbound email}}">{{email}}</a></div>
{{#unless active}}
<div class='controls'>{{i18n admin.users.not_verified}}</div>
{{/unless}}
</div>
{{#if canCheckEmails}}
<div class='display-row email'>
<div class='field'>{{i18n user.email.title}}</div>
<div class='value'>
{{#unless active}}
<div class='controls'>{{i18n admin.users.not_verified}}</div>
{{/unless}}
{{#if email}}
<a href="mailto:{{unbound email}}">{{email}}</a>
{{else}}
<button class="btn" title="{{i18n admin.users.check_email.title}}" {{action checkEmail this}}>{{fa-icon "envelope-o"}} {{i18n admin.users.check_email.text}}</button>
{{/if}}
</div>
</div>
<div class='display-row associations'>
<div class='field'>{{i18n user.associated_accounts}}</div>
<div class='value'>{{associated_accounts}}</div>
</div>
<div class='display-row associations'>
<div class='field'>{{i18n user.associated_accounts}}</div>
<div class='value'>
{{#if associated_accounts}}
{{associated_accounts}}
{{else}}
<button class="btn" title="{{i18n admin.users.check_email.title}}" {{action checkEmail this}}>{{fa-icon "envelope-o"}} {{i18n admin.users.check_email.text}}</button>
{{/if}}
</div>
</div>
{{/if}}
<div class='display-row'>
<div class='field'>{{i18n user.avatar.title}}</div>

6
app/assets/javascripts/admin/templates/users_list.hbs
View File

@ -42,7 +42,6 @@
{{/if}}
<th>&nbsp;</th>
<th>{{i18n username}}</th>
<th>{{i18n email}}</th>
<th>{{i18n admin.users.last_emailed}}</th>
<th>{{i18n last_seen}}</th>
<th>{{i18n admin.user.topics_entered}}</th>
@ -67,11 +66,6 @@
{{/if}}
<td>{{#link-to 'adminUser' this}}{{avatar this imageSize="small"}}{{/link-to}}</td>
<td>{{#link-to 'adminUser' this}}{{unbound username}}{{/link-to}}</td>
{{#if active}}
<td>{{shorten-text email}}</td>
{{else}}
<td title="{{i18n admin.users.not_verified}}">{{shorten-text email}}</td>
{{/if}}
<td>{{{unbound last_emailed_age}}}</td>
<td>{{{unbound last_seen_age}}}</td>
<td>{{{unbound topics_entered}}}</td>

9
app/assets/javascripts/discourse/controllers/preferences.js.es6
View File

@ -1,6 +1,7 @@
import ObjectController from 'discourse/controllers/object';
import CanCheckEmails from 'discourse/mixins/can-check-emails';
export default ObjectController.extend({
export default ObjectController.extend(CanCheckEmails, {
allowAvatarUpload: Discourse.computed.setting('allow_uploaded_avatars'),
allowUserLocale: Discourse.computed.setting('allow_user_locale'),
@ -17,12 +18,6 @@ export default ObjectController.extend({
newNameInput: null,
saveDisabled: function() {
if (this.get('saving')) return true;
if (this.blank('email')) return true;
return false;
}.property('saving', 'email'),
cannotDeleteAccount: Em.computed.not('can_delete_account'),
deleteDisabled: Em.computed.or('saving', 'deleting', 'cannotDeleteAccount'),

8
app/assets/javascripts/discourse/controllers/user.js.es6
View File

@ -1,6 +1,7 @@
import ObjectController from 'discourse/controllers/object';
import CanCheckEmails from 'discourse/mixins/can-check-emails';
export default ObjectController.extend({
export default ObjectController.extend(CanCheckEmails, {
viewingSelf: function() {
return this.get('content.username') === Discourse.User.currentProp('username');
@ -8,10 +9,6 @@ export default ObjectController.extend({
collapsedInfo: Em.computed.not('indexStream'),
showEmailOnProfile: Discourse.computed.setting('show_email_on_profile'),
showEmail: Ember.computed.and('email', 'showEmailOnProfile'),
websiteName: function() {
var website = this.get('website');
if (Em.isEmpty(website)) { return; }
@ -47,5 +44,4 @@ export default ObjectController.extend({
privateMessagesActive: Em.computed.equal('pmView', 'index'),
privateMessagesMineActive: Em.computed.equal('pmView', 'mine'),
privateMessagesUnreadActive: Em.computed.equal('pmView', 'unread')
});

7
app/assets/javascripts/discourse/mixins/can-check-emails.js.es6 Normal file
View File

@ -0,0 +1,7 @@
export default Ember.Mixin.create({
isOwnEmail: Discourse.computed.propertyEqual("id", "currentUser.id"),
showEmailOnProfile: Discourse.computed.setting("show_email_on_profile"),
canStaffCheckEmails: Em.computed.and("showEmailOnProfile", "currentUser.staff"),
canAdminCheckEmails: Em.computed.alias("currentUser.admin"),
canCheckEmails: Em.computed.or("isOwnEmail", "canStaffCheckEmails", "canAdminCheckEmails"),
});

16
app/assets/javascripts/discourse/models/user.js
View File

@ -411,6 +411,22 @@ Discourse.User = Discourse.Model.extend({
type: 'PUT',
data: { dismissed_banner_key: bannerKey }
});
},
checkEmail: function () {
var self = this;
return Discourse.ajax("/users/" + this.get("username_lower") + "/emails.json", {
type: "PUT",
data: { context: window.location.pathname }
})
.then(function (result) {
if (result) {
self.setProperties({
email: result.email,
associated_accounts: result.associated_accounts
});
}
});
}
});

4
app/assets/javascripts/discourse/routes/application.js.es6
View File

@ -146,6 +146,10 @@ var ApplicationRoute = Em.Route.extend({
deleteSpammer: function (user) {
this.send('closeModal');
user.deleteAsSpammer(function() { window.location.reload(); });
},
checkEmail: function (user) {
user.checkEmail();
}
},

28
app/assets/javascripts/discourse/templates/user/preferences.hbs
View File

@ -47,18 +47,26 @@
</div>
{{/if}}
<div class="control-group pref-email">
<label class="control-label">{{i18n user.email.title}}</label>
<div class="controls">
<span class='static'>{{email}}</span>
{{#if can_edit_email}}
{{#link-to "preferences.email" class="btn btn-small pad-left no-text"}}<i class="fa fa-pencil"></i>{{/link-to}}
{{#if canCheckEmails}}
<div class="control-group pref-email">
<label class="control-label">{{i18n user.email.title}}</label>
{{#if email}}
<div class="controls">
<span class='static'>{{email}}</span>
{{#if can_edit_email}}
{{#link-to "preferences.email" class="btn btn-small pad-left no-text"}}<i class="fa fa-pencil"></i>{{/link-to}}
{{/if}}
</div>
<div class='instructions'>
{{i18n user.email.instructions}}
</div>
{{else}}
<div class="controls">
<button class="btn" title="{{i18n admin.users.check_email.title}}" {{action checkEmail this}}>{{fa-icon "envelope-o"}} {{i18n admin.users.check_email.text}}</button>
</div>
{{/if}}
</div>
<div class='instructions'>
{{i18n user.email.instructions}}
</div>
</div>
{{/if}}
{{#if canChangePassword}}
<div class="control-group pref-password">

2
app/assets/javascripts/discourse/templates/user/preferences/_save_button.hbs
View File

@ -1,2 +1,2 @@
<button {{action save}} {{bind-attr disabled="saveDisabled"}} class="btn btn-primary">{{saveButtonText}}</button>
<button {{action save}} {{bind-attr disabled="saving"}} class="btn btn-primary">{{saveButtonText}}</button>
{{#if saved}}{{i18n saved}}{{/if}}

11
app/assets/javascripts/discourse/templates/user/user.hbs
View File

@ -144,8 +144,15 @@
{{#if invited_by}}
<dt>{{i18n user.invited_by}}</dt><dd>{{#link-to 'user' invited_by}}{{invited_by.username}}{{/link-to}}</dd>
{{/if}}
{{#if showEmail}}
<dt>{{i18n user.email.title}}</dt><dd {{bind-attr title="email"}}>{{email}}</dd>
{{#if canCheckEmails}}
<dt>{{i18n user.email.title}}</dt>
<dd {{bind-attr title="email"}}>
{{#if email}}
{{email}}
{{else}}
<button class="btn" title="{{i18n admin.users.check_email.title}}" {{action checkEmail this}}>{{fa-icon "envelope-o"}} {{i18n admin.users.check_email.text}}</button>
{{/if}}
</dd>
{{/if}}
<dt>{{i18n user.trust_level}}</dt><dd>{{trustLevel.name}}</dd>
</dl>

2
app/assets/javascripts/main_include.js
View File

@ -8,10 +8,10 @@
//= require highlight.pack.js
// Stuff we need to load first
//= require ./discourse/lib/computed
//= require ./discourse/mixins/scrolling
//= require_tree ./discourse/mixins
//= require ./discourse/lib/markdown
//= require ./discourse/lib/computed
//= require ./discourse/lib/search-for-term
//= require ./discourse/views/view
//= require ./discourse/views/container

2
app/assets/stylesheets/common/admin/admin_base.scss
View File

@ -308,7 +308,7 @@ section.details {
}
.display-row.associations .value {
width: 800px;
width: 750px;
}
.display-row {

16
app/controllers/users_controller.rb
View File

@ -8,7 +8,7 @@ class UsersController < ApplicationController
skip_before_filter :authorize_mini_profiler, only: [:avatar]
skip_before_filter :check_xhr, only: [:show, :password_reset, :update, :account_created, :activate_account, :perform_account_activation, :authorize_email, :user_preferences_redirect, :avatar, :my_redirect]
before_filter :ensure_logged_in, only: [:username, :update, :change_email, :user_preferences_redirect, :upload_user_image, :pick_avatar, :destroy_user_image, :destroy]
before_filter :ensure_logged_in, only: [:username, :update, :change_email, :user_preferences_redirect, :upload_user_image, :pick_avatar, :destroy_user_image, :destroy, :check_emails]
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
@ -64,6 +64,20 @@ class UsersController < ApplicationController
render nothing: true
end
def check_emails
user = fetch_user_from_params
guardian.ensure_can_check_emails!(user)
StaffActionLogger.new(current_user).log_check_email(user, context: params[:context])
render json: {
email: user.email,
associated_accounts: user.associated_accounts
}
rescue Discourse::InvalidAccess => e
render json: failed_json, status: 403
end
def badge_title
params.require(:user_badge_id)

18
app/models/user.rb
View File

@ -636,21 +636,11 @@ class User < ActiveRecord::Base
def associated_accounts
result = []
if twitter_user_info
result << "Twitter(#{twitter_user_info.screen_name})"
end
if facebook_user_info
result << "Facebook(#{facebook_user_info.username})"
end
if google_user_info
result << "Google(#{google_user_info.email})"
end
if github_user_info
result << "Github(#{github_user_info.screen_name})"
end
result << "Twitter(#{twitter_user_info.screen_name})" if twitter_user_info
result << "Facebook(#{facebook_user_info.username})" if facebook_user_info
result << "Google(#{google_user_info.email})" if google_user_info
result << "Github(#{github_user_info.screen_name})" if github_user_info
user_open_ids.each do |oid|
result << "OpenID #{oid.url[0..20]}...(#{oid.email})"

6
app/models/user_history.rb
View File

@ -26,7 +26,8 @@ class UserHistory < ActiveRecord::Base
:facebook_no_email,
:grant_badge,
:revoke_badge,
:auto_trust_level_change)
:auto_trust_level_change,
:check_email)
end
# Staff actions is a subset of all actions, used to audit actions taken by staff users.
@ -39,7 +40,8 @@ class UserHistory < ActiveRecord::Base
:suspend_user,
:unsuspend_user,
:grant_badge,
:revoke_badge]
:revoke_badge,
:check_email]
end
def self.staff_action_ids

7
app/serializers/admin_user_serializer.rb
View File

@ -36,6 +36,13 @@ class AdminUserSerializer < BasicUserSerializer
end
end
def include_email?
# staff members can always see their email
scope.is_staff? && object.id == scope.user.id
end
alias_method :include_associated_accounts?, :include_email?
def suspended
object.suspended?
end

9
app/serializers/user_serializer.rb
View File

@ -52,16 +52,13 @@ class UserSerializer < BasicUserSerializer
has_many :custom_groups, embed: :object, serializer: BasicGroupSerializer
has_many :featured_user_badges, embed: :ids, serializer: UserBadgeSerializer, root: :user_badges
staff_attributes :number_of_deleted_posts,
:number_of_flagged_posts,
:number_of_flags_given,
:number_of_suspensions,
:number_of_warnings
private_attributes :email,
:locale,
private_attributes :locale,
:email_digests,
:email_private_messages,
:email_direct,
@ -87,6 +84,10 @@ class UserSerializer < BasicUserSerializer
### ATTRIBUTES
###
def include_email?
object.id && object.id == scope.user.try(:id)
end
def bio_raw
object.user_profile.bio_raw
end

8
app/services/staff_action_logger.rb
View File

@ -92,6 +92,14 @@ class StaffActionLogger
}))
end
def log_check_email(user, opts={})
raise Discourse::InvalidParameters.new('user is nil') unless user
UserHistory.create(params(opts).merge({
action: UserHistory.actions[:check_email],
target_user_id: user.id
}))
end
private
def params(opts)

6
config/locales/client.en.yml
View File

@ -574,7 +574,7 @@ en:
created: 'Created'
created_lowercase: 'created'
trust_level: 'Trust Level'
search_hint: 'username or email'
search_hint: 'username'
create_account:
title: "Create New Account"
@ -1801,6 +1801,7 @@ en:
unsuspend_user: "unsuspend user"
grant_badge: "grant badge"
revoke_badge: "revoke badge"
check_email: "check email"
screened_emails:
title: "Screened Emails"
description: "When someone tries to create a new account, the following email addresses will be checked and the registration will be blocked, or some other action performed."
@ -1872,6 +1873,9 @@ en:
one: "Failed to reject 1 user."
other: "Failed to reject %{count} users."
not_verified: "Not verified"
check_email:
title: "Click this button to retrieve this user's email"
text: "Retrieve Email"
user:
suspend_failed: "Something went wrong suspending this user {{error}}"

1
config/routes.rb
View File

@ -219,6 +219,7 @@ Discourse::Application.routes.draw do
get "users/:username/private-messages/:filter" => "user_actions#private_messages", constraints: {username: USERNAME_ROUTE_FORMAT}
get "users/:username" => "users#show", as: 'user', constraints: {username: USERNAME_ROUTE_FORMAT}
put "users/:username" => "users#update", constraints: {username: USERNAME_ROUTE_FORMAT}
put "users/:username/emails" => "users#check_emails", constraints: {username: USERNAME_ROUTE_FORMAT}
get "users/:username/preferences" => "users#preferences", constraints: {username: USERNAME_ROUTE_FORMAT}, as: :email_preferences
get "users/:username/preferences/email" => "users#preferences", constraints: {username: USERNAME_ROUTE_FORMAT}
put "users/:username/preferences/email" => "users#change_email", constraints: {username: USERNAME_ROUTE_FORMAT}

17
lib/admin_user_index_query.rb
View File

@ -26,17 +26,17 @@ class AdminUserIndexQuery
def filter_by_query_classification
case params[:query]
when 'admins' then @query.where('admin = ?', true)
when 'moderators' then @query.where('moderator = ?', true)
when 'admins' then @query.where(admin: true)
when 'moderators' then @query.where(moderator: true)
when 'blocked' then @query.blocked
when 'suspended' then @query.suspended
when 'pending' then @query.not_suspended.where('approved = false')
when 'pending' then @query.not_suspended.where(approved: false)
end
end
def filter_by_search
if params[:filter].present?
@query.where('username_lower ILIKE :filter or email ILIKE :filter', filter: "%#{params[:filter]}%")
@query.where('username_lower ILIKE :filter', filter: "%#{params[:filter]}%")
end
end
@ -67,6 +67,13 @@ class AdminUserIndexQuery
end
def find_users
find_users_query.includes(:user_stat).take(100)
find_users_query.includes(:user_stat)
.includes(:single_sign_on_record)
.includes(:facebook_user_info)
.includes(:twitter_user_info)
.includes(:github_user_info)
.includes(:google_user_info)
.includes(:oauth2_user_info)
.take(100)
end
end

4
lib/guardian/user_guardian.rb
View File

@ -47,4 +47,8 @@ module UserGuardian
end
end
def can_check_emails?(user)
is_admin? || (is_staff? && SiteSetting.show_email_on_profile)
end
end

13
spec/components/admin_user_index_query_spec.rb
View File

@ -95,19 +95,6 @@ describe AdminUserIndexQuery do
end
describe "filtering" do
context "by email fragment" do
before(:each) { Fabricate(:user, email: "test1@example.com") }
it "matches the email" do
query = ::AdminUserIndexQuery.new({ filter: "est1" })
expect(query.find_users.count).to eq(1)
end
it "matches the email using any case" do
query = ::AdminUserIndexQuery.new({ filter: "Test1" })
expect(query.find_users.count).to eq(1)
end
end
context "by username fragment" do
before(:each) { Fabricate(:user, username: "test_user_1") }

28
spec/controllers/users_controller_spec.rb
View File

@ -1245,4 +1245,32 @@ describe UsersController do
end
end
describe '.check_emails' do
it 'raises an error when not logged in' do
lambda { xhr :get, :check_emails, username: 'zogstrip' }.should raise_error(Discourse::NotLoggedIn)
end
context 'while logged in' do
let!(:user) { log_in }
it "raises an error when you aren't allowed to check emails" do
Guardian.any_instance.expects(:can_check_emails?).returns(false)
xhr :get, :check_emails, username: Fabricate(:user).username
response.should be_forbidden
end
it "returns both email and associated_accounts when you're allowed to see them" do
Guardian.any_instance.expects(:can_check_emails?).returns(true)
xhr :get, :check_emails, username: Fabricate(:user).username
response.should be_success
json = JSON.parse(response.body)
json["email"].should be_present
json["associated_accounts"].should be_present
end
end
end
end