Skip to content
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.

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

Rename /api/security/oidc to /api/security/oidc/callback. #53886

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ describe('OIDCAuthenticationProvider', () => {

describe('`login` method', () => {
it('redirects third party initiated login attempts to the OpenId Connect Provider.', async () => {
const request = httpServerMock.createKibanaRequest({ path: '/api/security/oidc' });
const request = httpServerMock.createKibanaRequest({ path: '/api/security/oidc/callback' });

mockOptions.client.callAsInternalUser.withArgs('shield.oidcPrepare').resolves({
state: 'statevalue',
Expand Down Expand Up @@ -205,21 +205,22 @@ describe('OIDCAuthenticationProvider', () => {
describe('authorization code flow', () => {
defineAuthenticationFlowTests(() => ({
request: httpServerMock.createKibanaRequest({
path: '/api/security/oidc?code=somecodehere&state=somestatehere',
path: '/api/security/oidc/callback?code=somecodehere&state=somestatehere',
}),
attempt: {
flow: OIDCAuthenticationFlow.AuthorizationCode,
authenticationResponseURI: '/api/security/oidc?code=somecodehere&state=somestatehere',
authenticationResponseURI:
'/api/security/oidc/callback?code=somecodehere&state=somestatehere',
},
expectedRedirectURI: '/api/security/oidc?code=somecodehere&state=somestatehere',
expectedRedirectURI: '/api/security/oidc/callback?code=somecodehere&state=somestatehere',
}));
});

describe('implicit flow', () => {
defineAuthenticationFlowTests(() => ({
request: httpServerMock.createKibanaRequest({
path:
'/api/security/oidc?authenticationResponseURI=http://kibana/api/security/oidc/implicit#id_token=sometoken',
'/api/security/oidc/callback?authenticationResponseURI=http://kibana/api/security/oidc/implicit#id_token=sometoken',
}),
attempt: {
flow: OIDCAuthenticationFlow.Implicit,
Expand Down
10 changes: 5 additions & 5 deletions x-pack/plugins/security/server/routes/authentication/oidc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export function defineOIDCRoutes({ router, logger, authc, csp, basePath }: Route
/**
* The route should be configured as a redirect URI in OP when OpenID Connect implicit flow
* is used, so that we can extract authentication response from URL fragment and send it to
* the `/api/security/oidc` route.
* the `/api/security/oidc/callback` route.
*/
router.get(
{
Expand Down Expand Up @@ -57,7 +57,7 @@ export function defineOIDCRoutes({ router, logger, authc, csp, basePath }: Route

/**
* The route that accompanies `/api/security/oidc/implicit` and renders a JavaScript snippet
* that extracts fragment part from the URL and send it to the `/api/security/oidc` route.
* that extracts fragment part from the URL and send it to the `/api/security/oidc/callback` route.
* We need this separate endpoint because of default CSP policy that forbids inline scripts.
*/
router.get(
Expand All @@ -72,7 +72,7 @@ export function defineOIDCRoutes({ router, logger, authc, csp, basePath }: Route
createCustomResourceResponse(
`
window.location.replace(
'${serverBasePath}/api/security/oidc?authenticationResponseURI=' + encodeURIComponent(window.location.href)
'${serverBasePath}/api/security/oidc/callback?authenticationResponseURI=' + encodeURIComponent(window.location.href)
);
`,
'text/javascript',
Expand All @@ -83,7 +83,7 @@ export function defineOIDCRoutes({ router, logger, authc, csp, basePath }: Route
);

// Generate two identical routes with new and deprecated URL and issue a warning if route with deprecated URL is ever used.
for (const path of ['/api/security/oidc', '/api/security/v1/oidc']) {
for (const path of ['/api/security/oidc/callback', '/api/security/v1/oidc']) {
router.get(
{
path,
Expand Down Expand Up @@ -124,7 +124,7 @@ export function defineOIDCRoutes({ router, logger, authc, csp, basePath }: Route
} else if (request.query.code || request.query.error) {
if (path === '/api/security/v1/oidc') {
logger.warn(
`The "${serverBasePath}${path}" URL is deprecated and will stop working in the next major version, please use "${serverBasePath}/api/security/oidc" URL instead.`,
`The "${serverBasePath}${path}" URL is deprecated and will stop working in the next major version, please use "${serverBasePath}/api/security/oidc/callback" URL instead.`,
{ tags: ['deprecation'] }
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,22 +125,22 @@ export default function({ getService }) {

it('should fail if OpenID Connect response is not complemented with handshake cookie', async () => {
await supertest
.get(`/api/security/oidc?code=thisisthecode&state=${stateAndNonce.state}`)
.get(`/api/security/oidc/callback?code=thisisthecode&state=${stateAndNonce.state}`)
.set('kbn-xsrf', 'xxx')
.expect(401);
});

it('should fail if state is not matching', async () => {
await supertest
.get(`/api/security/oidc?code=thisisthecode&state=someothervalue`)
.get(`/api/security/oidc/callback?code=thisisthecode&state=someothervalue`)
.set('kbn-xsrf', 'xxx')
.set('Cookie', handshakeCookie.cookieString())
.expect(401);
});

it('should succeed if both the OpenID Connect response and the cookie are provided', async () => {
const oidcAuthenticationResponse = await supertest
.get(`/api/security/oidc?code=code1&state=${stateAndNonce.state}`)
.get(`/api/security/oidc/callback?code=code1&state=${stateAndNonce.state}`)
.set('kbn-xsrf', 'xxx')
.set('Cookie', handshakeCookie.cookieString())
.expect(302);
Expand Down Expand Up @@ -195,7 +195,7 @@ export default function({ getService }) {
.expect(200);

const oidcAuthenticationResponse = await supertest
.get(`/api/security/oidc?code=code2&state=${stateAndNonce.state}`)
.get(`/api/security/oidc/callback?code=code2&state=${stateAndNonce.state}`)
.set('kbn-xsrf', 'xxx')
.set('Cookie', handshakeCookie.cookieString())
.expect(302);
Expand Down Expand Up @@ -245,7 +245,7 @@ export default function({ getService }) {
.expect(200);

const oidcAuthenticationResponse = await supertest
.get(`/api/security/oidc?code=code1&state=${stateAndNonce.state}`)
.get(`/api/security/oidc/callback?code=code1&state=${stateAndNonce.state}`)
.set('kbn-xsrf', 'xxx')
.set('Cookie', sessionCookie.cookieString())
.expect(302);
Expand Down Expand Up @@ -320,7 +320,7 @@ export default function({ getService }) {
.expect(200);

const oidcAuthenticationResponse = await supertest
.get(`/api/security/oidc?code=code1&state=${stateAndNonce.state}`)
.get(`/api/security/oidc/callback?code=code1&state=${stateAndNonce.state}`)
.set('kbn-xsrf', 'xxx')
.set('Cookie', handshakeCookie.cookieString())
.expect(302);
Expand Down Expand Up @@ -409,7 +409,7 @@ export default function({ getService }) {
.expect(200);

const oidcAuthenticationResponse = await supertest
.get(`/api/security/oidc?code=code1&state=${stateAndNonce.state}`)
.get(`/api/security/oidc/callback?code=code1&state=${stateAndNonce.state}`)
.set('kbn-xsrf', 'xxx')
.set('Cookie', handshakeCookie.cookieString())
.expect(302);
Expand Down Expand Up @@ -499,7 +499,7 @@ export default function({ getService }) {
.expect(200);

const oidcAuthenticationResponse = await supertest
.get(`/api/security/oidc?code=code1&state=${stateAndNonce.state}`)
.get(`/api/security/oidc/callback?code=code1&state=${stateAndNonce.state}`)
.set('kbn-xsrf', 'xxx')
.set('Cookie', handshakeCookie.cookieString())
.expect(302);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export default function({ getService }: FtrProviderContext) {

// Check that script that forwards URL fragment worked correctly.
expect(dom.window.location.href).to.be(
'/api/security/oidc?authenticationResponseURI=https%3A%2F%2Fkibana.com%2Fapi%2Fsecurity%2Foidc%2Fimplicit%23token%3Dsome_token%26access_token%3Dsome_access_token'
'/api/security/oidc/callback?authenticationResponseURI=https%3A%2F%2Fkibana.com%2Fapi%2Fsecurity%2Foidc%2Fimplicit%23token%3Dsome_token%26access_token%3Dsome_access_token'
);
});

Expand All @@ -76,7 +76,7 @@ export default function({ getService }: FtrProviderContext) {

await supertest
.get(
`/api/security/oidc?authenticationResponseURI=${encodeURIComponent(
`/api/security/oidc/callback?authenticationResponseURI=${encodeURIComponent(
authenticationResponse
)}`
)
Expand All @@ -90,7 +90,7 @@ export default function({ getService }: FtrProviderContext) {

await supertest
.get(
`/api/security/oidc?authenticationResponseURI=${encodeURIComponent(
`/api/security/oidc/callback?authenticationResponseURI=${encodeURIComponent(
authenticationResponse
)}`
)
Expand All @@ -106,7 +106,7 @@ export default function({ getService }: FtrProviderContext) {

const oidcAuthenticationResponse = await supertest
.get(
`/api/security/oidc?authenticationResponseURI=${encodeURIComponent(
`/api/security/oidc/callback?authenticationResponseURI=${encodeURIComponent(
authenticationResponse
)}`
)
Expand Down
2 changes: 1 addition & 1 deletion x-pack/test/oidc_api_integration/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export default async function({ readConfigFile }: FtrConfigProviderContext) {
`xpack.security.authc.realms.oidc.oidc1.rp.client_id=0oa8sqpov3TxMWJOt356`,
`xpack.security.authc.realms.oidc.oidc1.rp.client_secret=0oa8sqpov3TxMWJOt356`,
`xpack.security.authc.realms.oidc.oidc1.rp.response_type=code`,
`xpack.security.authc.realms.oidc.oidc1.rp.redirect_uri=http://localhost:${kibanaPort}/api/security/oidc`,
`xpack.security.authc.realms.oidc.oidc1.rp.redirect_uri=http://localhost:${kibanaPort}/api/security/oidc/callback`,
`xpack.security.authc.realms.oidc.oidc1.op.authorization_endpoint=https://test-op.elastic.co/oauth2/v1/authorize`,
`xpack.security.authc.realms.oidc.oidc1.op.endsession_endpoint=https://test-op.elastic.co/oauth2/v1/endsession`,
`xpack.security.authc.realms.oidc.oidc1.op.token_endpoint=http://localhost:${kibanaPort}/api/oidc_provider/token_endpoint`,
Expand Down