Merge pull request #15757 from CartoDB/bug/ch80981/joinzoe-change-on-custom-api-key-after-import

Fix changes on custom API Key after import collision_strategy=overwrite

NEWS.md

@@ -9,6 +9,7 @@ Development
 
 ### Bug fixes / enhancements
 - Downgrade bundler to 1.17.3 to avoid problems with Rails version
+- Fix to prevent removing datasets from api_keys when it is replaced using overwrite as collision_strategy ([80981](https://app.clubhouse.io/cartoteam/story/80981/joinzoe-change-on-custom-api-key-after-import-collision-strategy-overwrite))
 
 4.39.0 (2020-07-20)
 -------------------

app/connectors/importer.rb

@@ -234,6 +234,24 @@ module CartoDB
         raise e
       end
 
+      # Restore the permissions (roles) for the table.
+      # These roles were dropped after the original table was
+      # replaced by the importer.
+      def restore_permissions_for(table_name)
+        # get the api keys of the original table
+        api_keys_table = Carto::ApiKey.where(user_id: user.id).select do |api_key|
+          api_key.grants.any? do |grant|
+            tables = grant[:tables]
+            next unless tables
+            tables.any? do |table|
+              table[:name] == table_name
+            end
+          end
+        end
+
+        api_keys_table.each { |api_key| api_key.setup_table_permissions }
+      end
+
       # Renames table from current_name to new_name.
       # It doesn't check if `new_name` is valid. To get a valid name use `Carto::ValidTableNameProposer`
       def rename(result, current_name, new_name, schema)
@@ -324,6 +342,7 @@ module CartoDB
         @table_setup.run_index_statements(index_statements, @database)
         @table_setup.recreate_overviews(name)
         @table_setup.update_cdb_tablemetadata(name)
+        restore_permissions_for(name)
       end
 
       private

app/models/carto/api_key.rb

@@ -362,6 +362,19 @@ module Carto
       db_run("GRANT \"#{effective_ownership_role_name}\" TO \"#{db_role}\"") if effective_ownership_role_name.present?
     end
 
+    def setup_table_permissions
+      setup_permissions(table_permissions) do |tp|
+        Carto::TableAndFriends.apply(db_connection, tp.schema, tp.name) do |schema, table_name, qualified_name|
+          db_run("GRANT #{tp.permissions.join(', ')} ON TABLE #{qualified_name} TO \"#{db_role}\"")
+          sequences_for_table(schema, table_name).each do |seq|
+            db_run("GRANT USAGE, SELECT ON SEQUENCE #{seq} TO \"#{db_role}\"")
+          end
+        end
+      end
+
+      schemas_from_granted_tables.each { |s| grant_usage_privileges_for_schema(s) }
+    end
+
     private
 
     PASSWORD_LENGTH = 40
@@ -505,19 +518,6 @@ module Carto
       grant_ownership_role_privileges
     end
 
-    def setup_table_permissions
-      setup_permissions(table_permissions) do |tp|
-        Carto::TableAndFriends.apply(db_connection, tp.schema, tp.name) do |schema, table_name, qualified_name|
-          db_run("GRANT #{tp.permissions.join(', ')} ON TABLE #{qualified_name} TO \"#{db_role}\"")
-          sequences_for_table(schema, table_name).each do |seq|
-            db_run("GRANT USAGE, SELECT ON SEQUENCE #{seq} TO \"#{db_role}\"")
-          end
-        end
-      end
-
-      schemas_from_granted_tables.each { |s| grant_usage_privileges_for_schema(s) }
-    end
-
     def setup_schema_permissions
       setup_permissions(schema_permissions) do |sp|
         db_run("GRANT #{sp.permissions.join(', ')} ON SCHEMA \"#{sp.name}\" TO \"#{db_role}\"")

spec/requests/api/imports_spec.rb

@@ -16,6 +16,10 @@ describe "Imports API" do
     @user.destroy
   end
 
+  def auth_params
+    { user_domain: @user.username, api_key: @user.api_key }
+  end
+
   let(:params) { { :api_key => @user.api_key } }
 
   it 'performs asynchronous imports' do
@@ -244,4 +248,64 @@ describe "Imports API" do
     @user.update max_import_table_row_count: old_max_import_row_count
   end
 
+  it 'keeps api_key grants for replaced tables' do
+    # imports two files
+    post api_v1_imports_create_url,
+      params.merge(:filename => upload_file('spec/support/data/csv_with_lat_lon.csv', 'application/octet-stream'))
+    post api_v1_imports_create_url,
+      params.merge(:filename => upload_file('spec/support/data/csv_with_number_columns.csv', 'application/octet-stream'))
+
+    # get the last one
+    @table_from_import = UserTable.all.last.service
+
+    # creates an api_key for both files
+    grants = [
+      {
+        type: "apis",
+        apis: ["sql", "maps"]
+      },
+      {
+        type: "database",
+        tables: [
+          {
+            schema: @user.database_schema,
+            name: 'csv_with_lat_lon',
+            permissions: [
+              "insert",
+              "select",
+              "update",
+              "delete"
+            ]
+          },
+          {
+            schema: @user.database_schema,
+            name: 'csv_with_number_columns',
+            permissions: [
+              "select"
+            ]
+          }
+        ]
+      }
+    ]
+    name = 'wadus'
+    payload = {
+      name: name,
+      grants: grants
+    }
+    post_json api_keys_url, auth_params.merge(payload), http_json_headers
+
+    # replace the file
+    post api_v1_imports_create_url,
+      params.merge(
+        :filename => upload_file('spec/support/data/csv_with_number_columns.csv', 'application/octet-stream'),
+        :collision_strategy => 'overwrite'
+      )
+
+    # gets the api_keys
+    get_json api_key_url(id: 'wadus'), auth_params, http_json_headers do |response|
+      response.status.should eq 200
+      response.body[:grants][1][:tables][1][:name] = @table_from_import
+    end
+  end
+
 end