From 024d05cddb39fc98af14791816bf5c181d034a9c Mon Sep 17 00:00:00 2001 From: Nivedha Date: Mon, 5 Sep 2022 16:43:57 +0530 Subject: [PATCH] feat(spanner): Support fine grained access control (#19067) --- .../spanner/client/batch_update_test.rb | 1 - .../spanner/fine_grain_access_control_test.rb | 80 +++++++++++++++++++ .../lib/google/cloud/spanner/client.rb | 15 +++- .../lib/google/cloud/spanner/pool.rb | 3 +- .../lib/google/cloud/spanner/project.rb | 5 +- .../lib/google/cloud/spanner/service.rb | 8 +- .../test/google/cloud/spanner/project_test.rb | 21 +++++ .../test/google/cloud/spanner/service_test.rb | 67 ++++++++++++---- 8 files changed, 173 insertions(+), 27 deletions(-) create mode 100644 google-cloud-spanner/acceptance/spanner/fine_grain_access_control_test.rb diff --git a/google-cloud-spanner/acceptance/spanner/client/batch_update_test.rb b/google-cloud-spanner/acceptance/spanner/client/batch_update_test.rb index eae641ede3fc..b85140dd0307 100644 --- a/google-cloud-spanner/acceptance/spanner/client/batch_update_test.rb +++ b/google-cloud-spanner/acceptance/spanner/client/batch_update_test.rb @@ -121,7 +121,6 @@ it "executes multiple DML statements in a batch with syntax error for #{dialect}" do prior_results = db[dialect].execute_sql "SELECT * FROM accounts" - p prior_results _(prior_results.rows.count).must_equal 3 timestamp = db[dialect].transaction do |tx| diff --git a/google-cloud-spanner/acceptance/spanner/fine_grain_access_control_test.rb b/google-cloud-spanner/acceptance/spanner/fine_grain_access_control_test.rb new file mode 100644 index 000000000000..11de75db3cde --- /dev/null +++ b/google-cloud-spanner/acceptance/spanner/fine_grain_access_control_test.rb @@ -0,0 +1,80 @@ +# Copyright 2022 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + + +require "spanner_helper" + +describe "Fine Grained Access Control", :spanner do + let(:table_name) { "stuffs" } + let(:db) { spanner } + let(:db_client) { spanner_client } + let(:admin) { $spanner_db_admin } + let(:instance_id) { $spanner_instance_id } + let(:database_id) { $spanner_database_id } + let(:role) { "selector" } + + before do + skip if emulator_enabled? + db_client.delete table_name # remove all data + db_client.insert table_name, [ + { id: 1, bool: false }, + { id: 2, bool: false }, + { id: 3, bool: true }, + { id: 4, bool: false }, + { id: 5, bool: true } + ] + + db_path = admin.database_path project: db.project_id, + instance: instance_id, + database: database_id + + db_job = admin.update_database_ddl database: db_path, statements: [ + "CREATE ROLE #{role}", + "GRANT SELECT ON TABLE #{table_name} TO ROLE #{role}" + ] + db_job.wait_until_done! + end + + it "should be able to do granted actions for role" do + skip if emulator_enabled? + selector_client = db.client $spanner_instance_id, $spanner_database_id, database_role: role + _(selector_client.read(table_name, [:id]).rows.map(&:to_h)).must_equal [{ id: 1 }, + { id: 2 }, + { id: 3 }, + { id: 4 }, + { id: 5 }] + end + + it "should give error for actions without access" do + skip if emulator_enabled? + selector_client = db.client $spanner_instance_id, $spanner_database_id, database_role: role + error = assert_raises Google::Cloud::PermissionDeniedError do + selector_client.insert table_name, [ + { id: 1, bool: false } + ] + end + + assert_includes error.message, "Role selector does not have required privileges on table #{table_name}" + end + + it "should give error when database role does not exists" do + skip if emulator_enabled? + + error = assert_raises Google::Cloud::PermissionDeniedError do + db.client $spanner_instance_id, $spanner_database_id, database_role: "unknown" + end + + assert_includes error.message, "Role not found: unknown" + end +end diff --git a/google-cloud-spanner/lib/google/cloud/spanner/client.rb b/google-cloud-spanner/lib/google/cloud/spanner/client.rb index ffca31a79298..14ba8645f9b8 100644 --- a/google-cloud-spanner/lib/google/cloud/spanner/client.rb +++ b/google-cloud-spanner/lib/google/cloud/spanner/client.rb @@ -52,10 +52,11 @@ class Client ## # @private Creates a new Spanner Client instance. def initialize project, instance_id, database_id, session_labels: nil, - pool_opts: {}, query_options: nil + pool_opts: {}, query_options: nil, database_role: nil @project = project @instance_id = instance_id @database_id = database_id + @database_role = database_role @session_labels = session_labels @pool = Pool.new self, **pool_opts @query_options = query_options @@ -97,6 +98,12 @@ def database @project.database instance_id, database_id end + # The Spanner session creator role. + # @return [String] + def database_role + @database_role + end + # A hash of values to specify the custom query options for executing # SQL query. # @return [Hash] @@ -2115,7 +2122,8 @@ def create_new_session Admin::Database::V1::DatabaseAdmin::Paths.database_path( project: project_id, instance: instance_id, database: database_id ), - labels: @session_labels + labels: @session_labels, + database_role: @database_role Session.from_grpc grpc, @project.service, query_options: @query_options end @@ -2144,7 +2152,8 @@ def batch_create_sessions session_count project: project_id, instance: instance_id, database: database_id ), session_count, - labels: @session_labels + labels: @session_labels, + database_role: @database_role resp.session.map { |grpc| Session.from_grpc grpc, @project.service, query_options: @query_options } end diff --git a/google-cloud-spanner/lib/google/cloud/spanner/pool.rb b/google-cloud-spanner/lib/google/cloud/spanner/pool.rb index 63efd42ebc4e..8b21888dee5b 100644 --- a/google-cloud-spanner/lib/google/cloud/spanner/pool.rb +++ b/google-cloud-spanner/lib/google/cloud/spanner/pool.rb @@ -277,8 +277,7 @@ def can_allocate_more_sessions? end def create_keepalive_task! - @keepalive_task = Concurrent::TimerTask.new(execution_interval: 300, - timeout_interval: 60) do + @keepalive_task = Concurrent::TimerTask.new execution_interval: 300 do keepalive_or_release! end @keepalive_task.execute diff --git a/google-cloud-spanner/lib/google/cloud/spanner/project.rb b/google-cloud-spanner/lib/google/cloud/spanner/project.rb index 89fe0275107d..c2b5a5e3fb40 100644 --- a/google-cloud-spanner/lib/google/cloud/spanner/project.rb +++ b/google-cloud-spanner/lib/google/cloud/spanner/project.rb @@ -569,7 +569,7 @@ def create_database instance_id, database_id, statements: [], # end # def client instance_id, database_id, pool: {}, labels: nil, - query_options: nil + query_options: nil, database_role: nil # Convert from possible Google::Protobuf::Map labels = labels.to_h { |k, v| [String(k), String(v)] } if labels # Configs set by environment variables take over client-level configs. @@ -581,7 +581,8 @@ def client instance_id, database_id, pool: {}, labels: nil, Client.new self, instance_id, database_id, session_labels: labels, pool_opts: valid_session_pool_options(pool), - query_options: query_options + query_options: query_options, + database_role: database_role end ## diff --git a/google-cloud-spanner/lib/google/cloud/spanner/service.rb b/google-cloud-spanner/lib/google/cloud/spanner/service.rb index 14dca2a18f48..7af25a9f02ad 100644 --- a/google-cloud-spanner/lib/google/cloud/spanner/service.rb +++ b/google-cloud-spanner/lib/google/cloud/spanner/service.rb @@ -293,20 +293,20 @@ def get_session session_name, call_options: nil end def create_session database_name, labels: nil, - call_options: nil + call_options: nil, database_role: nil opts = default_options session_name: database_name, call_options: call_options - session = V1::Session.new labels: labels if labels + session = V1::Session.new labels: labels, creator_role: database_role if labels || database_role service.create_session( { database: database_name, session: session }, opts ) end def batch_create_sessions database_name, session_count, labels: nil, - call_options: nil + call_options: nil, database_role: nil opts = default_options session_name: database_name, call_options: call_options - session = V1::Session.new labels: labels if labels + session = V1::Session.new labels: labels, creator_role: database_role if labels || database_role # The response may have fewer sessions than requested in the RPC. request = { database: database_name, diff --git a/google-cloud-spanner/test/google/cloud/spanner/project_test.rb b/google-cloud-spanner/test/google/cloud/spanner/project_test.rb index 6b8f493c7544..4ebdb67fbdc2 100644 --- a/google-cloud-spanner/test/google/cloud/spanner/project_test.rb +++ b/google-cloud-spanner/test/google/cloud/spanner/project_test.rb @@ -15,8 +15,29 @@ require "helper" describe Google::Cloud::Spanner::Project, :mock_spanner do + let(:instance_id) { "my-instance-id" } + let(:database_id) { "my-database-id" } + let(:session_id) { "session123" } + let(:session_grpc) { + Google::Cloud::Spanner::V1::Session.new name: session_path(instance_id, database_id, session_id) + } + let(:session) { Google::Cloud::Spanner::Session.from_grpc session_grpc, spanner.service } + let(:batch_create_sessions_grpc) { + Google::Cloud::Spanner::V1::BatchCreateSessionsResponse.new session: [session_grpc] + } + it "knows the project identifier" do _(spanner).must_be_kind_of Google::Cloud::Spanner::Project _(spanner.project_id).must_equal project end + + it "creates client with database role" do + mock = Minitest::Mock.new + request_session = Google::Cloud::Spanner::V1::Session.new labels: nil, creator_role: "test_role" + mock.expect :batch_create_sessions, batch_create_sessions_grpc, [Hash,::Gapic::CallOptions] + spanner.service.mocked_service = mock + + client = spanner.client instance_id, database_id, pool: { min: 1, max: 1 }, database_role: "test-role" + _(client.database_role).must_equal "test-role" + end end diff --git a/google-cloud-spanner/test/google/cloud/spanner/service_test.rb b/google-cloud-spanner/test/google/cloud/spanner/service_test.rb index 86e015c11b9e..d0d373b717c7 100644 --- a/google-cloud-spanner/test/google/cloud/spanner/service_test.rb +++ b/google-cloud-spanner/test/google/cloud/spanner/service_test.rb @@ -12,22 +12,59 @@ # See the License for the specific language governing permissions and # limitations under the License. -describe Google::Cloud::Spanner::Service do +require "helper" + +describe Google::Cloud::Spanner::Service, :mock_spanner do + let(:instance_id) { "my-instance-id" } + let(:database_id) { "my-database-id" } + let(:session_id) { "session123" } + let(:default_options) { ::Gapic::CallOptions.new metadata: { "google-cloud-resource-prefix" => database_path(instance_id, database_id) } } + let(:session_grpc) { Google::Cloud::Spanner::V1::Session.new name: session_path(instance_id, database_id, session_id) } + describe ".new" do - it "sets quota_project with given value" do - expected_quota_project = "test_quota_project" - service = Google::Cloud::Spanner::Service.new( - "test_project", nil, quota_project: expected_quota_project - ) - assert_equal expected_quota_project, service.quota_project - end + it "sets quota_project with given value" do + expected_quota_project = "test_quota_project" + service = Google::Cloud::Spanner::Service.new( + "test_project", nil, quota_project: expected_quota_project + ) + assert_equal expected_quota_project, service.quota_project + end + + it "sets quota_project from credentials if not given from config" do + expected_quota_project = "test_quota_project" + service = Google::Cloud::Spanner::Service.new( + "test_project", OpenStruct.new(quota_project_id: expected_quota_project) + ) + assert_equal expected_quota_project, service.quota_project + end + + end + + describe ".create_session" do + it "creates session with given database role" do + mock = Minitest::Mock.new + session = Google::Cloud::Spanner::V1::Session.new labels: nil, creator_role: "test_role" + mock.expect :create_session, session_grpc, [{ database: database_path(instance_id, database_id), session: session }, default_options] + service = Google::Cloud::Spanner::Service.new( + "test_project", OpenStruct.new(client: OpenStruct.new(updater_proc: Proc.new{""})) + ) + service.mocked_service = mock + service.create_session database_path(instance_id, database_id), database_role: "test_role" + mock.verify + end + end - it "sets quota_project from credentials if not given from config" do - expected_quota_project = "test_quota_project" - service = Google::Cloud::Spanner::Service.new( - "test_project", OpenStruct.new(quota_project_id: expected_quota_project) - ) - assert_equal expected_quota_project, service.quota_project - end + describe ".batch_create_sessions" do + it "batch creates session with given database role" do + mock = Minitest::Mock.new + session = Google::Cloud::Spanner::V1::Session.new labels: nil, creator_role: "test_role" + mock.expect :batch_create_sessions, OpenStruct.new(session: Array.new(10) { session_grpc }), [{database: database_path(instance_id, database_id), session_count: 10, session_template: session }, default_options] + service = Google::Cloud::Spanner::Service.new( + "test_project", OpenStruct.new(client: OpenStruct.new(updater_proc: Proc.new{""})) + ) + service.mocked_service = mock + service.batch_create_sessions database_path(instance_id, database_id), 10, database_role: "test_role" + mock.verify + end end end \ No newline at end of file