Admins can't lock themselves out of a site by setting approval.

app/controllers/users/omniauth_callbacks_controller.rb

@@ -58,8 +58,12 @@ class Users::OmniauthCallbacksController < ApplicationController
 
     if user_info
       if user_info.user.active
-        log_on_user(user_info.user)
+        if Guardian.new(user_info.user).can_access_forum?
-        @data[:authenticated] = true
+          log_on_user(user_info.user)
+          @data[:authenticated] = true
+        else
+          @data[:awaiting_approval] = true
+        end
       else
         @data[:awaiting_activation] = true
         # send another email ?
@@ -114,8 +118,14 @@ class Users::OmniauthCallbacksController < ApplicationController
           user.active = true
           user.save
         end
-        log_on_user(user)
+
-        @data[:authenticated] = true
+        # If we have to approve users
+        if Guardian.new(user).can_access_forum?
+          log_on_user(user)
+          @data[:authenticated] = true
+        else
+          @data[:awaiting_approval] = true
+        end
       end
     else
       user = User.where(email: email).first
@@ -156,11 +166,11 @@ class Users::OmniauthCallbacksController < ApplicationController
       user = user_open_id.user
 
       # If we have to approve users
-      if SiteSetting.must_approve_users? && !user.approved?
+      if Guardian.new(user).can_access_forum?
-        @data = {awaiting_approval: true}
-      else
         log_on_user(user)
         @data = {authenticated: true}
+      else
+        @data = {awaiting_approval: true}
       end
 
     else
@@ -203,8 +213,14 @@ class Users::OmniauthCallbacksController < ApplicationController
 
     if user_info
       if user_info.user.active
-        log_on_user(user_info.user)
+
-        @data[:authenticated] = true
+        if Guardian.new(user_info.user).can_access_forum?
+          log_on_user(user_info.user)
+          @data[:authenticated] = true
+        else
+          @data[:awaiting_approval] = true
+        end
+
       else
         @data[:awaiting_activation] = true
         # send another email ?
@@ -222,12 +238,14 @@ class Users::OmniauthCallbacksController < ApplicationController
     user = User.find_by_email(email)
 
     if user
-      if SiteSetting.must_approve_users? && !user.approved?
+
-        @data = {awaiting_approval: true}
+      if Guardian.new(user).can_access_forum?
-      else
         log_on_user(user)
         @data = {authenticated: true}
+      else
+        @data = {awaiting_approval: true}
       end
+
     else
       @data = {
         email: email,

app/controllers/users_controller.rb

@@ -246,13 +246,13 @@ class UsersController < ApplicationController
         @user.password = params[:password]
         if @user.save
 
-          if SiteSetting.must_approve_users? && !@user.approved?
+          if Guardian.new(@user).can_access_forum?
-            @requires_approval = true
-            flash[:success] = I18n.t('password_reset.success_unapproved')
-          else
             # Log in the user
             log_on_user(@user)
             flash[:success] = I18n.t('password_reset.success')
+          else
+            @requires_approval = true
+            flash[:success] = I18n.t('password_reset.success_unapproved')
           end
         end
       end
@@ -293,11 +293,11 @@ class UsersController < ApplicationController
     if @user = EmailToken.confirm(params[:token])
 
       # Log in the user unless they need to be approved
-      if SiteSetting.must_approve_users?
+      if Guardian.new(@user).can_access_forum?
-        @needs_approval = true
-      else
         @user.enqueue_welcome_message('welcome_user') if @user.send_welcome_message
         log_on_user(@user)
+      else
+        @needs_approval = true
       end
 
     else

lib/guardian.rb

@@ -164,6 +164,17 @@ class Guardian
     true
   end
 
+  # Support sites that have to approve users
+  def can_access_forum?
+    return true unless SiteSetting.must_approve_users?
+    return false if user.blank?
+
+    # Admins can't lock themselves out of a site
+    return true if user.admin?
+
+    user.approved?
+  end
+
   def can_see_pending_invites_from?(user)
     return false if user.blank?
     return false if @user.blank?

spec/components/guardian_spec.rb

@@ -815,6 +815,50 @@ describe Guardian do
     it 'is true when looking at your own invites' do
       Guardian.new(user).can_see_pending_invites_from?(user).should be_true
     end
+  end
+
+  context "can_access_forum?" do
+
+    let(:unapproved_user) { Fabricate.build(:user) }
+
+    context "when must_approve_users is false" do
+      before do
+        SiteSetting.stubs(:must_approve_users?).returns(false)
+      end
+
+      it "returns true for a nil user" do
+        Guardian.new(nil).can_access_forum?.should be_true
+      end
+
+      it "returns true for an unapproved user" do
+        Guardian.new(unapproved_user).can_access_forum?.should be_true
+      end
+    end
+
+    context 'when must_approve_users is true' do
+      before do
+        SiteSetting.stubs(:must_approve_users?).returns(true)
+      end
+
+      it "returns false for a nil user" do
+        Guardian.new(nil).can_access_forum?.should be_false
+      end
+
+      it "returns false for an unapproved user" do
+        Guardian.new(unapproved_user).can_access_forum?.should be_false
+      end
+
+      it "returns true for an admin user" do
+        unapproved_user.admin = true
+        Guardian.new(unapproved_user).can_access_forum?.should be_true
+      end
+
+      it "returns true for an approved user" do
+        unapproved_user.approved = true
+        Guardian.new(unapproved_user).can_access_forum?.should be_true
+      end
+
+    end
 
   end
 

spec/controllers/users_controller_spec.rb

@@ -117,6 +117,7 @@ describe UsersController do
 
       context 'reponse' do
         before do
+          Guardian.any_instance.expects(:can_access_forum?).returns(true)
           EmailToken.expects(:confirm).with('asdfasdf').returns(user)
           get :activate_account, token: 'asdfasdf'
         end
@@ -139,9 +140,9 @@ describe UsersController do
 
       end
 
-      context 'must_approve_users' do
+      context 'user is not approved' do
         before do
-          SiteSetting.expects(:must_approve_users?).returns(true)
+          Guardian.any_instance.expects(:can_access_forum?).returns(false)
           EmailToken.expects(:confirm).with('asdfasdf').returns(user)
           get :activate_account, token: 'asdfasdf'
         end