diff --git a/app/assets/javascripts/admin/controllers/admin-user-index.js.es6 b/app/assets/javascripts/admin/controllers/admin-user-index.js.es6 index 8b932957e06..ea3093ff6ce 100644 --- a/app/assets/javascripts/admin/controllers/admin-user-index.js.es6 +++ b/app/assets/javascripts/admin/controllers/admin-user-index.js.es6 @@ -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, diff --git a/app/assets/javascripts/admin/templates/user_index.hbs b/app/assets/javascripts/admin/templates/user_index.hbs index c45f0cf7d65..0959956ffb1 100644 --- a/app/assets/javascripts/admin/templates/user_index.hbs +++ b/app/assets/javascripts/admin/templates/user_index.hbs @@ -32,18 +32,32 @@ -
-
{{i18n user.email.title}}
-
{{email}}
- {{#unless active}} -
{{i18n admin.users.not_verified}}
- {{/unless}} -
+ {{#if canCheckEmails}} +
+
{{i18n user.email.title}}
+
+ {{#unless active}} +
{{i18n admin.users.not_verified}}
+ {{/unless}} + {{#if email}} + {{email}} + {{else}} + + {{/if}} +
+
-
-
{{i18n user.associated_accounts}}
-
{{associated_accounts}}
-
+
+
{{i18n user.associated_accounts}}
+
+ {{#if associated_accounts}} + {{associated_accounts}} + {{else}} + + {{/if}} +
+
+ {{/if}}
{{i18n user.avatar.title}}
diff --git a/app/assets/javascripts/admin/templates/users_list.hbs b/app/assets/javascripts/admin/templates/users_list.hbs index c8ee3b22df8..d37213e9237 100644 --- a/app/assets/javascripts/admin/templates/users_list.hbs +++ b/app/assets/javascripts/admin/templates/users_list.hbs @@ -42,7 +42,6 @@ {{/if}}   {{i18n username}} - {{i18n email}} {{i18n admin.users.last_emailed}} {{i18n last_seen}} {{i18n admin.user.topics_entered}} @@ -67,11 +66,6 @@ {{/if}} {{#link-to 'adminUser' this}}{{avatar this imageSize="small"}}{{/link-to}} {{#link-to 'adminUser' this}}{{unbound username}}{{/link-to}} - {{#if active}} - {{shorten-text email}} - {{else}} - {{shorten-text email}} - {{/if}} {{{unbound last_emailed_age}}} {{{unbound last_seen_age}}} {{{unbound topics_entered}}} diff --git a/app/assets/javascripts/discourse/controllers/preferences.js.es6 b/app/assets/javascripts/discourse/controllers/preferences.js.es6 index 0109ff8bd84..b70f843ebcc 100644 --- a/app/assets/javascripts/discourse/controllers/preferences.js.es6 +++ b/app/assets/javascripts/discourse/controllers/preferences.js.es6 @@ -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'), diff --git a/app/assets/javascripts/discourse/controllers/user.js.es6 b/app/assets/javascripts/discourse/controllers/user.js.es6 index bc4e753e00a..39b00bc03c7 100644 --- a/app/assets/javascripts/discourse/controllers/user.js.es6 +++ b/app/assets/javascripts/discourse/controllers/user.js.es6 @@ -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') - }); diff --git a/app/assets/javascripts/discourse/mixins/can-check-emails.js.es6 b/app/assets/javascripts/discourse/mixins/can-check-emails.js.es6 new file mode 100644 index 00000000000..2fb98297f84 --- /dev/null +++ b/app/assets/javascripts/discourse/mixins/can-check-emails.js.es6 @@ -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"), +}); diff --git a/app/assets/javascripts/discourse/models/user.js b/app/assets/javascripts/discourse/models/user.js index dcd64e7cb2a..a854a14ddb5 100644 --- a/app/assets/javascripts/discourse/models/user.js +++ b/app/assets/javascripts/discourse/models/user.js @@ -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 + }); + } + }); } }); diff --git a/app/assets/javascripts/discourse/routes/application.js.es6 b/app/assets/javascripts/discourse/routes/application.js.es6 index bf0f8078b79..fbafb966f03 100644 --- a/app/assets/javascripts/discourse/routes/application.js.es6 +++ b/app/assets/javascripts/discourse/routes/application.js.es6 @@ -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(); } }, diff --git a/app/assets/javascripts/discourse/templates/user/preferences.hbs b/app/assets/javascripts/discourse/templates/user/preferences.hbs index 0b8a075e8a2..b5a1c83f45f 100644 --- a/app/assets/javascripts/discourse/templates/user/preferences.hbs +++ b/app/assets/javascripts/discourse/templates/user/preferences.hbs @@ -47,18 +47,26 @@
{{/if}} -
- -
- {{email}} - {{#if can_edit_email}} - {{#link-to "preferences.email" class="btn btn-small pad-left no-text"}}{{/link-to}} + {{#if canCheckEmails}} +
+ + {{#if email}} +
+ {{email}} + {{#if can_edit_email}} + {{#link-to "preferences.email" class="btn btn-small pad-left no-text"}}{{/link-to}} + {{/if}} +
+
+ {{i18n user.email.instructions}} +
+ {{else}} +
+ +
{{/if}}
-
- {{i18n user.email.instructions}} -
-
+ {{/if}} {{#if canChangePassword}}
diff --git a/app/assets/javascripts/discourse/templates/user/preferences/_save_button.hbs b/app/assets/javascripts/discourse/templates/user/preferences/_save_button.hbs index 787624fedd4..ce4c3df3672 100644 --- a/app/assets/javascripts/discourse/templates/user/preferences/_save_button.hbs +++ b/app/assets/javascripts/discourse/templates/user/preferences/_save_button.hbs @@ -1,2 +1,2 @@ - + {{#if saved}}{{i18n saved}}{{/if}} diff --git a/app/assets/javascripts/discourse/templates/user/user.hbs b/app/assets/javascripts/discourse/templates/user/user.hbs index a0c6627e9c3..b613dc16f26 100644 --- a/app/assets/javascripts/discourse/templates/user/user.hbs +++ b/app/assets/javascripts/discourse/templates/user/user.hbs @@ -144,8 +144,15 @@ {{#if invited_by}}
{{i18n user.invited_by}}
{{#link-to 'user' invited_by}}{{invited_by.username}}{{/link-to}}
{{/if}} - {{#if showEmail}} -
{{i18n user.email.title}}
{{email}}
+ {{#if canCheckEmails}} +
{{i18n user.email.title}}
+
+ {{#if email}} + {{email}} + {{else}} + + {{/if}} +
{{/if}}
{{i18n user.trust_level}}
{{trustLevel.name}}
diff --git a/app/assets/javascripts/main_include.js b/app/assets/javascripts/main_include.js index d1d93bef18f..d3ee8aada6e 100644 --- a/app/assets/javascripts/main_include.js +++ b/app/assets/javascripts/main_include.js @@ -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 diff --git a/app/assets/stylesheets/common/admin/admin_base.scss b/app/assets/stylesheets/common/admin/admin_base.scss index 45c3c7ff716..390e1454c9a 100644 --- a/app/assets/stylesheets/common/admin/admin_base.scss +++ b/app/assets/stylesheets/common/admin/admin_base.scss @@ -308,7 +308,7 @@ section.details { } .display-row.associations .value { - width: 800px; + width: 750px; } .display-row { diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index e541b1a000d..ea1298e377c 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -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) diff --git a/app/models/user.rb b/app/models/user.rb index 80dc73dd245..b00762de5b5 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -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})" diff --git a/app/models/user_history.rb b/app/models/user_history.rb index 6f3c393cc54..db5e4202334 100644 --- a/app/models/user_history.rb +++ b/app/models/user_history.rb @@ -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 diff --git a/app/serializers/admin_user_serializer.rb b/app/serializers/admin_user_serializer.rb index 169ecd301ba..3a765e20aa2 100644 --- a/app/serializers/admin_user_serializer.rb +++ b/app/serializers/admin_user_serializer.rb @@ -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 diff --git a/app/serializers/user_serializer.rb b/app/serializers/user_serializer.rb index 8f916c9e964..f6f4f8f8057 100644 --- a/app/serializers/user_serializer.rb +++ b/app/serializers/user_serializer.rb @@ -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 diff --git a/app/services/staff_action_logger.rb b/app/services/staff_action_logger.rb index adeb51edcb1..5da07806f38 100644 --- a/app/services/staff_action_logger.rb +++ b/app/services/staff_action_logger.rb @@ -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) diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index e9d6ce46ab5..f6cbf7c12d1 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -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}}" diff --git a/config/routes.rb b/config/routes.rb index 9d734b34905..1b8d92f8940 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -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} diff --git a/lib/admin_user_index_query.rb b/lib/admin_user_index_query.rb index 1568e1ff90a..a8c3b3e1f96 100644 --- a/lib/admin_user_index_query.rb +++ b/lib/admin_user_index_query.rb @@ -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 diff --git a/lib/guardian/user_guardian.rb b/lib/guardian/user_guardian.rb index af6596b9bb8..ba8689f6e1c 100644 --- a/lib/guardian/user_guardian.rb +++ b/lib/guardian/user_guardian.rb @@ -47,4 +47,8 @@ module UserGuardian end end + def can_check_emails?(user) + is_admin? || (is_staff? && SiteSetting.show_email_on_profile) + end + end diff --git a/spec/components/admin_user_index_query_spec.rb b/spec/components/admin_user_index_query_spec.rb index fc312173f29..56f116fbe60 100644 --- a/spec/components/admin_user_index_query_spec.rb +++ b/spec/components/admin_user_index_query_spec.rb @@ -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") } diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 7d72c48aacd..bdb085bbf83 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -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