Sync DO API keys between clouds

NEWS.md

@@ -29,6 +29,7 @@ sudo make install
 - Cleanup after [#16189](https://github.com/CartoDB/cartodb/pull/16189). See [#16200](https://github.com/CartoDB/cartodb/pull/16200)
 - Split configuration for Message Broker & Central login redirection [#16150](https://github.com/CartoDB/cartodb/pull/16150)
 - Remove Data Library gallery page (now redirected to Spatial Data Catalog) [#16133](https://github.com/CartoDB/cartodb/pull/16133)
+- Sync DO API keys betwenn onpremises & CARTO-managed clouds [#16205](https://github.com/CartoDB/cartodb/pull/16205)
 
 ### Bug fixes / enhancements
 

app/commands/remote_do_api_key_commands/create.rb

@@ -0,0 +1,12 @@
+module RemoteDoApiKeyCommands
+  class Create < ::CartoCommand
+
+    private
+
+    def run_command
+      api_key = Carto::RemoteDoApiKey.new(params)
+      api_key.save!
+    end
+
+  end
+end

app/commands/remote_do_api_key_commands/destroy.rb

@@ -0,0 +1,12 @@
+module RemoteDoApiKeyCommands
+  class Destroy < ::CartoCommand
+
+    private
+
+    def run_command
+      api_key = Carto::RemoteDoApiKey.new(params)
+      api_key.destroy!
+    end
+
+  end
+end

app/helpers/message_broker_helper.rb

@@ -0,0 +1,11 @@
+module MessageBrokerHelper
+
+  def message_broker
+    Carto::Common::MessageBroker.new(logger: Rails.logger)
+  end
+
+  def cartodb_central_topic
+    message_broker.get_topic(:cartodb_central)
+  end
+
+end

app/models/carto/api_key.rb

@@ -9,6 +9,9 @@ module Carto
   class ApiKey < ActiveRecord::Base
 
     include Carto::AuthTokenGenerator
+    include ::MessageBrokerHelper
+
+    REDIS_KEY_PREFIX = 'api_keys:'.freeze
 
     TYPE_REGULAR = 'regular'.freeze
     TYPE_MASTER = 'master'.freeze
@@ -65,15 +68,19 @@ class ApiKey < ActiveRecord::Base
     validate :valid_default_public_key, if: :default_public?
 
     after_create :setup_db_role, if: ->(k) { k.needs_setup? && !k.skip_role_setup }
+    after_create :create_remote_do_api_key, if: ->(api_key) { api_key.master? }
+
     after_save { remove_from_redis(redis_key(token_was)) if token_changed? }
     after_save { invalidate_cache if token_changed? }
     after_save :add_to_redis, if: :valid_user?
     after_save :save_cdb_conf_info, unless: :skip_cdb_conf_info?
+    after_save :regenerate_remote_do_api_key, if: ->(k) { k.master? && k.token_changed? }
 
     after_destroy :reassign_owner, :drop_db_role, if: ->(k) { k.needs_setup? && !k.skip_role_setup }
     after_destroy :remove_from_redis
     after_destroy :invalidate_cache
     after_destroy :remove_cdb_conf_info, unless: :skip_cdb_conf_info?
+    after_destroy :destroy_remote_do_api_key, if: ->(api_key) { api_key.master? }
 
     scope :master, -> { where(type: TYPE_MASTER) }
     scope :default_public, -> { where(type: TYPE_DEFAULT_PUBLIC) }
@@ -150,6 +157,35 @@ def self.new_from_hash(api_key_hash)
       )
     end
 
+    def create_remote_do_api_key
+      cartodb_central_topic.publish(
+        :create_remote_do_api_key,
+        { type: type, token: token, user_id: user_id, username: user.username }
+      )
+    end
+
+    def regenerate_remote_do_api_key
+      original_token, saved_token = token_change
+
+      # Order of execution is not guaranteed, but since the token is different there's no danger
+      # of race conditions
+      cartodb_central_topic.publish(
+        :destroy_remote_do_api_key,
+        { token: original_token, user_id: user_id, username: user.username }
+      )
+      cartodb_central_topic.publish(
+        :create_remote_do_api_key,
+        { type: type, token: saved_token, user_id: user_id, username: user.username }
+      )
+    end
+
+    def destroy_remote_do_api_key
+      cartodb_central_topic.publish(
+        :destroy_remote_do_api_key,
+        { token: token, user_id: user_id, username: user.username }
+      )
+    end
+
     def revoke_permissions(table, revoked_permissions)
       Carto::TableAndFriends.apply(db_connection, table.database_schema, table.name) do |s, t, qualified_name|
         query = %{
@@ -379,8 +415,6 @@ def setup_table_permissions
 
     PASSWORD_LENGTH = 40
 
-    REDIS_KEY_PREFIX = 'api_keys:'.freeze
-
     def raise_unprocessable_entity_error(error)
       raise Carto::UnprocesableEntityError.new(/PG::Error: ERROR:  (.+)/ =~ error.message && $1 || 'Unexpected error')
     end

app/models/carto/remote_do_api_key.rb

@@ -0,0 +1,34 @@
+module Carto
+  class RemoteDoApiKey
+
+    attr_accessor :token, :type, :username
+    attr_reader :redis_client
+
+    def initialize(attributes = {})
+      attributes = attributes.with_indifferent_access
+      @token = attributes[:token]
+      @username = attributes[:username]
+      @type = attributes[:type]
+      @redis_client = $users_metadata
+    end
+
+    def save!
+      redis_client.hmset(redis_key, redis_hash_as_array)
+    end
+
+    def destroy!
+      redis_client.del(redis_key)
+    end
+
+    private
+
+    def redis_key
+      "#{Carto::ApiKey::REDIS_KEY_PREFIX}#{username}:#{token}"
+    end
+
+    def redis_hash_as_array
+      ['user', username, 'type', type]
+    end
+
+  end
+end

app/models/concerns/cartodb_central_synchronizable.rb

@@ -1,5 +1,7 @@
 module CartodbCentralSynchronizable
 
+  include ::MessageBrokerHelper
+
   def user?
     is_a?(::User) || is_a?(Carto::User)
   end
@@ -59,12 +61,10 @@ def update_in_central
         cartodb_central_client.update_user(username, allowed_attributes_to_central(:update))
       end
     elsif organization?
-      Carto::Common::MessageBroker.new(logger: Rails.logger)
-                                  .get_topic(:cartodb_central)
-                                  .publish(
-                                    :update_organization,
-                                    { organization: allowed_attributes_to_central(:update) }
-                                  )
+      cartodb_central_topic.publish(
+        :update_organization,
+        { organization: allowed_attributes_to_central(:update) }
+      )
     end
 
     true

lib/cartodb/central.rb

@@ -1,8 +1,11 @@
 require_relative '../carto/http/client'
+require './app/helpers/message_broker_helper'
 
 module Cartodb
   class Central
 
+    include ::MessageBrokerHelper
+
     def self.message_broker_sync_enabled?
       Cartodb.get_config(:message_broker, 'project_id').present? &&
         Cartodb.get_config(:message_broker, 'central_subscription_name').present?
@@ -199,10 +202,5 @@ def delete_oauth_app(username, app_id)
       send_request("api/users/#{username}/oauth_apps/#{app_id}", nil, :delete, [204])
     end
 
-    private
-
-    def cartodb_central_topic
-      Carto::Common::MessageBroker.new(logger: Rails.logger).get_topic(:cartodb_central)
-    end
   end
 end

lib/tasks/central_updates_subscriber.rake

@@ -107,6 +107,20 @@ namespace :message_broker do
         ).run
       end
 
+      subscription.register_callback(:create_do_api_key) do |message|
+        RemoteDoApiKeyCommands::Create.new(
+          message.payload,
+          { logger: logger, request_id: message.request_id }
+        ).run
+      end
+
+      subscription.register_callback(:destroy_do_api_key) do |message|
+        RemoteDoApiKeyCommands::Destroy.new(
+          message.payload,
+          { logger: logger, request_id: message.request_id }
+        ).run
+      end
+
       subscription.register_callback(:set_do_gcloud_settings) do |message|
         GcloudUserSettingsCommands::Set.new(
           message.payload,

script/ci/cloudbuild-tests-pr-pg12.yaml

@@ -83,7 +83,7 @@ steps:
     - -cx
     - |
       docker pull gcr.io/cartodb-on-gcp-main-artifacts/builder:${_BRANCH_TAG}
-      if [ $? -ne 0 ] 
+      if [ $? -ne 0 ]
       then
           docker pull gcr.io/cartodb-on-gcp-main-artifacts/builder:latest
           docker tag gcr.io/cartodb-on-gcp-main-artifacts/builder:latest gcr.io/cartodb-on-gcp-main-artifacts/builder:${_BRANCH_TAG}
@@ -133,7 +133,7 @@ steps:
       fi
 
 
-substitutions: 
+substitutions:
     _BRANCH_TAG: ${BRANCH_NAME//\//-}
 options:
     machineType: 'N1_HIGHCPU_32'

spec/commands/remote_do_api_key_commands/create_spec.rb

@@ -0,0 +1,18 @@
+require 'spec_helper'
+
+describe RemoteDoApiKeyCommands::Create do
+  let(:command) { described_class.new(params) }
+  let(:params) do
+    {
+      token: '1234-abcd-5678',
+      username: 'perico',
+      type: Carto::ApiKey::TYPE_MASTER
+    }
+  end
+
+  describe '#run' do
+    it 'runs OK' do
+      command.run
+    end
+  end
+end

spec/commands/remote_do_api_key_commands/destroy_spec.rb

@@ -0,0 +1,11 @@
+require 'spec_helper'
+
+describe RemoteDoApiKeyCommands::Destroy do
+  let(:command) { described_class.new(token: '1234-abcd-5678', username: 'perico') }
+
+  describe '#run' do
+    it 'runs OK' do
+      command.run
+    end
+  end
+end

spec/factories/users.rb

@@ -12,10 +12,10 @@
   factory :user, class: ::User do
     to_create(&:save)
 
-    username               { unique_name('user') }
-    email                  { unique_email }
-    password               { email.split('@').first }
-    password_confirmation  { email.split('@').first }
+    sequence(:username) { |i| "#{Faker::Internet.username(separators: [])}#{i}" }
+    email                  { "#{username}@example.com" }
+    password               { "#{username}123" }
+    password_confirmation  { password }
     table_quota            5
     quota_in_bytes         5000000
     id                     { Carto::UUIDHelper.random_uuid }
@@ -78,8 +78,8 @@
   end
 
   factory :carto_user, class: Carto::User do
-    username { unique_name('user') }
-    email { unique_email }
+    username { Faker::Internet.username(separators: ['-']) }
+    email { Faker::Internet.safe_email }
 
     password { email.split('@').first }
     password_confirmation { email.split('@').first }
@@ -131,8 +131,8 @@
 
   # Light user: database creation etc is skipped
   factory :carto_user_light, class: Carto::User do
-    username { unique_name('user') }
-    email { unique_email }
+    username { Faker::Internet.username(separators: ['-']) }
+    email { Faker::Internet.safe_email }
 
     password { email.split('@').first }
     password_confirmation { email.split('@').first }

spec/models/carto/remote_do_api_key_spec.rb

@@ -0,0 +1,40 @@
+require 'spec_helper_min'
+
+describe Carto::RemoteDoApiKey do
+  let(:username) { 'random-user' }
+  let(:type) { Carto::ApiKey::TYPE_MASTER }
+  let(:token) { '1234-abcd-5678' }
+  let(:redis_key) { 'api_keys:random-user:1234-abcd-5678' }
+  let(:api_key) { described_class.new(token: token, username: username, type: type) }
+
+  after { clean_redis_databases }
+
+  describe '#initialize' do
+    it 'correctly initializes the record' do
+      expect(api_key.token).to eq(token)
+      expect(api_key.username).to eq(username)
+      expect(api_key.type).to eq(type)
+      expect(api_key.redis_client).to be_present
+    end
+  end
+
+  describe '#save!' do
+    it 'saves the API key to redis' do
+      api_key.save!
+
+      expect($users_metadata.hgetall(redis_key)).to(
+        eq('user' => 'random-user', 'type' => 'master')
+      )
+    end
+  end
+
+  describe '#destroy!' do
+    before { api_key.save! }
+
+    it 'destroys the API key from redis' do
+      api_key.destroy!
+
+      expect($users_metadata.hgetall(redis_key)).to eq({})
+    end
+  end
+end