You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by harshach <gi...@git.apache.org> on 2014/09/29 22:59:16 UTC
[GitHub] storm pull request: Storm 509. Make groups checking specific for S...
GitHub user harshach opened a pull request:
https://github.com/apache/storm/pull/273
Storm 509. Make groups checking specific for SimpleACLAuthorizer.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/harshach/incubator-storm STORM-509
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/273.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #273
----
commit 02f6db5783f75a88ba305430991f6fda0bfc7b1a
Author: Sriharsha Chintalapani <ma...@harsha.io>
Date: 2014-09-22T18:16:45Z
STORM-502. storm native worker-launcher tests fail.
commit 84002179cf7ae5879c93e900f64a7e576ce4cce1
Author: Sriharsha Chintalapani <ma...@harsha.io>
Date: 2014-09-29T17:17:06Z
Merge remote-tracking branch 'upstream/security' into security
commit 11e6fc9821d7d7e676d573836c08272ce18385f2
Author: Sriharsha Chintalapani <ma...@harsha.io>
Date: 2014-09-29T20:57:07Z
STORM-509. Make groups checking specific for SimpleACLAuthorizer.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: Storm 509. Make groups checking specific for S...
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the pull request:
https://github.com/apache/storm/pull/273#issuecomment-57362203
@revans2 sorry didn't notice I left them in the commit.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: Storm 509. Make groups checking specific for S...
Posted by Parth-Brahmbhatt <gi...@git.apache.org>.
Github user Parth-Brahmbhatt commented on a diff in the pull request:
https://github.com/apache/storm/pull/273#discussion_r18183885
--- Diff: storm-core/src/jvm/backtype/storm/security/auth/authorizer/SimpleACLAuthorizer.java ---
@@ -107,12 +108,18 @@ public boolean permit(ReqContext context, String operation, Map topology_conf) {
if (topoUsers.contains(principal) || topoUsers.contains(user)) {
return true;
}
- if(_groups != null) {
+
+ Set<String> topoGroups = new HashSet<String>();
+ if (topology_conf.containsKey(Config.TOPOLOGY_GROUPS)) {
+ topoGroups.addAll((Collection<String>)topology_conf.get(Config.TOPOLOGY_GROUPS));
+ }
+
+ if(_groupMappingProvider != null && topoGroups.size() > 0) {
+ System.out.println("hello in if");
try {
- String topologySubmitterUser = (String) topology_conf.get(Config.TOPOLOGY_SUBMITTER_USER);
- Set<String> userGroups = _groups.getGroups(user);
- Set<String> topoUserGroups = _groups.getGroups(topologySubmitterUser);
- for (String tgroup : topoUserGroups) {
+ Set<String> userGroups = _groupMappingProvider.getGroups(user);
+ for (String tgroup : topoGroups) {
+ System.out.println("tgroup "+tgroup);
--- End diff --
remove.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: Storm 509. Make groups checking specific for S...
Posted by Parth-Brahmbhatt <gi...@git.apache.org>.
Github user Parth-Brahmbhatt commented on the pull request:
https://github.com/apache/storm/pull/273#issuecomment-57230604
+1.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: Storm 509. Make groups checking specific for S...
Posted by Parth-Brahmbhatt <gi...@git.apache.org>.
Github user Parth-Brahmbhatt commented on a diff in the pull request:
https://github.com/apache/storm/pull/273#discussion_r18183880
--- Diff: storm-core/src/jvm/backtype/storm/security/auth/authorizer/SimpleACLAuthorizer.java ---
@@ -107,12 +108,18 @@ public boolean permit(ReqContext context, String operation, Map topology_conf) {
if (topoUsers.contains(principal) || topoUsers.contains(user)) {
return true;
}
- if(_groups != null) {
+
+ Set<String> topoGroups = new HashSet<String>();
+ if (topology_conf.containsKey(Config.TOPOLOGY_GROUPS)) {
+ topoGroups.addAll((Collection<String>)topology_conf.get(Config.TOPOLOGY_GROUPS));
+ }
+
+ if(_groupMappingProvider != null && topoGroups.size() > 0) {
+ System.out.println("hello in if");
--- End diff --
remove.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: Storm 509. Make groups checking specific for S...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/273#discussion_r18237335
--- Diff: storm-core/src/jvm/backtype/storm/security/auth/authorizer/SimpleACLAuthorizer.java ---
@@ -107,12 +108,18 @@ public boolean permit(ReqContext context, String operation, Map topology_conf) {
if (topoUsers.contains(principal) || topoUsers.contains(user)) {
return true;
}
- if(_groups != null) {
+
+ Set<String> topoGroups = new HashSet<String>();
+ if (topology_conf.containsKey(Config.TOPOLOGY_GROUPS)) {
+ topoGroups.addAll((Collection<String>)topology_conf.get(Config.TOPOLOGY_GROUPS));
+ }
+
+ if(_groupMappingProvider != null && topoGroups.size() > 0) {
+ System.out.println("hello in if");
--- End diff --
Yes please remove the System.out.println calls.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: Storm 509. Make groups checking specific for S...
Posted by ptgoetz <gi...@git.apache.org>.
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/273#issuecomment-57414452
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: Storm 509. Make groups checking specific for S...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/273
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: Storm 509. Make groups checking specific for S...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/273#issuecomment-57360464
Once your remove the debug logging I too am +1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: Storm 509. Make groups checking specific for S...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/273#issuecomment-57383504
Looks great thanks for the quick turn around.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---