From 939e8505a99c5071b42e1432af5f72a462ade285 Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Wed, 16 Jul 2014 12:25:24 -0400 Subject: [PATCH] Remove hub username integration --- app/controllers/users_controller.rb | 6 +- app/models/admin_dashboard_data.rb | 7 +- app/models/invite_redeemer.rb | 7 - app/models/site_setting.rb | 4 - app/models/user.rb | 13 +- app/services/user_activator.rb | 10 +- app/services/user_destroyer.rb | 1 - app/services/username_checker_service.rb | 59 +--- config/application.rb | 3 +- config/locales/server.en.yml | 6 - config/site_settings.yml | 8 +- lib/discourse_hub.rb | 76 +---- spec/components/discourse_hub_spec.rb | 94 ------ spec/controllers/users_controller_spec.rb | 275 ++++-------------- spec/models/admin_dashboard_data_spec.rb | 22 -- spec/models/site_setting_spec.rb | 26 -- spec/models/user_spec.rb | 7 +- spec/services/user_destroyer_spec.rb | 22 -- .../services/username_checker_service_spec.rb | 107 +------ 19 files changed, 77 insertions(+), 676 deletions(-) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index abc0b53191e..c624fd6fb60 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -126,7 +126,7 @@ class UsersController < ApplicationController def check_username if !params[:username].present? params.require(:username) if !params[:email].present? - return render(json: success_json) unless SiteSetting.call_discourse_hub? + return render(json: success_json) end username = params[:username] @@ -138,8 +138,6 @@ class UsersController < ApplicationController checker = UsernameCheckerService.new email = params[:email] || target_user.try(:email) render json: checker.check_username(username, email) - rescue RestClient::Forbidden - render json: {errors: [I18n.t("discourse_hub.access_token_problem")]} end def user_from_params_or_current_user @@ -195,8 +193,6 @@ class UsersController < ApplicationController success: false, message: I18n.t("login.something_already_taken") } - rescue DiscourseHub::UsernameUnavailable => e - render json: e.response_message rescue RestClient::Forbidden render json: { errors: [I18n.t("discourse_hub.access_token_problem")] } end diff --git a/app/models/admin_dashboard_data.rb b/app/models/admin_dashboard_data.rb index e6dbd86966f..e182de003ba 100644 --- a/app/models/admin_dashboard_data.rb +++ b/app/models/admin_dashboard_data.rb @@ -43,8 +43,7 @@ class AdminDashboardData site_description_check, access_password_removal, site_contact_username_check, - notification_email_check, - enforce_global_nicknames_check + notification_email_check ].compact end @@ -186,10 +185,6 @@ class AdminDashboardData I18n.t('dashboard.ruby_version_warning') if RUBY_VERSION == '2.0.0' and RUBY_PATCHLEVEL < 247 end - def enforce_global_nicknames_check - I18n.t('dashboard.enforce_global_nicknames_warning') if SiteSetting.enforce_global_nicknames and !SiteSetting.discourse_org_access_key.present? - end - # TODO: generalize this method of putting i18n keys with expiry in redis # that should be reported on the admin dashboard: def access_password_removal diff --git a/app/models/invite_redeemer.rb b/app/models/invite_redeemer.rb index c7eb58711c9..a722c80e94d 100644 --- a/app/models/invite_redeemer.rb +++ b/app/models/invite_redeemer.rb @@ -29,16 +29,9 @@ InviteRedeemer = Struct.new(:invite, :username, :name) do end available_name = name || available_username - DiscourseHub.username_operation do - match, available, suggestion = DiscourseHub.username_match?(available_username, invite.email) - available_username = suggestion unless match || available - end - user = User.new(email: invite.email, username: available_username, name: available_name, active: true, trust_level: SiteSetting.default_invitee_trust_level) user.save! - DiscourseHub.username_operation { DiscourseHub.register_username(username, invite.email) } - user end diff --git a/app/models/site_setting.rb b/app/models/site_setting.rb index 98dfe84f73c..108230589fb 100644 --- a/app/models/site_setting.rb +++ b/app/models/site_setting.rb @@ -29,10 +29,6 @@ class SiteSetting < ActiveRecord::Base LocaleSiteSetting.values.map{ |e| e[:value] }.join('|') end - def self.call_discourse_hub? - self.enforce_global_nicknames? && self.discourse_org_access_key.present? - end - def self.topic_title_length min_topic_title_length..max_topic_title_length end diff --git a/app/models/user.rb b/app/models/user.rb index 856354fe85a..8df23499ff0 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -106,14 +106,8 @@ class User < ActiveRecord::Base LAST_VISIT = -2 end - GLOBAL_USERNAME_LENGTH_RANGE = 3..15 - def self.username_length - if SiteSetting.enforce_global_nicknames - GLOBAL_USERNAME_LENGTH_RANGE - else - SiteSetting.min_username_length.to_i..SiteSetting.max_username_length.to_i - end + SiteSetting.min_username_length.to_i..SiteSetting.max_username_length.to_i end def custom_groups @@ -176,11 +170,6 @@ class User < ActiveRecord::Base def change_username(new_username) current_username = self.username self.username = new_username - - if current_username.downcase != new_username.downcase && valid? - DiscourseHub.username_operation { DiscourseHub.change_username(current_username, new_username) } - end - save end diff --git a/app/services/user_activator.rb b/app/services/user_activator.rb index 7d20c6d7544..d96c8ca981c 100644 --- a/app/services/user_activator.rb +++ b/app/services/user_activator.rb @@ -6,13 +6,10 @@ class UserActivator @session = session @cookies = cookies @request = request - @settings = SiteSetting - @hub = DiscourseHub @message = nil end def start - register_username end def finish @@ -26,7 +23,7 @@ class UserActivator end def factory - if @settings.must_approve_users? + if SiteSetting.must_approve_users? ApprovalActivator elsif !user.active? EmailActivator @@ -35,11 +32,6 @@ class UserActivator end end - def register_username - if user.valid? && @settings.call_discourse_hub? - @hub.register_username(user.username, user.email) - end - end end class ApprovalActivator < UserActivator diff --git a/app/services/user_destroyer.rb b/app/services/user_destroyer.rb index f2d5505cedd..55f88a00458 100644 --- a/app/services/user_destroyer.rb +++ b/app/services/user_destroyer.rb @@ -61,7 +61,6 @@ class UserDestroyer end StaffActionLogger.new(@actor == user ? Discourse.system_user : @actor).log_user_deletion(user, opts.slice(:context)) - DiscourseHub.unregister_username(user.username) if SiteSetting.call_discourse_hub? MessageBus.publish "/file-change", ["refresh"], user_ids: [user.id] end end diff --git a/app/services/username_checker_service.rb b/app/services/username_checker_service.rb index 83f9562db53..caeea977aef 100644 --- a/app/services/username_checker_service.rb +++ b/app/services/username_checker_service.rb @@ -5,54 +5,13 @@ class UsernameCheckerService validator = UsernameValidator.new(username) if !validator.valid_format? {errors: validator.errors} - elsif !SiteSetting.call_discourse_hub? - check_username_locally(username) else - check_username_with_hub_server(username, email) - end - elsif email and SiteSetting.call_discourse_hub? - username_from_hub = DiscourseHub.username_for_email(email) - if username_from_hub && User.username_available?(username_from_hub) - {suggestion: username_from_hub} - else - {suggestion: nil} + check_username_availability(username) end end end - # Contact the Discourse Hub server - def check_username_with_hub_server(username, email) - available_locally = User.username_available?(username) - info = available_globally_and_suggestion_from_hub(username, email) - available_globally = info[:available_globally] - suggestion_from_discourse_hub = info[:suggestion_from_discourse_hub] - global_match = info[:global_match] - if available_globally && available_locally - { available: true, global_match: (global_match ? true : false) } - elsif available_locally && !available_globally - if email.present? - # Nickname and email do not match what's registered on the discourse hub. - { available: false, global_match: false, suggestion: suggestion_from_discourse_hub } - else - # The username is available locally, but is registered on the discourse hub. - # We need an email to see if the username belongs to this person. - # Don't give a suggestion until we get the email and try to match it with on the discourse hub. - { available: false } - end - elsif available_globally && !available_locally - # Already registered on this site with the matching username and email address. Why are you signing up again? - { available: false, suggestion: UserNameSuggester.suggest(username) } - else - # Not available anywhere. - render_unavailable_with_suggestion(suggestion_from_discourse_hub) - end - end - - def render_unavailable_with_suggestion(suggestion) - { available: false, suggestion: suggestion } - end - - def check_username_locally(username) + def check_username_availability(username) if User.username_available?(username) { available: true } else @@ -60,18 +19,4 @@ class UsernameCheckerService end end - def available_globally_and_suggestion_from_hub(username, email) - if email.present? - global_match, available, suggestion = - DiscourseHub.username_match?(username, email) - { available_globally: available || global_match, - suggestion_from_discourse_hub: suggestion, - global_match: global_match } - else - args = DiscourseHub.username_available?(username) - { available_globally: args[0], - suggestion_from_discourse_hub: args[1], - global_match: false } - end - end end diff --git a/config/application.rb b/config/application.rb index 1922a73d5a5..b7b75d5a377 100644 --- a/config/application.rb +++ b/config/application.rb @@ -93,8 +93,7 @@ module Discourse :s3_secret_access_key, :twitter_consumer_secret, :facebook_app_secret, - :github_client_secret, - :discourse_org_access_key, + :github_client_secret ] # Enable the asset pipeline diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index f9472290fa4..131a6466c2c 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -574,7 +574,6 @@ en: access_password_removal: "Your site was using the access_password setting, which has been removed. The login_required and must_approve_users settings have been enabled, which should be used instead. You can change them in the Site Settings. Be sure to approve users in the Pending Users list. (This message will go away after 2 days.)" site_contact_username_warning: "The site_contact_username setting is blank. Please update it in the Site Settings. Set it to the username of an admin user who should be the sender of system messages." notification_email_warning: "The notification_email setting is blank. Please update it in the Site Settings." - enforce_global_nicknames_warning: "The enforce_global_nicknames setting is checked, but the discourse_org_access_key is blank. A Discourse.org access key is required to use the enforce_global_nicknames setting. Please update your Site Settings." content_types: education_new_reply: @@ -640,8 +639,6 @@ en: uncategorized_description: "The description of the uncategorized category. Leave blank for no description." allow_duplicate_topic_titles: "Allow topics with identical, duplicate titles." unique_posts_mins: "How many minutes before a user can make a post with the same content again" - enforce_global_nicknames: "Enforce global username uniqueness (WARNING: only change this during initial setup)" - discourse_org_access_key: "The access key used to access the Discourse Hub username registry at discourse.org" educate_until_posts: "When the user starts typing their first (n) new posts, show the pop-up new user education panel in the composer." title: "Brief title of this site, used in the title tag." site_description: "Describe this site in one sentence, used in the meta description tag." @@ -1606,9 +1603,6 @@ en: If the above link is not clickable, try copying and pasting it into the address bar of your web browser. - discourse_hub: - access_token_problem: "Tell an admin: Please update the site settings to include the correct discourse_org_access_key." - page_not_found: title: "The page you requested doesn't exist or is private." popular_topics: "Popular" diff --git a/config/site_settings.yml b/config/site_settings.yml index 255baac5ebd..f9f012f8b96 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -246,8 +246,12 @@ users: default: 8 min: 1 block_common_passwords: true - enforce_global_nicknames: false - discourse_org_access_key: '' + enforce_global_nicknames: + default: false + hidden: true + discourse_org_access_key: + default: '' + hidden: true username_change_period: 3 auto_track_topics_after: 240000 email_editable: true diff --git a/lib/discourse_hub.rb b/lib/discourse_hub.rb index 9087f782c9c..e36768620d2 100644 --- a/lib/discourse_hub.rb +++ b/lib/discourse_hub.rb @@ -3,63 +3,6 @@ require_dependency 'version' module DiscourseHub - class UsernameUnavailable < RuntimeError - def initialize(username) - @username = username - end - - def response_message - { - success: false, - message: I18n.t( - "login.errors", - errors:I18n.t( - "login.not_available", suggestion: UserNameSuggester.suggest(@username) - ) - ) - } - end - - end - - def self.username_available?(username) - json = get('/users/username_available', {username: username}) - [json['available'], json['suggestion']] - end - - def self.username_match?(username, email) - json = get('/users/username_match', {username: username, email: email}) - [json['match'], json['available'] || false, json['suggestion']] - end - - def self.username_for_email(email) - json = get('/users/username_match', {email: email}) - json['suggestion'] - end - - def self.register_username(username, email) - json = post('/users', {username: username, email: email}) - if json.has_key?('success') - true - else - raise UsernameUnavailable.new(username) # TODO: report ALL the errors - end - end - - def self.unregister_username(username) - json = delete('/memberships/' + username) - json.has_key?('success') - end - - def self.change_username(current_username, new_username) - json = put("/users/#{current_username}/username", {username: new_username}) - if json.has_key?('success') - true - else - raise UsernameUnavailable.new(new_username) # TODO: report ALL the errors - end - end - def self.version_check_payload { installed_version: Discourse::VERSION::STRING, @@ -103,11 +46,11 @@ module DiscourseHub end def self.singular_action(action, rel_url, params={}) - JSON.parse RestClient.send(action, "#{hub_base_url}#{rel_url}", {params: {access_token: access_token}.merge(params), accept: accepts } ) + JSON.parse RestClient.send(action, "#{hub_base_url}#{rel_url}", {params: params, accept: accepts } ) end def self.collection_action(action, rel_url, params={}) - JSON.parse RestClient.send(action, "#{hub_base_url}#{rel_url}", {access_token: access_token}.merge(params), content_type: :json, accept: accepts ) + JSON.parse RestClient.send(action, "#{hub_base_url}#{rel_url}", params, content_type: :json, accept: accepts ) end def self.hub_base_url @@ -118,23 +61,8 @@ module DiscourseHub end end - def self.access_token - SiteSetting.discourse_org_access_key - end - def self.accepts [:json, 'application/vnd.discoursehub.v1'] end - def self.username_operation - if SiteSetting.call_discourse_hub? - begin - yield - rescue DiscourseHub::UsernameUnavailable - false - rescue => e - Rails.logger.error e.message + "\n" + e.backtrace.join("\n") - end - end - end end diff --git a/spec/components/discourse_hub_spec.rb b/spec/components/discourse_hub_spec.rb index ead432b8b84..ea579d0e839 100644 --- a/spec/components/discourse_hub_spec.rb +++ b/spec/components/discourse_hub_spec.rb @@ -2,71 +2,6 @@ require 'spec_helper' require_dependency 'discourse_hub' describe DiscourseHub do - describe '#username_available?' do - it 'should return true when username is available and no suggestion' do - RestClient.stubs(:get).returns( {success: 'OK', available: true}.to_json ) - DiscourseHub.username_available?('MacGyver').should == [true, nil] - end - - it 'should return false and a suggestion when username is not available' do - RestClient.stubs(:get).returns( {success: 'OK', available: false, suggestion: 'MacGyver1'}.to_json ) - available, suggestion = DiscourseHub.username_available?('MacGyver') - available.should be_false - suggestion.should_not be_nil - end - - # How to handle connect errors? timeout? 401? 403? 429? - end - - describe '#username_match?' do - it 'should return true when it is a match and no suggestion' do - RestClient.stubs(:get).returns( {success: 'OK', match: true, available: false}.to_json ) - DiscourseHub.username_match?('MacGyver', 'macg@example.com').should == [true, false, nil] - end - - it 'should return false and a suggestion when it is not a match and the username is not available' do - RestClient.stubs(:get).returns( {success: 'OK', match: false, available: false, suggestion: 'MacGyver1'}.to_json ) - match, available, suggestion = DiscourseHub.username_match?('MacGyver', 'macg@example.com') - match.should be_false - available.should be_false - suggestion.should_not be_nil - end - - it 'should return false and no suggestion when it is not a match and the username is available' do - RestClient.stubs(:get).returns( {success: 'OK', match: false, available: true}.to_json ) - match, available, suggestion = DiscourseHub.username_match?('MacGyver', 'macg@example.com') - match.should be_false - available.should be_true - suggestion.should be_nil - end - end - - describe '#register_username' do - it 'should return true when registration succeeds' do - RestClient.stubs(:post).returns( {success: 'OK'}.to_json ) - DiscourseHub.register_username('MacGyver', 'macg@example.com').should be_true - end - - it 'should return raise an exception when registration fails' do - RestClient.stubs(:post).returns( {failed: -200}.to_json ) - expect { - DiscourseHub.register_username('MacGyver', 'macg@example.com') - }.to raise_error(DiscourseHub::UsernameUnavailable) - end - end - - describe '#unregister_username' do - it 'should return true when unregister succeeds' do - RestClient.stubs(:delete).returns( {success: 'OK'}.to_json ) - DiscourseHub.unregister_username('byebye').should be_true - end - - it 'should return false when unregister fails' do - RestClient.stubs(:delete).returns( {failed: -20}.to_json ) - DiscourseHub.unregister_username('byebye').should be_false - end - end - describe '#discourse_version_check' do it 'should return just return the json that the hub returns' do hub_response = {'success' => 'OK', 'latest_version' => '0.8.1', 'critical_updates' => false} @@ -74,33 +9,4 @@ describe DiscourseHub do DiscourseHub.discourse_version_check.should == hub_response end end - - describe '#change_username' do - it 'should return true when username is changed successfully' do - RestClient.stubs(:put).returns( {success: 'OK'}.to_json ) - DiscourseHub.change_username('MacGyver', 'MacG').should be_true - end - - it 'should return raise UsernameUnavailable when username is not available' do - RestClient.stubs(:put).returns( {failed: -200}.to_json ) - expect { - DiscourseHub.change_username('MacGyver', 'MacG') - }.to raise_error(DiscourseHub::UsernameUnavailable) - end - - - # it 'should return raise UsernameUnavailable when username does not belong to this forum' do - # RestClient.stubs(:put).returns( {failed: -13}.to_json ) - # expect { - # DiscourseHub.change_username('MacGyver', 'MacG') - # }.to raise_error(DiscourseHub::ActionForbidden) - # end - - # it 'should return raise UsernameUnavailable when username does not belong to this forum' do - # RestClient.stubs(:put).returns( {failed: -13}.to_json ) - # expect { - # DiscourseHub.change_username('MacGyver', 'MacG') - # }.to raise_error(DiscourseHub::ActionForbidden) - # end - end end diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 7c618a2ef41..8f127939df9 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -288,7 +288,6 @@ describe UsersController do SiteSetting.stubs(:allow_new_registrations).returns(true) @user = Fabricate.build(:user) @user.password = "strongpassword" - DiscourseHub.stubs(:register_username).returns([true, nil]) end def post_user @@ -494,25 +493,9 @@ describe UsersController do include_examples 'failed signup' end - context 'when username is unavailable in DiscourseHub' do - before do - SiteSetting.stubs(:call_discourse_hub?).returns(true) - DiscourseHub.stubs(:register_username).raises(DiscourseHub::UsernameUnavailable.new(@user.name)) - end - let(:create_params) {{ - name: @user.name, - username: @user.username, - password: 'strongpassword', - email: @user.email - }} - - include_examples 'failed signup' - end - context 'when an Exception is raised' do [ ActiveRecord::StatementInvalid, - DiscourseHub::UsernameUnavailable, RestClient::Forbidden ].each do |exception| before { User.any_instance.stubs(:save).raises(exception) } @@ -561,15 +544,11 @@ describe UsersController do end context '.check_username' do - before do - DiscourseHub.stubs(:username_available?).returns([true, nil]) - end - it 'raises an error without any parameters' do lambda { xhr :get, :check_username }.should raise_error(ActionController::ParameterMissing) end - shared_examples 'when username is unavailable locally' do + shared_examples 'when username is unavailable' do it 'should return success' do response.should be_success end @@ -583,7 +562,7 @@ describe UsersController do end end - shared_examples 'when username is available everywhere' do + shared_examples 'when username is available' do it 'should return success' do response.should be_success end @@ -593,211 +572,59 @@ describe UsersController do end end - context 'when call_discourse_hub is disabled' do - before do - SiteSetting.stubs(:call_discourse_hub?).returns(false) - DiscourseHub.expects(:username_available?).never - DiscourseHub.expects(:username_match?).never - end + it 'returns nothing when given an email param but no username' do + xhr :get, :check_username, email: 'dood@example.com' + response.should be_success + end - it 'returns nothing when given an email param but no username' do - xhr :get, :check_username, email: 'dood@example.com' + context 'username is available' do + before do + xhr :get, :check_username, username: 'BruceWayne' + end + include_examples 'when username is available' + end + + context 'username is unavailable' do + let!(:user) { Fabricate(:user) } + before do + xhr :get, :check_username, username: user.username + end + include_examples 'when username is unavailable' + end + + shared_examples 'checking an invalid username' do + it 'should return success' do response.should be_success end - context 'username is available' do - before do - xhr :get, :check_username, username: 'BruceWayne' - end - include_examples 'when username is available everywhere' - end - - context 'username is unavailable' do - let!(:user) { Fabricate(:user) } - before do - xhr :get, :check_username, username: user.username - end - include_examples 'when username is unavailable locally' - end - - shared_examples 'checking an invalid username' do - it 'should return success' do - response.should be_success - end - - it 'should not return an available key' do - ::JSON.parse(response.body)['available'].should be_nil - end - - it 'should return an error message' do - ::JSON.parse(response.body)['errors'].should_not be_empty - end - end - - context 'has invalid characters' do - before do - xhr :get, :check_username, username: 'bad username' - end - include_examples 'checking an invalid username' - - it 'should return the invalid characters message' do - ::JSON.parse(response.body)['errors'].should include(I18n.t(:'user.username.characters')) - end - end - - context 'is too long' do - before do - xhr :get, :check_username, username: generate_username(User.username_length.last + 1) - end - include_examples 'checking an invalid username' - - it 'should return the "too long" message' do - ::JSON.parse(response.body)['errors'].should include(I18n.t(:'user.username.long', max: User.username_length.end)) - end - end - end - - context 'when call_discourse_hub is enabled' do - before do - SiteSetting.stubs(:call_discourse_hub?).returns(true) - end - - context 'available locally and globally' do - before do - DiscourseHub.stubs(:username_available?).returns([true, nil]) - DiscourseHub.stubs(:username_match?).returns([false, true, nil]) # match = false, available = true, suggestion = nil - end - - shared_examples 'check_username when username is available everywhere' do - it 'should return success' do - response.should be_success - end - - it 'should return available in the JSON' do - ::JSON.parse(response.body)['available'].should be_true - end - - it 'should return global_match false in the JSON' do - ::JSON.parse(response.body)['global_match'].should be_false - end - end - - context 'and email is not given' do - before do - xhr :get, :check_username, username: 'BruceWayne' - end - include_examples 'check_username when username is available everywhere' - end - - context 'both username and email is given' do - before do - xhr :get, :check_username, username: 'BruceWayne', email: 'brucie@gmail.com' - end - include_examples 'check_username when username is available everywhere' - end - - context 'only email is given' do - it "should check for a matching username" do - UsernameCheckerService.any_instance.expects(:check_username).with(nil, 'brucie@gmail.com').returns({json: 'blah'}) - xhr :get, :check_username, email: 'brucie@gmail.com' - response.should be_success - end - end - end - - shared_examples 'when email is needed to check username match' do - it 'should return success' do - response.should be_success - end - - it 'should return available as false in the JSON' do - ::JSON.parse(response.body)['available'].should be_false - end - - it 'should not return a suggested username' do - ::JSON.parse(response.body)['suggestion'].should_not be_present - end - end - - context 'available locally but not globally' do - before do - DiscourseHub.stubs(:username_available?).returns([false, 'suggestion']) - end - - context 'email param is not given' do - before do - xhr :get, :check_username, username: 'BruceWayne' - end - include_examples 'when email is needed to check username match' - end - - context 'email param is an empty string' do - before do - xhr :get, :check_username, username: 'BruceWayne', email: '' - end - include_examples 'when email is needed to check username match' - end - - context 'email matches global username' do - before do - DiscourseHub.stubs(:username_match?).returns([true, false, nil]) - xhr :get, :check_username, username: 'BruceWayne', email: 'brucie@example.com' - end - include_examples 'when username is available everywhere' - - it 'should indicate a global match' do - ::JSON.parse(response.body)['global_match'].should be_true - end - end - - context 'email does not match global username' do - before do - DiscourseHub.stubs(:username_match?).returns([false, false, 'suggestion']) - xhr :get, :check_username, username: 'BruceWayne', email: 'brucie@example.com' - end - include_examples 'when username is unavailable locally' - - it 'should not indicate a global match' do - ::JSON.parse(response.body)['global_match'].should be_false - end - end - end - - context 'unavailable locally and globally' do - let!(:user) { Fabricate(:user) } - - before do - DiscourseHub.stubs(:username_available?).returns([false, 'suggestion']) - xhr :get, :check_username, username: user.username - end - - include_examples 'when username is unavailable locally' - end - - context 'unavailable locally and available globally' do - let!(:user) { Fabricate(:user) } - - before do - DiscourseHub.stubs(:username_available?).returns([true, nil]) - xhr :get, :check_username, username: user.username - end - - include_examples 'when username is unavailable locally' - end - end - - context 'when discourse_org_access_key is wrong' do - before do - SiteSetting.stubs(:call_discourse_hub?).returns(true) - DiscourseHub.stubs(:username_available?).raises(RestClient::Forbidden) - DiscourseHub.stubs(:username_match?).raises(RestClient::Forbidden) + it 'should not return an available key' do + ::JSON.parse(response.body)['available'].should be_nil end it 'should return an error message' do - xhr :get, :check_username, username: 'horsie' - json = JSON.parse(response.body) - json['errors'].should_not be_nil - json['errors'][0].should_not be_nil + ::JSON.parse(response.body)['errors'].should_not be_empty + end + end + + context 'has invalid characters' do + before do + xhr :get, :check_username, username: 'bad username' + end + include_examples 'checking an invalid username' + + it 'should return the invalid characters message' do + ::JSON.parse(response.body)['errors'].should include(I18n.t(:'user.username.characters')) + end + end + + context 'is too long' do + before do + xhr :get, :check_username, username: generate_username(User.username_length.last + 1) + end + include_examples 'checking an invalid username' + + it 'should return the "too long" message' do + ::JSON.parse(response.body)['errors'].should include(I18n.t(:'user.username.long', max: User.username_length.end)) end end @@ -808,7 +635,7 @@ describe UsersController do log_in_user(user) xhr :get, :check_username, username: 'HanSolo' end - include_examples 'when username is available everywhere' + include_examples 'when username is available' end context "it's someone else's username" do @@ -817,7 +644,7 @@ describe UsersController do log_in xhr :get, :check_username, username: 'HanSolo' end - include_examples 'when username is unavailable locally' + include_examples 'when username is unavailable' end context "an admin changing it for someone else" do @@ -826,7 +653,7 @@ describe UsersController do log_in_user(Fabricate(:admin)) xhr :get, :check_username, username: 'HanSolo', for_user_id: user.id end - include_examples 'when username is available everywhere' + include_examples 'when username is available' end end end diff --git a/spec/models/admin_dashboard_data_spec.rb b/spec/models/admin_dashboard_data_spec.rb index b8da3fc38ff..414c296c261 100644 --- a/spec/models/admin_dashboard_data_spec.rb +++ b/spec/models/admin_dashboard_data_spec.rb @@ -244,26 +244,4 @@ describe AdminDashboardData do end end - describe "enforce_global_nicknames_check" do - subject { described_class.new.enforce_global_nicknames_check } - - it 'returns nil when enforce_global_nicknames and discourse_org_access_key are set' do - SiteSetting.stubs(:enforce_global_nicknames).returns(true) - SiteSetting.stubs(:discourse_org_access_key).returns('123') - subject.should be_nil - end - - it 'returns a string when enforce_global_nicknames is true but discourse_org_access_key is not' do - SiteSetting.stubs(:enforce_global_nicknames).returns(true) - SiteSetting.stubs(:discourse_org_access_key).returns('') - subject.should_not be_nil - end - - it 'returns nil when enforce_global_nicknames is false and discourse_org_access_key is set' do - SiteSetting.stubs(:enforce_global_nicknames).returns(false) - SiteSetting.stubs(:discourse_org_access_key).returns('123') - subject.should be_nil - end - end - end diff --git a/spec/models/site_setting_spec.rb b/spec/models/site_setting_spec.rb index e95ca790b51..778cc3536b9 100644 --- a/spec/models/site_setting_spec.rb +++ b/spec/models/site_setting_spec.rb @@ -4,32 +4,6 @@ require_dependency 'site_setting_extension' describe SiteSetting do - describe 'call_discourse_hub?' do - it 'should be true when enforce_global_nicknames is true and discourse_org_access_key is set' do - SiteSetting.stubs(:enforce_global_nicknames).returns(true) - SiteSetting.stubs(:discourse_org_access_key).returns('asdfasfsafd') - SiteSetting.call_discourse_hub?.should == true - end - - it 'should be false when enforce_global_nicknames is false and discourse_org_access_key is set' do - SiteSetting.stubs(:enforce_global_nicknames).returns(false) - SiteSetting.stubs(:discourse_org_access_key).returns('asdfasfsafd') - SiteSetting.call_discourse_hub?.should == false - end - - it 'should be false when enforce_global_nicknames is true and discourse_org_access_key is not set' do - SiteSetting.stubs(:enforce_global_nicknames).returns(true) - SiteSetting.stubs(:discourse_org_access_key).returns('') - SiteSetting.call_discourse_hub?.should == false - end - - it 'should be false when enforce_global_nicknames is false and discourse_org_access_key is not set' do - SiteSetting.stubs(:enforce_global_nicknames).returns(false) - SiteSetting.stubs(:discourse_org_access_key).returns('') - SiteSetting.call_discourse_hub?.should == false - end - end - describe "normalized_embeddable_host" do it 'returns the `embeddable_host` value' do SiteSetting.stubs(:embeddable_host).returns("eviltrout.com") diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index b39ce620ca6..fe55d6415db 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -143,7 +143,7 @@ describe User do describe 'allow custom minimum username length from site settings' do before do - @custom_min = User::GLOBAL_USERNAME_LENGTH_RANGE.begin - 1 + @custom_min = 2 SiteSetting.min_username_length = @custom_min end @@ -161,11 +161,6 @@ describe User do result = user.change_username('a' * (User.username_length.end + 1)) result.should be_false end - - it 'should use default length for validation if enforce_global_nicknames is true' do - SiteSetting.enforce_global_nicknames = true - User::username_length.should == User::GLOBAL_USERNAME_LENGTH_RANGE - end end end diff --git a/spec/services/user_destroyer_spec.rb b/spec/services/user_destroyer_spec.rb index 4e9ebf613cb..1d1d10db452 100644 --- a/spec/services/user_destroyer_spec.rb +++ b/spec/services/user_destroyer_spec.rb @@ -50,18 +50,6 @@ describe UserDestroyer do StaffActionLogger.any_instance.expects(:log_user_deletion).with(@user, anything).once destroy end - - it 'should unregister the nickname as the discourse hub if hub integration is enabled' do - SiteSetting.stubs(:call_discourse_hub?).returns(true) - DiscourseHub.expects(:unregister_username).with(@user.username) - destroy - end - - it 'should not try to unregister the nickname as the discourse hub if hub integration is disabled' do - SiteSetting.stubs(:call_discourse_hub?).returns(false) - DiscourseHub.expects(:unregister_username).never - destroy - end end shared_examples "email block list" do @@ -106,11 +94,6 @@ describe UserDestroyer do StaffActionLogger.any_instance.expects(:log_user_deletion).never destroy rescue nil end - - it 'should not unregister the user at the discourse hub' do - DiscourseHub.expects(:unregister_username).never - destroy rescue nil - end end context "delete_posts is true" do @@ -190,11 +173,6 @@ describe UserDestroyer do StaffActionLogger.any_instance.expects(:log_user_deletion).never destroy end - - it 'should not unregister the user at the discourse hub' do - DiscourseHub.expects(:unregister_username).never - destroy rescue nil - end end end diff --git a/spec/services/username_checker_service_spec.rb b/spec/services/username_checker_service_spec.rb index 46c95701290..6c05448e483 100644 --- a/spec/services/username_checker_service_spec.rb +++ b/spec/services/username_checker_service_spec.rb @@ -31,105 +31,18 @@ describe UsernameCheckerService do end end - context 'Using Discourse Hub' do - before do - SiteSetting.stubs(:call_discourse_hub?).returns(true) - end - - context 'username and email is given' do - it 'username is available locally but not globally' do - DiscourseHub.expects(:username_available?).never - DiscourseHub.expects(:username_match?).returns([false, false, 'porkchop']) - result = @service.check_username('vincent', @email) - expected = { available: false, global_match: false, suggestion: 'porkchop' } - expect(result).to eq(expected) - end - - it 'username is available both locally and globally' do - DiscourseHub.expects(:username_available?).never - DiscourseHub.stubs(:username_match?).returns([true, true, nil]) - result = @service.check_username('vincent', @email) - expected = { available: true, global_match: true } - expect(result).to eq(expected) - end - - it 'username is available locally but not globally' do - DiscourseHub.stubs(:username_match?).returns([false, false, 'suggestion']) - result = @service.check_username('vincent', @email) - expected = { available: false, global_match: false, suggestion: 'suggestion' } - expect(result).to eq(expected) - end - - it 'username is available globally but not locally' do - DiscourseHub.stubs(:username_match?).returns([false, true, nil]) - User.stubs(:username_available?).returns(false) - UserNameSuggester.stubs(:suggest).returns('einar-j') - expected = { available: false, suggestion: 'einar-j' } - result = @service.check_username('vincent', @email) - expect(result).to eq(expected) - end - - it 'username not available anywhere' do - DiscourseHub.stubs(:username_match?).returns([false, false, 'suggestion']) - expected = { available: false, suggestion: 'suggestion', global_match: false } - @nil_email = nil - result = @service.check_username('vincent', @email) - expect(result).to eq(expected) - end - end - - shared_examples "only email is given" do - it "should call the correct api" do - DiscourseHub.expects(:username_available?).never - DiscourseHub.expects(:username_match?).never - DiscourseHub.stubs(:username_for_email).returns(nil) - result - end - - it 'no match on email' do - DiscourseHub.stubs(:username_for_email).returns(nil) - result.should == {suggestion: nil} - end - - it 'match found for email' do - DiscourseHub.stubs(:username_for_email).returns('vincent') - result.should == {suggestion: 'vincent'} - end - - it 'match found for email, but username is taken' do - # This case can happen when you've already signed up on the site, - # or enforce_global_nicknames used to be disabled. - DiscourseHub.stubs(:username_for_email).returns('taken') - User.stubs(:username_available?).with('taken').returns(false) - result.should == {suggestion: nil} - end - end - - context 'username is nil' do - subject(:result) { @service.check_username(nil, @email) } - include_examples "only email is given" - end - - context 'username is empty string' do - subject(:result) { @service.check_username('', @email) } - include_examples "only email is given" - end + it 'username not available locally' do + User.stubs(:username_available?).returns(false) + UserNameSuggester.stubs(:suggest).returns('einar-j') + result = @service.check_username('vincent', @nil_email) + result[:available].should be_false + result[:suggestion].should eq('einar-j') end - context 'Discourse Hub disabled' do - it 'username not available locally' do - User.stubs(:username_available?).returns(false) - UserNameSuggester.stubs(:suggest).returns('einar-j') - result = @service.check_username('vincent', @nil_email) - result[:available].should be_false - result[:suggestion].should eq('einar-j') - end - - it 'username available locally' do - User.stubs(:username_available?).returns(true) - result = @service.check_username('vincent', @nil_email) - result[:available].should be_true - end + it 'username available locally' do + User.stubs(:username_available?).returns(true) + result = @service.check_username('vincent', @nil_email) + result[:available].should be_true end end