maxmustermann/kibana
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Allow IdP initiated SAML login with session containing expired token. (#59686)

Browse source
This commit is contained in:
Aleh Zasypkin 2020-04-23 19:15:45 +02:00 committed by GitHub
parent 09c2727d78
commit 95ac47d3d2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 299 additions and 229 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

210
x-pack/plugins/security/server/authentication/providers/saml.test.ts
View file

@ -315,117 +315,123 @@ describe('SAMLAuthenticationProvider', () => {
});
});
it('redirects to the home page if new SAML Response is for the same user.', async () => {
const request = httpServerMock.createKibanaRequest({ headers: {} });
const state = {
username: 'user',
accessToken: 'existing-valid-token',
refreshToken: 'existing-valid-refresh-token',
realm: 'test-realm',
};
const authorization = `Bearer ${state.accessToken}`;
for (const [description, response] of [
['session is valid', Promise.resolve({ username: 'user' })],
[
'session is is expired',
Promise.reject(ElasticsearchErrorHelpers.decorateNotAuthorizedError(new Error())),
],
] as Array<[string, Promise<any>]>) {
it(`redirects to the home page if new SAML Response is for the same user if ${description}.`, async () => {
const request = httpServerMock.createKibanaRequest({ headers: {} });
const state = {
username: 'user',
accessToken: 'existing-token',
refreshToken: 'existing-refresh-token',
realm: 'test-realm',
};
const authorization = `Bearer ${state.accessToken}`;
const user = { username: 'user' };
const mockScopedClusterClient = elasticsearchServiceMock.createScopedClusterClient();
mockScopedClusterClient.callAsCurrentUser.mockResolvedValue(user);
mockOptions.client.asScoped.mockReturnValue(mockScopedClusterClient);
const mockScopedClusterClient = elasticsearchServiceMock.createScopedClusterClient();
mockScopedClusterClient.callAsCurrentUser.mockImplementation(() => response);
mockOptions.client.asScoped.mockReturnValue(mockScopedClusterClient);
mockOptions.client.callAsInternalUser.mockResolvedValue({
username: 'user',
access_token: 'new-valid-token',
refresh_token: 'new-valid-refresh-token',
mockOptions.client.callAsInternalUser.mockResolvedValue({
username: 'user',
access_token: 'new-valid-token',
refresh_token: 'new-valid-refresh-token',
});
mockOptions.tokens.invalidate.mockResolvedValue(undefined);
await expect(
provider.login(
request,
{ type: SAMLLogin.LoginWithSAMLResponse, samlResponse: 'saml-response-xml' },
state
)
).resolves.toEqual(
AuthenticationResult.redirectTo('/base-path/', {
state: {
username: 'user',
accessToken: 'new-valid-token',
refreshToken: 'new-valid-refresh-token',
realm: 'test-realm',
},
})
);
expectAuthenticateCall(mockOptions.client, { headers: { authorization } });
expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledWith(
'shield.samlAuthenticate',
{
body: { ids: [], content: 'saml-response-xml', realm: 'test-realm' },
}
);
expect(mockOptions.tokens.invalidate).toHaveBeenCalledTimes(1);
expect(mockOptions.tokens.invalidate).toHaveBeenCalledWith({
accessToken: state.accessToken,
refreshToken: state.refreshToken,
});
});
mockOptions.tokens.invalidate.mockResolvedValue(undefined);
it(`redirects to \`overwritten_session\` if new SAML Response is for the another user if ${description}.`, async () => {
const request = httpServerMock.createKibanaRequest({ headers: {} });
const state = {
username: 'user',
accessToken: 'existing-token',
refreshToken: 'existing-refresh-token',
realm: 'test-realm',
};
const authorization = `Bearer ${state.accessToken}`;
await expect(
provider.login(
request,
{ type: SAMLLogin.LoginWithSAMLResponse, samlResponse: 'saml-response-xml' },
state
)
).resolves.toEqual(
AuthenticationResult.redirectTo('/base-path/', {
state: {
username: 'user',
accessToken: 'new-valid-token',
refreshToken: 'new-valid-refresh-token',
realm: 'test-realm',
},
})
);
const mockScopedClusterClient = elasticsearchServiceMock.createScopedClusterClient();
mockScopedClusterClient.callAsCurrentUser.mockImplementation(() => response);
mockOptions.client.asScoped.mockReturnValue(mockScopedClusterClient);
expectAuthenticateCall(mockOptions.client, { headers: { authorization } });
mockOptions.client.callAsInternalUser.mockResolvedValue({
username: 'new-user',
access_token: 'new-valid-token',
refresh_token: 'new-valid-refresh-token',
});
expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledWith(
'shield.samlAuthenticate',
{
body: { ids: [], content: 'saml-response-xml', realm: 'test-realm' },
}
);
mockOptions.tokens.invalidate.mockResolvedValue(undefined);
expect(mockOptions.tokens.invalidate).toHaveBeenCalledTimes(1);
expect(mockOptions.tokens.invalidate).toHaveBeenCalledWith({
accessToken: state.accessToken,
refreshToken: state.refreshToken,
await expect(
provider.login(
request,
{ type: SAMLLogin.LoginWithSAMLResponse, samlResponse: 'saml-response-xml' },
state
)
).resolves.toEqual(
AuthenticationResult.redirectTo('/mock-server-basepath/security/overwritten_session', {
state: {
username: 'new-user',
accessToken: 'new-valid-token',
refreshToken: 'new-valid-refresh-token',
realm: 'test-realm',
},
})
);
expectAuthenticateCall(mockOptions.client, { headers: { authorization } });
expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledWith(
'shield.samlAuthenticate',
{
body: { ids: [], content: 'saml-response-xml', realm: 'test-realm' },
}
);
expect(mockOptions.tokens.invalidate).toHaveBeenCalledTimes(1);
expect(mockOptions.tokens.invalidate).toHaveBeenCalledWith({
accessToken: state.accessToken,
refreshToken: state.refreshToken,
});
});
});
it('redirects to `overwritten_session` if new SAML Response is for the another user.', async () => {
const request = httpServerMock.createKibanaRequest({ headers: {} });
const state = {
username: 'user',
accessToken: 'existing-valid-token',
refreshToken: 'existing-valid-refresh-token',
realm: 'test-realm',
};
const authorization = `Bearer ${state.accessToken}`;
const existingUser = { username: 'user' };
const mockScopedClusterClient = elasticsearchServiceMock.createScopedClusterClient();
mockScopedClusterClient.callAsCurrentUser.mockResolvedValue(existingUser);
mockOptions.client.asScoped.mockReturnValue(mockScopedClusterClient);
mockOptions.client.callAsInternalUser.mockResolvedValue({
username: 'new-user',
access_token: 'new-valid-token',
refresh_token: 'new-valid-refresh-token',
});
mockOptions.tokens.invalidate.mockResolvedValue(undefined);
await expect(
provider.login(
request,
{ type: SAMLLogin.LoginWithSAMLResponse, samlResponse: 'saml-response-xml' },
state
)
).resolves.toEqual(
AuthenticationResult.redirectTo('/mock-server-basepath/security/overwritten_session', {
state: {
username: 'new-user',
accessToken: 'new-valid-token',
refreshToken: 'new-valid-refresh-token',
realm: 'test-realm',
},
})
);
expectAuthenticateCall(mockOptions.client, { headers: { authorization } });
expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledWith(
'shield.samlAuthenticate',
{
body: { ids: [], content: 'saml-response-xml', realm: 'test-realm' },
}
);
expect(mockOptions.tokens.invalidate).toHaveBeenCalledTimes(1);
expect(mockOptions.tokens.invalidate).toHaveBeenCalledWith({
accessToken: state.accessToken,
refreshToken: state.refreshToken,
});
});
}
});
describe('User initiated login with captured redirect URL', () => {

12
x-pack/plugins/security/server/authentication/providers/saml.ts
View file

@ -158,10 +158,14 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider {
return await this.loginWithSAMLResponse(request, samlResponse, state);
}
if (authenticationResult.succeeded()) {
// If user has been authenticated via session, but request also includes SAML payload
// we should check whether this payload is for the exactly same user and if not
// we'll re-authenticate user and forward to a page with the respective warning.
// If user has been authenticated via session or failed to do so because of expired access token,
// but request also includes SAML payload we should check whether this payload is for the exactly
// same user and if not we'll re-authenticate user and forward to a page with the respective warning.
if (
authenticationResult.succeeded() ||
(authenticationResult.failed() &&
Tokens.isAccessTokenExpiredError(authenticationResult.error))
) {
return await this.loginWithNewSAMLResponse(
request,
samlResponse,

133
x-pack/plugins/security/server/authentication/tokens.test.ts
View file

@ -25,7 +25,7 @@ describe('Tokens', () => {
tokens = new Tokens(tokensOptions);
});
it('isAccessTokenExpiredError() returns `true` only if token expired or its document is missing', () => {
it('isAccessTokenExpiredError() returns `true` only if token expired', () => {
const nonExpirationErrors = [
{},
new Error(),
@ -91,55 +91,66 @@ describe('Tokens', () => {
});
describe('invalidate()', () => {
it('throws if call to delete access token responds with an error', async () => {
const tokenPair = { accessToken: 'foo', refreshToken: 'bar' };
for (const [description, failureReason] of [
['an unknown error', new Error('failed to delete token')],
['a 404 error without body', { statusCode: 404 }],
] as Array<[string, object]>) {
it(`throws if call to delete access token responds with ${description}`, async () => {
const tokenPair = { accessToken: 'foo', refreshToken: 'bar' };
const failureReason = new Error('failed to delete token');
mockClusterClient.callAsInternalUser.mockImplementation((methodName, args: any) => {
if (args && args.body && args.body.token) {
return Promise.reject(failureReason);
}
mockClusterClient.callAsInternalUser.mockImplementation((methodName, args: any) => {
if (args && args.body && args.body.token) {
return Promise.reject(failureReason);
}
return Promise.resolve({ invalidated_tokens: 1 });
return Promise.resolve({ invalidated_tokens: 1 });
});
await expect(tokens.invalidate(tokenPair)).rejects.toBe(failureReason);
expect(mockClusterClient.callAsInternalUser).toHaveBeenCalledTimes(2);
expect(mockClusterClient.callAsInternalUser).toHaveBeenCalledWith(
'shield.deleteAccessToken',
{
body: { token: tokenPair.accessToken },
}
);
expect(mockClusterClient.callAsInternalUser).toHaveBeenCalledWith(
'shield.deleteAccessToken',
{
body: { refresh_token: tokenPair.refreshToken },
}
);
});
await expect(tokens.invalidate(tokenPair)).rejects.toBe(failureReason);
it(`throws if call to delete refresh token responds with ${description}`, async () => {
const tokenPair = { accessToken: 'foo', refreshToken: 'bar' };
expect(mockClusterClient.callAsInternalUser).toHaveBeenCalledTimes(2);
expect(mockClusterClient.callAsInternalUser).toHaveBeenCalledWith(
'shield.deleteAccessToken',
{ body: { token: tokenPair.accessToken } }
);
expect(mockClusterClient.callAsInternalUser).toHaveBeenCalledWith(
'shield.deleteAccessToken',
{ body: { refresh_token: tokenPair.refreshToken } }
);
});
mockClusterClient.callAsInternalUser.mockImplementation((methodName, args: any) => {
if (args && args.body && args.body.refresh_token) {
return Promise.reject(failureReason);
}
it('throws if call to delete refresh token responds with an error', async () => {
const tokenPair = { accessToken: 'foo', refreshToken: 'bar' };
return Promise.resolve({ invalidated_tokens: 1 });
});
const failureReason = new Error('failed to delete token');
mockClusterClient.callAsInternalUser.mockImplementation((methodName, args: any) => {
if (args && args.body && args.body.refresh_token) {
return Promise.reject(failureReason);
}
await expect(tokens.invalidate(tokenPair)).rejects.toBe(failureReason);
return Promise.resolve({ invalidated_tokens: 1 });
expect(mockClusterClient.callAsInternalUser).toHaveBeenCalledTimes(2);
expect(mockClusterClient.callAsInternalUser).toHaveBeenCalledWith(
'shield.deleteAccessToken',
{
body: { token: tokenPair.accessToken },
}
);
expect(mockClusterClient.callAsInternalUser).toHaveBeenCalledWith(
'shield.deleteAccessToken',
{
body: { refresh_token: tokenPair.refreshToken },
}
);
});
await expect(tokens.invalidate(tokenPair)).rejects.toBe(failureReason);
expect(mockClusterClient.callAsInternalUser).toHaveBeenCalledTimes(2);
expect(mockClusterClient.callAsInternalUser).toHaveBeenCalledWith(
'shield.deleteAccessToken',
{ body: { token: tokenPair.accessToken } }
);
expect(mockClusterClient.callAsInternalUser).toHaveBeenCalledWith(
'shield.deleteAccessToken',
{ body: { refresh_token: tokenPair.refreshToken } }
);
});
}
it('invalidates all provided tokens', async () => {
const tokenPair = { accessToken: 'foo', refreshToken: 'bar' };
@ -187,23 +198,35 @@ describe('Tokens', () => {
);
});
it('does not fail if none of the tokens were invalidated', async () => {
const tokenPair = { accessToken: 'foo', refreshToken: 'bar' };
for (const [description, response] of [
['none of the tokens were invalidated', Promise.resolve({ invalidated_tokens: 0 })],
[
'404 error is returned',
Promise.reject({ statusCode: 404, body: { invalidated_tokens: 0 } }),
],
] as Array<[string, Promise<any>]>) {
it(`does not fail if ${description}`, async () => {
const tokenPair = { accessToken: 'foo', refreshToken: 'bar' };
mockClusterClient.callAsInternalUser.mockResolvedValue({ invalidated_tokens: 0 });
mockClusterClient.callAsInternalUser.mockImplementation(() => response);
await expect(tokens.invalidate(tokenPair)).resolves.toBe(undefined);
await expect(tokens.invalidate(tokenPair)).resolves.toBe(undefined);
expect(mockClusterClient.callAsInternalUser).toHaveBeenCalledTimes(2);
expect(mockClusterClient.callAsInternalUser).toHaveBeenCalledWith(
'shield.deleteAccessToken',
{ body: { token: tokenPair.accessToken } }
);
expect(mockClusterClient.callAsInternalUser).toHaveBeenCalledWith(
'shield.deleteAccessToken',
{ body: { refresh_token: tokenPair.refreshToken } }
);
});
expect(mockClusterClient.callAsInternalUser).toHaveBeenCalledTimes(2);
expect(mockClusterClient.callAsInternalUser).toHaveBeenCalledWith(
'shield.deleteAccessToken',
{
body: { token: tokenPair.accessToken },
}
);
expect(mockClusterClient.callAsInternalUser).toHaveBeenCalledWith(
'shield.deleteAccessToken',
{
body: { refresh_token: tokenPair.refreshToken },
}
);
});
}
it('does not fail if more than one token per access or refresh token were invalidated', async () => {
const tokenPair = { accessToken: 'foo', refreshToken: 'bar' };

20
x-pack/plugins/security/server/authentication/tokens.ts
View file

@ -103,8 +103,15 @@ export class Tokens {
).invalidated_tokens;
} catch (err) {
this.logger.debug(`Failed to invalidate refresh token: ${err.message}`);
// We don't re-throw the error here to have a chance to invalidate access token if it's provided.
invalidationError = err;
// When using already deleted refresh token, Elasticsearch responds with 404 and a body that
// shows that no tokens were invalidated.
if (getErrorStatusCode(err) === 404 && err.body?.invalidated_tokens === 0) {
invalidatedTokensCount = err.body.invalidated_tokens;
} else {
// We don't re-throw the error here to have a chance to invalidate access token if it's provided.
invalidationError = err;
}
}
if (invalidatedTokensCount === 0) {
@ -128,7 +135,14 @@ export class Tokens {
).invalidated_tokens;
} catch (err) {
this.logger.debug(`Failed to invalidate access token: ${err.message}`);
invalidationError = err;
// When using already deleted access token, Elasticsearch responds with 404 and a body that
// shows that no tokens were invalidated.
if (getErrorStatusCode(err) === 404 && err.body?.invalidated_tokens === 0) {
invalidatedTokensCount = err.body.invalidated_tokens;
} else {
invalidationError = err;
}
}
if (invalidatedTokensCount === 0) {

153
x-pack/test/saml_api_integration/apis/security/saml_login.ts
View file

@ -35,7 +35,7 @@ export default function({ getService }: FtrProviderContext) {
});
}
async function checkSessionCookie(sessionCookie: Cookie) {
async function checkSessionCookie(sessionCookie: Cookie, username = 'a@b.c') {
expect(sessionCookie.key).to.be('sid');
expect(sessionCookie.value).to.not.be.empty();
expect(sessionCookie.path).to.be('/');
@ -59,7 +59,7 @@ export default function({ getService }: FtrProviderContext) {
'authentication_provider',
]);
expect(apiResponse.body.username).to.be('a@b.c');
expect(apiResponse.body.username).to.be(username);
}
describe('SAML authentication', () => {
@ -668,6 +668,29 @@ export default function({ getService }: FtrProviderContext) {
const existingUsername = 'a@b.c';
let existingSessionCookie: Cookie;
const testScenarios: Array<[string, () => Promise<void>]> = [
// Default scenario when active cookie has an active access token.
['when access token is valid', async () => {}],
// Scenario when active cookie has an expired access token. Access token expiration is set
// to 15s for API integration tests so we need to wait for 20s to make sure token expires.
['when access token is expired', async () => await delay(20000)],
// Scenario when active cookie references to access/refresh token pair that were already
// removed from Elasticsearch (to simulate 24h when expired tokens are removed).
[
'when access token document is missing',
async () => {
const esResponse = await getService('legacyEs').deleteByQuery({
index: '.security-tokens',
q: 'doc_type:token',
refresh: true,
});
expect(esResponse)
.to.have.property('deleted')
.greaterThan(0);
},
],
];
beforeEach(async () => {
const captureURLResponse = await supertest
.get('/abc/xyz/handshake?one=two three')
@ -701,76 +724,76 @@ export default function({ getService }: FtrProviderContext) {
)!;
});
it('should renew session and redirect to the home page if login is for the same user', async () => {
const samlAuthenticationResponse = await supertest
.post('/api/security/saml/callback')
.set('kbn-xsrf', 'xxx')
.set('Cookie', existingSessionCookie.cookieString())
.send({ SAMLResponse: await createSAMLResponse({ username: existingUsername }) })
.expect('location', '/')
.expect(302);
for (const [description, setup] of testScenarios) {
it(`should renew session and redirect to the home page if login is for the same user ${description}`, async () => {
await setup();
const newSessionCookie = request.cookie(
samlAuthenticationResponse.headers['set-cookie'][0]
)!;
expect(newSessionCookie.value).to.not.be.empty();
expect(newSessionCookie.value).to.not.equal(existingSessionCookie.value);
const samlAuthenticationResponse = await supertest
.post('/api/security/saml/callback')
.set('kbn-xsrf', 'xxx')
.set('Cookie', existingSessionCookie.cookieString())
.send({ SAMLResponse: await createSAMLResponse({ username: existingUsername }) })
.expect(302);
// Tokens from old cookie are invalidated.
const rejectedResponse = await supertest
.get('/internal/security/me')
.set('kbn-xsrf', 'xxx')
.set('Cookie', existingSessionCookie.cookieString())
.expect(400);
expect(rejectedResponse.body).to.have.property(
'message',
'Both access and refresh tokens are expired.'
);
expect(samlAuthenticationResponse.headers.location).to.be('/');
// Only tokens from new session are valid.
const acceptedResponse = await supertest
.get('/internal/security/me')
.set('kbn-xsrf', 'xxx')
.set('Cookie', newSessionCookie.cookieString())
.expect(200);
expect(acceptedResponse.body).to.have.property('username', existingUsername);
});
const newSessionCookie = request.cookie(
samlAuthenticationResponse.headers['set-cookie'][0]
)!;
expect(newSessionCookie.value).to.not.be.empty();
expect(newSessionCookie.value).to.not.equal(existingSessionCookie.value);
it('should create a new session and redirect to the `overwritten_session` if login is for another user', async () => {
const newUsername = 'c@d.e';
const samlAuthenticationResponse = await supertest
.post('/api/security/saml/callback')
.set('kbn-xsrf', 'xxx')
.set('Cookie', existingSessionCookie.cookieString())
.send({ SAMLResponse: await createSAMLResponse({ username: newUsername }) })
.expect('location', '/security/overwritten_session')
.expect(302);
// Tokens from old cookie are invalidated.
const rejectedResponse = await supertest
.get('/internal/security/me')
.set('kbn-xsrf', 'xxx')
.set('Cookie', existingSessionCookie.cookieString())
.expect(400);
expect(rejectedResponse.body).to.have.property(
'message',
'Both access and refresh tokens are expired.'
);
const newSessionCookie = request.cookie(
samlAuthenticationResponse.headers['set-cookie'][0]
)!;
expect(newSessionCookie.value).to.not.be.empty();
expect(newSessionCookie.value).to.not.equal(existingSessionCookie.value);
// Only tokens from new session are valid.
await checkSessionCookie(newSessionCookie);
});
// Tokens from old cookie are invalidated.
const rejectedResponse = await supertest
.get('/internal/security/me')
.set('kbn-xsrf', 'xxx')
.set('Cookie', existingSessionCookie.cookieString())
.expect(400);
expect(rejectedResponse.body).to.have.property(
'message',
'Both access and refresh tokens are expired.'
);
it(`should create a new session and redirect to the \`overwritten_session\` if login is for another user ${description}`, async () => {
await setup();
// Only tokens from new session are valid.
const acceptedResponse = await supertest
.get('/internal/security/me')
.set('kbn-xsrf', 'xxx')
.set('Cookie', newSessionCookie.cookieString())
.expect(200);
expect(acceptedResponse.body).to.have.property('username', newUsername);
});
const newUsername = 'c@d.e';
const samlAuthenticationResponse = await supertest
.post('/api/security/saml/callback')
.set('kbn-xsrf', 'xxx')
.set('Cookie', existingSessionCookie.cookieString())
.send({ SAMLResponse: await createSAMLResponse({ username: newUsername }) })
.expect(302);
expect(samlAuthenticationResponse.headers.location).to.be(
'/security/overwritten_session'
);
const newSessionCookie = request.cookie(
samlAuthenticationResponse.headers['set-cookie'][0]
)!;
expect(newSessionCookie.value).to.not.be.empty();
expect(newSessionCookie.value).to.not.equal(existingSessionCookie.value);
// Tokens from old cookie are invalidated.
const rejectedResponse = await supertest
.get('/internal/security/me')
.set('kbn-xsrf', 'xxx')
.set('Cookie', existingSessionCookie.cookieString())
.expect(400);
expect(rejectedResponse.body).to.have.property(
'message',
'Both access and refresh tokens are expired.'
);
// Only tokens from new session are valid.
await checkSessionCookie(newSessionCookie, newUsername);
});
}
});
describe('handshake with very long URL path or fragment', () => {