Skip to content

Commit

Permalink
Rename /api/security/oidc to /api/security/oidc/callback. (#53886)
Browse files Browse the repository at this point in the history
  • Loading branch information
azasypkin committed Jan 3, 2020
1 parent 86df421 commit 6cf7ece
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 23 deletions.
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

0 comments on commit 6cf7ece

Please sign in to comment.