Skip to content

[Enterprise Search] Fix users being logged out of Kibana if Enterprise Search returns a 401 #80757

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 16, 2020
Merged

[Enterprise Search] Fix users being logged out of Kibana if Enterprise Search returns a 401 #80757

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions x-pack/plugins/enterprise_search/server/lib/enterprise_search_request_handler.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,10 @@ describe('EnterpriseSearchRequestHandler', () => {
expect(mockLogger.error).toHaveBeenCalled();
});

it('errors when receiving a 401 response', async () => {
EnterpriseSearchAPI.mockReturn({}, { status: 401 });
});

it('errors when redirected to /login', async () => {
EnterpriseSearchAPI.mockReturn({}, { url: 'http://localhost:3002/login' });
});
Expand Down
12 changes: 10 additions & 2 deletions x-pack/plugins/enterprise_search/server/lib/enterprise_search_request_handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,12 @@ export class EnterpriseSearchRequestHandler {
// Handle response headers
this.setResponseHeaders(apiResponse);

// Handle authentication redirects
if (apiResponse.url.endsWith('/login') || apiResponse.url.endsWith('/ent/select')) {
// Handle unauthenticated users / authentication redirects
if (
apiResponse.status === 401 ||
Copy link
Contributor

@scottybollinger scottybollinger Oct 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Since this is destructured above, we can get rid of apiResponse. here. I'd also submit that you can also destructure url and clan this up a bit below

EDIT: I see that url is already declared in scope. Perhaps:

const { status, url: responseUrl } = apiResponse;
...
responseUrl.endsWith('/login') ||
responseUrl.endsWith('/ent/select')

... but that doesn't really help much

apiResponse.url.endsWith('/login') ||
apiResponse.url.endsWith('/ent/select')
Comment on lines +90 to +91
Copy link
Contributor Author

@cee-chen cee-chen Oct 15, 2020

Choose a reason for hiding this comment

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

@orhantoy Let me know if you think we can remove redirect checks going forward - I might leave it in for now just in case users are somehow using mismatched Enterprise Search / Kibana versions, but would be good to potentially clean up down the road

) {
return this.handleAuthenticationError(response);
}

Expand Down Expand Up @@ -213,6 +217,10 @@ export class EnterpriseSearchRequestHandler {
return response.customError({ statusCode: 502, headers: this.headers, body: errorMessage });
}

/**
* Note: Kibana auto logs users out when it receives a 401 response, so we want to catch and
* return 401 responses from Enterprise Search as a 502 so Kibana sessions aren't interrupted
*/
handleAuthenticationError(response: KibanaResponseFactory) {
const errorMessage = 'Cannot authenticate Enterprise Search user';

Expand Down