Skip to content

Reset roles by "SET ROLE NONE" #1649

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
May 26, 2024
Merged

Reset roles by "SET ROLE NONE" #1649

merged 9 commits into from
May 26, 2024

Conversation

chernser
Copy link
Contributor

@chernser chernser commented May 25, 2024

Summary

Closes #1647

Checklist

Delete items not relevant to your PR:

  • Unit and integration tests covering the common scenarios were added
  • A human-readable description of the changes was provided to include in CHANGELOG

@CLAassistant
Copy link

CLAassistant commented May 25, 2024

CLA assistant check
All committers have signed the CLA.

@chernser chernser requested review from mzitnik and slvrtrn May 25, 2024 03:15
@chernser chernser changed the title Rest roles by "SET ROLE NONE" Reset roles by "SET ROLE NONE" May 25, 2024
@slvrtrn
Copy link

slvrtrn commented May 25, 2024

The following simulation of an app with the impersonation feature based on row policies could be an OK integration test.

Setup:

CREATE OR REPLACE TABLE test_table
(
    `s` String
)
ENGINE = MergeTree
ORDER BY tuple();
INSERT INTO test_table VALUES ('a'), ('b');

CREATE USER test_user NOT IDENTIFIED;
CREATE ROLE row_a;
CREATE ROLE row_b;
GRANT row_a, row_b TO test_user;

GRANT SELECT ON default.test_table TO test_user;
CREATE ROW POLICY OR REPLACE policy_row_b ON test_table FOR SELECT USING s = 'b' TO row_b;
CREATE ROW POLICY OR REPLACE policy_row_a ON test_table FOR SELECT USING s = 'a' TO row_a;

Test:

➜ curl -H "Authorization: Basic dGVzdF91c2VyOg==" "http://localhost:8123" --data-binary "SELECT * FROM test_table"
a
b
➜ curl -H "Authorization: Basic dGVzdF91c2VyOg==" "http://localhost:8123?role=row_b" --data-binary "SELECT * FROM test_table"
b
➜ curl -H "Authorization: Basic dGVzdF91c2VyOg==" "http://localhost:8123?role=row_a" --data-binary "SELECT * FROM test_table"
a
➜ curl -H "Authorization: Basic dGVzdF91c2VyOg==" "http://localhost:8123?role=row_a&role=row_b" --data-binary "SELECT * FROM test_table"
a
b

In Java code, that could be:

-- no roles set - a select query will get both rows A and B
SET ROLE row_a;
-- gets only row A
SET ROLE row_b;
-- gets both rows A and B
SET ROLE NONE;
-- gets both rows A and B
SET ROLE row_b;
-- gets only row B

Properties properties = new Properties();
properties.setProperty(ClickHouseHttpOption.REMEMBER_LAST_SET_ROLES.getKey(), "true");
properties.setProperty(ClickHouseHttpOption.CONNECTION_PROVIDER.getKey(),
HttpConnectionProvider.APACHE_HTTP_CLIENT.name());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we are setting this, it is the default

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've forgot this after testing. Will remove.

@chernser chernser merged commit 8f03f0d into main May 26, 2024
1 of 2 checks passed
@chernser chernser deleted the fix_1647 branch June 27, 2024 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add resseting memorized roles
4 participants