You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Na Li via Review Board <no...@reviews.apache.org> on 2018/10/22 22:04:38 UTC
Review Request 69122: SENTRY-2432: The case of a username is ignored
when determining object ownership
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69122/
-----------------------------------------------------------
Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Sergio Pena.
Bugs: sentry-2432
https://issues.apache.org/jira/browse/sentry-2432
Repository: sentry
Description
-------
Keep user name case when saving into sentry DB and fetching privileges for that user.
Diffs
-----
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java f29c455
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java b387a22
sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 4a9afe3
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java d3294f4
Diff: https://reviews.apache.org/r/69122/diff/1/
Testing
-------
add new test case to check authorization based on user name case
Thanks,
Na Li
Re: Review Request 69122: SENTRY-2432: The case of a username is
ignored when determining object ownership
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69122/#review209972
-----------------------------------------------------------
Ship it!
Ship It!
- kalyan kumar kalvagadda
On Oct. 24, 2018, 4:14 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69122/
> -----------------------------------------------------------
>
> (Updated Oct. 24, 2018, 4:14 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Sergio Pena.
>
>
> Bugs: sentry-2432
> https://issues.apache.org/jira/browse/sentry-2432
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Keep user name case when saving into sentry DB and fetching privileges for that user.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java f29c455
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java b387a22
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 4a9afe3
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java d3294f4
>
>
> Diff: https://reviews.apache.org/r/69122/diff/2/
>
>
> Testing
> -------
>
> add new test case to check authorization based on user name case
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 69122: SENTRY-2432: The case of a username is
ignored when determining object ownership
Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69122/#review209978
-----------------------------------------------------------
Ship it!
Ship It!
- Arjun Mishra
On Oct. 24, 2018, 4:14 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69122/
> -----------------------------------------------------------
>
> (Updated Oct. 24, 2018, 4:14 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Sergio Pena.
>
>
> Bugs: sentry-2432
> https://issues.apache.org/jira/browse/sentry-2432
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Keep user name case when saving into sentry DB and fetching privileges for that user.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java f29c455
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java b387a22
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 4a9afe3
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java d3294f4
>
>
> Diff: https://reviews.apache.org/r/69122/diff/2/
>
>
> Testing
> -------
>
> add new test case to check authorization based on user name case
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 69122: SENTRY-2432: The case of a username is
ignored when determining object ownership
Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69122/
-----------------------------------------------------------
(Updated Oct. 24, 2018, 4:14 p.m.)
Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Sergio Pena.
Bugs: sentry-2432
https://issues.apache.org/jira/browse/sentry-2432
Repository: sentry
Description
-------
Keep user name case when saving into sentry DB and fetching privileges for that user.
Diffs (updated)
-----
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java f29c455
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java b387a22
sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 4a9afe3
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java d3294f4
Diff: https://reviews.apache.org/r/69122/diff/2/
Changes: https://reviews.apache.org/r/69122/diff/1-2/
Testing
-------
add new test case to check authorization based on user name case
Thanks,
Na Li
Re: Review Request 69122: SENTRY-2432: The case of a username is
ignored when determining object ownership
Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
> On Oct. 23, 2018, 3:42 p.m., Arjun Mishra wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Line 925 (original), 928 (patched)
> > <https://reviews.apache.org/r/69122/diff/1/?file=2101902#file2101902line928>
> >
> > Here?
>
> Na Li wrote:
> what do you mean "Here?"?
I mean in the method "alterSentryUserGrantPrivileges" we don't trim the userName before doing any grant
- Arjun
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69122/#review209922
-----------------------------------------------------------
On Oct. 22, 2018, 10:04 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69122/
> -----------------------------------------------------------
>
> (Updated Oct. 22, 2018, 10:04 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Sergio Pena.
>
>
> Bugs: sentry-2432
> https://issues.apache.org/jira/browse/sentry-2432
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Keep user name case when saving into sentry DB and fetching privileges for that user.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java f29c455
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java b387a22
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 4a9afe3
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java d3294f4
>
>
> Diff: https://reviews.apache.org/r/69122/diff/1/
>
>
> Testing
> -------
>
> add new test case to check authorization based on user name case
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 69122: SENTRY-2432: The case of a username is
ignored when determining object ownership
Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
> On Oct. 23, 2018, 3:42 p.m., Arjun Mishra wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Line 929 (original), 932 (patched)
> > <https://reviews.apache.org/r/69122/diff/1/?file=2101902#file2101902line932>
> >
> > Should we add userName.trim() here?
>
> Na Li wrote:
> It is trimmed in "MSentryUser getMSentryUserByName(final String userName, final boolean throwExceptionIfNotExist) "
Ok. Got it.
- Arjun
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69122/#review209922
-----------------------------------------------------------
On Oct. 22, 2018, 10:04 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69122/
> -----------------------------------------------------------
>
> (Updated Oct. 22, 2018, 10:04 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Sergio Pena.
>
>
> Bugs: sentry-2432
> https://issues.apache.org/jira/browse/sentry-2432
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Keep user name case when saving into sentry DB and fetching privileges for that user.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java f29c455
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java b387a22
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 4a9afe3
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java d3294f4
>
>
> Diff: https://reviews.apache.org/r/69122/diff/1/
>
>
> Testing
> -------
>
> add new test case to check authorization based on user name case
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 69122: SENTRY-2432: The case of a username is
ignored when determining object ownership
Posted by Na Li via Review Board <no...@reviews.apache.org>.
> On Oct. 23, 2018, 3:42 p.m., Arjun Mishra wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Line 925 (original), 928 (patched)
> > <https://reviews.apache.org/r/69122/diff/1/?file=2101902#file2101902line928>
> >
> > Here?
what do you mean "Here?"?
- Na
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69122/#review209922
-----------------------------------------------------------
On Oct. 22, 2018, 10:04 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69122/
> -----------------------------------------------------------
>
> (Updated Oct. 22, 2018, 10:04 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Sergio Pena.
>
>
> Bugs: sentry-2432
> https://issues.apache.org/jira/browse/sentry-2432
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Keep user name case when saving into sentry DB and fetching privileges for that user.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java f29c455
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java b387a22
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 4a9afe3
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java d3294f4
>
>
> Diff: https://reviews.apache.org/r/69122/diff/1/
>
>
> Testing
> -------
>
> add new test case to check authorization based on user name case
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 69122: SENTRY-2432: The case of a username is
ignored when determining object ownership
Posted by Na Li via Review Board <no...@reviews.apache.org>.
> On Oct. 23, 2018, 3:42 p.m., Arjun Mishra wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Line 925 (original), 928 (patched)
> > <https://reviews.apache.org/r/69122/diff/1/?file=2101902#file2101902line928>
> >
> > Here?
>
> Na Li wrote:
> what do you mean "Here?"?
>
> Arjun Mishra wrote:
> I mean in the method "alterSentryUserGrantPrivileges" we don't trim the userName before doing any grant
userName is trimmed in getMSentryUserByName
> On Oct. 23, 2018, 3:42 p.m., Arjun Mishra wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Line 984 (original), 990 (patched)
> > <https://reviews.apache.org/r/69122/diff/1/?file=2101902#file2101902line990>
> >
> > Here?
>
> Na Li wrote:
> what do you mean?
userName is trimmed in getMSentryUserByName
- Na
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69122/#review209922
-----------------------------------------------------------
On Oct. 22, 2018, 10:04 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69122/
> -----------------------------------------------------------
>
> (Updated Oct. 22, 2018, 10:04 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Sergio Pena.
>
>
> Bugs: sentry-2432
> https://issues.apache.org/jira/browse/sentry-2432
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Keep user name case when saving into sentry DB and fetching privileges for that user.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java f29c455
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java b387a22
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 4a9afe3
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java d3294f4
>
>
> Diff: https://reviews.apache.org/r/69122/diff/1/
>
>
> Testing
> -------
>
> add new test case to check authorization based on user name case
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 69122: SENTRY-2432: The case of a username is
ignored when determining object ownership
Posted by Na Li via Review Board <no...@reviews.apache.org>.
> On Oct. 23, 2018, 3:42 p.m., Arjun Mishra wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Line 929 (original), 932 (patched)
> > <https://reviews.apache.org/r/69122/diff/1/?file=2101902#file2101902line932>
> >
> > Should we add userName.trim() here?
It is trimmed in "MSentryUser getMSentryUserByName(final String userName, final boolean throwExceptionIfNotExist) "
> On Oct. 23, 2018, 3:42 p.m., Arjun Mishra wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Line 984 (original), 990 (patched)
> > <https://reviews.apache.org/r/69122/diff/1/?file=2101902#file2101902line990>
> >
> > Here?
what do you mean?
- Na
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69122/#review209922
-----------------------------------------------------------
On Oct. 22, 2018, 10:04 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69122/
> -----------------------------------------------------------
>
> (Updated Oct. 22, 2018, 10:04 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Sergio Pena.
>
>
> Bugs: sentry-2432
> https://issues.apache.org/jira/browse/sentry-2432
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Keep user name case when saving into sentry DB and fetching privileges for that user.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java f29c455
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java b387a22
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 4a9afe3
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java d3294f4
>
>
> Diff: https://reviews.apache.org/r/69122/diff/1/
>
>
> Testing
> -------
>
> add new test case to check authorization based on user name case
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 69122: SENTRY-2432: The case of a username is
ignored when determining object ownership
Posted by Na Li via Review Board <no...@reviews.apache.org>.
> On Oct. 23, 2018, 3:42 p.m., Arjun Mishra wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Line 2837 (original), 2847 (patched)
> > <https://reviews.apache.org/r/69122/diff/1/?file=2101902#file2101902line2847>
> >
> > What about this method? trim and lower?
good catch! I will add trim and lower in alterSentryGrantPrivilegeCore
- Na
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69122/#review209922
-----------------------------------------------------------
On Oct. 22, 2018, 10:04 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69122/
> -----------------------------------------------------------
>
> (Updated Oct. 22, 2018, 10:04 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Sergio Pena.
>
>
> Bugs: sentry-2432
> https://issues.apache.org/jira/browse/sentry-2432
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Keep user name case when saving into sentry DB and fetching privileges for that user.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java f29c455
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java b387a22
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 4a9afe3
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java d3294f4
>
>
> Diff: https://reviews.apache.org/r/69122/diff/1/
>
>
> Testing
> -------
>
> add new test case to check authorization based on user name case
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 69122: SENTRY-2432: The case of a username is
ignored when determining object ownership
Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69122/#review209922
-----------------------------------------------------------
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Line 925 (original), 928 (patched)
<https://reviews.apache.org/r/69122/#comment294561>
Here?
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Line 929 (original), 932 (patched)
<https://reviews.apache.org/r/69122/#comment294559>
Should we add userName.trim() here?
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Line 984 (original), 990 (patched)
<https://reviews.apache.org/r/69122/#comment294562>
Here?
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Line 2837 (original), 2847 (patched)
<https://reviews.apache.org/r/69122/#comment294560>
What about this method? trim and lower?
- Arjun Mishra
On Oct. 22, 2018, 10:04 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69122/
> -----------------------------------------------------------
>
> (Updated Oct. 22, 2018, 10:04 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Sergio Pena.
>
>
> Bugs: sentry-2432
> https://issues.apache.org/jira/browse/sentry-2432
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Keep user name case when saving into sentry DB and fetching privileges for that user.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java f29c455
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java b387a22
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 4a9afe3
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java d3294f4
>
>
> Diff: https://reviews.apache.org/r/69122/diff/1/
>
>
> Testing
> -------
>
> add new test case to check authorization based on user name case
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 69122: SENTRY-2432: The case of a username is
ignored when determining object ownership
Posted by Na Li via Review Board <no...@reviews.apache.org>.
> On Oct. 23, 2018, 4:38 p.m., kalyan kumar kalvagadda wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Line 1118 (original), 1124 (patched)
> > <https://reviews.apache.org/r/69122/diff/1/?file=2101902#file2101902line1124>
> >
> > you can just call princilaNames.safeTrim()
I removed the trim in this function. In alterSentryRevokePrivilegeCore, I check if the variable is null and throw exception if so. Then just trim
- Na
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69122/#review209932
-----------------------------------------------------------
On Oct. 24, 2018, 4:14 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69122/
> -----------------------------------------------------------
>
> (Updated Oct. 24, 2018, 4:14 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Sergio Pena.
>
>
> Bugs: sentry-2432
> https://issues.apache.org/jira/browse/sentry-2432
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Keep user name case when saving into sentry DB and fetching privileges for that user.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java f29c455
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java b387a22
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 4a9afe3
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java d3294f4
>
>
> Diff: https://reviews.apache.org/r/69122/diff/2/
>
>
> Testing
> -------
>
> add new test case to check authorization based on user name case
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 69122: SENTRY-2432: The case of a username is
ignored when determining object ownership
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69122/#review209932
-----------------------------------------------------------
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java
Lines 375-379 (patched)
<https://reviews.apache.org/r/69122/#comment294571>
It can be simply changed to
addCustomParam(prefix + ":" + vName, vName, (toLowerCase) ? name.trim().toLowerCase() : name.trim());
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Line 1118 (original), 1124 (patched)
<https://reviews.apache.org/r/69122/#comment294572>
you can just call princilaNames.safeTrim()
- kalyan kumar kalvagadda
On Oct. 22, 2018, 10:04 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69122/
> -----------------------------------------------------------
>
> (Updated Oct. 22, 2018, 10:04 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Sergio Pena.
>
>
> Bugs: sentry-2432
> https://issues.apache.org/jira/browse/sentry-2432
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Keep user name case when saving into sentry DB and fetching privileges for that user.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java f29c455
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java b387a22
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 4a9afe3
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java d3294f4
>
>
> Diff: https://reviews.apache.org/r/69122/diff/1/
>
>
> Testing
> -------
>
> add new test case to check authorization based on user name case
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 69122: SENTRY-2432: The case of a username is
ignored when determining object ownership
Posted by Na Li via Review Board <no...@reviews.apache.org>.
> On Oct. 23, 2018, 3:44 p.m., Arjun Mishra wrote:
> > Let me know what you think about this. I think we could minimize the change set by keeping changes to the core methods? alterSentryRevokePrivilegeCore and alterSentryGrantPrivilegeCore?
> > For example there are methods like dropOrRenamePrivilegeForAllEntities, and importRolePrivilegeMapping that don't have a trim and lower change to it
agree. It is cleaner and can make sure we process input name correctly. I will make change accordingly
- Na
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69122/#review209923
-----------------------------------------------------------
On Oct. 22, 2018, 10:04 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69122/
> -----------------------------------------------------------
>
> (Updated Oct. 22, 2018, 10:04 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Sergio Pena.
>
>
> Bugs: sentry-2432
> https://issues.apache.org/jira/browse/sentry-2432
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Keep user name case when saving into sentry DB and fetching privileges for that user.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java f29c455
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java b387a22
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 4a9afe3
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java d3294f4
>
>
> Diff: https://reviews.apache.org/r/69122/diff/1/
>
>
> Testing
> -------
>
> add new test case to check authorization based on user name case
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 69122: SENTRY-2432: The case of a username is
ignored when determining object ownership
Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69122/#review209923
-----------------------------------------------------------
Let me know what you think about this. I think we could minimize the change set by keeping changes to the core methods? alterSentryRevokePrivilegeCore and alterSentryGrantPrivilegeCore?
For example there are methods like dropOrRenamePrivilegeForAllEntities, and importRolePrivilegeMapping that don't have a trim and lower change to it
- Arjun Mishra
On Oct. 22, 2018, 10:04 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69122/
> -----------------------------------------------------------
>
> (Updated Oct. 22, 2018, 10:04 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Sergio Pena.
>
>
> Bugs: sentry-2432
> https://issues.apache.org/jira/browse/sentry-2432
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Keep user name case when saving into sentry DB and fetching privileges for that user.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java f29c455
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java b387a22
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 4a9afe3
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java d3294f4
>
>
> Diff: https://reviews.apache.org/r/69122/diff/1/
>
>
> Testing
> -------
>
> add new test case to check authorization based on user name case
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 69122: SENTRY-2432: The case of a username is
ignored when determining object ownership
Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69122/#review209929
-----------------------------------------------------------
We could also refactor the trimAndLower() method to minimize changes. What do you think?
- Arjun Mishra
On Oct. 22, 2018, 10:04 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69122/
> -----------------------------------------------------------
>
> (Updated Oct. 22, 2018, 10:04 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Sergio Pena.
>
>
> Bugs: sentry-2432
> https://issues.apache.org/jira/browse/sentry-2432
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Keep user name case when saving into sentry DB and fetching privileges for that user.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java f29c455
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java b387a22
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 4a9afe3
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java d3294f4
>
>
> Diff: https://reviews.apache.org/r/69122/diff/1/
>
>
> Testing
> -------
>
> add new test case to check authorization based on user name case
>
>
> Thanks,
>
> Na Li
>
>