From 6cb0b4e9422e22c8fe0554248aec0b8fd7b2849a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Cie=C5=9Blak?= Date: Wed, 4 Sep 2024 15:21:15 +0200 Subject: [PATCH] chore: Improve user test and add manual test for user default database and role (#3035) ## Changes - Address https://github.com/Snowflake-Labs/terraform-provider-snowflake/pull/3034#discussion_r1742132935 - Add a manual test that shows the behavior of the default namespace and default role for a user when logging in with different conditions. --- pkg/acceptance/helpers/user_client.go | 6 + pkg/manual_tests/README.md | 8 + .../user_default_database_and_role/test.md | 218 ++++++++++++++++++ pkg/resources/user_acceptance_test.go | 46 ++++ 4 files changed, 278 insertions(+) create mode 100644 pkg/manual_tests/README.md create mode 100644 pkg/manual_tests/user_default_database_and_role/test.md diff --git a/pkg/acceptance/helpers/user_client.go b/pkg/acceptance/helpers/user_client.go index dae89b0e39..6316ff26de 100644 --- a/pkg/acceptance/helpers/user_client.go +++ b/pkg/acceptance/helpers/user_client.go @@ -45,6 +45,12 @@ func (c *UserClient) CreateUserWithOptions(t *testing.T, id sdk.AccountObjectIde return user, c.DropUserFunc(t, id) } +func (c *UserClient) Alter(t *testing.T, id sdk.AccountObjectIdentifier, opts *sdk.AlterUserOptions) { + t.Helper() + err := c.client().Alter(context.Background(), id, opts) + require.NoError(t, err) +} + func (c *UserClient) DropUserFunc(t *testing.T, id sdk.AccountObjectIdentifier) func() { t.Helper() ctx := context.Background() diff --git a/pkg/manual_tests/README.md b/pkg/manual_tests/README.md new file mode 100644 index 0000000000..69fc83e54d --- /dev/null +++ b/pkg/manual_tests/README.md @@ -0,0 +1,8 @@ +# Manual tests + +This directory is dedicated to hold steps for manual tests that are not possible to re-recreate in automated tests (or very hard to set up). +Every test should be placed in the subfolder representing a particular test (mostly because multiple files can be necessary for a single test) +and should contain a file describing the manual steps to perform the test. + +Here's the list of cases we currently cannot reproduce and write acceptance tests for: +- `user_default_database_and_role`: Setting up a user with default_namespace and default_role, then logging into that user to see what happens with those values in various scenarios (e.g. insufficient privileges on the role). \ No newline at end of file diff --git a/pkg/manual_tests/user_default_database_and_role/test.md b/pkg/manual_tests/user_default_database_and_role/test.md new file mode 100644 index 0000000000..5803a9b767 --- /dev/null +++ b/pkg/manual_tests/user_default_database_and_role/test.md @@ -0,0 +1,218 @@ +# User default database and role + +This test shows how Snowflake behaves whenever you log into a user that has default namespace (database) and default role set up. +The cases are covering the situations where: +- Either database or role is not present. +- Database and role are present, but not granted to the user. +- Database and role are granted to the user, but the casing is not matched. +- Database and role are granted to the user and the casing is matching. +- Only role is granted to the user and the casing is matching. +- Database and role are created with lowercase, granted to the user. + +## Setup + +As a testing environment, I chose VSCode with the Snowflake extension for being able to quickly go between ACCOUNTADMIN account +and the tested one. For testing account, I added this configuration for the tested user: + +```toml +[connections.Test] +accountname = '***' +username = '***' +password= '***' +host = '***' +``` + +The only thing that won't change between the tests is role and database, so we can create it beforehand: + +```sql +CREATE ROLE TEST_ROLE; +CREATE DATABASE TEST_DATABASE; +CREATE ROLE "test_role"; +CREATE DATABASE "test_database"; +``` + +> Note: We have to use `TYPE = LEGACY_SERVICE` to be able to quickly log in using `login_name` + `password`. + +> Note: It doesn't matter if you use single or double quotes for user properties (they result in the same behavior). + +### 1. Either database or role is not present. + +1. Create user with the non-existing default database and role. + +```sql +CREATE OR REPLACE USER TEST_USER + TYPE = LEGACY_SERVICE + LOGIN_NAME = 'login_name' + PASSWORD = 'password' + DEFAULT_NAMESPACE = 'NON_EXISTING_TEST_DATABASE' + DEFAULT_ROLE = 'NON_EXISTING_TEST_ROLE'; +``` + +2. Log into the user (logs in successfully, but no database and role is selected in the context). + +### 2. Database and role are present, but not granted to the user. + +1. Replace the user with existing default database and role. + +```sql +CREATE OR REPLACE USER TEST_USER + TYPE = LEGACY_SERVICE + LOGIN_NAME = 'login_name' + PASSWORD = 'password' + DEFAULT_NAMESPACE = 'TEST_DATABASE' + DEFAULT_ROLE = 'TEST_ROLE'; +``` + +2. Log into the user (logs in successfully, but no database and role is selected in the context). + +### 3. Database and role are granted to the user, but the casing is not matched. + +1. Replace the user with existing default database and role, but with lowercase names. + +```sql +CREATE OR REPLACE USER TEST_USER + TYPE = LEGACY_SERVICE + LOGIN_NAME = 'login_name' + PASSWORD = 'password' + DEFAULT_NAMESPACE = 'test_database' + DEFAULT_ROLE = 'test_role'; +``` + +2. Grant the role to the user and grant usage on the database to the role. + +```sql +GRANT ROLE TEST_ROLE TO USER TEST_USER; +GRANT USAGE ON DATABASE TEST_DATABASE TO ROLE TEST_ROLE; +``` + +3. Log into the user (logs in successfully, but no database and role is selected in the context). + +### 4. Database and role are granted to the user and the casing is matching. + +1. Replace the user with existing default database and role and exact casing. + +```sql +CREATE OR REPLACE USER TEST_USER + TYPE = LEGACY_SERVICE + LOGIN_NAME = 'login_name' + PASSWORD = 'password' + DEFAULT_NAMESPACE = 'TEST_DATABASE' + DEFAULT_ROLE = 'TEST_ROLE'; +``` + +2. Grant the role to the user and grant usage on the database to the role. + +```sql +GRANT ROLE TEST_ROLE TO USER TEST_USER; +GRANT USAGE ON DATABASE TEST_DATABASE TO ROLE TEST_ROLE; +``` + +3. Log into the user (logs in successfully and the database and role are selected in the context). + +### 5. Only role is granted to the user and the casing is matching. + +1. Replace the user with existing default database and role and exact casing. + +```sql +CREATE OR REPLACE USER TEST_USER + TYPE = LEGACY_SERVICE + LOGIN_NAME = 'login_name' + PASSWORD = 'password' + DEFAULT_NAMESPACE = 'TEST_DATABASE' + DEFAULT_ROLE = 'TEST_ROLE'; +``` + +2. Grant the role to the user and revoke usage on the database to the role. + +```sql +GRANT ROLE TEST_ROLE TO USER TEST_USER; +REVOKE USAGE ON DATABASE TEST_DATABASE FROM ROLE TEST_ROLE; +``` + +3. Log into the user (logs in successfully and role is selected in the context, but the database is not). + +### 6. Database and role are created with lowercase, granted to the user. + +#### 1. Matching casing + +1. Replace the user with existing default database and role and exact casing. + +```sql +CREATE OR REPLACE USER TEST_USER + TYPE = LEGACY_SERVICE + LOGIN_NAME = 'login_name' + PASSWORD = 'password' + DEFAULT_NAMESPACE = 'test_database' + DEFAULT_ROLE = 'test_role'; +``` + +2. Grant the role to the user and grant usage on the database to the role. + +```sql +GRANT ROLE "test_role" TO USER TEST_USER; +GRANT USAGE ON DATABASE "test_database" TO ROLE "test_role"; +``` + +3. Log into the user (logs in successfully and role is selected in the context, but the database is not). + +#### 2. Additionally quoting the database + +1. Replace the user with existing default database and role and exact casing. + +```sql +CREATE OR REPLACE USER TEST_USER + TYPE = LEGACY_SERVICE + LOGIN_NAME = 'login_name' + PASSWORD = 'password' + DEFAULT_NAMESPACE = '"test_database"' + DEFAULT_ROLE = 'test_role'; +``` + +2. Grant the role to the user and grant usage on the database to the role. + +```sql +GRANT ROLE "test_role" TO USER TEST_USER; +GRANT USAGE ON DATABASE "test_database" TO ROLE "test_role"; +``` + +3. Log into the user (logs in successfully and the database and role are selected in the context). + +#### 3. Additionally quoting the role + +1. Replace the user with existing default database and role and exact casing. + +```sql +CREATE OR REPLACE USER TEST_USER + TYPE = LEGACY_SERVICE + LOGIN_NAME = 'login_name' + PASSWORD = 'password' + DEFAULT_NAMESPACE = 'test_database' + DEFAULT_ROLE = '"test_role"'; +``` + +2. Grant the role to the user and grant usage on the database to the role. + +```sql +GRANT ROLE "test_role" TO USER TEST_USER; +GRANT USAGE ON DATABASE "test_database" TO ROLE "test_role"; +``` + +3. Log into the user (logs in successfully and the database and role are not selected in the context, because the value of `"test_role"` is saved as role name which is **not** perceived as a valid role name). + +## Clean up + +To clean up all the objects used in the tests, run the following commands. + +```sql +DROP DATABASE TEST_DATABASE; +DROP ROLE TEST_ROLE; +DROP DATABASE "test_database"; +DROP ROLE "test_role"; +DROP USER TEST_USER; +``` + +## Summary + +When specifying `DEFAULT_NAMESPACE` and `DEFAULT_ROLE` we have to take into the account that: +- `DEFAULT_NAMESPACE` is always being uppercased in Snowflake unless it's wrapped into double quotes, e.g. `DEFAULT_NAMESPACE = '"test_database"'`. +- `DEFAULT_ROLE` is always saving the input you are passing as is. This may cause issues when double-quoted id is passed, e.g. `DEFAULT_ROLE = '"test_role"'` will be saved as `"test_role"` (which won't work when logging into that user), and not `test_role` like in the case of `DEFAULT_NAMESPACE`. diff --git a/pkg/resources/user_acceptance_test.go b/pkg/resources/user_acceptance_test.go index b93585654d..74ece5ee28 100644 --- a/pkg/resources/user_acceptance_test.go +++ b/pkg/resources/user_acceptance_test.go @@ -1206,6 +1206,28 @@ func TestAcc_User_LoginNameAndDisplayName(t *testing.T) { HasLoginName("LOGIN_NAME"), ), }, + // Unset externally + { + PreConfig: func() { + acc.TestClient().User.Alter(t, newId, &sdk.AlterUserOptions{ + Unset: &sdk.UserUnset{ + ObjectProperties: &sdk.UserObjectPropertiesUnset{ + LoginName: sdk.Bool(true), + DisplayName: sdk.Bool(true), + }, + }, + }) + }, + Config: config.FromModel(t, userModelWithBoth), + Check: assert.AssertThat(t, + resourceassert.UserResource(t, userModelWithBoth.ResourceReference()). + HasDisplayNameString("display_name"). + HasLoginNameString("login_name"), + objectassert.User(t, newId). + HasDisplayName("display_name"). + HasLoginName("LOGIN_NAME"), + ), + }, // Unset both params { Config: config.FromModel(t, userModelWithNewId), @@ -1218,6 +1240,30 @@ func TestAcc_User_LoginNameAndDisplayName(t *testing.T) { HasLoginName(strings.ToUpper(newId.Name())), ), }, + // Set externally + { + PreConfig: func() { + acc.TestClient().User.Alter(t, newId, &sdk.AlterUserOptions{ + Set: &sdk.UserSet{ + ObjectProperties: &sdk.UserAlterObjectProperties{ + UserObjectProperties: sdk.UserObjectProperties{ + LoginName: sdk.String("external_login_name"), + DisplayName: sdk.String("external_display_name"), + }, + }, + }, + }) + }, + Config: config.FromModel(t, userModelWithNewId), + Check: assert.AssertThat(t, + resourceassert.UserResource(t, userModelWithNewId.ResourceReference()). + HasDisplayNameString(""). + HasLoginNameString(""), + objectassert.User(t, newId). + HasDisplayName(""). + HasLoginName(strings.ToUpper(newId.Name())), + ), + }, }, }) }