Skip to content

Commit

Permalink
Adjust validation for OIDC endpoint (#37303)
Browse files Browse the repository at this point in the history
This change adjusts validation of query parameters in the
/api/security/v1/oidc endpoint. It was discovered during manual
testing that Google's OP is sending extra parameters than the ones
identified in https://tools.ietf.org/html/rfc6749#section-4.1.2
which is refernced by
https://openid.net/specs/openid-connect-core-1_0.html#AuthResponse
(for instance auth_user and session_state). The existing validation
rules only allowed the expected query parameters but this
means that Kibana wouldn't be able to complete OpenID Connect
authentication with Google acting as the OP.
As dictated in the standard (RFC6749), "The client MUST ignore
unrecognized response parameters." so we should allow but discard
any extra parameters we do not recognize and not throw an error.
Furthermore, it adds stricter validation for the issuer and all
parameters of type URI when these are present.
  • Loading branch information
jkakavas committed May 29, 2019
1 parent 4146c6b commit 2d668db
Showing 1 changed file with 6 additions and 42 deletions.
48 changes: 6 additions & 42 deletions x-pack/plugins/security/server/routes/api/v1/authenticate.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,21 +72,22 @@ export function initAuthenticateApi(server) {
});

server.route({
method: 'GET',
// POST is only allowed for Third Party initiated authentication
method: ['GET', 'POST'],
path: '/api/security/v1/oidc',
config: {
auth: false,
validate: {
query: Joi.object().keys({
iss: Joi.string(),
iss: Joi.string().uri({ scheme: 'https' }),
login_hint: Joi.string(),
target_link_uri: Joi.string(),
target_link_uri: Joi.string().uri(),
code: Joi.string(),
error: Joi.string(),
error_description: Joi.string(),
error_uri: Joi.string(),
error_uri: Joi.string().uri(),
state: Joi.string()
})
}).unknown()
}
},
async handler(request, h) {
Expand All @@ -112,43 +113,6 @@ export function initAuthenticateApi(server) {
}
});

server.route({
// POST is only allowed for Third Party initiated authentication
method: 'POST',
path: '/api/security/v1/oidc',
config: {
auth: false,
validate: {
query: Joi.object().keys({
iss: Joi.string(),
login_hint: Joi.string(),
target_link_uri: Joi.string()
})
}
},
async handler(request, h) {
try {
// We handle the fact that the user might get redirected to Kibana while already having an session
// in the same exact manner as with saml. Return an error notifying the user they are already logged in.
const authenticationResult = await server.plugins.security.authenticate(request);
if (authenticationResult.succeeded()) {
return Boom.forbidden(
'Sorry, you already have an active Kibana session. ' +
'If you want to start a new one, please logout from the existing session first.'
);
}

if (authenticationResult.redirected()) {
return h.redirect(authenticationResult.redirectURL);
}

throw Boom.unauthorized(authenticationResult.error);
} catch (err) {
throw wrapError(err);
}
}
});

server.route({
method: 'GET',
path: '/api/security/v1/logout',
Expand Down

0 comments on commit 2d668db

Please sign in to comment.