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

Use regex expression for authorization #32

Merged

Conversation

Chaho12
Copy link
Member

@Chaho12 Chaho12 commented Sep 11, 2023

Use regex instead of contains to support more cases where more than 1 type of user members can exist and not all but some of the groups/members are allowed for User role.

@cla-bot cla-bot bot added the cla-signed label Sep 11, 2023
Copy link
Contributor

@willmostly willmostly left a comment

Choose a reason for hiding this comment

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

Logic looks good, just a couple notes on the docs and I think this will be ready.

docs/security.md Outdated Show resolved Hide resolved
docs/security.md Show resolved Hide resolved
@Chaho12 Chaho12 force-pushed the feature/jaeho.yoo/allow_regex_authorization branch 2 times, most recently from 2172b7d to d4d5de7 Compare September 22, 2023 10:41
Copy link
Member

@vishalya vishalya left a comment

Choose a reason for hiding this comment

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

LGTM

docs/security.md Show resolved Hide resolved
@Chaho12 Chaho12 force-pushed the feature/jaeho.yoo/allow_regex_authorization branch from d4d5de7 to 68e922b Compare October 5, 2023 07:57
@Chaho12
Copy link
Member Author

Chaho12 commented Oct 5, 2023

Rebased and pushed to be ready for merge

Copy link
Member

@mosabua mosabua left a comment

Choose a reason for hiding this comment

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

Looks good to me as well.

Any further input @willmostly ?

@willmostly
Copy link
Contributor

Sorry for the delay @Chaho12 - I had a review pending that I didn't realize never got submitted

@mosabua
Copy link
Member

mosabua commented Oct 10, 2023

Could you rebase and adjust to review comments please @Chaho12 ? Or maybe wait until the test refactor to use junit is in .. and then adapt.

@Chaho12 Chaho12 force-pushed the feature/jaeho.yoo/allow_regex_authorization branch 2 times, most recently from ced88bb to 8a9da6d Compare October 18, 2023 11:15
@Chaho12 Chaho12 force-pushed the feature/jaeho.yoo/allow_regex_authorization branch from 8a9da6d to e772a0b Compare October 18, 2023 11:19
Copy link
Contributor

@willmostly willmostly left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@mosabua mosabua merged commit fc6b48f into trinodb:main Oct 19, 2023
2 checks passed
@Chaho12 Chaho12 deleted the feature/jaeho.yoo/allow_regex_authorization branch October 20, 2023 00:19
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.

4 participants