Skip to content

Commit

Permalink
chore: New Builder Auth (#4010)
Browse files Browse the repository at this point in the history
## Description

### Full track


webstudio-is/webstudio-saas#317 (comment)

So thoughts about session

webstudio-is/webstudio-saas#317 (comment)

Some explanations what is fixed and how

webstudio-is/webstudio-saas#317 (comment)

Next PR:
- [x] - Old share links auto redirect
- [x] - Try make better error message having that current PR will logout
all users.

## Steps for reproduction

1. Login using `google login` or `dev login` try to open projects on
dahsboard, create new, clone delete etc.
2. Try to open projects on dahboard, see each is on it's own domain and
work
3. Try to open project link (without share token) with other user -
should fail.
4. Try publish/clone any links and operations.
5. Share project from `userA`:
 - try to open it with other `userB`
   - see `userB` is not logged in to that project
- try to get using fetch from console anything about `userB` or `userA`
i.e rest etc endpoints on apps.webstudio.is domain
 - try to open incognito, check it works.
6. Open few project, then logout from dashboard, reload project page
 - See login page
 - Try to login as same uer - see it works
 - Or try to login as another user - see auth error


## 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
  • Loading branch information
istarkov authored and kof committed Aug 30, 2024
1 parent a0d96fa commit e1053d1
Show file tree
Hide file tree
Showing 75 changed files with 1,676 additions and 713 deletions.
2 changes: 1 addition & 1 deletion .github/actions/add-status/action.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ runs:
using: "composite"
steps:
- name: Add URL to vercel deployment through *.prs.webstudio.is
uses: actions/github-script@v6
uses: actions/github-script@v7
with:
script: |
const branch = context.payload.pull_request?.head?.ref ?? context.payload.ref?.replace('refs/heads/', '')
Expand Down
7 changes: 7 additions & 0 deletions .github/workflows/cli-r2.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ permissions:
contents: read # to fetch code (actions/checkout)
statuses: write # This is required for the GitHub Script createCommitStatus to work
packages: write
deployments: write

jobs:
build:
Expand Down Expand Up @@ -115,3 +116,9 @@ jobs:
'./functions/[[path]].ts'
working-directory: ${{ github.workspace }}/cloudflare-template

delete-github-deployments:
needs: checks
uses: ./.github/workflows/delete-github-deployments.yml
with:
ref: ${{ github.ref_name }}
16 changes: 11 additions & 5 deletions .github/workflows/delete-github-deployments.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ name: Delete github deployments
on:
workflow_call:
inputs:
sha:
ref:
type: string
required: true

Expand All @@ -19,17 +19,23 @@ jobs:
- name: Delete Previous deployments
uses: actions/github-script@v7
env:
SHA_HEAD: ${{ inputs.sha }}
REF: ${{ inputs.ref }}
with:
script: |
const { SHA_HEAD } = process.env
const { REF } = process.env;
console.log(REF);
const deployments = await github.rest.repos.listDeployments({
owner: context.repo.owner,
repo: context.repo.repo,
sha: SHA_HEAD
ref: REF,
per_page: 100
});
await Promise.all(
console.log(deployments);
await Promise.allSettled(
deployments.data.map(async (deployment) => {
await github.rest.repos.createDeploymentStatus({
owner: context.repo.owner,
Expand Down
7 changes: 7 additions & 0 deletions .github/workflows/vercel-deploy-staging.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ concurrency:
permissions:
contents: read # to fetch code (actions/checkout)
statuses: write # This is required for the GitHub Script createCommitStatus to work
deployments: write

jobs:
deployment:
Expand Down Expand Up @@ -82,3 +83,9 @@ jobs:
uses: ./.github/workflows/fixtures-test.yml
with:
builder-url: ${{ needs.deployment.outputs.builder-url }}

delete-github-deployments:
needs: fixtures-test
uses: ./.github/workflows/delete-github-deployments.yml
with:
ref: ${{ github.ref_name }}
10 changes: 10 additions & 0 deletions @types/canvas-iframe.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
declare namespace JSX {
interface IntrinsicElements {
iframe: React.DetailedHTMLProps<
React.IframeHTMLAttributes<HTMLIFrameElement> & {
credentialless?: "true";
},
HTMLIFrameElement
>;
}
}
4 changes: 1 addition & 3 deletions apps/builder/app/builder/builder.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -311,9 +311,7 @@ export const Builder = ({
return unsubscribe;
}, []);

const canvasUrl = getCanvasUrl({
project,
});
const canvasUrl = getCanvasUrl();

/**
* Prevents Lexical text editor from stealing focus during rendering.
Expand Down
6 changes: 2 additions & 4 deletions apps/builder/app/builder/features/topbar/menu/menu.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { useNavigate } from "@remix-run/react";
import { useStore } from "@nanostores/react";
import {
theme,
Expand All @@ -21,7 +20,7 @@ import {
$isShareDialogOpen,
$isPublishDialogOpen,
} from "~/builder/shared/nano-states";
import { dashboardPath } from "~/shared/router-utils";
import { dashboardUrl } from "~/shared/router-utils";
import { $authPermit, $authTokenPermissions } from "~/shared/nano-states";
import { emitCommand } from "~/builder/shared/commands";
import { MenuButton } from "./menu-button";
Expand Down Expand Up @@ -52,7 +51,6 @@ const ViewMenuItem = () => {
};

export const Menu = () => {
const navigate = useNavigate();
const { hasProPlan } = useStore($userPlanFeatures);
const authPermit = useStore($authPermit);
const authTokenPermission = useStore($authTokenPermissions);
Expand Down Expand Up @@ -80,7 +78,7 @@ export const Menu = () => {
>
<DropdownMenuItem
onSelect={() => {
navigate(dashboardPath());
window.location.href = dashboardUrl({ origin: window.origin });
}}
>
Dashboard
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,14 @@ export const CanvasIframe = forwardRef<HTMLIFrameElement, CanvasIframeProps>(
$canvasIframeState.set("idle");
});

return <iframe {...props} ref={ref} className={iframeStyle()} />;
return (
<iframe
{...props}
ref={ref}
className={iframeStyle()}
credentialless="true"
/>
);
}
);

Expand Down
5 changes: 4 additions & 1 deletion apps/builder/app/builder/shared/sync/sync-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,10 @@ const syncServer = async function () {
}
// when versions mismatched ask user to reload
// user may cancel to copy own state before reloading
if (result.status === "version_mismatched") {
if (
result.status === "version_mismatched" ||
result.status === "authorization_error"
) {
const error =
result.errors ?? "Unknown version mismatch. Please reload.";

Expand Down
5 changes: 3 additions & 2 deletions apps/builder/app/dashboard/projects/project-card.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
import { InfoCircleIcon, EllipsesIcon } from "@webstudio-is/icons";
import { type KeyboardEvent, useRef, useState } from "react";
import type { ImageLoader } from "@webstudio-is/image";
import { builderPath } from "~/shared/router-utils";
import { builderUrl } from "~/shared/router-utils";
import {
RenameProjectDialog,
DeleteProjectDialog,
Expand Down Expand Up @@ -181,7 +181,8 @@ export const ProjectCard = ({
const { thumbnailRef, handleKeyDown } = useProjectCard();
const handleCloneProject = useCloneProject(id);
const { state, location } = useNavigation();
const linkPath = builderPath({ projectId: id });

const linkPath = builderUrl({ origin: window.origin, projectId: id });
// Transition to the project has started, we may need to show a spinner
const isTransitioning = state !== "idle" && linkPath === location.pathname;
return (
Expand Down
13 changes: 7 additions & 6 deletions apps/builder/app/dashboard/projects/project-dialogs.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { useRevalidator } from "@remix-run/react";
import { useEffect, useState } from "react";
import { useNavigate } from "@remix-run/react";
import {
Box,
Button,
Expand All @@ -19,7 +18,7 @@ import {
} from "@webstudio-is/design-system";
import { PlusIcon } from "@webstudio-is/icons";
import { Title } from "@webstudio-is/project";
import { builderPath } from "~/shared/router-utils";
import { builderUrl } from "~/shared/router-utils";
import { ShareProjectContainer } from "~/shared/share-project";
import { trpcClient } from "~/shared/trpc/trpc-client";

Expand Down Expand Up @@ -118,10 +117,8 @@ const DialogContent = ({
};

const useCreateProject = () => {
const navigate = useNavigate();
const { send, state } = trpcClient.dashboardProject.create.useMutation();
const [errors, setErrors] = useState<string>();
const revalidator = useRevalidator();

const handleSubmit = ({ title }: { title: string }) => {
const parsed = Title.safeParse(title);
Expand All @@ -132,9 +129,13 @@ const useCreateProject = () => {
setErrors(errors);
if (parsed.success) {
send({ title }, (data) => {
revalidator.revalidate();
console.info("data");

if (data?.id) {
navigate(builderPath({ projectId: data.id }));
window.location.href = builderUrl({
origin: window.origin,
projectId: data.id,
});
}
});
}
Expand Down
3 changes: 1 addition & 2 deletions apps/builder/app/env/env.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,7 @@ const env = {
POSTGREST_URL: process.env.POSTGREST_URL ?? "http://localhost:3000",
POSTGREST_API_KEY: process.env.POSTGREST_API_KEY ?? "",

SECURE_COOKIE:
process.env.SSL === "true" || process.env.NODE_ENV === "production",
SECURE_COOKIE: true,

// Used for project oauth login flow @todo remove ??
AUTH_WS_CLIENT_ID: process.env.AUTH_WS_CLIENT_ID ?? "12345",
Expand Down
39 changes: 34 additions & 5 deletions apps/builder/app/routes/$.tsx
Original file line number Diff line number Diff line change
@@ -1,31 +1,48 @@
import {
type HeadersFunction,
type LoaderFunctionArgs,
redirect,
} from "@remix-run/server-runtime";

import { dashboardPath } from "~/shared/router-utils";
import { isRouteErrorResponse, useRouteError } from "@remix-run/react";
import { ErrorMessage } from "~/shared/error/error-message";
import { preventCrossOriginCookie } from "~/services/no-cross-origin-cookie";

export const loader = async ({ request }: LoaderFunctionArgs) => {
preventCrossOriginCookie(request);

const url = new URL(request.url);

// Redirecting asset files (e.g., .js, .css) to the dashboard should be avoided.
// This is because immutable caching rules apply to redirects, causing these files
// to become permanently inaccessible. Ensure asset files are served correctly
// without redirects to maintain availability and proper caching behavior.
const publicPaths = ["/cgi/", "/assets/"];
const publicPaths = ["/cgi/", "/assets/", "/apple-touch-icon-"];

// In case of 404 on static assets, this route will be executed
if (publicPaths.some((publicPath) => url.pathname.startsWith(publicPath))) {
throw new Response("Not found", {
return new Response("Not found", {
status: 404,
headers: {
"Cache-Control": "public, max-age=0, must-revalidate",
},
});
}

return redirect(dashboardPath());
const contentType = request.headers.get("Content-Type");

if (contentType?.includes("application/json")) {
// Return an error to not trigger the ErrorBoundary rendering (api request)
return new Response(null, {
status: 404,
statusText: "Not Found",
});
}

// Throw an error to trigger the ErrorBoundary rendering
throw new Response(null, {
status: 404,
statusText: "Not Found",
});
};

export const headers: HeadersFunction = ({ errorHeaders, loaderHeaders }) => {
Expand All @@ -43,3 +60,15 @@ export const headers: HeadersFunction = ({ errorHeaders, loaderHeaders }) => {
}
return loaderHeaders;
};

export const ErrorBoundary = () => {
const error = useRouteError();
console.error({ error });
const message = isRouteErrorResponse(error)
? `${error.status} ${error.statusText} ${error.data.message ?? error.data}`
: error instanceof Error
? error.message
: String(error);

return <ErrorMessage message={`${message}`} />;
};
29 changes: 7 additions & 22 deletions apps/builder/app/routes/_canvas.canvas.tsx
Original file line number Diff line number Diff line change
@@ -1,21 +1,19 @@
import { lazy } from "react";
import { type LoaderFunctionArgs, redirect } from "@remix-run/server-runtime";
import {
isRouteErrorResponse,
useLoaderData,
useRouteError,
} from "@remix-run/react";
import { type LoaderFunctionArgs } from "@remix-run/server-runtime";
import { useLoaderData } from "@remix-run/react";
import type { Params } from "@webstudio-is/react-sdk";
import { Body } from "@webstudio-is/sdk-components-react-remix";
import { createImageLoader } from "@webstudio-is/image";
import env from "~/env/env.server";
import { ErrorMessage } from "~/shared/error";
import { dashboardPath, isCanvas } from "~/shared/router-utils";
import { isCanvas } from "~/shared/router-utils";
import { ClientOnly } from "~/shared/client-only";

export const loader = async ({ request }: LoaderFunctionArgs) => {
if (isCanvas(request) === false) {
throw redirect(dashboardPath());
throw new Response(null, {
status: 404,
statusText: "Not Found",
});
}

const params: Params = {
Expand All @@ -26,19 +24,6 @@ export const loader = async ({ request }: LoaderFunctionArgs) => {
return { params };
};

export const ErrorBoundary = () => {
const error = useRouteError();

console.error({ error });
const message = isRouteErrorResponse(error)
? (error.data.message ?? error.data)
: error instanceof Error
? error.message
: String(error);

return <ErrorMessage message={message} />;
};

const Canvas = lazy(async () => {
const { Canvas } = await import("~/canvas/index.client");
return { default: Canvas };
Expand Down
33 changes: 31 additions & 2 deletions apps/builder/app/routes/_canvas.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { Links, Meta, Outlet } from "@remix-run/react";
import { ErrorMessage } from "~/shared/error";
import { isRouteErrorResponse, useRouteError } from "@remix-run/react";

export default function CanvasLayout() {
const Document = (props: { children: React.ReactNode }) => {
return (
<html lang="en">
<head>
Expand All @@ -9,7 +11,34 @@ export default function CanvasLayout() {
<Meta />
<Links />
</head>
<Outlet />
{props.children}
</html>
);
};

export const ErrorBoundary = () => {
const error = useRouteError();

console.error({ error });
const message = isRouteErrorResponse(error)
? (error.data.message ?? error.data)
: error instanceof Error
? error.message
: String(error);

return (
<Document>
<body>
<ErrorMessage message={message} />
</body>
</Document>
);
};

export default function CanvasLayout() {
return (
<Document>
<Outlet />
</Document>
);
}
Loading

0 comments on commit e1053d1

Please sign in to comment.