HBase
  1. HBase
  2. HBASE-9227

RESTServer should handle the loginUser correctly

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 0.95.0
    • Fix Version/s: 0.95.2
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      HBASE-8662 introduced a change by which the realUser in the method RESTServer.main() gets assigned to the loginUser only when the config hbase.rest.authentication.type is set to something (like "kerberos").

      I think we should set the realUser to loginUser even when the config hbase.rest.authentication.type is null. Without that the regular (non-impersonated) accesses also fail.

      1. 9227-4.txt
        1 kB
        Devaraj Das
      2. 9227-3.txt
        1 kB
        Devaraj Das
      3. 9227-2.txt
        2 kB
        Devaraj Das
      4. 9227-1.txt
        1 kB
        Devaraj Das

        Activity

        Hide
        Devaraj Das added a comment -

        Straightforward fix. Jimmy Xiang mind taking a look.

        Show
        Devaraj Das added a comment - Straightforward fix. Jimmy Xiang mind taking a look.
        Hide
        Jimmy Xiang added a comment -

        For the regular secure access, why do we need to manage the connection ourselves? HBaseAdmin doesn't handle it any more? It should use the login user by default, right?

        Show
        Jimmy Xiang added a comment - For the regular secure access, why do we need to manage the connection ourselves? HBaseAdmin doesn't handle it any more? It should use the login user by default, right?
        Hide
        Devaraj Das added a comment - - edited

        The problem is that in the RESTServer code, we call loginUserFromKeytabAndReturnUGI. This doesn't set the loginUser in the UGI. Thereafter, when the getCurrentUser would be called (which at some point would be called in the RPC client), it would try to login, but the keytabFile wouldn't be set and the login will be as a regular user (non-keytab). This will work if someone had done a kinit outside the process prior to the first RPC invocation from the REST server. But this is not what we want.. [Note that the keytabFile would be set only when loginUserFromKeytab is called.]

        Show
        Devaraj Das added a comment - - edited The problem is that in the RESTServer code, we call loginUserFromKeytabAndReturnUGI. This doesn't set the loginUser in the UGI. Thereafter, when the getCurrentUser would be called (which at some point would be called in the RPC client), it would try to login, but the keytabFile wouldn't be set and the login will be as a regular user (non-keytab). This will work if someone had done a kinit outside the process prior to the first RPC invocation from the REST server. But this is not what we want.. [Note that the keytabFile would be set only when loginUserFromKeytab is called.]
        Hide
        Devaraj Das added a comment -

        Maybe this is a better fix. I am still debating (and digging in) between the two..

        Show
        Devaraj Das added a comment - Maybe this is a better fix. I am still debating (and digging in) between the two..
        Hide
        Jimmy Xiang added a comment -

        +1. Thanks for the explanation.

        Show
        Jimmy Xiang added a comment - +1. Thanks for the explanation.
        Hide
        Jimmy Xiang added a comment -

        I prefer v2. It is better to set realUser to User.getCurrent().getUGI() only if hbase.rest.authentication.type is not null?

        Show
        Jimmy Xiang added a comment - I prefer v2. It is better to set realUser to User.getCurrent().getUGI() only if hbase.rest.authentication.type is not null?
        Hide
        Devaraj Das added a comment -

        I thought I had submitted the patch earlier for hadoopqa to take a look but I hadn't. Doing it now.

        Jimmy Xiang, I didn't quite get what you said (in the patch v2 realUser is inlined and the variable is not really there). I have left the patch as it is.

        Show
        Devaraj Das added a comment - I thought I had submitted the patch earlier for hadoopqa to take a look but I hadn't. Doing it now. Jimmy Xiang , I didn't quite get what you said (in the patch v2 realUser is inlined and the variable is not really there). I have left the patch as it is.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12598141/9227-2.txt
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +1 hadoop1.0. The patch compiles against the hadoop 1.0 profile.

        +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

        -1 javadoc. The javadoc tool appears to have generated 1 warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        -1 findbugs. The patch appears to introduce 1 new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 lineLengths. The patch does not introduce lines longer than 100

        +1 site. The mvn site goal succeeds with this patch.

        +1 core tests. The patch passed unit tests in .

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/6757//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6757//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6757//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6757//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6757//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6757//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6757//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6757//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6757//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/6757//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12598141/9227-2.txt against trunk revision . +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 hadoop1.0 . The patch compiles against the hadoop 1.0 profile. +1 hadoop2.0 . The patch compiles against the hadoop 2.0 profile. -1 javadoc . The javadoc tool appears to have generated 1 warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. -1 findbugs . The patch appears to introduce 1 new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 lineLengths . The patch does not introduce lines longer than 100 +1 site . The mvn site goal succeeds with this patch. +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/6757//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6757//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6757//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6757//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6757//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6757//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6757//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6757//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6757//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/6757//console This message is automatically generated.
        Hide
        Devaraj Das added a comment -

        This is more close to the code we presently have.

        Show
        Devaraj Das added a comment - This is more close to the code we presently have.
        Hide
        stack added a comment -

        Jimmy Xiang Mighty Jimmy! Any chance of a review of the last patch? Thanks.

        Show
        stack added a comment - Jimmy Xiang Mighty Jimmy! Any chance of a review of the last patch? Thanks.
        Hide
        Jimmy Xiang added a comment - - edited

        Devaraj Das, User.getCurrent().getUGI() login again? I think that's why I used login and return UGI. My understanding is that the issue is not setting the user after login in, instead of the way to log in, right?

        Show
        Jimmy Xiang added a comment - - edited Devaraj Das , User.getCurrent().getUGI() login again? I think that's why I used login and return UGI. My understanding is that the issue is not setting the user after login in, instead of the way to log in, right?
        Hide
        Devaraj Das added a comment -

        Jimmy Xiang, User.login would do the login and save the state in loginUser. Later when User.getCurrent() is called, it would get that UGI (since getCurrentUser in Hadoop's UGI class returns the loginUser UGI unless called within a doAs for a different user). I'll also try to test the patch for the regular user (non-impersonation case).

        Show
        Devaraj Das added a comment - Jimmy Xiang , User.login would do the login and save the state in loginUser. Later when User.getCurrent() is called, it would get that UGI (since getCurrentUser in Hadoop's UGI class returns the loginUser UGI unless called within a doAs for a different user). I'll also try to test the patch for the regular user (non-impersonation case).
        Hide
        Jimmy Xiang added a comment -

        +1 if the test is ok. Thanks.

        Show
        Jimmy Xiang added a comment - +1 if the test is ok. Thanks.
        Hide
        Devaraj Das added a comment -

        Testing was needed! I was using the wrong strings for the arguments to User.login. This test fixes that.
        This is also tested for the regular secure case case (no impersonation, no hbase.rest.authentication.type set).

        Show
        Devaraj Das added a comment - Testing was needed! I was using the wrong strings for the arguments to User.login. This test fixes that. This is also tested for the regular secure case case (no impersonation, no hbase.rest.authentication.type set).
        Hide
        Jimmy Xiang added a comment -

        +1, can we remove those parameters not needed any more when commit it?

        Show
        Jimmy Xiang added a comment - +1, can we remove those parameters not needed any more when commit it?
        Hide
        stack added a comment -

        I committed the patch as is. Lets do cleanup in another issue.

        Show
        stack added a comment - I committed the patch as is. Lets do cleanup in another issue.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12598259/9227-4.txt
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +1 hadoop1.0. The patch compiles against the hadoop 1.0 profile.

        +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 lineLengths. The patch does not introduce lines longer than 100

        +1 site. The mvn site goal succeeds with this patch.

        -1 core tests. The patch failed these unit tests:

        -1 core zombie tests. There are 1 zombie test(s): at org.apache.hadoop.hbase.TestZooKeeper.testRegionAssignmentAfterMasterRecoveryDueToZKExpiry(TestZooKeeper.java:486)

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/6763//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6763//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6763//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6763//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6763//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6763//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6763//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6763//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6763//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/6763//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12598259/9227-4.txt against trunk revision . +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 hadoop1.0 . The patch compiles against the hadoop 1.0 profile. +1 hadoop2.0 . The patch compiles against the hadoop 2.0 profile. +1 javadoc . The javadoc tool did not generate any warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 lineLengths . The patch does not introduce lines longer than 100 +1 site . The mvn site goal succeeds with this patch. -1 core tests . The patch failed these unit tests: -1 core zombie tests . There are 1 zombie test(s): at org.apache.hadoop.hbase.TestZooKeeper.testRegionAssignmentAfterMasterRecoveryDueToZKExpiry(TestZooKeeper.java:486) Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/6763//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6763//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6763//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6763//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6763//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6763//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6763//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6763//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6763//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/6763//console This message is automatically generated.
        Hide
        Devaraj Das added a comment -

        Thanks, stack and Jimmy Xiang for commit/reviews.

        can we remove those parameters not needed any more when commit it

        Jimmy, which parameters are you referring to here?

        Show
        Devaraj Das added a comment - Thanks, stack and Jimmy Xiang for commit/reviews. can we remove those parameters not needed any more when commit it Jimmy, which parameters are you referring to here?
        Hide
        Jimmy Xiang added a comment -

        No parameter, just one import not used any more: SecurityUtil. We can do it later.

        Show
        Jimmy Xiang added a comment - No parameter, just one import not used any more: SecurityUtil. We can do it later.
        Hide
        Hudson added a comment -

        SUCCESS: Integrated in hbase-0.95-on-hadoop2 #244 (See https://builds.apache.org/job/hbase-0.95-on-hadoop2/244/)
        HBASE-9227 RESTServer should handle the loginUser correctly (stack: rev 1514439)

        • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/rest/RESTServer.java
        Show
        Hudson added a comment - SUCCESS: Integrated in hbase-0.95-on-hadoop2 #244 (See https://builds.apache.org/job/hbase-0.95-on-hadoop2/244/ ) HBASE-9227 RESTServer should handle the loginUser correctly (stack: rev 1514439) /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/rest/RESTServer.java
        Hide
        Hudson added a comment -

        SUCCESS: Integrated in HBase-TRUNK #4398 (See https://builds.apache.org/job/HBase-TRUNK/4398/)
        HBASE-9227 RESTServer should handle the loginUser correctly (stack: rev 1514440)

        • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/rest/RESTServer.java
        Show
        Hudson added a comment - SUCCESS: Integrated in HBase-TRUNK #4398 (See https://builds.apache.org/job/HBase-TRUNK/4398/ ) HBASE-9227 RESTServer should handle the loginUser correctly (stack: rev 1514440) /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/rest/RESTServer.java
        Hide
        Hudson added a comment -

        FAILURE: Integrated in hbase-0.95 #455 (See https://builds.apache.org/job/hbase-0.95/455/)
        HBASE-9227 RESTServer should handle the loginUser correctly (stack: rev 1514439)

        • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/rest/RESTServer.java
        Show
        Hudson added a comment - FAILURE: Integrated in hbase-0.95 #455 (See https://builds.apache.org/job/hbase-0.95/455/ ) HBASE-9227 RESTServer should handle the loginUser correctly (stack: rev 1514439) /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/rest/RESTServer.java
        Hide
        Hudson added a comment -

        SUCCESS: Integrated in HBase-TRUNK-on-Hadoop-2.0.0 #678 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-2.0.0/678/)
        HBASE-9227 RESTServer should handle the loginUser correctly (stack: rev 1514440)

        • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/rest/RESTServer.java
        Show
        Hudson added a comment - SUCCESS: Integrated in HBase-TRUNK-on-Hadoop-2.0.0 #678 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-2.0.0/678/ ) HBASE-9227 RESTServer should handle the loginUser correctly (stack: rev 1514440) /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/rest/RESTServer.java

          People

          • Assignee:
            Devaraj Das
            Reporter:
            Devaraj Das
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development