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
This commit is contained in:
Ivan Starkov authored and GitHub committed 2024-09-05 23:30:48 +03:00
1 parent cafba33352
commit ca41c505bd
1 file changed
+37 -3
+37 -3
View File
@@ -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`
);