[GitHub] storm pull request: Storm 509. Make groups checking specific for S...

2014-10-27 Thread asfgit
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: Storm 509. Make groups checking specific for S...

2014-09-30 Thread ptgoetz
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: Storm 509. Make groups checking specific for S...

2014-09-30 Thread revans2
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: Storm 509. Make groups checking specific for S...

2014-09-30 Thread harshach
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: Storm 509. Make groups checking specific for S...

2014-09-30 Thread revans2
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: Storm 509. Make groups checking specific for S...

2014-09-30 Thread revans2
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 topoGroups = new HashSet();
+if (topology_conf.containsKey(Config.TOPOLOGY_GROUPS)) {
+
topoGroups.addAll((Collection)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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: Storm 509. Make groups checking specific for S...

2014-09-29 Thread Parth-Brahmbhatt
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 topoGroups = new HashSet();
+if (topology_conf.containsKey(Config.TOPOLOGY_GROUPS)) {
+
topoGroups.addAll((Collection)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 userGroups = _groups.getGroups(user);
-Set topoUserGroups = 
_groups.getGroups(topologySubmitterUser);
-for (String tgroup : topoUserGroups) {
+Set 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: Storm 509. Make groups checking specific for S...

2014-09-29 Thread Parth-Brahmbhatt
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 topoGroups = new HashSet();
+if (topology_conf.containsKey(Config.TOPOLOGY_GROUPS)) {
+
topoGroups.addAll((Collection)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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: Storm 509. Make groups checking specific for S...

2014-09-29 Thread Parth-Brahmbhatt
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: Storm 509. Make groups checking specific for S...

2014-09-29 Thread harshach
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 
Date:   2014-09-22T18:16:45Z

STORM-502. storm native worker-launcher tests fail.

commit 84002179cf7ae5879c93e900f64a7e576ce4cce1
Author: Sriharsha Chintalapani 
Date:   2014-09-29T17:17:06Z

Merge remote-tracking branch 'upstream/security' into security

commit 11e6fc9821d7d7e676d573836c08272ce18385f2
Author: Sriharsha Chintalapani 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---