Skip to content

Commit 7827fa5

Browse files
azasypkinazasypkin
authored
[7.7] Allow IdP initiated SAML login with session containing expired token. (#64339)
1 parent 81df9ab commit 7827fa5

File tree

5 files changed

+387
-308
lines changed

5 files changed

+387
-308
lines changed

‎x-pack/plugins/security/server/authentication/providers/saml.test.ts‎

Lines changed: 181 additions & 166 deletions
Original file line numberDiff line numberDiff line change
@@ -396,193 +396,208 @@ describe('SAMLAuthenticationProvider', () => {
396396
});
397397
});
398398

399-
it('redirects to the home page if new SAML Response is for the same user.', async () => {
400-
const request = httpServerMock.createKibanaRequest({ headers: {} });
401-
const state = {
402-
username: 'user',
403-
accessToken: 'existing-valid-token',
404-
refreshToken: 'existing-valid-refresh-token',
405-
realm: 'test-realm',
406-
};
407-
408-
const user = { username: 'user', authentication_realm: { name: 'saml1' } };
409-
const mockScopedClusterClient = elasticsearchServiceMock.createScopedClusterClient();
410-
mockScopedClusterClient.callAsCurrentUser.mockResolvedValue(user);
411-
mockOptions.client.asScoped.mockReturnValue(mockScopedClusterClient);
399+
for (const [description, response] of [
400+
[
401+
'session is valid',
402+
Promise.resolve({ username: 'user', authentication_realm: { name: 'saml1' } }),
403+
],
404+
[
405+
'session is is expired',
406+
Promise.reject(ElasticsearchErrorHelpers.decorateNotAuthorizedError(new Error())),
407+
],
408+
] as Array<[string, Promise<any>]>) {
409+
it(`redirects to the home page if new SAML Response is for the same user if ${description}.`, async () => {
410+
const request = httpServerMock.createKibanaRequest({ headers: {} });
411+
const state = {
412+
username: 'user',
413+
accessToken: 'existing-token',
414+
refreshToken: 'existing-refresh-token',
415+
realm: 'test-realm',
416+
};
417+
const authorization = `Bearer ${state.accessToken}`;
412418

413-
mockOptions.client.callAsInternalUser.mockResolvedValue({
414-
username: 'user',
415-
access_token: 'new-valid-token',
416-
refresh_token: 'new-valid-refresh-token',
417-
realm: 'test-realm',
418-
});
419+
const mockScopedClusterClient = elasticsearchServiceMock.createScopedClusterClient();
420+
mockScopedClusterClient.callAsCurrentUser.mockImplementation(() => response);
421+
mockOptions.client.asScoped.mockReturnValue(mockScopedClusterClient);
419422

420-
mockOptions.tokens.invalidate.mockResolvedValue(undefined);
423+
mockOptions.client.callAsInternalUser.mockResolvedValue({
424+
username: 'user',
425+
access_token: 'new-valid-token',
426+
refresh_token: 'new-valid-refresh-token',
427+
realm: 'test-realm',
428+
});
429+
430+
mockOptions.tokens.invalidate.mockResolvedValue(undefined);
431+
432+
await expect(
433+
provider.login(
434+
request,
435+
{ type: SAMLLogin.LoginWithSAMLResponse, samlResponse: 'saml-response-xml' },
436+
state
437+
)
438+
).resolves.toEqual(
439+
AuthenticationResult.redirectTo('/base-path/', {
440+
state: {
441+
username: 'user',
442+
accessToken: 'new-valid-token',
443+
refreshToken: 'new-valid-refresh-token',
444+
realm: 'test-realm',
445+
},
446+
})
447+
);
421448

422-
await expect(
423-
provider.login(
424-
request,
425-
{ type: SAMLLogin.LoginWithSAMLResponse, samlResponse: 'saml-response-xml' },
426-
state
427-
)
428-
).resolves.toEqual(
429-
AuthenticationResult.redirectTo('/base-path/', {
430-
state: {
431-
username: 'user',
432-
accessToken: 'new-valid-token',
433-
refreshToken: 'new-valid-refresh-token',
434-
realm: 'test-realm',
435-
},
436-
})
437-
);
449+
expectAuthenticateCall(mockOptions.client, { headers: { authorization } });
438450

439-
expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledWith(
440-
'shield.samlAuthenticate',
441-
{
442-
body: { ids: [], content: 'saml-response-xml' },
443-
}
444-
);
451+
expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledWith(
452+
'shield.samlAuthenticate',
453+
{
454+
body: { ids: [], content: 'saml-response-xml' },
455+
}
456+
);
445457

446-
expect(mockOptions.tokens.invalidate).toHaveBeenCalledTimes(1);
447-
expect(mockOptions.tokens.invalidate).toHaveBeenCalledWith({
448-
accessToken: state.accessToken,
449-
refreshToken: state.refreshToken,
458+
expect(mockOptions.tokens.invalidate).toHaveBeenCalledTimes(1);
459+
expect(mockOptions.tokens.invalidate).toHaveBeenCalledWith({
460+
accessToken: state.accessToken,
461+
refreshToken: state.refreshToken,
462+
});
450463
});
451-
});
452464

453-
it('redirects to `overwritten_session` if new SAML Response is for the another user.', async () => {
454-
const request = httpServerMock.createKibanaRequest({ headers: {} });
455-
const state = {
456-
username: 'user',
457-
accessToken: 'existing-valid-token',
458-
refreshToken: 'existing-valid-refresh-token',
459-
realm: 'test-realm',
460-
};
461-
462-
const existingUser = { username: 'user', authentication_realm: { name: 'saml1' } };
463-
const newUser = { username: 'new-user', authentication_realm: { name: 'saml1' } };
464-
mockOptions.client.asScoped.mockImplementation(scopeableRequest => {
465-
if (scopeableRequest?.headers.authorization === `Bearer ${state.accessToken}`) {
466-
const mockScopedClusterClient = elasticsearchServiceMock.createScopedClusterClient();
467-
mockScopedClusterClient.callAsCurrentUser.mockResolvedValue(existingUser);
468-
return mockScopedClusterClient;
469-
}
465+
it(`redirects to \`overwritten_session\` if new SAML Response is for the another user if ${description}.`, async () => {
466+
const request = httpServerMock.createKibanaRequest({ headers: {} });
467+
const state = {
468+
username: 'user',
469+
accessToken: 'existing-token',
470+
refreshToken: 'existing-refresh-token',
471+
realm: 'test-realm',
472+
};
473+
const authorization = `Bearer ${state.accessToken}`;
474+
475+
const newUser = { username: 'new-user', authentication_realm: { name: 'saml1' } };
476+
mockOptions.client.asScoped.mockImplementation(scopeableRequest => {
477+
if (scopeableRequest?.headers.authorization === `Bearer ${state.accessToken}`) {
478+
const mockScopedClusterClient = elasticsearchServiceMock.createScopedClusterClient();
479+
mockScopedClusterClient.callAsCurrentUser.mockImplementation(() => response);
480+
return mockScopedClusterClient;
481+
}
470482

471-
if (scopeableRequest?.headers.authorization === 'Bearer new-valid-token') {
472-
const mockScopedClusterClient = elasticsearchServiceMock.createScopedClusterClient();
473-
mockScopedClusterClient.callAsCurrentUser.mockRejectedValue(newUser);
474-
return mockScopedClusterClient;
475-
}
483+
if (scopeableRequest?.headers.authorization === 'Bearer new-valid-token') {
484+
const mockScopedClusterClient = elasticsearchServiceMock.createScopedClusterClient();
485+
mockScopedClusterClient.callAsCurrentUser.mockRejectedValue(newUser);
486+
return mockScopedClusterClient;
487+
}
476488

477-
throw new Error('Unexpected call');
478-
});
489+
throw new Error('Unexpected call');
490+
});
479491

480-
mockOptions.client.callAsInternalUser.mockResolvedValue({
481-
username: 'new-user',
482-
access_token: 'new-valid-token',
483-
refresh_token: 'new-valid-refresh-token',
484-
realm: 'test-realm',
485-
});
486-
487-
mockOptions.tokens.invalidate.mockResolvedValue(undefined);
492+
mockOptions.client.callAsInternalUser.mockResolvedValue({
493+
username: 'new-user',
494+
access_token: 'new-valid-token',
495+
refresh_token: 'new-valid-refresh-token',
496+
realm: 'test-realm',
497+
});
498+
499+
mockOptions.tokens.invalidate.mockResolvedValue(undefined);
500+
501+
await expect(
502+
provider.login(
503+
request,
504+
{ type: SAMLLogin.LoginWithSAMLResponse, samlResponse: 'saml-response-xml' },
505+
state
506+
)
507+
).resolves.toEqual(
508+
AuthenticationResult.redirectTo('/mock-server-basepath/security/overwritten_session', {
509+
state: {
510+
username: 'new-user',
511+
accessToken: 'new-valid-token',
512+
refreshToken: 'new-valid-refresh-token',
513+
realm: 'test-realm',
514+
},
515+
})
516+
);
488517

489-
await expect(
490-
provider.login(
491-
request,
492-
{ type: SAMLLogin.LoginWithSAMLResponse, samlResponse: 'saml-response-xml' },
493-
state
494-
)
495-
).resolves.toEqual(
496-
AuthenticationResult.redirectTo('/mock-server-basepath/security/overwritten_session', {
497-
state: {
498-
username: 'new-user',
499-
accessToken: 'new-valid-token',
500-
refreshToken: 'new-valid-refresh-token',
501-
realm: 'test-realm',
502-
},
503-
})
504-
);
518+
expectAuthenticateCall(mockOptions.client, { headers: { authorization } });
505519

506-
expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledWith(
507-
'shield.samlAuthenticate',
508-
{
509-
body: { ids: [], content: 'saml-response-xml' },
510-
}
511-
);
520+
expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledWith(
521+
'shield.samlAuthenticate',
522+
{
523+
body: { ids: [], content: 'saml-response-xml' },
524+
}
525+
);
512526

513-
expect(mockOptions.tokens.invalidate).toHaveBeenCalledTimes(1);
514-
expect(mockOptions.tokens.invalidate).toHaveBeenCalledWith({
515-
accessToken: state.accessToken,
516-
refreshToken: state.refreshToken,
527+
expect(mockOptions.tokens.invalidate).toHaveBeenCalledTimes(1);
528+
expect(mockOptions.tokens.invalidate).toHaveBeenCalledWith({
529+
accessToken: state.accessToken,
530+
refreshToken: state.refreshToken,
531+
});
517532
});
518-
});
519533

520-
it('redirects to `overwritten_session` if new SAML Response is for another realm.', async () => {
521-
const request = httpServerMock.createKibanaRequest();
522-
const state = {
523-
username: 'user',
524-
accessToken: 'existing-valid-token',
525-
refreshToken: 'existing-valid-refresh-token',
526-
realm: 'saml1',
527-
};
528-
529-
const existingUser = { username: 'user', authentication_realm: { name: 'saml1' } };
530-
const newUser = { username: 'user', authentication_realm: { name: 'saml2' } };
531-
mockOptions.client.asScoped.mockImplementation(scopeableRequest => {
532-
if (scopeableRequest?.headers.authorization === `Bearer ${state.accessToken}`) {
533-
const mockScopedClusterClient = elasticsearchServiceMock.createScopedClusterClient();
534-
mockScopedClusterClient.callAsCurrentUser.mockResolvedValue(existingUser);
535-
return mockScopedClusterClient;
536-
}
537-
538-
if (scopeableRequest?.headers.authorization === 'Bearer new-valid-token') {
539-
const mockScopedClusterClient = elasticsearchServiceMock.createScopedClusterClient();
540-
mockScopedClusterClient.callAsCurrentUser.mockResolvedValue(newUser);
541-
return mockScopedClusterClient;
542-
}
543-
544-
throw new Error('Unexpected call');
545-
});
534+
it(`redirects to \`overwritten_session\` if new SAML Response is for another realm if ${description}.`, async () => {
535+
const request = httpServerMock.createKibanaRequest();
536+
const state = {
537+
username: 'user',
538+
accessToken: 'existing-valid-token',
539+
refreshToken: 'existing-valid-refresh-token',
540+
realm: 'saml1',
541+
};
542+
543+
const existingUser = { username: 'user', authentication_realm: { name: 'saml1' } };
544+
const newUser = { username: 'user', authentication_realm: { name: 'saml2' } };
545+
mockOptions.client.asScoped.mockImplementation(scopeableRequest => {
546+
if (scopeableRequest?.headers.authorization === `Bearer ${state.accessToken}`) {
547+
const mockScopedClusterClient = elasticsearchServiceMock.createScopedClusterClient();
548+
mockScopedClusterClient.callAsCurrentUser.mockResolvedValue(existingUser);
549+
return mockScopedClusterClient;
550+
}
546551

547-
mockOptions.client.callAsInternalUser.mockResolvedValue({
548-
username: 'user',
549-
access_token: 'new-valid-token',
550-
refresh_token: 'new-valid-refresh-token',
551-
realm: 'saml2',
552-
});
552+
if (scopeableRequest?.headers.authorization === 'Bearer new-valid-token') {
553+
const mockScopedClusterClient = elasticsearchServiceMock.createScopedClusterClient();
554+
mockScopedClusterClient.callAsCurrentUser.mockResolvedValue(newUser);
555+
return mockScopedClusterClient;
556+
}
553557

554-
mockOptions.tokens.invalidate.mockResolvedValue(undefined);
558+
throw new Error('Unexpected call');
559+
});
555560

556-
await expect(
557-
provider.login(
558-
request,
559-
{ type: SAMLLogin.LoginWithSAMLResponse, samlResponse: 'saml-response-xml' },
560-
state
561-
)
562-
).resolves.toEqual(
563-
AuthenticationResult.redirectTo('/mock-server-basepath/security/overwritten_session', {
564-
state: {
565-
username: 'user',
566-
accessToken: 'new-valid-token',
567-
refreshToken: 'new-valid-refresh-token',
568-
realm: 'saml2',
569-
},
570-
})
571-
);
561+
mockOptions.client.callAsInternalUser.mockResolvedValue({
562+
username: 'user',
563+
access_token: 'new-valid-token',
564+
refresh_token: 'new-valid-refresh-token',
565+
realm: 'saml2',
566+
});
567+
568+
mockOptions.tokens.invalidate.mockResolvedValue(undefined);
569+
570+
await expect(
571+
provider.login(
572+
request,
573+
{ type: SAMLLogin.LoginWithSAMLResponse, samlResponse: 'saml-response-xml' },
574+
state
575+
)
576+
).resolves.toEqual(
577+
AuthenticationResult.redirectTo('/mock-server-basepath/security/overwritten_session', {
578+
state: {
579+
username: 'user',
580+
accessToken: 'new-valid-token',
581+
refreshToken: 'new-valid-refresh-token',
582+
realm: 'saml2',
583+
},
584+
})
585+
);
572586

573-
expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledWith(
574-
'shield.samlAuthenticate',
575-
{
576-
body: { ids: [], content: 'saml-response-xml' },
577-
}
578-
);
587+
expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledWith(
588+
'shield.samlAuthenticate',
589+
{
590+
body: { ids: [], content: 'saml-response-xml' },
591+
}
592+
);
579593

580-
expect(mockOptions.tokens.invalidate).toHaveBeenCalledTimes(1);
581-
expect(mockOptions.tokens.invalidate).toHaveBeenCalledWith({
582-
accessToken: state.accessToken,
583-
refreshToken: state.refreshToken,
594+
expect(mockOptions.tokens.invalidate).toHaveBeenCalledTimes(1);
595+
expect(mockOptions.tokens.invalidate).toHaveBeenCalledWith({
596+
accessToken: state.accessToken,
597+
refreshToken: state.refreshToken,
598+
});
584599
});
585-
});
600+
}
586601
});
587602

588603
describe('User initiated login with captured redirect URL', () => {

‎x-pack/plugins/security/server/authentication/providers/saml.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -156,10 +156,14 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider {
156156
return await this.loginWithSAMLResponse(request, samlResponse, state);
157157
}
158158

159-
if (authenticationResult.succeeded()) {
160-
// If user has been authenticated via session, but request also includes SAML payload
161-
// we should check whether this payload is for the exactly same user and if not
162-
// we'll re-authenticate user and forward to a page with the respective warning.
159+
// If user has been authenticated via session or failed to do so because of expired access token,
160+
// but request also includes SAML payload we should check whether this payload is for the exactly
161+
// same user and if not we'll re-authenticate user and forward to a page with the respective warning.
162+
if (
163+
authenticationResult.succeeded() ||
164+
(authenticationResult.failed() &&
165+
Tokens.isAccessTokenExpiredError(authenticationResult.error))
166+
) {
163167
return await this.loginWithNewSAMLResponse(
164168
request,
165169
samlResponse,

0 commit comments

Comments
 (0)