Sometimes wrong user is used to delete the namespace in explore.

Description

The issue I observed:
Alice was logged in and had the privilege to delete the namespace but the operation failed with following stacktrace, indicating eve is making query, not sure why this happens and how to reproduce:

Release Notes

Fixed a bug that sometimes wrong user is used in explore, which results in the failure of deleting namespace.

Activity

Show:
Yaojie Feng
June 26, 2017, 10:45 PM

There is a race condition that ExploreClient is getting the wrong user from SecurityRequestContext for the http request, which may result in failure of execution on the request on an authorized cluster.
Steps to reproduce(Not entirely sure this will reproduce, need to do it back and forth):
1. in cdap cli, log in as eve, create ns1
2. log out and delete the access token file
3. log in as alice, try to create then delete the namespace
4. if delete succeeds, try to go through 1-3 and try couple more times.

Eventually, the deletion will throw exception as shown above, the explore service is complaining eve is trying to get the namespace while the actual user is actually alice.

After some investigation, found that before the call get to the explore service, we got the correct user alice sending the request. But in the callback of explore service, which tries to get the namespace meta, the ExploreClient is getting eve from SecurityRequestContext. Since eve does not have access to the namespace, the deletion will fail.

Poorna Chandra
June 27, 2017, 2:34 AM

The userid of the user making calls to a CDAP service is stored as an InheritableThreadLocal variable. Explore client makes an async call to Explore Service for all operations including namespace deletion. It uses a thread pool to submit the calls. According to the javadoc of InheritableThreadLocal, the value is inherited only on thread creation. Once a thread is created, all subsequent calls made using that thread will use the userid of the first call. This is the reason for the behavior seen above.

Poorna Chandra
June 27, 2017, 7:22 AM

One option to fix this issue is to store the user id as a new Principal (as part of the active Subject), instead of storing the user id in a InheritableThreadLocal variable. This way the AuthenticationChannelHandler can that set this principal, and the enclose the subsequent handler's method with a doAs block . We can then use a privilegedThreadFactory to propagate this principal to all asnyc calls. Any impersonation calls will have to propagate this principal to the new subjects it creates.

Note: whether we go this route or not for storing the user id, we will still need to use a privilegedThreadFactory to propagate the impersonation context to async calls.

Yaojie Feng
June 29, 2017, 9:06 PM
Yaojie Feng
June 29, 2017, 11:06 PM
Fixed

Assignee

Yaojie Feng

Reporter

Yaojie Feng

Labels

Docs Impact

None

UX Impact

None

Components

Fix versions

Affects versions

Priority

Blocker
Configure