invite only forums had very wonky logic, invited users were not being activated, invite_only forums were still registering users

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);

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
-
+    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

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|

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

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 <b>{{sentTo}}</b>. 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 <b>{{currentEmail}}</b>. It might take a few minutes for it to arrive; be sure to check your spam folder."

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,

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

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

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) }