[GitHub] incubator-hawq pull request #1250: Hawq 1477. Ranger-plugin connect to Range...

2017-06-06 Thread interma
Github user interma closed the pull request at:

https://github.com/apache/incubator-hawq/pull/1250


---
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] incubator-hawq pull request #1250: Hawq 1477. Ranger-plugin connect to Range...

2017-06-06 Thread interma
Github user interma commented on a diff in the pull request:

https://github.com/apache/incubator-hawq/pull/1250#discussion_r120286370
  
--- Diff: 
ranger-plugin/service/src/main/java/org/apache/hawq/ranger/authorization/RangerHawqPluginResource.java
 ---
@@ -45,8 +47,43 @@
 /**
  * Constructor. Creates a new instance of the resource that uses 
RangerHawqAuthorizer.
  */
-public RangerHawqPluginResource() {
+public RangerHawqPluginResource()
+{
+// set UserGroupInformation under kerberos authentication
+if (Utils.getAuth().equals("kerberos"))
+{
+Configuration conf = new Configuration();
+conf.set("hadoop.security.authentication", "kerberos");
+UserGroupInformation.setConfiguration(conf);
+
+String prin = Utils.getPrincipal();
+String keytab = Utils.getKeytab();
+
+if ( !prin.equals("") && !keytab.equals("") )
+{
+try
+{
+UserGroupInformation.loginUserFromKeytab(prin, keytab);
+}
+catch (Exception e)
+{
+LOG.warn(String.format("loginUserFromKeytab failed, 
user[%s], keytab[%s]", prin, keytab));
+}
+}
+}
+
+try
+{
+UserGroupInformation user = 
UserGroupInformation.getLoginUser();
+LOG.info(String.format("login user: %s", user));
+}
+catch (Exception e)
+{
+LOG.warn("get login user failed");
--- End diff --

Two reasons:
1. Abort immediately may be not appropriate, sometimes it can run happily 
in below code (e.g. set simple auth and get login_user() failed by some 
reasons, but will call a no-secure webservice later)
2. Another small reason: this code locates in a constructor, I didn't find 
a easy way to deal with exception... 

So I just leave it as it is, it will fail in the first place which need 
auth in the below code (I tested, it usually prints a warning message contains 
"HTTP 401/403" in the log, looks well).



---
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] incubator-hawq pull request #1250: Hawq 1477. Ranger-plugin connect to Range...

2017-06-06 Thread stanlyxiang
Github user stanlyxiang commented on a diff in the pull request:

https://github.com/apache/incubator-hawq/pull/1250#discussion_r120274829
  
--- Diff: 
ranger-plugin/service/src/main/java/org/apache/hawq/ranger/authorization/RangerHawqPluginResource.java
 ---
@@ -45,8 +47,43 @@
 /**
  * Constructor. Creates a new instance of the resource that uses 
RangerHawqAuthorizer.
  */
-public RangerHawqPluginResource() {
+public RangerHawqPluginResource()
+{
+// set UserGroupInformation under kerberos authentication
+if (Utils.getAuth().equals("kerberos"))
+{
+Configuration conf = new Configuration();
+conf.set("hadoop.security.authentication", "kerberos");
+UserGroupInformation.setConfiguration(conf);
+
+String prin = Utils.getPrincipal();
+String keytab = Utils.getKeytab();
+
+if ( !prin.equals("") && !keytab.equals("") )
+{
+try
+{
+UserGroupInformation.loginUserFromKeytab(prin, keytab);
+}
+catch (Exception e)
+{
+LOG.warn(String.format("loginUserFromKeytab failed, 
user[%s], keytab[%s]", prin, keytab));
+}
+}
+}
+
+try
+{
+UserGroupInformation user = 
UserGroupInformation.getLoginUser();
+LOG.info(String.format("login user: %s", user));
+}
+catch (Exception e)
+{
+LOG.warn("get login user failed");
--- End diff --

So if it usually fail in the below code due to try use simple auth user to 
login a kerberized service, why do we let it go failure and why don't we abort 
here ? 


---
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] incubator-hawq pull request #1250: Hawq 1477. Ranger-plugin connect to Range...

2017-06-06 Thread interma
Github user interma commented on a diff in the pull request:

https://github.com/apache/incubator-hawq/pull/1250#discussion_r120274725
  
--- Diff: 
ranger-plugin/service/src/main/java/org/apache/hawq/ranger/authorization/RangerHawqPluginResource.java
 ---
@@ -45,8 +47,43 @@
 /**
  * Constructor. Creates a new instance of the resource that uses 
RangerHawqAuthorizer.
  */
-public RangerHawqPluginResource() {
+public RangerHawqPluginResource()
+{
+// set UserGroupInformation under kerberos authentication
+if (Utils.getAuth().equals("kerberos"))
--- End diff --

Yes, but I think use 
`org.apache.hadoop.security.UserGroupInformation.AuthenticationMethod` is 
better:
```
  public static enum AuthenticationMethod {
// currently we support only one auth per method, but eventually a 
// subtype is needed to differentiate, ex. if digest is token or ldap
SIMPLE(AuthMethod.SIMPLE,
HadoopConfiguration.SIMPLE_CONFIG_NAME),
KERBEROS(AuthMethod.KERBEROS,
HadoopConfiguration.USER_KERBEROS_CONFIG_NAME),
TOKEN(AuthMethod.TOKEN),
CERTIFICATE(null),
KERBEROS_SSL(null),
PROXY(null);
```
Let's refactor it in future, keep it as it is now. 


---
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] incubator-hawq pull request #1250: Hawq 1477. Ranger-plugin connect to Range...

2017-06-06 Thread interma
Github user interma commented on a diff in the pull request:

https://github.com/apache/incubator-hawq/pull/1250#discussion_r120274082
  
--- Diff: 
ranger-plugin/service/src/main/java/org/apache/hawq/ranger/authorization/RangerHawqPluginResource.java
 ---
@@ -45,8 +47,43 @@
 /**
  * Constructor. Creates a new instance of the resource that uses 
RangerHawqAuthorizer.
  */
-public RangerHawqPluginResource() {
+public RangerHawqPluginResource()
+{
+// set UserGroupInformation under kerberos authentication
+if (Utils.getAuth().equals("kerberos"))
+{
+Configuration conf = new Configuration();
+conf.set("hadoop.security.authentication", "kerberos");
+UserGroupInformation.setConfiguration(conf);
+
+String prin = Utils.getPrincipal();
+String keytab = Utils.getKeytab();
+
+if ( !prin.equals("") && !keytab.equals("") )
+{
+try
+{
+UserGroupInformation.loginUserFromKeytab(prin, keytab);
+}
+catch (Exception e)
+{
+LOG.warn(String.format("loginUserFromKeytab failed, 
user[%s], keytab[%s]", prin, keytab));
+}
+}
+}
+
+try
+{
+UserGroupInformation user = 
UserGroupInformation.getLoginUser();
+LOG.info(String.format("login user: %s", user));
+}
+catch (Exception e)
+{
+LOG.warn("get login user failed");
--- End diff --

Yes, if login failed here, it just uses the simple auth method(e.g. 
gpadmin), but it usually fail in the below code (try use simple auth user to 
login a kerberized sevice). 


---
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] incubator-hawq pull request #1250: Hawq 1477. Ranger-plugin connect to Range...

2017-06-06 Thread interma
Github user interma commented on a diff in the pull request:

https://github.com/apache/incubator-hawq/pull/1250#discussion_r120273701
  
--- Diff: 
ranger-plugin/service/src/main/java/org/apache/hawq/ranger/authorization/RangerHawqPluginResource.java
 ---
@@ -45,8 +47,43 @@
 /**
  * Constructor. Creates a new instance of the resource that uses 
RangerHawqAuthorizer.
  */
-public RangerHawqPluginResource() {
+public RangerHawqPluginResource()
+{
+// set UserGroupInformation under kerberos authentication
+if (Utils.getAuth().equals("kerberos"))
+{
+Configuration conf = new Configuration();
+conf.set("hadoop.security.authentication", "kerberos");
+UserGroupInformation.setConfiguration(conf);
+
+String prin = Utils.getPrincipal();
+String keytab = Utils.getKeytab();
+
+if ( !prin.equals("") && !keytab.equals("") )
+{
+try
+{
+UserGroupInformation.loginUserFromKeytab(prin, keytab);
+}
+catch (Exception e)
+{
+LOG.warn(String.format("loginUserFromKeytab failed, 
user[%s], keytab[%s]", prin, keytab));
+}
+}
+}
--- End diff --

It uses the kinit credential cache to when auth method set to 
authentication and two of them are empty. 
So it's ok here.


---
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] incubator-hawq pull request #1250: Hawq 1477. Ranger-plugin connect to Range...

2017-06-06 Thread interma
Github user interma commented on a diff in the pull request:

https://github.com/apache/incubator-hawq/pull/1250#discussion_r120273006
  
--- Diff: 
ranger-plugin/service/src/main/java/org/apache/hawq/ranger/authorization/RangerHawqPluginResource.java
 ---
@@ -45,8 +47,43 @@
 /**
  * Constructor. Creates a new instance of the resource that uses 
RangerHawqAuthorizer.
  */
-public RangerHawqPluginResource() {
+public RangerHawqPluginResource()
+{
+// set UserGroupInformation under kerberos authentication
+if (Utils.getAuth().equals("kerberos"))
+{
+Configuration conf = new Configuration();
+conf.set("hadoop.security.authentication", "kerberos");
+UserGroupInformation.setConfiguration(conf);
+
+String prin = Utils.getPrincipal();
+String keytab = Utils.getKeytab();
+
+if ( !prin.equals("") && !keytab.equals("") )
+{
+try
+{
+UserGroupInformation.loginUserFromKeytab(prin, keytab);
+}
+catch (Exception e)
+{
+LOG.warn(String.format("loginUserFromKeytab failed, 
user[%s], keytab[%s]", prin, keytab));
+}
+}
+}
+
+try
+{
+UserGroupInformation user = 
UserGroupInformation.getLoginUser();
+LOG.info(String.format("login user: %s", user));
+}
+catch (Exception e)
+{
+LOG.warn("get login user failed");
--- End diff --

Thanks, I will fix them!


---
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] incubator-hawq pull request #1250: Hawq 1477. Ranger-plugin connect to Range...

2017-06-05 Thread denalex
Github user denalex commented on a diff in the pull request:

https://github.com/apache/incubator-hawq/pull/1250#discussion_r120161076
  
--- Diff: 
ranger-plugin/service/src/main/java/org/apache/hawq/ranger/authorization/RangerHawqPluginResource.java
 ---
@@ -45,8 +47,43 @@
 /**
  * Constructor. Creates a new instance of the resource that uses 
RangerHawqAuthorizer.
  */
-public RangerHawqPluginResource() {
+public RangerHawqPluginResource()
+{
+// set UserGroupInformation under kerberos authentication
+if (Utils.getAuth().equals("kerberos"))
+{
+Configuration conf = new Configuration();
+conf.set("hadoop.security.authentication", "kerberos");
+UserGroupInformation.setConfiguration(conf);
+
+String prin = Utils.getPrincipal();
+String keytab = Utils.getKeytab();
+
+if ( !prin.equals("") && !keytab.equals("") )
+{
+try
+{
+UserGroupInformation.loginUserFromKeytab(prin, keytab);
+}
+catch (Exception e)
+{
+LOG.warn(String.format("loginUserFromKeytab failed, 
user[%s], keytab[%s]", prin, keytab));
+}
+}
+}
+
+try
+{
+UserGroupInformation user = 
UserGroupInformation.getLoginUser();
+LOG.info(String.format("login user: %s", user));
+}
+catch (Exception e)
+{
+LOG.warn("get login user failed");
--- End diff --

always log more details, like exception type and message and context 
information, current message doesn't help to troubleshoot.


---
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] incubator-hawq pull request #1250: Hawq 1477. Ranger-plugin connect to Range...

2017-06-05 Thread denalex
Github user denalex commented on a diff in the pull request:

https://github.com/apache/incubator-hawq/pull/1250#discussion_r120160818
  
--- Diff: 
ranger-plugin/service/src/main/java/org/apache/hawq/ranger/authorization/RangerHawqPluginResource.java
 ---
@@ -45,8 +47,43 @@
 /**
  * Constructor. Creates a new instance of the resource that uses 
RangerHawqAuthorizer.
  */
-public RangerHawqPluginResource() {
+public RangerHawqPluginResource()
+{
+// set UserGroupInformation under kerberos authentication
+if (Utils.getAuth().equals("kerberos"))
+{
+Configuration conf = new Configuration();
+conf.set("hadoop.security.authentication", "kerberos");
+UserGroupInformation.setConfiguration(conf);
+
+String prin = Utils.getPrincipal();
+String keytab = Utils.getKeytab();
+
+if ( !prin.equals("") && !keytab.equals("") )
+{
+try
+{
+UserGroupInformation.loginUserFromKeytab(prin, keytab);
+}
+catch (Exception e)
+{
+LOG.warn(String.format("loginUserFromKeytab failed, 
user[%s], keytab[%s]", prin, keytab));
+}
+}
+}
+
+try
+{
+UserGroupInformation user = 
UserGroupInformation.getLoginUser();
+LOG.info(String.format("login user: %s", user));
--- End diff --

this should be debug level, we do not want to log into log file every time 
request is processed normally


---
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] incubator-hawq pull request #1250: Hawq 1477. Ranger-plugin connect to Range...

2017-06-04 Thread stanlyxiang
Github user stanlyxiang commented on a diff in the pull request:

https://github.com/apache/incubator-hawq/pull/1250#discussion_r120044242
  
--- Diff: 
ranger-plugin/service/src/main/java/org/apache/hawq/ranger/authorization/RangerHawqPluginResource.java
 ---
@@ -45,8 +47,43 @@
 /**
  * Constructor. Creates a new instance of the resource that uses 
RangerHawqAuthorizer.
  */
-public RangerHawqPluginResource() {
+public RangerHawqPluginResource()
+{
+// set UserGroupInformation under kerberos authentication
+if (Utils.getAuth().equals("kerberos"))
+{
+Configuration conf = new Configuration();
+conf.set("hadoop.security.authentication", "kerberos");
+UserGroupInformation.setConfiguration(conf);
+
+String prin = Utils.getPrincipal();
+String keytab = Utils.getKeytab();
+
+if ( !prin.equals("") && !keytab.equals("") )
+{
+try
+{
+UserGroupInformation.loginUserFromKeytab(prin, keytab);
+}
+catch (Exception e)
+{
+LOG.warn(String.format("loginUserFromKeytab failed, 
user[%s], keytab[%s]", prin, keytab));
+}
+}
+}
--- End diff --

To my understanding for this logic, principal and keytab are needed for 
loginUserFromKeytab.But the default value of the 2 configurations are empty, 
empty values are not work for kerberos, if user set RPS_AUTH to kerberos and 
leave them empty, he will only get a warning of "get login user failed" until 
the program tries to getLoginUser(). So why don't we check them earlier ? 


---
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] incubator-hawq pull request #1250: Hawq 1477. Ranger-plugin connect to Range...

2017-06-04 Thread stanlyxiang
Github user stanlyxiang commented on a diff in the pull request:

https://github.com/apache/incubator-hawq/pull/1250#discussion_r120044685
  
--- Diff: 
ranger-plugin/service/src/main/java/org/apache/hawq/ranger/authorization/RangerHawqPluginResource.java
 ---
@@ -45,8 +47,43 @@
 /**
  * Constructor. Creates a new instance of the resource that uses 
RangerHawqAuthorizer.
  */
-public RangerHawqPluginResource() {
+public RangerHawqPluginResource()
+{
+// set UserGroupInformation under kerberos authentication
+if (Utils.getAuth().equals("kerberos"))
+{
+Configuration conf = new Configuration();
+conf.set("hadoop.security.authentication", "kerberos");
+UserGroupInformation.setConfiguration(conf);
+
+String prin = Utils.getPrincipal();
+String keytab = Utils.getKeytab();
+
+if ( !prin.equals("") && !keytab.equals("") )
+{
+try
+{
+UserGroupInformation.loginUserFromKeytab(prin, keytab);
+}
+catch (Exception e)
+{
+LOG.warn(String.format("loginUserFromKeytab failed, 
user[%s], keytab[%s]", prin, keytab));
+}
+}
+}
+
+try
+{
+UserGroupInformation user = 
UserGroupInformation.getLoginUser();
+LOG.info(String.format("login user: %s", user));
+}
+catch (Exception e)
+{
+LOG.warn("get login user failed");
--- End diff --

If get login user failed, this constructor can run well anyway ? 


---
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] incubator-hawq pull request #1250: Hawq 1477. Ranger-plugin connect to Range...

2017-06-04 Thread stanlyxiang
Github user stanlyxiang commented on a diff in the pull request:

https://github.com/apache/incubator-hawq/pull/1250#discussion_r120043142
  
--- Diff: 
ranger-plugin/service/src/main/java/org/apache/hawq/ranger/authorization/Utils.java
 ---
@@ -68,6 +77,30 @@ public static String getVersion() {
 }
 
 /**
+ * Retrieves the authentication
+ * @return kerberos or simple[default]
+ */
+public static String getAuth() {
+return System.getProperty(AUTH_KEY_ENV, 
properties.getProperty(AUTH_KEY_FILE, "simple"));
--- End diff --

public static final String SIMPLE="simple"; and use SIMPLE here. So does 
EMPTY_STRING for ""


---
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] incubator-hawq pull request #1250: Hawq 1477. Ranger-plugin connect to Range...

2017-06-04 Thread stanlyxiang
Github user stanlyxiang commented on a diff in the pull request:

https://github.com/apache/incubator-hawq/pull/1250#discussion_r120042673
  
--- Diff: 
ranger-plugin/service/src/main/java/org/apache/hawq/ranger/authorization/RangerHawqPluginResource.java
 ---
@@ -45,8 +47,43 @@
 /**
  * Constructor. Creates a new instance of the resource that uses 
RangerHawqAuthorizer.
  */
-public RangerHawqPluginResource() {
+public RangerHawqPluginResource()
+{
+// set UserGroupInformation under kerberos authentication
+if (Utils.getAuth().equals("kerberos"))
--- End diff --

using variable instead of using "kerberos" string everywhere.


---
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.
---