You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Dapeng Sun <da...@intel.com> on 2015/01/15 09:09:48 UTC
Review Request 29921: SENTRY-608: Add simple authorization support to
SentryWebserver
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29921/
-----------------------------------------------------------
Review request for sentry, Xiaomeng Huang, Arun Suresh, Colin Ma, shen guoquan, Lenni Kuff, Prasad Mujumdar, and Sravya Tirukkovalur.
Bugs: SENTRY-608
https://issues.apache.org/jira/browse/SENTRY-608
Repository: sentry
Description
-------
Add **allow.connect.groups** to conf, and using **GroupMappingService** get user's groups, only user in allow groups can pass.
Diffs
-----
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryAuthFilter.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryWebServer.java 090917c
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 47794bc
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryWebServerWithKerberos.java PRE-CREATION
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java 0bdc3a2
Diff: https://reviews.apache.org/r/29921/diff/
Testing
-------
Unit Tests in local
Thanks,
Dapeng Sun
Re: Review Request 29921: SENTRY-608: Add simple authorization
support to SentryWebserver
Posted by shen guoquan <gu...@intel.com>.
> On 一月 15, 2015, 8:43 a.m., shen guoquan wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryWebServer.java, line 99
> > <https://reviews.apache.org/r/29921/diff/1/?file=822479#file822479line99>
> >
> > you get the allowGroups from the configuration here, why not directly transfer the allowGropus to SentryAuthFilter class?
>
> Dapeng Sun wrote:
> Hi Guoquan, thank you for your review, I tried before but it seems we can only use "filterHolder.setInitParameters(Map<String,String> map)" to pass the parameters. If you have any other idea, please let me know.
Ok, I know the reason. These patch looks good to me. It's a good job, Thanks dapeng. By the way, you can close the issue.
- shen
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29921/#review68221
-----------------------------------------------------------
On 一月 15, 2015, 8:09 a.m., Dapeng Sun wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29921/
> -----------------------------------------------------------
>
> (Updated 一月 15, 2015, 8:09 a.m.)
>
>
> Review request for sentry, Xiaomeng Huang, Arun Suresh, Colin Ma, shen guoquan, Lenni Kuff, Prasad Mujumdar, and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-608
> https://issues.apache.org/jira/browse/SENTRY-608
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add **allow.connect.groups** to conf, and using **GroupMappingService** get user's groups, only user in allow groups can pass.
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryAuthFilter.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryWebServer.java 090917c
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 47794bc
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryWebServerWithKerberos.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java 0bdc3a2
>
> Diff: https://reviews.apache.org/r/29921/diff/
>
>
> Testing
> -------
>
> Unit Tests in local
>
>
> Thanks,
>
> Dapeng Sun
>
>
Re: Review Request 29921: SENTRY-608: Add simple authorization
support to SentryWebserver
Posted by Dapeng Sun <da...@intel.com>.
> On 一月 15, 2015, 4:43 p.m., shen guoquan wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryWebServer.java, line 99
> > <https://reviews.apache.org/r/29921/diff/1/?file=822479#file822479line99>
> >
> > you get the allowGroups from the configuration here, why not directly transfer the allowGropus to SentryAuthFilter class?
Hi Guoquan, thank you for your review, I tried before but it seems we can only use "filterHolder.setInitParameters(Map<String,String> map)" to pass the parameters. If you have any other idea, please let me know.
- Dapeng
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29921/#review68221
-----------------------------------------------------------
On 一月 15, 2015, 4:09 p.m., Dapeng Sun wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29921/
> -----------------------------------------------------------
>
> (Updated 一月 15, 2015, 4:09 p.m.)
>
>
> Review request for sentry, Xiaomeng Huang, Arun Suresh, Colin Ma, shen guoquan, Lenni Kuff, Prasad Mujumdar, and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-608
> https://issues.apache.org/jira/browse/SENTRY-608
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add **allow.connect.groups** to conf, and using **GroupMappingService** get user's groups, only user in allow groups can pass.
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryAuthFilter.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryWebServer.java 090917c
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 47794bc
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryWebServerWithKerberos.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java 0bdc3a2
>
> Diff: https://reviews.apache.org/r/29921/diff/
>
>
> Testing
> -------
>
> Unit Tests in local
>
>
> Thanks,
>
> Dapeng Sun
>
>
Re: Review Request 29921: SENTRY-608: Add simple authorization
support to SentryWebserver
Posted by shen guoquan <gu...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29921/#review68221
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryWebServer.java
<https://reviews.apache.org/r/29921/#comment112366>
you get the allowGroups from the configuration here, why not directly transfer the allowGropus to SentryAuthFilter class?
- shen guoquan
On 一月 15, 2015, 8:09 a.m., Dapeng Sun wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29921/
> -----------------------------------------------------------
>
> (Updated 一月 15, 2015, 8:09 a.m.)
>
>
> Review request for sentry, Xiaomeng Huang, Arun Suresh, Colin Ma, shen guoquan, Lenni Kuff, Prasad Mujumdar, and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-608
> https://issues.apache.org/jira/browse/SENTRY-608
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add **allow.connect.groups** to conf, and using **GroupMappingService** get user's groups, only user in allow groups can pass.
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryAuthFilter.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryWebServer.java 090917c
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 47794bc
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryWebServerWithKerberos.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java 0bdc3a2
>
> Diff: https://reviews.apache.org/r/29921/diff/
>
>
> Testing
> -------
>
> Unit Tests in local
>
>
> Thanks,
>
> Dapeng Sun
>
>
Re: Review Request 29921: SENTRY-608: Add simple authorization
support to SentryWebserver
Posted by Dapeng Sun <da...@intel.com>.
> On 一月 19, 2015, 3:28 p.m., Lenni Kuff wrote:
> >
Thank you very much for your review, Lenni.
> On 一月 19, 2015, 3:28 p.m., Lenni Kuff wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java, line 165
> > <https://reviews.apache.org/r/29921/diff/1/?file=822480#file822480line165>
> >
> > Do we need a new config option, or can you reuse the exisitng "admin user"/"allow connect" config options?
Good suggestion! using "allow connect users" would be simple. I will fix it in next patch.
- Dapeng
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29921/#review68591
-----------------------------------------------------------
On 一月 15, 2015, 4:09 p.m., Dapeng Sun wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29921/
> -----------------------------------------------------------
>
> (Updated 一月 15, 2015, 4:09 p.m.)
>
>
> Review request for sentry, Xiaomeng Huang, Arun Suresh, Colin Ma, shen guoquan, Lenni Kuff, Prasad Mujumdar, and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-608
> https://issues.apache.org/jira/browse/SENTRY-608
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add **allow.connect.groups** to conf, and using **GroupMappingService** get user's groups, only user in allow groups can pass.
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryAuthFilter.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryWebServer.java 090917c
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 47794bc
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryWebServerWithKerberos.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java 0bdc3a2
>
> Diff: https://reviews.apache.org/r/29921/diff/
>
>
> Testing
> -------
>
> Unit Tests in local
>
>
> Thanks,
>
> Dapeng Sun
>
>
Re: Review Request 29921: SENTRY-608: Add simple authorization
support to SentryWebserver
Posted by Lenni Kuff <ls...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29921/#review68591
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
<https://reviews.apache.org/r/29921/#comment112914>
Do we need a new config option, or can you reuse the exisitng "admin user"/"allow connect" config options?
- Lenni Kuff
On Jan. 15, 2015, 8:09 a.m., Dapeng Sun wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29921/
> -----------------------------------------------------------
>
> (Updated Jan. 15, 2015, 8:09 a.m.)
>
>
> Review request for sentry, Xiaomeng Huang, Arun Suresh, Colin Ma, shen guoquan, Lenni Kuff, Prasad Mujumdar, and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-608
> https://issues.apache.org/jira/browse/SENTRY-608
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add **allow.connect.groups** to conf, and using **GroupMappingService** get user's groups, only user in allow groups can pass.
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryAuthFilter.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryWebServer.java 090917c
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 47794bc
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryWebServerWithKerberos.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java 0bdc3a2
>
> Diff: https://reviews.apache.org/r/29921/diff/
>
>
> Testing
> -------
>
> Unit Tests in local
>
>
> Thanks,
>
> Dapeng Sun
>
>
Re: Review Request 29921: SENTRY-608: Add simple authorization
support to SentryWebserver
Posted by Dapeng Sun <da...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29921/
-----------------------------------------------------------
(Updated 一月 20, 2015, 5:05 p.m.)
Review request for sentry, Xiaomeng Huang, Arun Suresh, Colin Ma, shen guoquan, Lenni Kuff, Prasad Mujumdar, and Sravya Tirukkovalur.
Changes
-------
Update
Bugs: SENTRY-608
https://issues.apache.org/jira/browse/SENTRY-608
Repository: sentry
Description
-------
Add **allow.connect.groups** to conf, and using **GroupMappingService** get user's groups, only user in allow groups can pass.
Diffs (updated)
-----
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryAuthFilter.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryWebServer.java 090917c
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 47794bc
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryWebServerWithKerberos.java PRE-CREATION
Diff: https://reviews.apache.org/r/29921/diff/
Testing
-------
Unit Tests in local
Thanks,
Dapeng Sun
Re: Review Request 29921: SENTRY-608: Add simple authorization
support to SentryWebserver
Posted by Dapeng Sun <da...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29921/
-----------------------------------------------------------
(Updated 一月 19, 2015, 8:59 p.m.)
Review request for sentry, Xiaomeng Huang, Arun Suresh, Colin Ma, shen guoquan, Lenni Kuff, Prasad Mujumdar, and Sravya Tirukkovalur.
Changes
-------
Update patch according Lenni's feedback
Bugs: SENTRY-608
https://issues.apache.org/jira/browse/SENTRY-608
Repository: sentry
Description
-------
Add **allow.connect.groups** to conf, and using **GroupMappingService** get user's groups, only user in allow groups can pass.
Diffs (updated)
-----
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryAuthFilter.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryWebServer.java 090917c
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 47794bc
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryWebServerWithKerberos.java PRE-CREATION
Diff: https://reviews.apache.org/r/29921/diff/
Testing
-------
Unit Tests in local
Thanks,
Dapeng Sun