Account for server base path that can be numeric

Signed-off-by: Craig Perkins <[email protected]>
opensearch-project · Jul 8, 2024 · 8410e6d · 8410e6d
1 parent 2926dce
commit 8410e6d
Show file tree

Hide file tree

Showing 6 changed files with 44 additions and 19 deletions.
diff --git a/server/auth/types/basic/routes.ts b/server/auth/types/basic/routes.ts
@@ -52,7 +52,9 @@ export class BasicAuthRoutes {
           query: schema.object({
             nextUrl: schema.maybe(
               schema.string({
-                validate: validateNextUrl,
+                validate: (nexturl) => {
+                  return validateNextUrl(nexturl, this.coreSetup.http.basePath.serverBasePath);
+                },
               })
             ),
           }),

diff --git a/server/auth/types/openid/routes.ts b/server/auth/types/openid/routes.ts
@@ -96,7 +96,9 @@ export class OpenIdAuthRoutes {
               code: schema.maybe(schema.string()),
               nextUrl: schema.maybe(
                 schema.string({
-                  validate: validateNextUrl,
+                  validate: (nexturl) => {
+                    return validateNextUrl(nexturl, this.core.http.basePath.serverBasePath);
+                  },
                 })
               ),
               redirectHash: schema.maybe(schema.boolean()),
@@ -293,7 +295,9 @@ export class OpenIdAuthRoutes {
           query: schema.object({
             nextUrl: schema.maybe(
               schema.string({
-                validate: validateNextUrl,
+                validate: (nexturl) => {
+                  return validateNextUrl(nexturl, this.core.http.basePath.serverBasePath);
+                },
               })
             ),
           }),

diff --git a/server/auth/types/proxy/routes.ts b/server/auth/types/proxy/routes.ts
@@ -38,7 +38,9 @@ export class ProxyAuthRoutes {
           query: schema.object({
             nextUrl: schema.maybe(
               schema.string({
-                validate: validateNextUrl,
+                validate: (nexturl) => {
+                  return validateNextUrl(nexturl, this.coreSetup.http.basePath.serverBasePath);
+                },
               })
             ),
           }),

diff --git a/server/auth/types/saml/routes.ts b/server/auth/types/saml/routes.ts
@@ -55,7 +55,9 @@ export class SamlAuthRoutes {
           query: schema.object({
             nextUrl: schema.maybe(
               schema.string({
-                validate: validateNextUrl,
+                validate: (nexturl) => {
+                  return validateNextUrl(nexturl, this.coreSetup.http.basePath.serverBasePath);
+                },
               })
             ),
             redirectHash: schema.string(),
@@ -270,7 +272,9 @@ export class SamlAuthRoutes {
           query: schema.object({
             nextUrl: schema.maybe(
               schema.string({
-                validate: validateNextUrl,
+                validate: (nexturl) => {
+                  return validateNextUrl(nexturl, this.coreSetup.http.basePath.serverBasePath);
+                },
               })
             ),
           }),

diff --git a/server/utils/next_url.test.ts b/server/utils/next_url.test.ts
@@ -69,43 +69,48 @@ describe('test composeNextUrlQueryParam', () => {
 describe('test validateNextUrl', () => {
   test('accept relative url', () => {
     const url = '/relative/path';
-    expect(validateNextUrl(url)).toEqual(undefined);
+    expect(validateNextUrl(url, '')).toEqual(undefined);
   });
 
   test('accept relative url with # and query', () => {
     const url = '/relative/path#hash?a=b';
-    expect(validateNextUrl(url)).toEqual(undefined);
+    expect(validateNextUrl(url, undefined)).toEqual(undefined);
   });
 
   test('reject url not start with /', () => {
     const url = 'exmaple.com/relative/path';
-    expect(validateNextUrl(url)).toEqual(INVALID_NEXT_URL_PARAMETER_MESSAGE);
+    expect(validateNextUrl(url, '')).toEqual(INVALID_NEXT_URL_PARAMETER_MESSAGE);
   });
 
   test('reject absolute url', () => {
     const url = 'https://exmaple.com/relative/path';
-    expect(validateNextUrl(url)).toEqual(INVALID_NEXT_URL_PARAMETER_MESSAGE);
+    expect(validateNextUrl(url, '')).toEqual(INVALID_NEXT_URL_PARAMETER_MESSAGE);
   });
 
   test('reject url starts with //', () => {
     const url = '//exmaple.com/relative/path';
-    expect(validateNextUrl(url)).toEqual(INVALID_NEXT_URL_PARAMETER_MESSAGE);
+    expect(validateNextUrl(url, '')).toEqual(INVALID_NEXT_URL_PARAMETER_MESSAGE);
   });
 
   test('accpet url has @ in query parameters', () => {
     const url = '/test/path?key=a@b&k2=v';
-    expect(validateNextUrl(url)).toEqual(undefined);
+    expect(validateNextUrl(url, '')).toEqual(undefined);
   });
 
   test('allow slash', () => {
     const url = '/';
-    expect(validateNextUrl(url)).toEqual(undefined);
+    expect(validateNextUrl(url, '')).toEqual(undefined);
   });
 
   test('allow dashboard url', () => {
     const url =
       '/_plugin/opensearch-dashboards/app/opensearch-dashboards#dashbard/dashboard-id?_g=(param=a&p=b)';
-    expect(validateNextUrl(url)).toEqual(undefined);
+    expect(validateNextUrl(url, '')).toEqual(undefined);
+  });
+
+  test('allow basePath with numbers', () => {
+    const url = '/123/app/dashboards';
+    expect(validateNextUrl(url, '/123')).toEqual(undefined);
   });
 
   // test cases from https://pentester.land/cheatsheets/2018/11/02/open-redirect-cheatsheet.html
@@ -635,7 +640,7 @@ describe('test validateNextUrl', () => {
     ];
     for (const url of urlList) {
       if (url) {
-        expect(validateNextUrl(url)).toEqual(INVALID_NEXT_URL_PARAMETER_MESSAGE);
+        expect(validateNextUrl(url, '')).toEqual(INVALID_NEXT_URL_PARAMETER_MESSAGE);
       }
     }
   });

diff --git a/server/utils/next_url.ts b/server/utils/next_url.ts
@@ -54,14 +54,22 @@ export const INVALID_NEXT_URL_PARAMETER_MESSAGE = 'Invalid nextUrl parameter.';
  * @param url url string.
  * @returns error message if nextUrl is invalid, otherwise void.
  */
-export const validateNextUrl = (url: string | undefined): string | void => {
+export function validateNextUrl(
+  url: string | undefined,
+  basePath: string | undefined
+): string | void {
   if (url) {
     const path = url.split(/\?|#/)[0];
+    const bp = basePath || '';
+    if (!path.startsWith(bp)) {
+      return INVALID_NEXT_URL_PARAMETER_MESSAGE;
+    }
+    const pathMinusBase = path.replace(bp, '');
     if (
-      !path.startsWith('/') ||
-      (path.length >= 2 && !/^\/[a-zA-Z_][\/a-zA-Z0-9-_]+$/.test(path))
+      !pathMinusBase.startsWith('/') ||
+      (pathMinusBase.length >= 2 && !/^\/[a-zA-Z_][\/a-zA-Z0-9-_]+$/.test(pathMinusBase))
     ) {
       return INVALID_NEXT_URL_PARAMETER_MESSAGE;
     }
   }
-};
+}