From 61281a3c81343e49d43933d784f3e48017a2d286 Mon Sep 17 00:00:00 2001 From: Sam Date: Wed, 28 Aug 2013 17:18:31 +1000 Subject: [PATCH] invite only forums had very wonky logic, invited users were not being activated, invite_only forums were still registering users --- .../discourse/controllers/login_controller.js | 5 ++++ .../users/omniauth_callbacks_controller.rb | 12 +++++--- app/models/invite_redeemer.rb | 23 +++++++++++--- app/models/user.rb | 30 +------------------ config/locales/client.en.yml | 1 + lib/auth/result.rb | 7 +++-- lib/discourse_hub.rb | 12 ++++++++ spec/models/invite_redeemer_spec.rb | 14 +++++++++ spec/models/user_spec.rb | 9 ------ 9 files changed, 65 insertions(+), 48 deletions(-) create mode 100644 spec/models/invite_redeemer_spec.rb diff --git a/app/assets/javascripts/discourse/controllers/login_controller.js b/app/assets/javascripts/discourse/controllers/login_controller.js index 3cdbf674f..58664f6ff 100644 --- a/app/assets/javascripts/discourse/controllers/login_controller.js +++ b/app/assets/javascripts/discourse/controllers/login_controller.js @@ -95,6 +95,11 @@ Discourse.LoginController = Discourse.Controller.extend(Discourse.ModalFunctiona }, authenticationComplete: function(options) { + if (options.requires_invite) { + this.flash(I18n.t('login.requires_invite'), 'success'); + this.set('authenticate', null); + return; + } if (options.awaiting_approval) { this.flash(I18n.t('login.awaiting_approval'), 'success'); this.set('authenticate', null); diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb index 5118378ac..42fc78f3b 100644 --- a/app/controllers/users/omniauth_callbacks_controller.rb +++ b/app/controllers/users/omniauth_callbacks_controller.rb @@ -37,9 +37,13 @@ class Users::OmniauthCallbacksController < ApplicationController @data = authenticator.after_authenticate(auth) @data.authenticator_name = authenticator.name - user_found(@data.user) if @data.user - - session[:authentication] = @data.session_data + if @data.user + user_found(@data.user) + elsif SiteSetting.invite_only? + @data.requires_invite = true + else + session[:authentication] = @data.session_data + end respond_to do |format| format.html @@ -87,7 +91,7 @@ class Users::OmniauthCallbacksController < ApplicationController session[:authentication] = nil @data.authenticated = true else - if SiteSetting.invite_only? + if SiteSetting.must_approve_users? && !user.approved? @data.awaiting_approval = true else @data.awaiting_activation = true diff --git a/app/models/invite_redeemer.rb b/app/models/invite_redeemer.rb index 91fb3aefa..45623474b 100644 --- a/app/models/invite_redeemer.rb +++ b/app/models/invite_redeemer.rb @@ -8,6 +8,24 @@ InviteRedeemer = Struct.new(:invite) do invited_user end + # extracted from User cause it is very specific to invites + def self.create_user_for_email(email) + username = UserNameSuggester.suggest(email) + + DiscourseHub.nickname_operation do + match, available, suggestion = DiscourseHub.nickname_match?(username, email) + username = suggestion unless match || available + end + + user = User.new(email: email, username: username, name: username, active: true) + user.trust_level = SiteSetting.default_invitee_trust_level + user.save! + + DiscourseHub.nickname_operation { DiscourseHub.register_nickname(username, email) } + + user + end + private def invited_user @@ -34,7 +52,7 @@ InviteRedeemer = Struct.new(:invite) do def get_invited_user result = get_existing_user - result ||= create_new_user + result ||= InviteRedeemer.create_user_for_email(invite.email) result.send_welcome_message = false result end @@ -43,9 +61,6 @@ InviteRedeemer = Struct.new(:invite) do User.where(email: invite.email).first end - def create_new_user - User.create_for_email(invite.email, trust_level: SiteSetting.default_invitee_trust_level) - end def add_to_private_topics_if_invited invite.topics.private_messages.each do |t| diff --git a/app/models/user.rb b/app/models/user.rb index 58c1411b9..fb2870798 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -96,23 +96,6 @@ class User < ActiveRecord::Base user end - def self.create_for_email(email, opts={}) - username = UserNameSuggester.suggest(email) - - discourse_hub_nickname_operation do - match, available, suggestion = DiscourseHub.nickname_match?(username, email) - username = suggestion unless match || available - end - - user = User.new(email: email, username: username, name: username) - user.trust_level = opts[:trust_level] if opts[:trust_level].present? - user.save! - - discourse_hub_nickname_operation { DiscourseHub.register_nickname(username, email) } - - user - end - def self.suggest_name(email) return "" unless email name = email.split(/[@\+]/)[0] @@ -154,7 +137,7 @@ class User < ActiveRecord::Base self.username = new_username if current_username.downcase != new_username.downcase && valid? - User.discourse_hub_nickname_operation { DiscourseHub.change_nickname(current_username, new_username) } + DiscourseHub.nickname_operation { DiscourseHub.change_nickname(current_username, new_username) } end save @@ -612,17 +595,6 @@ class User < ActiveRecord::Base private - def self.discourse_hub_nickname_operation - if SiteSetting.call_discourse_hub? - begin - yield - rescue DiscourseHub::NicknameUnavailable - false - rescue => e - Rails.logger.error e.message + "\n" + e.backtrace.join("\n") - end - end - end end # == Schema Information diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 07a961884..204b4d13f 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -396,6 +396,7 @@ en: authenticating: "Authenticating..." awaiting_confirmation: "Your account is awaiting activation, use the forgot password link to issue another activation email." awaiting_approval: "Your account has not been approved by a staff member yet. You will be sent an email when it is approved." + requires_invite: "Sorry, access to this forum is by invite only." not_activated: "You can't log in yet. We previously sent an activation email to you at {{sentTo}}. Please follow the instructions in that email to activate your account." resend_activation_email: "Click here to send the activation email again." sent_activation_email_again: "We sent another activation email to you at {{currentEmail}}. It might take a few minutes for it to arrive; be sure to check your spam folder." diff --git a/lib/auth/result.rb b/lib/auth/result.rb index d76f3e045..26e067dce 100644 --- a/lib/auth/result.rb +++ b/lib/auth/result.rb @@ -1,7 +1,8 @@ class Auth::Result attr_accessor :user, :name, :username, :email, :user, :email_valid, :extra_data, :awaiting_activation, - :awaiting_approval, :authenticated, :authenticator_name + :awaiting_approval, :authenticated, :authenticator_name, + :requires_invite def session_data { @@ -15,7 +16,9 @@ class Auth::Result end def to_client_hash - if user + if requires_invite + { requires_invite: true } + elsif user { authenticated: !!authenticated, awaiting_activation: !!awaiting_activation, diff --git a/lib/discourse_hub.rb b/lib/discourse_hub.rb index 60ec2fea4..eb0e85ddc 100644 --- a/lib/discourse_hub.rb +++ b/lib/discourse_hub.rb @@ -110,4 +110,16 @@ module DiscourseHub def self.accepts [:json, 'application/vnd.discoursehub.v1'] end + + def self.nickname_operation + if SiteSetting.call_discourse_hub? + begin + yield + rescue DiscourseHub::NicknameUnavailable + false + rescue => e + Rails.logger.error e.message + "\n" + e.backtrace.join("\n") + end + end + end end diff --git a/spec/models/invite_redeemer_spec.rb b/spec/models/invite_redeemer_spec.rb new file mode 100644 index 000000000..4d1d0bb96 --- /dev/null +++ b/spec/models/invite_redeemer_spec.rb @@ -0,0 +1,14 @@ +require 'spec_helper' + +describe InviteRedeemer do + + describe '#create_for_email' do + let(:user) { InviteRedeemer.create_user_for_email('walter.white@email.com') } + it "should be created correctly" do + user.username.should == 'walter_white' + user.name.should == 'walter_white' + user.should be_active + user.email.should == 'walter.white@email.com' + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index ec844be63..757cd636e 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -731,15 +731,6 @@ describe User do end end - describe '#create_for_email' do - let(:subject) { User.create_for_email('walter.white@email.com') } - it { should be_present } - its(:username) { should == 'walter_white' } - its(:name) { should == 'walter_white'} - it { should_not be_active } - its(:email) { should == 'walter.white@email.com' } - end - describe 'email_confirmed?' do let(:user) { Fabricate(:user) }