Merge branch 'master' into overviews-remove-rake

NEWS.md

@@ -2,13 +2,14 @@ Development
 -----------
 
 ### NOTICES
-- None yet
+- Ensuring right `search_path` for non organization `publicuser`
 
 ### Features
-- None yet
+- Improve dropping db role of an API key (#14307)
 
 ### Bug fixes / enhancements
-* Add `remove_overview_tables` rake
+- Add `remove_overview_tables` rake
+- Allowing views in API Keys (#14309)
 
 4.22.0 (2018-10-04)
 -------------------

app/models/carto/api_key.rb

@@ -320,27 +320,61 @@ module Carto
       return unless databases.present?
 
       databases[:tables].each do |table|
-        check_table(table)
+        if !check_table(table) && !check_view(table) && !check_materilized_view(table)
+          raise Carto::UnprocesableEntityError.new("relation \"#{table[:schema]}.#{table[:name]}\" does not exist")
+        end
       end
     end
 
     def check_table(table)
       begin
         result = db_run(%{
-                   SELECT *
-                   FROM
-                     pg_tables
-                   WHERE
-                     schemaname = #{db_connection.quote(table[:schema])} AND
-                     tablename = #{db_connection.quote(table[:name])}
-                 })
+          SELECT *
+          FROM
+            pg_tables
+          WHERE
+            schemaname = #{db_connection.quote(table[:schema])} AND
+            tablename = #{db_connection.quote(table[:name])}
+        })
       rescue StandardError => e
         raise_unprocessable_entity_error(e)
       end
 
-      if result && result.count.zero?
-        raise Carto::UnprocesableEntityError.new("relation \"#{table[:schema]}.#{table[:name]}\" does not exist")
+      result && !result.count.zero?
+    end
+
+    def check_view(view)
+      begin
+        result = db_run(%{
+          SELECT *
+          FROM
+            pg_views
+          WHERE
+            schemaname = #{db_connection.quote(view[:schema])} AND
+            viewname = #{db_connection.quote(view[:name])}
+        })
+      rescue StandardError => e
+        raise_unprocessable_entity_error(e)
+      end
+
+      result && !result.count.zero?
+    end
+
+    def check_materilized_view(matview)
+      begin
+        result = db_run(%{
+          SELECT *
+          FROM
+            pg_matviews
+          WHERE
+            schemaname = #{db_connection.quote(matview[:schema])} AND
+            matviewname = #{db_connection.quote(matview[:name])}
+        })
+      rescue StandardError => e
+        raise_unprocessable_entity_error(e)
       end
+
+      result && !result.count.zero?
     end
 
     def invalidate_cache
@@ -431,7 +465,7 @@ module Carto
     end
 
     def drop_db_role
-      revoke_privileges
+      db_run("DROP OWNED BY  \"#{db_role}\"")
       db_run("DROP ROLE \"#{db_role}\"")
     end
 
@@ -497,18 +531,6 @@ module Carto
       $users_metadata
     end
 
-    def revoke_privileges
-      affected_schemas.uniq.each do |schema|
-        db_run("REVOKE ALL PRIVILEGES ON ALL TABLES IN SCHEMA \"#{schema}\" FROM \"#{db_role}\"")
-        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)
       db_run("GRANT USAGE ON SCHEMA \"#{s}\" TO \"#{db_role}\"")
     end

app/models/user/db_service.rb

@@ -467,9 +467,7 @@ module CartoDB
               db.run(build_geocoder_server_config_sql(geocoder_api_config))
               db.run(build_entity_config_sql)
               db.run("ALTER USER \"#{@user.database_username}\" SET search_path TO #{build_search_path}")
-              if @user.organization_user?
-                db.run("ALTER USER \"#{@user.database_public_username}\" SET search_path TO #{build_search_path}")
-              end
+              db.run("ALTER USER \"#{@user.database_public_username}\" SET search_path TO #{build_search_path}")
             end
           end
           return true

spec/models/carto/api_key_spec.rb

@@ -159,6 +159,43 @@ describe Carto::ApiKey do
       @user1.in_database.run("ALTER TABLE \"wadus\"\"wadus\" RENAME TO #{old_name}")
     end
 
+    it 'grants view' do
+      view_name = 'cool_view'
+
+      validate_view_api_key(
+        view_name,
+        "CREATE VIEW #{view_name} AS SELECT * FROM #{@table1.name}",
+        "DROP VIEW #{view_name}"
+      )
+
+      validate_view_api_key(
+        view_name,
+        "CREATE MATERIALIZED VIEW #{view_name} AS SELECT * FROM #{@table1.name}",
+        "DROP MATERIALIZED VIEW #{view_name}"
+      )
+    end
+
+    def validate_view_api_key(view_name, create_query, drop_query)
+      @user1.in_database.run(create_query)
+      grants = [apis_grant(['sql']), database_grant(@table1.database_schema, view_name)]
+      api_key = @carto_user1.api_keys.create_regular_key!(name: 'grants_view', grants: grants)
+
+      with_connection_from_api_key(api_key) do |connection|
+        begin
+          connection.execute("select count(1) from #{@table1.name}")
+        rescue Sequel::DatabaseError => e
+          e.message.should include "permission denied for relation #{@table1.name}"
+        end
+
+        connection.execute("select count(1) from #{view_name}") do |result|
+          result[0]['count'].should eq '0'
+        end
+      end
+
+      @user1.in_database.run(drop_query)
+      api_key.destroy
+    end
+
     let (:grants) { [database_grant(@table1.database_schema, @table1.name), apis_grant] }
 
     describe '#destroy' do
@@ -507,5 +544,23 @@ describe Carto::ApiKey do
       table.destroy
       other_user.destroy
     end
+
+    it 'drop role with grants of objects owned by other user' do
+      user2 = TestUserFactory.new.create_test_user(unique_name('user'), @auth_organization)
+      table_user2 = create_table(user_id: user2.id)
+      schema_and_table_user2 = "\"#{table_user2.database_schema}\".#{table_user2.name}"
+
+      table_user1 = create_table(user_id: @carto_user1.id)
+      grants = [database_grant(table_user1.database_schema, table_user1.name), apis_grant]
+      api_key = @carto_user1.api_keys.create_regular_key!(name: 'full', grants: grants)
+
+      user2.in_database.run("GRANT SELECT ON #{schema_and_table_user2} TO \"#{api_key.db_role}\"")
+
+      expect { api_key.destroy! }.to_not raise_error
+
+      table_user1.destroy
+      table_user2.destroy
+      user2.destroy
+    end
   end
 end