Merge branch 'master' into 876-ie11-drag&drop

NEWS.md

@@ -124,7 +124,7 @@ ion for time-series (#12670)
   * Header authentication (#13329)
   * Keep API Key permissions up to date when tables change (#13333)
   * Delete API keys on user deletion (#13470)
-  * Inherit from public user for API key permissions (#13464)
+  * Inherit from public user for API key permissions (#13464, #13550)
   * Sync master key with user model (#13540)
   * Do not allow empty api list in Auth API [#13291](https://github.com/CartoDB/cartodb/issues/13291)
   * Conventions (#13491)
@@ -142,6 +142,7 @@ ion for time-series (#12670)
 * Add titles (and description) to embeds in mobile viewports (#13517)
 * User feed renders google maps properly when user has it enabled
 * Prevent destroying modals with `keepOpenOnRouteChange` property enabled on Builder when route changes. ([Support#1293](https://github.com/CartoDB/support/issues/1293))
+* Import gpkg without coordinate system. ([Support#1303](https://github.com/CartoDB/support/issues/1303))
 * Improved bundling aliases
 * Remove Tangram's vector rendering support in Builder embeds ([#13461](https://github.com/CartoDB/cartodb/issues/13461))
 * Remove Tangram references (#13461)

UPGRADE

@@ -1,5 +1,32 @@
+Standard migration (default)
+----------------------------
+
+Unless stated otherwise, assume any new CARTO release requires calling these commands:
+
+ $ bundle exec rake db:migrate
+
+NOTES:
+ - Redis server must be listening on the configured port (see config/app_config.yml) for some of the steps below to work
+ - Rails must be restarted after an upgrade
+
+Extension migration
+-------------------
+
+Most CARTO releases need an updated cartodb-postgresql extension. After installing it, it must be upgraded in all existing databases:
+
+ $ bundle exec rake cartodb:db:upgrade_postgres_extension
+
+Specific version notes
+======================
+
+3.1.0 onwards
+-------------
+
+Upgrade notes are now kept in NEWS.md. Check the NOTICE section of the release you are upgrading to.
+
 3.0.0 -> 3.1.0
 --------------
+
 * Run rake task:
     bundle exec rake cartodb:overlays:create_overlays
 
@@ -11,30 +38,13 @@
 
 2.x -> 3.0.0
 ------------
+
 * Install CartoDB extension version 0.3.x
 * Run rake tasks:
   - bundle exec rake cartodb:db:create_default_vis_permissions
   - bundle exec rake cartodb:db:populate_permission_entity_id
 
 
-General notes
--------------
-
-NOTES:
- - Redis server must be listening on the configured
-   port (see config/app_config.yml) for some of the
-   steps below to work
- - Rails must be restarted after an upgrade
-
-
-Standard migration (default)
-----------------------------
-
-Unless stated otherwise, assume any new CartoDB release requires calling these commands:
-
- $ bundle exec rake db:migrate
-
-
 Mandatory migration
 -------------------
 

app/controllers/carto/api/api_key_presenter.rb

@@ -21,19 +21,27 @@ def to_poro
             },
             {
               type: 'database',
-              tables: @api_key.table_permissions_from_db.map do |p|
-                {
-                  schema: p.schema,
-                  name: p.name,
-                  permissions: p.permissions
-                }
-              end
+              tables: table_permissions_for_api_key
             }
           ],
           created_at: @api_key.created_at.to_s,
           updated_at: @api_key.updated_at.to_s
         }
       end
+
+      private
+
+      def table_permissions_for_api_key
+        return [] if @api_key.master? || @api_key.default_public?
+
+        @api_key.table_permissions_from_db.map do |p|
+          {
+            schema: p.schema,
+            name: p.name,
+            permissions: p.permissions
+          }
+        end
+      end
     end
   end
 end

app/models/carto/api_key.rb

@@ -147,10 +147,8 @@ def table_permissions_from_db
           string_agg(DISTINCT lower(privilege_type),',') privilege_types
         FROM
           information_schema.table_privileges tp
-        LEFT JOIN
-          information_schema.applicable_roles ar ON tp.grantee = ar.role_name
         WHERE
-          ar.grantee = '#{db_role}' OR tp.grantee = '#{db_role}'
+          tp.grantee = '#{db_role}'
         GROUP BY
           table_schema,
           table_name;
@@ -197,8 +195,13 @@ def role_creation_queries
         "ALTER ROLE \"#{db_role}\" SET search_path TO #{user.db_service.build_search_path}"
       ]
 
+      # This is GRANTED to the organizational role for organization users, and the PUBLIC users for non-orgs
+      # We do not want to grant the organization role to the Api Keys, since that also opens access to the analysis
+      # catalog and tablemetadata. To be more consistent, we should probably GRANT this to the organization public
+      # user instead, but that has the downside of leaking quotas to the public.
+      # This works for now, but if you are adding new permissions, please reconsider this decision.
       if user.organization_user?
-        queries << "GRANT \"#{user.service.organization_member_group_role_member_name}\" TO \"#{db_role}\""
+        queries << "GRANT ALL ON FUNCTION \"#{user.database_schema}\"._CDB_UserQuotaInBytes() TO \"#{db_role}\""
       end
       queries
     end
@@ -308,6 +311,10 @@ def revoke_privileges
         db_run("REVOKE USAGE ON SCHEMA \"#{schema}\" FROM \"#{db_role}\"")
         db_run("REVOKE USAGE, SELECT ON ALL SEQUENCES IN SCHEMA \"#{schema}\" FROM \"#{db_role}\"")
       end
+
+      if user.organization_user?
+        db_run("REVOKE ALL ON FUNCTION \"#{user.database_schema}\"._CDB_UserQuotaInBytes() FROM \"#{db_role}\"")
+      end
     end
 
     def grant_aux_write_privileges_for_schema(s)

lib/tasks/db_maintenance.rake

@@ -247,23 +247,24 @@ namespace :cartodb do
 
     desc 'Upgrade cartodb postgresql extension'
     task :upgrade_postgres_extension, [:database_host, :version, :sleep, :statement_timeout] => :environment do |task_name, args|
-      raise "Sample usage: rake cartodb:db:upgrade_postgres_extension['127.0.0.1','0.5.2']" if args[:database_host].blank? or args[:version].blank?
-
       # Send this as string, not as number
       extension_version = args[:version]
       database_host = args[:database_host]
       sleep = args[:sleep].blank? ? 0.5 : args[:sleep].to_i
       statement_timeout = args[:statement_timeout].blank? ? 180000 : args[:statement_timeout] # 3 min by default
 
       puts "Upgrading cartodb extension with following config:"
-      puts "extension_version: #{extension_version}"
-      puts "database_host: #{database_host}"
+      puts "extension_version: #{extension_version || 'LATEST'}"
+      puts "database_host: #{database_host || 'ALL'}"
       puts "sleep: #{sleep}"
       puts "statement_timeout: #{statement_timeout}"
 
-      count = ::User.where(database_host: database_host).count
+      query = User
+      query = query.where(database_host: database_host) if database_host
+
+      count = query.count
 
-      ::User.where(database_host: database_host).order(Sequel.asc(:created_at)).each_with_index do |user, i|
+      query.order(Sequel.asc(:created_at)).each_with_index do |user, i|
         begin
           # We grant 2 x statement_timeout, by default 6 min
           Timeout::timeout(statement_timeout/1000 * 2) do

package.json

@@ -1,6 +1,6 @@
 {
   "name": "cartodb-ui",
-  "version": "4.11.43-876",
+  "version": "4.11.43",
   "description": "CARTO UI frontend",
   "repository": {
     "type": "git",

services/importer/lib/importer/loader.rb

@@ -254,7 +254,7 @@ def run_ogr2ogr(append_mode=false)
         ogr2ogr.run(append_mode)
 
         #In case there are not an specific error we try to fix it
-        if ogr2ogr.generic_error? && ogr2ogr.exit_code == 0
+        if ogr2ogr.generic_error? && ogr2ogr.exit_code.zero? || ogr2ogr.missing_srs?
           try_fallback(append_mode)
         end
 
@@ -291,6 +291,12 @@ def try_fallback(append_mode)
           ogr2ogr.overwrite = true
           ogr2ogr.encoding = "ISO-8859-1"
           ogr2ogr.run(append_mode)
+        elsif ogr2ogr.missing_srs?
+          job.log "Fallback: Source dataset has no coordinate system, forcing -s_srs 4326"
+          @job.fallback_executed = "srs 4326"
+          ogr2ogr.overwrite = true
+          ogr2ogr.shape_coordinate_system = '4326'
+          ogr2ogr.run(append_mode)
         end
         ogr2ogr.set_default_properties
       end

services/importer/lib/importer/ogr2ogr.rb

@@ -114,6 +114,10 @@ def too_many_columns?
         command_output =~ /tables can have at most 1600 columns/i
       end
 
+      def missing_srs?
+        exit_code == 256 && command_output =~ /Use -s_srs to set one/i
+      end
+
       def unsupported_format?
         exit_code == 256 && command_output =~ /Unable to open(.*)with the following drivers/i
       end

services/importer/spec/doubles/ogr2ogr.rb

@@ -14,21 +14,32 @@ def run(append_mode=false)
           Object.new
         end
 
-        def set_default_properties; return; end
-
-        def generic_error?; return; end
-        def encoding_error?; return; end
-        def invalid_dates?; return; end
-        def duplicate_column?; return; end
-        def invalid_geojson?; return; end
-        def too_many_columns?; return; end
-        def unsupported_format?; return; end
-        def file_too_big?; return; end
-        def statement_timeout?; return; end
-        def segfault_error?; return; end
-        def kml_style_missing?; return; end
+        def set_default_properties; end
+
+        def generic_error?; end
+
+        def encoding_error?; end
+
+        def invalid_dates?; end
+
+        def duplicate_column?; end
+
+        def invalid_geojson?; end
+
+        def too_many_columns?; end
+
+        def unsupported_format?; end
+
+        def file_too_big?; end
+
+        def statement_timeout?; end
+
+        def segfault_error?; end
+
+        def kml_style_missing?; end
+
+        def missing_srs?; end
       end
     end
   end
 end
-

services/importer/spec/unit/loader_spec.rb

@@ -49,6 +49,7 @@
       ogr2ogr_mock.stubs(:duplicate_column?).returns(false)
       ogr2ogr_mock.stubs(:segfault_error?).returns(false)
       ogr2ogr_mock.stubs(:kml_style_missing?).returns(false)
+      ogr2ogr_mock.stubs(:missing_srs?).returns(false)
       ogr2ogr_mock.stubs(:exit_code).returns(0)
       ogr2ogr_mock.stubs(:run).returns(Object.new).at_least_once
 

spec/models/carto/api_key_spec.rb

@@ -115,6 +115,18 @@ def with_connection_from_api_key(api_key)
       }.to raise_exception ActiveRecord::RecordInvalid
     end
 
+    it 'fails to access system tables' do
+      api_key = @carto_user1.api_keys.create_regular_key!(name: 'full', grants: [apis_grant])
+
+      with_connection_from_api_key(api_key) do |connection|
+        ['cdb_tablemetadata', 'cdb_analysis_catalog'].each do |table|
+          expect {
+            connection.execute("select count(1) from cartodb.#{table}")
+          }.to raise_exception /permission denied/
+        end
+      end
+    end
+
     describe '#destroy' do
       it 'removes the role from DB' do
         grants = [database_grant(@table1.database_schema, @table1.name), apis_grant]
@@ -254,8 +266,10 @@ def with_connection_from_api_key(api_key)
 
       it 'shows public tables' do
         api_key = @carto_user1.api_keys.default_public.first
-
-        api_key_permissions(api_key, @public_table.database_schema, @public_table.name).permissions.should eq ['select']
+        unless @carto_user1.has_organization?
+          api_key_permissions(api_key, @public_table.database_schema, @public_table.name)
+            .permissions.should eq ['select']
+        end
       end
     end
 

spec/models/data_import_spec.rb

@@ -347,6 +347,19 @@ def create_import_from_query(user: @user, overwrite:, from_query: '')
     table.should_not be_nil
   end
 
+  it 'imports a gpkg with no coordinate system' do
+    data_import = DataImport.create(
+      user_id: @user.id,
+      data_source: fake_data_path('no_coordinate_system_dataset.gpkg'),
+      updated_at: Time.now
+    ).run_import!
+
+    table = ::UserTable.where(id: data_import.table_id).first
+    table.should_not be_nil
+    table.name.should be == 'no_coordinate_system_dataset'
+    table.service.records[:rows].should have(1).items
+  end
+
   it 'should allow to create a table from a url' do
     data_import = nil
     CartoDB::Importer2::Downloader.any_instance.stubs(:validate_url!).returns(true)