Skip to content
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

Capitalize page permission key conf #352

Merged

Conversation

Chaho12
Copy link
Member

@Chaho12 Chaho12 commented May 22, 2024

Description

Since both roles in web & security context are in upper-case, page permission should also be in upper case to match with roles.

Without this patch, user has to set upper-case key's in yaml in their yaml file as below, which is awkward as all other auth related confs are in lower-case.

pagePermissions:
  ADMIN:
  USER: dashboard_history
  API:

Additional context and related issues

Release notes

(x) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

* 

Test

pagePermissions:
  admin:
  user: dashboard_history
  api:

스크린샷 2024-05-23 오전 10 32 40
스크린샷 2024-05-23 오전 10 32 51

@Chaho12 Chaho12 requested review from oneonestar, ebyhr and ytwp May 22, 2024 04:49
@cla-bot cla-bot bot added the cla-signed label May 22, 2024
@ebyhr
Copy link
Member

ebyhr commented May 22, 2024

Please check CI failure:

Error:    TestLbAuthenticator.testNoLdapNoPresetUsers:122 » NullPointer Cannot invoke "java.util.Map.forEach(java.util.function.BiConsumer)" because "pagePermissions" is null
Error:    TestLbAuthenticator.testWrongLdapConfig:131 » NullPointer Cannot invoke "java.util.Map.forEach(java.util.function.BiConsumer)" because "pagePermissions" is null

@Chaho12 Chaho12 force-pushed the feature/jyoo/set_page_permission_key_uppercase branch from 4616df0 to ca07cfd Compare May 22, 2024 08:50
@Chaho12 Chaho12 force-pushed the feature/jyoo/set_page_permission_key_uppercase branch from ca07cfd to 8ebeb13 Compare May 23, 2024 01:34
@ebyhr
Copy link
Member

ebyhr commented May 23, 2024

Please fix CI failure:

Error:    TestLbAuthenticator.testNoLdapNoPresetUsers:122 » NullPointer Cannot invoke "java.util.Map.entrySet()" because "pagePermissions" is null
Error:    TestLbAuthenticator.testWrongLdapConfig:131 » NullPointer Cannot invoke "java.util.Map.entrySet()" because "pagePermissions" is null

@Chaho12 Chaho12 force-pushed the feature/jyoo/set_page_permission_key_uppercase branch from 8ebeb13 to ee2212b Compare May 23, 2024 05:04
@Chaho12 Chaho12 force-pushed the feature/jyoo/set_page_permission_key_uppercase branch 2 times, most recently from 642b6ce to fa7204f Compare May 24, 2024 05:45
@Chaho12 Chaho12 force-pushed the feature/jyoo/set_page_permission_key_uppercase branch from fa7204f to 87a8302 Compare May 31, 2024 05:10
@Chaho12 Chaho12 requested a review from ebyhr May 31, 2024 05:11
@ebyhr
Copy link
Member

ebyhr commented May 31, 2024

Please fix CI failure:

Error:  src/main/java/io/trino/gateway/ha/security/LbOAuthManager.java:[52,1] (imports) ImportOrder: Wrong order for 'com.google.common.collect.ImmutableMap.toImmutableMap' import.

@Chaho12 Chaho12 force-pushed the feature/jyoo/set_page_permission_key_uppercase branch from 87a8302 to 1d94edb Compare May 31, 2024 05:27
@Chaho12 Chaho12 force-pushed the feature/jyoo/set_page_permission_key_uppercase branch 3 times, most recently from f02574b to acd34fb Compare May 31, 2024 05:52
@Chaho12 Chaho12 force-pushed the feature/jyoo/set_page_permission_key_uppercase branch from acd34fb to 4183359 Compare May 31, 2024 05:54
@Chaho12 Chaho12 requested a review from ebyhr May 31, 2024 11:16
@ebyhr ebyhr force-pushed the feature/jyoo/set_page_permission_key_uppercase branch from 4183359 to bbcfd6f Compare May 31, 2024 11:58
@Chaho12 Chaho12 force-pushed the feature/jyoo/set_page_permission_key_uppercase branch from bbcfd6f to a0e6bb4 Compare May 31, 2024 12:06
@ebyhr ebyhr force-pushed the feature/jyoo/set_page_permission_key_uppercase branch from a0e6bb4 to c38e90d Compare May 31, 2024 12:09
@ebyhr
Copy link
Member

ebyhr commented May 31, 2024

I pushed small changes because the indentation was wrong.

@ebyhr ebyhr merged commit 5315748 into trinodb:main May 31, 2024
2 checks passed
@github-actions github-actions bot added this to the 10 milestone May 31, 2024
@Chaho12 Chaho12 deleted the feature/jyoo/set_page_permission_key_uppercase branch July 1, 2024 02:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants