diff --git a/NEWS.md b/NEWS.md index d5793a3919..0f2357b7ca 100644 --- a/NEWS.md +++ b/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) ------------------- diff --git a/app/connectors/importer.rb b/app/connectors/importer.rb index 726f4c96fc..eb4b26b801 100644 --- a/app/connectors/importer.rb +++ b/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 diff --git a/app/models/carto/api_key.rb b/app/models/carto/api_key.rb index a239c8d27e..cc7e548bdd 100644 --- a/app/models/carto/api_key.rb +++ b/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}\"") diff --git a/spec/requests/api/imports_spec.rb b/spec/requests/api/imports_spec.rb index 00b0b5cda6..63af91a187 100644 --- a/spec/requests/api/imports_spec.rb +++ b/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