Skip to content

Commit

Permalink
fix(security, http): expose authentication headers in the authenticat…
Browse files Browse the repository at this point in the history
…ion result when HTTP authentication is used (elastic#190998)

## Summary

When Kibana tries to authenticate a request that already has an
`Authorization` header (not a cookie, client certificate, or Kerberos
ticket), the authentication is performed by the [HTTP authentication
provider](https://www.elastic.co/guide/en/kibana/current/kibana-authentication.html#http-authentication).

Unlike session/Kerberos/PKI providers, this provider returns an
authentication result that doesn't explicitly tell Core which
authorization headers should be used (e.g., `t.authenticated({ state:
authenticationResult.user, --> requestHeaders:
authenticationResult.authHeaders <-- ... });`), assuming that Core will
just use the headers from the request. The `Authorization` header is
forwarded to Elasticsearch by default, no additional configuration is
required.

This worked well previously, but I think with the introduction of the
the
[`getSecondaryAuthHeaders`](elastic#184901)
method this is the first time where this assumption doesn't hold.
Internally, this method tries to reuse authentication headers that were
provided to Core by the authentication provider during the request
authentication stage — headers that the HTTP authentication provider
never provided before.

This PR makes the HTTP authentication provider behave consistently with
the rest of the providers we support today.
  • Loading branch information
azasypkin authored Aug 22, 2024
1 parent 29a45fc commit 3ac931f
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(
() =>
Expand Down

0 comments on commit 3ac931f

Please sign in to comment.