Removes deprecated platform security v1 routes (elastic#203915)

## Summary Removes the v1 routes deprecated in elastic#199656 Part of Kibana 9.0.0 readiness elastic/kibana-team#1190
CAWilson94 · Jan 10, 2025 · 0c4b5be · 0c4b5be
1 parent 17906d6
commit 0c4b5be
Show file tree

Hide file tree

Showing 8 changed files with 280 additions and 430 deletions.
diff --git a/x-pack/platform/plugins/shared/security/server/authentication/authentication_service.ts b/x-pack/platform/plugins/shared/security/server/authentication/authentication_service.ts
@@ -194,9 +194,7 @@ export class AuthenticationService {
       }
 
       const isAuthRoute = request.route.options.tags.includes(ROUTE_TAG_AUTH_FLOW);
-      const isLogoutRoute =
-        request.route.path === '/api/security/logout' ||
-        request.route.path === '/api/v1/security/logout';
+      const isLogoutRoute = request.route.path === '/api/security/logout';
 
       // If users can eventually re-login we want to redirect them directly to the page they tried
       // to access initially, but we only want to do that for routes that aren't part of the various

diff --git a/x-pack/platform/plugins/shared/security/server/authentication/providers/saml.test.ts b/x-pack/platform/plugins/shared/security/server/authentication/providers/saml.test.ts
@@ -788,7 +788,7 @@ describe('SAMLAuthenticationProvider', () => {
           method: 'POST',
           path: '/_security/saml/prepare',
           body: {
-            acs: 'test-protocol://test-hostname:1234/mock-server-basepath/api/security/v1/saml',
+            acs: 'test-protocol://test-hostname:1234/mock-server-basepath/api/security/saml/callback',
           },
         });
 
@@ -830,7 +830,7 @@ describe('SAMLAuthenticationProvider', () => {
           method: 'POST',
           path: '/_security/saml/prepare',
           body: {
-            acs: 'test-protocol://test-hostname:1234/mock-server-basepath/api/security/v1/saml',
+            acs: 'test-protocol://test-hostname:1234/mock-server-basepath/api/security/saml/callback',
           },
         });
 
@@ -900,7 +900,7 @@ describe('SAMLAuthenticationProvider', () => {
           method: 'POST',
           path: '/_security/saml/prepare',
           body: {
-            acs: 'test-protocol://test-hostname:1234/mock-server-basepath/api/security/v1/saml',
+            acs: 'test-protocol://test-hostname:1234/mock-server-basepath/api/security/saml/callback',
           },
         });
       });
@@ -1003,7 +1003,7 @@ describe('SAMLAuthenticationProvider', () => {
         method: 'POST',
         path: '/_security/saml/prepare',
         body: {
-          acs: 'test-protocol://test-hostname:1234/mock-server-basepath/api/security/v1/saml',
+          acs: 'test-protocol://test-hostname:1234/mock-server-basepath/api/security/saml/callback',
         },
       });
     });
@@ -1294,7 +1294,7 @@ describe('SAMLAuthenticationProvider', () => {
         path: '/_security/saml/invalidate',
         body: {
           query_string: 'SAMLRequest=xxx%20yyy',
-          acs: 'test-protocol://test-hostname:1234/mock-server-basepath/api/security/v1/saml',
+          acs: 'test-protocol://test-hostname:1234/mock-server-basepath/api/security/saml/callback',
         },
       });
     });
@@ -1408,7 +1408,7 @@ describe('SAMLAuthenticationProvider', () => {
         path: '/_security/saml/invalidate',
         body: {
           query_string: 'SAMLRequest=xxx%20yyy',
-          acs: 'test-protocol://test-hostname:1234/mock-server-basepath/api/security/v1/saml',
+          acs: 'test-protocol://test-hostname:1234/mock-server-basepath/api/security/saml/callback',
         },
       });
     });
@@ -1430,7 +1430,7 @@ describe('SAMLAuthenticationProvider', () => {
         path: '/_security/saml/invalidate',
         body: {
           query_string: 'SAMLRequest=xxx%20yyy',
-          acs: 'test-protocol://test-hostname:1234/mock-server-basepath/api/security/v1/saml',
+          acs: 'test-protocol://test-hostname:1234/mock-server-basepath/api/security/saml/callback',
         },
       });
     });

diff --git a/x-pack/platform/plugins/shared/security/server/authentication/providers/saml.ts b/x-pack/platform/plugins/shared/security/server/authentication/providers/saml.ts
@@ -659,7 +659,7 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider {
   private getACS() {
     return `${this.options.getServerBaseURL()}${
       this.options.basePath.serverBasePath
-    }/api/security/v1/saml`;
+    }/api/security/saml/callback`;
   }
 
   /**

diff --git a/x-pack/platform/plugins/shared/security/server/routes/authentication/common.test.ts b/x-pack/platform/plugins/shared/security/server/routes/authentication/common.test.ts
@@ -78,6 +78,16 @@ describe('Common authentication routes', () => {
         query: expect.any(Type),
         params: undefined,
       });
+      expect(routeConfig.security).toEqual({
+        authz: {
+          enabled: false,
+          reason: 'This route must remain accessible to 3rd-party IdPs',
+        },
+        authc: {
+          enabled: false,
+          reason: 'This route must remain accessible to 3rd-party IdPs',
+        },
+      });
 
       const queryValidator = (routeConfig.validate as any).query as Type<any>;
       expect(queryValidator.validate({ someRandomField: 'some-random' })).toEqual({
@@ -211,6 +221,17 @@ describe('Common authentication routes', () => {
         query: undefined,
         params: undefined,
       });
+      expect(routeConfig.security).toEqual({
+        authz: {
+          enabled: false,
+          reason: `This route provides basic and token login capability, which is delegated to the internal authentication service`,
+        },
+        authc: {
+          enabled: false,
+          reason:
+            'This route is used for authentication - it does not require existing authentication',
+        },
+      });
 
       const bodyValidator = (routeConfig.validate as any).body as Type<any>;
       expect(

diff --git a/x-pack/platform/plugins/shared/security/server/routes/authentication/common.ts b/x-pack/platform/plugins/shared/security/server/routes/authentication/common.ts
@@ -7,7 +7,6 @@
 
 import type { TypeOf } from '@kbn/config-schema';
 import { schema } from '@kbn/config-schema';
-import { i18n } from '@kbn/i18n';
 import { parseNextURL } from '@kbn/std';
 
 import type { RouteDefinitionParams } from '..';
@@ -34,130 +33,71 @@ export function defineCommonRoutes({
   license,
   logger,
   buildFlavor,
-  docLinks,
 }: RouteDefinitionParams) {
-  // Generate two identical routes with new and deprecated URL and issue a warning if route with deprecated URL is ever used.
-  // For a serverless build, do not register deprecated versioned routes
-  for (const path of [
-    '/api/security/logout',
-    ...(buildFlavor !== 'serverless' ? ['/api/security/v1/logout'] : []),
-  ]) {
-    const isDeprecated = path === '/api/security/v1/logout';
-    router.get(
-      {
-        path,
-        security: {
-          authz: {
-            enabled: false,
-            reason: 'This route must remain accessible to 3rd-party IdPs',
-          },
-          authc: {
-            enabled: false,
-            reason:
-              'This route is used for authentication - it does not require existing authentication',
-          },
+  router.get(
+    {
+      path: '/api/security/logout',
+      security: {
+        authz: {
+          enabled: false,
+          reason: 'This route must remain accessible to 3rd-party IdPs',
         },
-        // Allow unknown query parameters as this endpoint can be hit by the 3rd-party with any
-        // set of query string parameters (e.g. SAML/OIDC logout request/response parameters).
-        validate: { query: schema.object({}, { unknowns: 'allow' }) },
-        options: {
-          access: 'public',
-          excludeFromOAS: true,
-          tags: [ROUTE_TAG_CAN_REDIRECT, ROUTE_TAG_AUTH_FLOW],
-          ...(isDeprecated && {
-            deprecated: {
-              documentationUrl: docLinks.links.security.deprecatedV1Endpoints,
-              severity: 'warning',
-              message: i18n.translate('xpack.security.deprecations.logoutRouteMessage', {
-                defaultMessage:
-                  'The "{path}" URL is deprecated and will be removed in the next major version. Use "/api/security/logout" instead.',
-                values: { path },
-              }),
-              reason: {
-                type: 'migrate',
-                newApiMethod: 'GET',
-                newApiPath: '/api/security/logout',
-              },
-            },
-          }),
+        authc: {
+          enabled: false,
+          reason: 'This route must remain accessible to 3rd-party IdPs',
         },
       },
-      async (context, request, response) => {
-        const serverBasePath = basePath.serverBasePath;
-        if (isDeprecated) {
-          logger.warn(
-            `The "${serverBasePath}${path}" URL is deprecated and will stop working in the next major version. Use "${serverBasePath}/api/security/logout" URL instead.`,
-            { tags: ['deprecation'] }
-          );
-        }
+      // Allow unknown query parameters as this endpoint can be hit by the 3rd-party with any
+      // set of query string parameters (e.g. SAML/OIDC logout request/response parameters).
+      validate: { query: schema.object({}, { unknowns: 'allow' }) },
+      options: {
+        access: 'public',
+        excludeFromOAS: true,
+        tags: [ROUTE_TAG_CAN_REDIRECT, ROUTE_TAG_AUTH_FLOW],
+      },
+    },
+    async (context, request, response) => {
+      const serverBasePath = basePath.serverBasePath;
+      if (!canRedirectRequest(request)) {
+        return response.badRequest({
+          body: 'Client should be able to process redirect response.',
+        });
+      }
 
-        if (!canRedirectRequest(request)) {
-          return response.badRequest({
-            body: 'Client should be able to process redirect response.',
-          });
+      try {
+        const deauthenticationResult = await getAuthenticationService().logout(request);
+        if (deauthenticationResult.failed()) {
+          return response.customError(wrapIntoCustomErrorResponse(deauthenticationResult.error));
         }
 
-        try {
-          const deauthenticationResult = await getAuthenticationService().logout(request);
-          if (deauthenticationResult.failed()) {
-            return response.customError(wrapIntoCustomErrorResponse(deauthenticationResult.error));
-          }
-
-          return response.redirected({
-            headers: { location: deauthenticationResult.redirectURL || `${serverBasePath}/` },
-          });
-        } catch (error) {
-          return response.customError(wrapIntoCustomErrorResponse(error));
-        }
+        return response.redirected({
+          headers: { location: deauthenticationResult.redirectURL || `${serverBasePath}/` },
+        });
+      } catch (error) {
+        return response.customError(wrapIntoCustomErrorResponse(error));
       }
-    );
-  }
+    }
+  );
 
-  // Generate two identical routes with new and deprecated URL and issue a warning if route with deprecated URL is ever used.
-  // For a serverless build, do not register deprecated versioned routes
-  for (const path of [
-    '/internal/security/me',
-    ...(buildFlavor !== 'serverless' ? ['/api/security/v1/me'] : []),
-  ]) {
-    const isDeprecated = path === '/api/security/v1/me';
-    router.get(
-      {
-        path,
-        security: {
-          authz: {
-            enabled: false,
-            reason: `This route delegates authorization to Core's security service; there must be an authenticated user for this route to return information`,
-          },
-        },
-        validate: false,
-        options: {
-          access: isDeprecated ? 'public' : 'internal',
-          ...(isDeprecated && {
-            deprecated: {
-              documentationUrl: docLinks.links.security.deprecatedV1Endpoints,
-              severity: 'warning',
-              message: i18n.translate('xpack.security.deprecations.meRouteMessage', {
-                defaultMessage:
-                  'The "{path}" endpoint is deprecated and will be removed in the next major version.',
-                values: { path },
-              }),
-              reason: { type: 'remove' },
-            },
-          }),
+  router.get(
+    {
+      path: '/internal/security/me',
+      security: {
+        authz: {
+          enabled: false,
+          reason: `This route delegates authorization to Core's security service; there must be an authenticated user for this route to return information`,
         },
       },
-      createLicensedRouteHandler(async (context, request, response) => {
-        if (isDeprecated) {
-          logger.warn(
-            `The "${basePath.serverBasePath}${path}" endpoint is deprecated and will be removed in the next major version.`,
-            { tags: ['deprecation'] }
-          );
-        }
-        const { security: coreSecurity } = await context.core;
-        return response.ok({ body: coreSecurity.authc.getCurrentUser()! });
-      })
-    );
-  }
+      validate: false,
+      options: {
+        access: 'internal',
+      },
+    },
+    createLicensedRouteHandler(async (context, request, response) => {
+      const { security: coreSecurity } = await context.core;
+      return response.ok({ body: coreSecurity.authc.getCurrentUser()! });
+    })
+  );
 
   const basicParamsSchema = schema.object({
     username: schema.string({ minLength: 1 }),