Skip to content

Commit

Permalink
[8.15] fix(security, http): expose authentication headers in the auth…
Browse files Browse the repository at this point in the history
…entication result when HTTP authentication is used (#190998) (#191051)

# Backport

This will backport the following commits from `main` to `8.15`:
- [fix(security, http): expose authentication headers in the
authentication result when HTTP authentication is used
(#190998)](#190998)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Aleh
Zasypkin","email":"aleh.zasypkin@elastic.co"},"sourceCommit":{"committedDate":"2024-08-22T07:48:35Z","message":"fix(security,
http): expose authentication headers in the authentication result when
HTTP authentication is used (#190998)\n\n## Summary\r\n\r\nWhen Kibana
tries to authenticate a request that already has an\r\n`Authorization`
header (not a cookie, client certificate, or Kerberos\r\nticket), the
authentication is performed by the [HTTP
authentication\r\nprovider](https://www.elastic.co/guide/en/kibana/current/kibana-authentication.html#http-authentication).\r\n\r\nUnlike
session/Kerberos/PKI providers, this provider returns
an\r\nauthentication result that doesn't explicitly tell Core
which\r\nauthorization headers should be used (e.g., `t.authenticated({
state:\r\nauthenticationResult.user, -->
requestHeaders:\r\nauthenticationResult.authHeaders <-- ... });`),
assuming that Core will\r\njust use the headers from the request. The
`Authorization` header is\r\nforwarded to Elasticsearch by default, no
additional configuration is\r\nrequired.\r\n\r\nThis worked well
previously, but I think with the introduction of
the\r\nthe\r\n[`getSecondaryAuthHeaders`](https://github.com/elastic/kibana/pull/184901)\r\nmethod
this is the first time where this assumption doesn't
hold.\r\nInternally, this method tries to reuse authentication headers
that were\r\nprovided to Core by the authentication provider during the
request\r\nauthentication stage — headers that the HTTP authentication
provider\r\nnever provided before.\r\n\r\nThis PR makes the HTTP
authentication provider behave consistently with\r\nthe rest of the
providers we support
today.","sha":"3ac931fb796b2053c68b454804e643788dc598d9","branchLabelMapping":{"^v8.16.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Security","release_note:skip","backport:prev-minor","v8.16.0"],"title":"fix(security,
http): expose authentication headers in the authentication result when
HTTP authentication is
used","number":190998,"url":"https://github.com/elastic/kibana/pull/190998","mergeCommit":{"message":"fix(security,
http): expose authentication headers in the authentication result when
HTTP authentication is used (#190998)\n\n## Summary\r\n\r\nWhen Kibana
tries to authenticate a request that already has an\r\n`Authorization`
header (not a cookie, client certificate, or Kerberos\r\nticket), the
authentication is performed by the [HTTP
authentication\r\nprovider](https://www.elastic.co/guide/en/kibana/current/kibana-authentication.html#http-authentication).\r\n\r\nUnlike
session/Kerberos/PKI providers, this provider returns
an\r\nauthentication result that doesn't explicitly tell Core
which\r\nauthorization headers should be used (e.g., `t.authenticated({
state:\r\nauthenticationResult.user, -->
requestHeaders:\r\nauthenticationResult.authHeaders <-- ... });`),
assuming that Core will\r\njust use the headers from the request. The
`Authorization` header is\r\nforwarded to Elasticsearch by default, no
additional configuration is\r\nrequired.\r\n\r\nThis worked well
previously, but I think with the introduction of
the\r\nthe\r\n[`getSecondaryAuthHeaders`](https://github.com/elastic/kibana/pull/184901)\r\nmethod
this is the first time where this assumption doesn't
hold.\r\nInternally, this method tries to reuse authentication headers
that were\r\nprovided to Core by the authentication provider during the
request\r\nauthentication stage — headers that the HTTP authentication
provider\r\nnever provided before.\r\n\r\nThis PR makes the HTTP
authentication provider behave consistently with\r\nthe rest of the
providers we support
today.","sha":"3ac931fb796b2053c68b454804e643788dc598d9"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/190998","number":190998,"mergeCommit":{"message":"fix(security,
http): expose authentication headers in the authentication result when
HTTP authentication is used (#190998)\n\n## Summary\r\n\r\nWhen Kibana
tries to authenticate a request that already has an\r\n`Authorization`
header (not a cookie, client certificate, or Kerberos\r\nticket), the
authentication is performed by the [HTTP
authentication\r\nprovider](https://www.elastic.co/guide/en/kibana/current/kibana-authentication.html#http-authentication).\r\n\r\nUnlike
session/Kerberos/PKI providers, this provider returns
an\r\nauthentication result that doesn't explicitly tell Core
which\r\nauthorization headers should be used (e.g., `t.authenticated({
state:\r\nauthenticationResult.user, -->
requestHeaders:\r\nauthenticationResult.authHeaders <-- ... });`),
assuming that Core will\r\njust use the headers from the request. The
`Authorization` header is\r\nforwarded to Elasticsearch by default, no
additional configuration is\r\nrequired.\r\n\r\nThis worked well
previously, but I think with the introduction of
the\r\nthe\r\n[`getSecondaryAuthHeaders`](https://github.com/elastic/kibana/pull/184901)\r\nmethod
this is the first time where this assumption doesn't
hold.\r\nInternally, this method tries to reuse authentication headers
that were\r\nprovided to Core by the authentication provider during the
request\r\nauthentication stage — headers that the HTTP authentication
provider\r\nnever provided before.\r\n\r\nThis PR makes the HTTP
authentication provider behave consistently with\r\nthe rest of the
providers we support
today.","sha":"3ac931fb796b2053c68b454804e643788dc598d9"}}]}]
BACKPORT-->

Co-authored-by: Aleh Zasypkin <aleh.zasypkin@elastic.co>
  • Loading branch information
kibanamachine and azasypkin authored Aug 22, 2024
1 parent ff7d8ce commit bd96220
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,10 @@ describe('HTTPAuthenticationProvider', () => {
});

await expect(provider.authenticate(request)).resolves.toEqual(
AuthenticationResult.succeeded({
...user,
authentication_provider: { type: 'http', name: 'http' },
})
AuthenticationResult.succeeded(
{ ...user, authentication_provider: { type: 'http', name: 'http' } },
{ authHeaders: { authorization: header } }
)
);

expectAuthenticateCall(mockOptions.client, { headers: { authorization: header } });
Expand All @@ -160,10 +160,10 @@ describe('HTTPAuthenticationProvider', () => {
});

await expect(provider.authenticate(request)).resolves.toEqual(
AuthenticationResult.succeeded({
...user,
authentication_provider: { type: 'http', name: 'http' },
})
AuthenticationResult.succeeded(
{ ...user, authentication_provider: { type: 'http', name: 'http' } },
{ authHeaders: { authorization: header } }
)
);

expectAuthenticateCall(mockOptions.client, { headers: { authorization: header } });
Expand All @@ -187,10 +187,10 @@ describe('HTTPAuthenticationProvider', () => {
});

await expect(provider.authenticate(request)).resolves.toEqual(
AuthenticationResult.succeeded({
...user,
authentication_provider: { type: 'http', name: 'http' },
})
AuthenticationResult.succeeded(
{ ...user, authentication_provider: { type: 'http', name: 'http' } },
{ authHeaders: { authorization: header } }
)
);

expectAuthenticateCall(mockOptions.client, { headers: { authorization: header } });
Expand All @@ -217,10 +217,10 @@ describe('HTTPAuthenticationProvider', () => {
});

await expect(provider.authenticate(request)).resolves.toEqual(
AuthenticationResult.succeeded({
...user,
authentication_provider: { type: 'http', name: 'http' },
})
AuthenticationResult.succeeded(
{ ...user, authentication_provider: { type: 'http', name: 'http' } },
{ authHeaders: { authorization: header } }
)
);

expectAuthenticateCall(mockOptions.client, { headers: { authorization: header } });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,11 @@ export class HTTPAuthenticationProvider extends BaseAuthenticationProvider {
return AuthenticationResult.notHandled();
}

return AuthenticationResult.succeeded(user);
return AuthenticationResult.succeeded(user, {
// Even though the `Authorization` header is already present in the HTTP headers of the original request,
// we still need to expose it to the Core authentication service for consistency.
authHeaders: { authorization: authorizationHeader.toString() },
});
} catch (err) {
this.logger.debug(
`Failed to authenticate request to ${request.url.pathname} via authorization header with "${
Expand Down

0 comments on commit bd96220

Please sign in to comment.