From ca41c505bdb9606bc5feb7fb3eabf9deb851c0fc Mon Sep 17 00:00:00 2001
From: Ivan Starkov <istarkov@gmail.com>
Date: Thu, 5 Sep 2024 23:30:48 +0300
Subject: [PATCH] chore: Additional checks for the redirect_uri to avoid
 authorization code leakage (#4066)

## Description

It was possible to use any builder subdomain for redirect_uri. What can
cause auth `code` leak.

Not a big deal having pkce but better to avoid as additional security
measure.


## Steps for reproduction

1. click button
2. expect xyz

## Code Review

- [ ] hi @kof, I need you to do
  - conceptual review (architecture, feature-correctness)
  - detailed review (read every line)
  - test it on preview

## Before requesting a review

- [ ] made a self-review
- [ ] added inline comments where things may be not obvious (the "why",
not "what")

## Before merging

- [ ] tested locally and on preview environment (preview dev login:
5de6)
- [ ] updated [test
cases](https://github.com/webstudio-is/webstudio/blob/main/apps/builder/docs/test-cases.md)
document
- [ ] added tests
- [ ] if any new env variables are added, added them to `.env` file
---
 .../builder/app/routes/oauth.ws.authorize.tsx | 40 +++++++++++++++++--
 1 file changed, 37 insertions(+), 3 deletions(-)

diff --git a/apps/builder/app/routes/oauth.ws.authorize.tsx b/apps/builder/app/routes/oauth.ws.authorize.tsx
index 8cc5a5c81b..6c4a6295db 100644
--- a/apps/builder/app/routes/oauth.ws.authorize.tsx
+++ b/apps/builder/app/routes/oauth.ws.authorize.tsx
@@ -2,12 +2,20 @@ import { json, type LoaderFunction } from "@remix-run/server-runtime";
 import { z } from "zod";
 import { createDebug } from "~/shared/debug";
 import { fromError } from "zod-validation-error";
-import { getAuthorizationServerOrigin } from "~/shared/router-utils/origins";
+import {
+  getAuthorizationServerOrigin,
+  isBuilderUrl,
+} from "~/shared/router-utils/origins";
 import env from "~/env/env.server";
 import { authenticator } from "~/services/auth.server";
 import { createCodeToken } from "~/services/token.server";
 import { isUserAuthorizedForProject } from "~/services/builder-access.server";
-import { isDashboard, loginPath } from "~/shared/router-utils";
+import {
+  builderUrl,
+  compareUrls,
+  isDashboard,
+  loginPath,
+} from "~/shared/router-utils";
 import { preventCrossOriginCookie } from "~/services/no-cross-origin-cookie";
 import * as session from "~/services/session.server";
 import { redirect } from "~/services/no-store-redirect";
@@ -113,7 +121,9 @@ export const loader: LoaderFunction = async ({ request }) => {
     // It is not pre-registered but it must match the AuthorizationServerOrigin
     if (
       getAuthorizationServerOrigin(request.url) !==
-      getAuthorizationServerOrigin(redirect_uri)
+        getAuthorizationServerOrigin(redirect_uri) ||
+      new URL(redirect_uri).pathname !== "/auth/ws/callback" ||
+      false === isBuilderUrl(redirect_uri)
     ) {
       debug("redirect_uri does not match the registered redirect URIs");
 
@@ -174,6 +184,30 @@ export const loader: LoaderFunction = async ({ request }) => {
         );
       }
 
+      // redirect_uri: Ensure the redirect_uri parameter value is valid and authorized
+      if (
+        false ===
+        compareUrls(
+          new URL(redirect_uri).origin,
+          builderUrl({
+            projectId: oAuthParams.scope.projectId,
+            origin: getAuthorizationServerOrigin(request.url),
+          })
+        )
+      ) {
+        debug("redirect_uri does not match the registered redirect URIs");
+
+        return json(
+          {
+            error: "invalid_request",
+            error_description:
+              "The redirect_uri provided does not match the registered redirect URIs.",
+            error_uri: "https://tools.ietf.org/html/rfc6749#section-3.1.2",
+          },
+          { status: 400 }
+        );
+      }
+
       debug(
         `User ${sessionData.userId} is the owner of ${oAuthParams.scope.projectId}, creating token`
       );