Validate authentication and whitelisting #10580

app/controllers/admin/organizations_controller.rb

@@ -97,10 +97,10 @@ class Admin::OrganizationsController < Admin::AdminController
   rescue CartoDB::CentralCommunicationFailure => e
     @organization.reload
     flash.now[:error] = "There was a problem while updating your organization. Please, try again and contact us if the problem persists. #{e.user_message}"
-    render action: 'settings'
+    render action: 'auth'
   rescue Sequel::ValidationFailed => e
     flash.now[:error] = "There's been a validation error, check your values"
-    render action: 'settings'
+    render action: 'auth'
   end
 
   private

app/models/organization.rb

@@ -70,6 +70,9 @@ class Organization < Sequel::Model
       errors.add(:default_quota_in_bytes, 'Default quota must be positive') if default_quota_in_bytes <= 0
     end
     errors.add(:name, 'cannot exist as user') if name_exists_in_users?
+    if whitelisted_email_domains.present? && !auth_enabled?
+      errors.add(:whitelisted_email_domains, 'enable at least one auth. system or clear whitelisted email domains')
+    end
   end
 
   def validate_new_user(user, errors)

app/views/admin/organizations/auth.html.erb

@@ -41,6 +41,9 @@
             </div>
             <%= f.hidden_field :whitelisted_email_domains, :value => "#{ @organization[:whitelisted_email_domains].join(',') if !@organization[:whitelisted_email_domains].blank? }", :class => "js-whitelist CDB-InputText CDB-Text", :placeholder => "Only valid domains will be added (ex. carto.com)" %>
             <div class="u-flex u-lSpace--xl">
+              <% if @organization.errors[:whitelisted_email_domains].present? %>
+                <p class="CDB-Text FormAccount-rowInfoText FormAccount-rowInfoText--error u-tSpace"><%= @organization.errors[:whitelisted_email_domains].first%></p>
+              <% end %>
               <p class="CDB-Text CDB-Size-small u-altTextColor">If empty, login and signup pages will be disabled</p>
             </div>
           </div>

spec/factories/organizations.rb

@@ -18,6 +18,11 @@ FactoryGirl.define do
     location 'Madrid'
     builder_enabled false # Most tests still assume editor
 
+    factory :organization_whitelist_carto do
+      whitelisted_email_domains ['carto.com']
+      auth_username_password_enabled true
+    end
+
     factory :organization_with_users do
       after(:create) do |org|
         owner = FactoryGirl.create(:user)

spec/requests/signup_controller_spec.rb

@@ -21,7 +21,7 @@ describe SignupController do
     end
 
     it 'returns 200 for organizations with signup_page_enabled' do
-      @fake_organization = FactoryGirl.create(:organization, whitelisted_email_domains: ['carto.com'])
+      @fake_organization = FactoryGirl.create(:organization_whitelist_carto)
       Organization.stubs(:where).returns([@fake_organization])
       get signup_url
       response.status.should == 200
@@ -35,11 +35,11 @@ describe SignupController do
     end
 
     it 'returns 404 for organizations with whitelisted domains but without any authentication enabled' do
-      @fake_organization = FactoryGirl.create(:organization,
-                                              whitelisted_email_domains: ['carto.com'],
-                                              auth_username_password_enabled: false,
+      @fake_organization = FactoryGirl.create(:organization_whitelist_carto,
+                                              auth_username_password_enabled: true,
                                               auth_google_enabled: false,
                                               auth_github_enabled: false)
+      @fake_organization.stubs(:auth_username_password_enabled).returns(false)
       Organization.stubs(:where).returns([@fake_organization])
       get signup_url
       response.status.should == 404
@@ -56,7 +56,7 @@ describe SignupController do
 
     it 'returns user error with admin mail if organization has not enough seats' do
       fake_owner = FactoryGirl.build(:valid_user)
-      @fake_organization = FactoryGirl.create(:organization, whitelisted_email_domains: ['carto.com'], seats: 0, owner: fake_owner)
+      @fake_organization = FactoryGirl.create(:organization_whitelist_carto, seats: 0, owner: fake_owner)
       Organization.stubs(:where).returns([@fake_organization])
       get signup_url
       response.status.should == 200