MonkeyCanCode opened a new pull request, #4396: URL: https://github.com/apache/polaris/pull/4396
<!-- ๐ Describe what changes you're proposing, especially breaking or user-facing changes. ๐ See https://github.com/apache/polaris/blob/main/CONTRIBUTING.md for more. --> Currently only root principal can perform principal reset (which contains both client id and client secret). The problem here is there is lack of check for client ID collision. When a client ID collision happened (used principal B's client id to reset principal A's client id), this will prevent principal B from perform auth request. This also means, if an admin who accidentally used root's client id to update a low privilege principal, we will no longer be able to obtain new token for principal root. For example: Assuming I accidentally reset principal `quickstart_user`'s client ID with value `root` (which is the default client ID for principal `root`), I won't be able to auth with principal `root` again (root can get itself locked out): ```sh โ polaris git:(main) ./polaris --profile dev principals list {"name": "root", "clientId": "root", "properties": {}, "createTimestamp": 1778384520325, "lastUpdateTimestamp": 1778384520325, "entityVersion": 1} {"name": "quickstart_user", "clientId": "a35358218e1a5458", "properties": {}, "createTimestamp": 1778384524349, "lastUpdateTimestamp": 1778384524349, "entityVersion": 1} โ polaris git:(main) ./polaris --profile dev principals reset quickstart_user --new-client-id root ... File "/Users/yong/Desktop/GitHome/polaris/client/python/apache_polaris/cli/api_client_builder.py", line 147, in _get_token raise Exception("Failed to get access token") ``` Here is the new behavior with this check: ```sh โ polaris git:(prevent_clientid_collison_during_reset) โ ./polaris --profile dev principals list {"name": "root", "clientId": "root", "properties": {}, "createTimestamp": 1778389833416, "lastUpdateTimestamp": 1778389833416, "entityVersion": 1} {"name": "quickstart_user", "clientId": "8c587de26b5cc0ee", "properties": {}, "createTimestamp": 1778389837767, "lastUpdateTimestamp": 1778389837767, "entityVersion": 1} โ polaris git:(prevent_clientid_collison_during_reset) โ ./polaris --profile dev principals reset quickstart_user --new-client-id root Exception when communicating with the Polaris server. AlreadyExistsException: Client ID already in used: root โ polaris git:(prevent_clientid_collison_during_reset) โ ./polaris --profile dev principals list {"name": "root", "clientId": "root", "properties": {}, "createTimestamp": 1778389833416, "lastUpdateTimestamp": 1778389833416, "entityVersion": 1} {"name": "quickstart_user", "clientId": "8c587de26b5cc0ee", "properties": {}, "createTimestamp": 1778389837767, "lastUpdateTimestamp": 1778389837767, "entityVersion": 1} ``` ## Checklist - [x] ๐ก๏ธ Don't disclose security issues! (contact [email protected]) - [x] ๐ Clearly explained why the changes are needed, or linked related issues: Fixes # - [x] ๐งช Added/updated tests with good coverage, or manually tested (and explained how) - [x] ๐ก Added comments for complex logic - [x] ๐งพ Updated `CHANGELOG.md` (if needed) - [x] ๐ Updated documentation in `site/content/in-dev/unreleased` (if needed) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
