Skip to content

Commit 9f4e851

Browse files
authored
Removes deprecated platform security v1 routes (#203915)
## Summary Removes the v1 routes deprecated in #199656 Part of Kibana 9.0.0 readiness elastic/kibana-team#1190
1 parent 95094b2 commit 9f4e851

File tree

8 files changed

+280
-430
lines changed

8 files changed

+280
-430
lines changed

x-pack/platform/plugins/shared/security/server/authentication/authentication_service.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -194,9 +194,7 @@ export class AuthenticationService {
194194
}
195195

196196
const isAuthRoute = request.route.options.tags.includes(ROUTE_TAG_AUTH_FLOW);
197-
const isLogoutRoute =
198-
request.route.path === '/api/security/logout' ||
199-
request.route.path === '/api/v1/security/logout';
197+
const isLogoutRoute = request.route.path === '/api/security/logout';
200198

201199
// If users can eventually re-login we want to redirect them directly to the page they tried
202200
// to access initially, but we only want to do that for routes that aren't part of the various

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -788,7 +788,7 @@ describe('SAMLAuthenticationProvider', () => {
788788
method: 'POST',
789789
path: '/_security/saml/prepare',
790790
body: {
791-
acs: 'test-protocol://test-hostname:1234/mock-server-basepath/api/security/v1/saml',
791+
acs: 'test-protocol://test-hostname:1234/mock-server-basepath/api/security/saml/callback',
792792
},
793793
});
794794

@@ -830,7 +830,7 @@ describe('SAMLAuthenticationProvider', () => {
830830
method: 'POST',
831831
path: '/_security/saml/prepare',
832832
body: {
833-
acs: 'test-protocol://test-hostname:1234/mock-server-basepath/api/security/v1/saml',
833+
acs: 'test-protocol://test-hostname:1234/mock-server-basepath/api/security/saml/callback',
834834
},
835835
});
836836

@@ -900,7 +900,7 @@ describe('SAMLAuthenticationProvider', () => {
900900
method: 'POST',
901901
path: '/_security/saml/prepare',
902902
body: {
903-
acs: 'test-protocol://test-hostname:1234/mock-server-basepath/api/security/v1/saml',
903+
acs: 'test-protocol://test-hostname:1234/mock-server-basepath/api/security/saml/callback',
904904
},
905905
});
906906
});
@@ -1003,7 +1003,7 @@ describe('SAMLAuthenticationProvider', () => {
10031003
method: 'POST',
10041004
path: '/_security/saml/prepare',
10051005
body: {
1006-
acs: 'test-protocol://test-hostname:1234/mock-server-basepath/api/security/v1/saml',
1006+
acs: 'test-protocol://test-hostname:1234/mock-server-basepath/api/security/saml/callback',
10071007
},
10081008
});
10091009
});
@@ -1294,7 +1294,7 @@ describe('SAMLAuthenticationProvider', () => {
12941294
path: '/_security/saml/invalidate',
12951295
body: {
12961296
query_string: 'SAMLRequest=xxx%20yyy',
1297-
acs: 'test-protocol://test-hostname:1234/mock-server-basepath/api/security/v1/saml',
1297+
acs: 'test-protocol://test-hostname:1234/mock-server-basepath/api/security/saml/callback',
12981298
},
12991299
});
13001300
});
@@ -1408,7 +1408,7 @@ describe('SAMLAuthenticationProvider', () => {
14081408
path: '/_security/saml/invalidate',
14091409
body: {
14101410
query_string: 'SAMLRequest=xxx%20yyy',
1411-
acs: 'test-protocol://test-hostname:1234/mock-server-basepath/api/security/v1/saml',
1411+
acs: 'test-protocol://test-hostname:1234/mock-server-basepath/api/security/saml/callback',
14121412
},
14131413
});
14141414
});
@@ -1430,7 +1430,7 @@ describe('SAMLAuthenticationProvider', () => {
14301430
path: '/_security/saml/invalidate',
14311431
body: {
14321432
query_string: 'SAMLRequest=xxx%20yyy',
1433-
acs: 'test-protocol://test-hostname:1234/mock-server-basepath/api/security/v1/saml',
1433+
acs: 'test-protocol://test-hostname:1234/mock-server-basepath/api/security/saml/callback',
14341434
},
14351435
});
14361436
});

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -659,7 +659,7 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider {
659659
private getACS() {
660660
return `${this.options.getServerBaseURL()}${
661661
this.options.basePath.serverBasePath
662-
}/api/security/v1/saml`;
662+
}/api/security/saml/callback`;
663663
}
664664

665665
/**

x-pack/platform/plugins/shared/security/server/routes/authentication/common.test.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,16 @@ describe('Common authentication routes', () => {
7878
query: expect.any(Type),
7979
params: undefined,
8080
});
81+
expect(routeConfig.security).toEqual({
82+
authz: {
83+
enabled: false,
84+
reason: 'This route must remain accessible to 3rd-party IdPs',
85+
},
86+
authc: {
87+
enabled: false,
88+
reason: 'This route must remain accessible to 3rd-party IdPs',
89+
},
90+
});
8191

8292
const queryValidator = (routeConfig.validate as any).query as Type<any>;
8393
expect(queryValidator.validate({ someRandomField: 'some-random' })).toEqual({
@@ -211,6 +221,17 @@ describe('Common authentication routes', () => {
211221
query: undefined,
212222
params: undefined,
213223
});
224+
expect(routeConfig.security).toEqual({
225+
authz: {
226+
enabled: false,
227+
reason: `This route provides basic and token login capability, which is delegated to the internal authentication service`,
228+
},
229+
authc: {
230+
enabled: false,
231+
reason:
232+
'This route is used for authentication - it does not require existing authentication',
233+
},
234+
});
214235

215236
const bodyValidator = (routeConfig.validate as any).body as Type<any>;
216237
expect(

x-pack/platform/plugins/shared/security/server/routes/authentication/common.ts

Lines changed: 54 additions & 114 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
import type { TypeOf } from '@kbn/config-schema';
99
import { schema } from '@kbn/config-schema';
10-
import { i18n } from '@kbn/i18n';
1110
import { parseNextURL } from '@kbn/std';
1211

1312
import type { RouteDefinitionParams } from '..';
@@ -34,130 +33,71 @@ export function defineCommonRoutes({
3433
license,
3534
logger,
3635
buildFlavor,
37-
docLinks,
3836
}: RouteDefinitionParams) {
39-
// Generate two identical routes with new and deprecated URL and issue a warning if route with deprecated URL is ever used.
40-
// For a serverless build, do not register deprecated versioned routes
41-
for (const path of [
42-
'/api/security/logout',
43-
...(buildFlavor !== 'serverless' ? ['/api/security/v1/logout'] : []),
44-
]) {
45-
const isDeprecated = path === '/api/security/v1/logout';
46-
router.get(
47-
{
48-
path,
49-
security: {
50-
authz: {
51-
enabled: false,
52-
reason: 'This route must remain accessible to 3rd-party IdPs',
53-
},
54-
authc: {
55-
enabled: false,
56-
reason:
57-
'This route is used for authentication - it does not require existing authentication',
58-
},
37+
router.get(
38+
{
39+
path: '/api/security/logout',
40+
security: {
41+
authz: {
42+
enabled: false,
43+
reason: 'This route must remain accessible to 3rd-party IdPs',
5944
},
60-
// Allow unknown query parameters as this endpoint can be hit by the 3rd-party with any
61-
// set of query string parameters (e.g. SAML/OIDC logout request/response parameters).
62-
validate: { query: schema.object({}, { unknowns: 'allow' }) },
63-
options: {
64-
access: 'public',
65-
excludeFromOAS: true,
66-
tags: [ROUTE_TAG_CAN_REDIRECT, ROUTE_TAG_AUTH_FLOW],
67-
...(isDeprecated && {
68-
deprecated: {
69-
documentationUrl: docLinks.links.security.deprecatedV1Endpoints,
70-
severity: 'warning',
71-
message: i18n.translate('xpack.security.deprecations.logoutRouteMessage', {
72-
defaultMessage:
73-
'The "{path}" URL is deprecated and will be removed in the next major version. Use "/api/security/logout" instead.',
74-
values: { path },
75-
}),
76-
reason: {
77-
type: 'migrate',
78-
newApiMethod: 'GET',
79-
newApiPath: '/api/security/logout',
80-
},
81-
},
82-
}),
45+
authc: {
46+
enabled: false,
47+
reason: 'This route must remain accessible to 3rd-party IdPs',
8348
},
8449
},
85-
async (context, request, response) => {
86-
const serverBasePath = basePath.serverBasePath;
87-
if (isDeprecated) {
88-
logger.warn(
89-
`The "${serverBasePath}${path}" URL is deprecated and will stop working in the next major version. Use "${serverBasePath}/api/security/logout" URL instead.`,
90-
{ tags: ['deprecation'] }
91-
);
92-
}
50+
// Allow unknown query parameters as this endpoint can be hit by the 3rd-party with any
51+
// set of query string parameters (e.g. SAML/OIDC logout request/response parameters).
52+
validate: { query: schema.object({}, { unknowns: 'allow' }) },
53+
options: {
54+
access: 'public',
55+
excludeFromOAS: true,
56+
tags: [ROUTE_TAG_CAN_REDIRECT, ROUTE_TAG_AUTH_FLOW],
57+
},
58+
},
59+
async (context, request, response) => {
60+
const serverBasePath = basePath.serverBasePath;
61+
if (!canRedirectRequest(request)) {
62+
return response.badRequest({
63+
body: 'Client should be able to process redirect response.',
64+
});
65+
}
9366

94-
if (!canRedirectRequest(request)) {
95-
return response.badRequest({
96-
body: 'Client should be able to process redirect response.',
97-
});
67+
try {
68+
const deauthenticationResult = await getAuthenticationService().logout(request);
69+
if (deauthenticationResult.failed()) {
70+
return response.customError(wrapIntoCustomErrorResponse(deauthenticationResult.error));
9871
}
9972

100-
try {
101-
const deauthenticationResult = await getAuthenticationService().logout(request);
102-
if (deauthenticationResult.failed()) {
103-
return response.customError(wrapIntoCustomErrorResponse(deauthenticationResult.error));
104-
}
105-
106-
return response.redirected({
107-
headers: { location: deauthenticationResult.redirectURL || `${serverBasePath}/` },
108-
});
109-
} catch (error) {
110-
return response.customError(wrapIntoCustomErrorResponse(error));
111-
}
73+
return response.redirected({
74+
headers: { location: deauthenticationResult.redirectURL || `${serverBasePath}/` },
75+
});
76+
} catch (error) {
77+
return response.customError(wrapIntoCustomErrorResponse(error));
11278
}
113-
);
114-
}
79+
}
80+
);
11581

116-
// Generate two identical routes with new and deprecated URL and issue a warning if route with deprecated URL is ever used.
117-
// For a serverless build, do not register deprecated versioned routes
118-
for (const path of [
119-
'/internal/security/me',
120-
...(buildFlavor !== 'serverless' ? ['/api/security/v1/me'] : []),
121-
]) {
122-
const isDeprecated = path === '/api/security/v1/me';
123-
router.get(
124-
{
125-
path,
126-
security: {
127-
authz: {
128-
enabled: false,
129-
reason: `This route delegates authorization to Core's security service; there must be an authenticated user for this route to return information`,
130-
},
131-
},
132-
validate: false,
133-
options: {
134-
access: isDeprecated ? 'public' : 'internal',
135-
...(isDeprecated && {
136-
deprecated: {
137-
documentationUrl: docLinks.links.security.deprecatedV1Endpoints,
138-
severity: 'warning',
139-
message: i18n.translate('xpack.security.deprecations.meRouteMessage', {
140-
defaultMessage:
141-
'The "{path}" endpoint is deprecated and will be removed in the next major version.',
142-
values: { path },
143-
}),
144-
reason: { type: 'remove' },
145-
},
146-
}),
82+
router.get(
83+
{
84+
path: '/internal/security/me',
85+
security: {
86+
authz: {
87+
enabled: false,
88+
reason: `This route delegates authorization to Core's security service; there must be an authenticated user for this route to return information`,
14789
},
14890
},
149-
createLicensedRouteHandler(async (context, request, response) => {
150-
if (isDeprecated) {
151-
logger.warn(
152-
`The "${basePath.serverBasePath}${path}" endpoint is deprecated and will be removed in the next major version.`,
153-
{ tags: ['deprecation'] }
154-
);
155-
}
156-
const { security: coreSecurity } = await context.core;
157-
return response.ok({ body: coreSecurity.authc.getCurrentUser()! });
158-
})
159-
);
160-
}
91+
validate: false,
92+
options: {
93+
access: 'internal',
94+
},
95+
},
96+
createLicensedRouteHandler(async (context, request, response) => {
97+
const { security: coreSecurity } = await context.core;
98+
return response.ok({ body: coreSecurity.authc.getCurrentUser()! });
99+
})
100+
);
161101

162102
const basicParamsSchema = schema.object({
163103
username: schema.string({ minLength: 1 }),

0 commit comments

Comments
 (0)