Skip to content

Fix Kerberos Authentication failing requests without cookies and excludedPaths config. - #5596

Merged
b-slim merged 5 commits into
apache:masterfrom
nishantmonu51:kerberos-fix
Apr 10, 2018
Merged

b-slim merged 5 commits into
apache:masterfrom
nishantmonu51:kerberos-fix

Conversation

@nishantmonu51

@nishantmonu51 nishantmonu51 commented Apr 9, 2018 •

Copy link
Copy Markdown
Member

KerberosAuthenticator was failing First request from the clients.
After authentication we were setting the cookie properly but not
setting the the authenticated flag in the request. This PR fixes that.

Additional Fixes -

  1. Excluded paths config - It was not working as expected. Requests for excluded paths were still failing. Fixed this by adding authentication result with anonymous user.
  2. Removing of Unused SpnegoFilterConfig - replaced by
    KerberosAuthenticator
  3. removed unused internalClientKeytab and principal configs from KerberosAuthenticator
  4. Fix docs accordingly and add docs for configuring an escalated
    client.

Reproducing the Issue -

  • Configure Kerberos Security
  • Perform Kinit on client side and send an HTTP request to any druid endpoint
  • Druid returns unauthorized error with a set-cookie flag. ----- HERE druid should have returned a 200 OK
  • Using the cookie on subsequent requests works fine.

KerberosAuthenticator was failing `First` request from the clients.
After authentication we were setting the cookie properly but not
setting the the authenticated flag in the request. This PR fixed that.

Additional Fixes -
* Removing of Unused SpnegoFilterConfig - replaced by
KerberosAuthenticator
* Unused internalClientKeytab and principal from KerberosAuthenticator
* Fix docs accordingly and add docs for configuring an escalated
client.
@nishantmonu51

Copy link
Copy Markdown
Member Author

@jon-wei @b-slim Please review.

@nishantmonu51 nishantmonu51 changed the title Fix Kerberos Authentication failing requests without cookies. Fix Kerberos Authentication failing requests without cookies and excludedPaths config. Apr 9, 2018

String path = ((HttpServletRequest) request).getRequestURI();
if (isExcluded(path)) {
// For excluded paths that does NOT need to be authenticated.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there's an authorizer configured + the Kerberos authenticator, this won't be enough to skip security checks on a request (the authorizer would check what permissions the "anonymous" user would have and deny the request unless the user has configured appropriate permissions for the "anonymous" user)

To support excludedPaths, there's a similar concept with "unsecured paths" in the various service initialization files (e.g., UNSECURED_PATHS in CoordinatorJettyServerInitializer), one option could be to extend that to allow user-specified whitelist entries (i.e., make excludedPaths a general concept applied across all authenticators, and remove excludedPaths as a Kerberos-specific config).

I think excludedPaths makes more sense as a system-wide property than an authenticator-specific property, I'll suggest taking this out of this PR and one of us could do follow-on PR to make it a general config.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jon-wei: sure, we can make it a general config.
Will raise a separate PR for that.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jon-wei, created #5614

@gianm gianm added this to the 0.12.1 milestone Apr 9, 2018
@jon-wei

jon-wei commented Apr 9, 2018

Copy link
Copy Markdown
Contributor

👍 after CI

@b-slim b-slim self-assigned this Apr 9, 2018
@b-slim

b-slim commented Apr 9, 2018

Copy link
Copy Markdown
Contributor

👍

@b-slim
b-slim merged commit 80fa509 into apache:master Apr 10, 2018
nishantmonu51 added a commit to nishantmonu51/druid that referenced this pull request Apr 10, 2018
…udedPaths config. (apache#5596)

* Fix Kerberos Authentication failing requests without cookies.

KerberosAuthenticator was failing `First` request from the clients.
After authentication we were setting the cookie properly but not
setting the the authenticated flag in the request. This PR fixed that.

Additional Fixes -
* Removing of Unused SpnegoFilterConfig - replaced by
KerberosAuthenticator
* Unused internalClientKeytab and principal from KerberosAuthenticator
* Fix docs accordingly and add docs for configuring an escalated
client.

* Fix excluded path config behavior

* spelling correction

* Revert "spelling correction"

This reverts commit fb754b4.

* Revert "Fix excluded path config behavior"

This reverts commit 3901047.
fjy pushed a commit that referenced this pull request Apr 10, 2018
…udedPaths config. (#5596) (#5621)

* Fix Kerberos Authentication failing requests without cookies.

KerberosAuthenticator was failing `First` request from the clients.
After authentication we were setting the cookie properly but not
setting the the authenticated flag in the request. This PR fixed that.

Additional Fixes -
* Removing of Unused SpnegoFilterConfig - replaced by
KerberosAuthenticator
* Unused internalClientKeytab and principal from KerberosAuthenticator
* Fix docs accordingly and add docs for configuring an escalated
client.

* Fix excluded path config behavior

* spelling correction

* Revert "spelling correction"

This reverts commit fb754b4.

* Revert "Fix excluded path config behavior"

This reverts commit 3901047.
gianm pushed a commit to implydata/druid-public that referenced this pull request Apr 16, 2018
…udedPaths config. (apache#5596) (apache#5621)

* Fix Kerberos Authentication failing requests without cookies.

KerberosAuthenticator was failing `First` request from the clients.
After authentication we were setting the cookie properly but not
setting the the authenticated flag in the request. This PR fixed that.

Additional Fixes -
* Removing of Unused SpnegoFilterConfig - replaced by
KerberosAuthenticator
* Unused internalClientKeytab and principal from KerberosAuthenticator
* Fix docs accordingly and add docs for configuring an escalated
client.

* Fix excluded path config behavior

* spelling correction

* Revert "spelling correction"

This reverts commit fb754b4.

* Revert "Fix excluded path config behavior"

This reverts commit 3901047.
sathishsri88 pushed a commit to sathishs/druid that referenced this pull request May 8, 2018
…udedPaths config. (apache#5596)

* Fix Kerberos Authentication failing requests without cookies.

KerberosAuthenticator was failing `First` request from the clients.
After authentication we were setting the cookie properly but not
setting the the authenticated flag in the request. This PR fixed that.

Additional Fixes -
* Removing of Unused SpnegoFilterConfig - replaced by
KerberosAuthenticator
* Unused internalClientKeytab and principal from KerberosAuthenticator
* Fix docs accordingly and add docs for configuring an escalated
client.

* Fix excluded path config behavior

* spelling correction

* Revert "spelling correction"

This reverts commit fb754b4.

* Revert "Fix excluded path config behavior"

This reverts commit 3901047.
riovic918data pushed a commit to riovic918data/druid that referenced this pull request Jun 12, 2026
…udedPaths config. (apache#5596)

* Fix Kerberos Authentication failing requests without cookies.

KerberosAuthenticator was failing `First` request from the clients.
After authentication we were setting the cookie properly but not
setting the the authenticated flag in the request. This PR fixed that.

Additional Fixes -
* Removing of Unused SpnegoFilterConfig - replaced by
KerberosAuthenticator
* Unused internalClientKeytab and principal from KerberosAuthenticator
* Fix docs accordingly and add docs for configuring an escalated
client.

* Fix excluded path config behavior

* spelling correction

* Revert "spelling correction"

This reverts commit fb754b4.

* Revert "Fix excluded path config behavior"

This reverts commit 3901047.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants