Merge pull request #15614 from CartoDB/2496_oauth_whitelist

check whitelist when signing up from login form with OAuth providers

Author: gonzaloriestra

Makefile

@@ -296,6 +296,7 @@ SPEC_HELPER_MIN_SPECS = \
 	spec/requests/carto/superadmin/user_migration_imports_spec.rb \
 	spec/requests/carto/superadmin/user_migration_exports_spec.rb \
 	spec/requests/carto/saml_controller_spec.rb \
+	spec/requests/carto/oauth_login_controller_spec.rb \
 	spec/services/carto/user_table_index_service_spec.rb \
 	spec/services/carto/user_metadata_export_service_spec.rb \
 	spec/services/carto/organization_metadata_export_service_spec.rb \

NEWS.md

@@ -40,6 +40,7 @@ sudo make install
 - Set node 10.15.1 as default and only for building assets, removing 6.9.2 ([#15530](https://github.com/CartoDB/cartodb/issues/15530))
 - Update toolkit libraries to fix case sensitive fields ([#15569](https://github.com/CartoDB/cartodb/pull/15569))
 - Fix to avoid locks when sorting rows in dataset table ([#2399](https://github.com/CartoDB/support/issues/2399))
+- Fix whitelisted domains for OAuth signup ([#2495]https://github.com/CartoDB/support/issues/2495))
 - Lazy loading of Dashboard routes ([#15581](https://github.com/CartoDB/cartodb/pull/15581))
 
 4.36.0 (2020-03-09)

app/controllers/carto/oauth_login_controller.rb

@@ -75,7 +75,7 @@ def login(api)
     def signup(api)
       org_name = @organization_name
       @organization = ::Organization.where(name: org_name).first if org_name.present?
-      unless @organization.present? && api.config.auth_enabled?(@organization)
+      unless @organization.present? && signup_page_enabled?(api)
         return redirect_to CartoDB.url(self, 'login')
       end
 
@@ -101,5 +101,9 @@ def signup(api)
         end
       end
     end
+
+    def signup_page_enabled?(api)
+      api.config.auth_enabled?(@organization) && @organization.whitelisted_email_domains.present?
+    end
   end
 end

spec/factories/organizations.rb

@@ -32,6 +32,11 @@
       auth_username_password_enabled true
     end
 
+    factory :organization_google_whitelist_empty do
+      whitelisted_email_domains []
+      auth_google_enabled true
+    end
+
     factory :organization_with_users do
       after(:create) do |org|
         create_account_type_fg('ORGANIZATION USER')

spec/requests/carto/oauth_login_controller_spec.rb

@@ -0,0 +1,42 @@
+require 'spec_helper_min'
+
+describe Carto::OauthLoginController do
+  before(:all) do
+    @organization = FactoryGirl.create(:organization_google_whitelist_empty)
+  end
+
+  after(:all) do
+    @organization.destroy
+  end
+
+  it 'does not allow Google signup if whitelisted domains is empty' do
+    Carto::Oauth::Google::Api.any_instance.stubs(:user).returns(nil)
+    Carto::Oauth::Google::Config.stubs(:config).returns('client_id' => '11')
+    Carto::Oauth::Client.any_instance.stubs(:exchange_code_for_token).returns('123')
+    Carto::OauthLoginController.any_instance.stubs(:valid_authenticity_token?).returns(true)
+
+    CartoDB::UserAccountCreator.any_instance.expects(:new).never
+    get google_oauth_url(user_domain: @organization.name,
+                        code: 'blabla',
+                        state: '{"organization_name": "' + @organization.name + '"}')
+    response.status.should eq 302
+    follow_redirect!
+    request.path.should eq '/login'
+  end
+
+  it 'allows Google signup with whitelisted domains' do
+    @organization.whitelisted_email_domains = ['*gmail.com']
+    @organization.save
+
+    Carto::Oauth::Google::Api.any_instance.stubs(:user).returns(nil)
+    Carto::Oauth::Google::Config.stubs(:config).returns('client_id' => '11')
+    Carto::Oauth::Client.any_instance.stubs(:exchange_code_for_token).returns('123')
+    Carto::OauthLoginController.any_instance.stubs(:valid_authenticity_token?).returns(true)
+
+    CartoDB::UserAccountCreator.any_instance.expects(:valid?).once
+    get google_oauth_url(user_domain: @organization.name,
+                        code: 'blabla',
+                        state: '{"organization_name": "' + @organization.name + '"}')
+    response.status.should eq 200
+  end
+end