joshelser commented on code in PR #123:
URL: 
https://github.com/apache/phoenix-queryserver/pull/123#discussion_r1140363900


##########
phoenix-queryserver/src/main/java/org/apache/phoenix/queryserver/server/QueryServer.java:
##########
@@ -633,6 +625,26 @@ SimpleLRUCache<String,UserGroupInformation> getCache() {
       }
   }
 
+  /**
+   * The new Jetty kerberos implementation that we use strips the realm from 
the principal, which
+   * and Hadoop cannot process that.
+   * This strips the hostname part as well, so that only the username remains.
+
+   * @param remoteUserName the principal as received from Jetty
+   * @return the principal without the hostname part
+   */
+  //FIXME We are probably compensating for a Jetty a bug, which should really 
be fixed in Jetty
+  private static String stripHostNameFromPrincipal(String remoteUserName) {
+      // realm got removed from remoteUserName in CALCITE-4152
+      // so we remove the instance name to avoid geting 
KerberosName$NoMatchingRule exception
+      int atSignIndex = remoteUserName.indexOf('@');
+      int separatorIndex = remoteUserName.indexOf('/');
+      if (atSignIndex == -1 && separatorIndex > 0) {
+        remoteUserName = remoteUserName.substring(0, separatorIndex);
+      }

Review Comment:
   > I just refactored it into a method.
   
   Gotcha. IIRC, the default `auth_to_local` rules will treat `foo/127.0.0.1`, 
`foo/1.2.3.4` and `foo` all as the `foo` "remote user name". This is more 
important for horizontally scalable services (e.g. treat all RegionServers in 
an HBase cluster uniformly), but I think preserving that idea in PQS is also a 
good idea.
   
   Doesn't have to be done in this PR.
   
   > I'm not sure what format principals we should support, and whether Haddop 
not accepting the user/host format without realm is a bug or not.
   
   I'd imagine that the code in hadoop that can't handle the realm is 
considered "private API" and you would get a "we don't want to touch it" kind 
of response :)
   
   Code processing principals should definitely be able to handle receiving a 
principal with a realm (and removing that as-necessary).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to