Create Site Preview JWT in the API #2554

fraxachun · 2024-09-19T14:03:00Z

Allows having the Site Preview unauthenticated

Description

Before: the JWT with the preview scope and setting was generated in the site. To check if the current user is allowed to preview the given scope the site had to make a request to the API and therefore needed the access token of the current user. With this setup the site has to be behind an authproxy.

Now: the JWT is generated by the API and submitted to the site. This way the site can be public.

BREAKING

The API now requires the SITE_PREVIEW_SECRET environment variable. To make it explicit to add it this env now is also mandatory for local development.

Open TODOs/questions

Changeset

COM-1087

Allows having the Site Preview unauthenticated

johnnyomair · 2024-09-19T14:27:14Z

Idea to make this non-breaking: Use an existing environment variable that we know to be available in both site and API. Could be the password of the system user. For v8 we can remove this.

Alternatives:

Update all of our projects that are on v7 (pragmatic approach to the breaking change)
Use old technique if secret isn't available

nsams · 2024-09-20T06:27:23Z

demo/api/schema.gql

@@ -748,6 +748,7 @@ type Query {
  pageTreeNodeList(scope: PageTreeNodeScopeInput!, category: String): [PageTreeNode!]!
  paginatedPageTreeNodes(scope: PageTreeNodeScopeInput!, category: String, sort: [PageTreeNodeSort!], documentType: String, offset: Int! = 0, limit: Int! = 25): PaginatedPageTreeNodes!
  pageTreeNodeSlugAvailable(scope: PageTreeNodeScopeInput!, parentId: ID, slug: String!): SlugAvailability!
+  sitePreviewJwt(scope: JSONObject!, path: String!, includeInvisible: Boolean!): String!


imho this should be a mutation, not a query:

it creates something new (although not stored, so it strictly speaking does not mutate a database or something)

it should never be cached by anything, that might even result into security issues

I think so too

We could call it createSitePreviewJwt or generateSitePreviewJwt

Currently the request is polled (due to expiration time of jwt), this is not possible for mutations. Furthermore, useMutation does not provide refetch, which is used when changing the preview settings.

Caching however is disabled by fetchPolicy: "network-only".

Changing to a mutation requires more handwork to simulate polling and refetching. Do you still think a mutation is necessary?

Caching however is disabled

I was thinking about a future cache solution we might add

You ave good arguments, leave the query!

nsams · 2024-09-20T06:29:26Z

Allows having the Site Preview unauthenticated

👏 why didn't we do that in the first place?

thomasdax98 · 2024-09-23T08:33:58Z

.env

@@ -66,6 +66,7 @@ NEXT_PUBLIC_API_URL=$API_URL
 # site-pages
 SITE_PAGES_PORT=3001
 SITE_PAGES_URL=http://localhost:$SITE_PAGES_PORT
+SITE_PREVIEW_SECRET=zPoURKLHPcMnV7E9


This change needs a changeset and this change should also be added to the migration guide for projects that are not on v7 yet

Changeset will be added.

I added another commit to mitigate the effort of handling the breaking change: dea0a1d

Changeset: 5d8c99a

thomasdax98 · 2024-09-23T08:34:03Z

demo/api/schema.gql

@@ -748,6 +748,7 @@ type Query {
  pageTreeNodeList(scope: PageTreeNodeScopeInput!, category: String): [PageTreeNode!]!
  paginatedPageTreeNodes(scope: PageTreeNodeScopeInput!, category: String, sort: [PageTreeNodeSort!], documentType: String, offset: Int! = 0, limit: Int! = 25): PaginatedPageTreeNodes!
  pageTreeNodeSlugAvailable(scope: PageTreeNodeScopeInput!, parentId: ID, slug: String!): SlugAvailability!
+  sitePreviewJwt(scope: JSONObject!, path: String!, includeInvisible: Boolean!): String!


I think so too

johnnyomair · 2024-09-25T06:35:23Z

packages/admin/cms-admin/src/preview/site/SitePreview.tsx

-    }).toString()}`;
+    const { data, error, refetch } = useQuery<GQLSitePreviewJwtQuery>(
+        gql`
+            query SitePreviewJwt($scope: JSONObject!, $path: String!, $includeInvisible: Boolean!) {


One downside to a mutation would be that it can't be queried like this here. But we could create it before even navigating to the site preview.

johnnyomair · 2024-09-25T06:36:09Z

packages/admin/cms-admin/src/preview/site/SitePreview.tsx

+                path: iframePath,
+                includeInvisible: showOnlyVisible ? false : true,
+            },
+            pollInterval: 1000 * 60 * 60 * 24, // due to expiration time of jwt


Wouldn't this cause a location reset when a user stays too long in the site preview?

Yes, it does. Do we ignore it? (as this only happens after 24 hours)

If it's 24 hours, do we even need polling?

The expiry date of the JWT (which is stored in the site as cookie) is validated with every request. If we don't reload the SitePreview every click would result in an error. The problem occurs when a user leaves the tab opened for more than 24 hours and then continues navigation in the preview.

packages/api/cms-api/src/page-tree/site-preview.resolver.ts

packages/site/cms-site/src/sitePreview/pagesRouter/legacyPagesRouterSitePreviewApiHandler.ts

…proxy

nsams · 2024-09-27T07:36:40Z

Regarding the breaking change:

we decided to accept an exceptional braking change in v7
should we add a comet-upgrade script that also upgrades the required deployment changes?
- currently we don't run those for minor upgrades

…proxy

fraxachun · 2024-09-27T09:41:52Z

should we add a comet-upgrade script that also upgrades the required deployment changes?

I don't think this will be necessary. After updating and startup in local dev the following error message will be shown:

BREAKING: this update of Comet v7 requires to have set sitePreviewSecret (which has to be the same value like possibly already set for site). Please refer to vivid-planet/comet-starter#371 for more information on how to upgrade.

The linked PR then shows exactly what to do.

nsams · 2024-09-27T09:52:01Z

The linked PR then shows exactly what to do.

but it doesn't show the required deployment changes

fraxachun · 2024-09-27T10:02:20Z

but it doesn't show the required deployment changes

This is true. But it's nothing more than copying one line. We could add this to the description of the PR.

johnnyomair · 2024-09-30T11:00:38Z

I don't think this will be necessary. After updating and startup in local dev the following error message will be shown:

BREAKING: this update of Comet v7 requires to have set sitePreviewSecret (which has to be the same value like possibly already set for site). Please refer to vivid-planet/comet-starter#371 for more information on how to upgrade.

Wouldn't it be even better if the error message said: Please run npx @comet/upgrade v7/add-site-preview-secret.ts?

An even more crazy thought: What if the library could patch itself (i.e. include the upgrade script). I'm thinking about something like Next patches the typescript file if both Pages and App Router are used: https://github.com/vercel/next.js/blob/canary/packages/next/src/lib/typescript/writeAppTypeDeclarations.ts#L52-L56

.changeset/cold-guests-train.md

…proxy

Script for the breaking change in vivid-planet/comet#2554.

## Description Add deployment based on [DigitalOcean's app platform](https://www.digitalocean.com/products/app-platform). The apps are split per domain to be compatible with routing from the app platform. The preview does not work yet, as I'm waiting for vivid-planet/comet#2554. I decided to not commit the symlinks and copy the files instead to keep the required changes minimal. The secrets are [safe](https://docs.digitalocean.com/products/app-platform/how-to/use-environment-variables/) inside the repo as they are encrypted. This does not include a secondary site, but could be added later.   ## Screenshots/screencasts  ## Related tasks and documents  ## Open TODOs/questions (Future PR) - [ ] Remove dependency to dkarnutsch/oauth2-proxy (no really trustworthy image from Docker Hub found and GitHub registry does not work yet on DigitalOcean) - [ ] Replace domains with starter.comet-dxp.com and admin.starter.comet-dxp.com - [ ] Add CI/CD based on GitHub Actions ## Further information

Since #2554 the site preview isn't anymore behind an authproxy. As a consequence preview urls do not work anymore, but creating normal hashed urls should be sufficient

Allows having the Site Preview unauthenticated ## Description Before: the JWT with the preview scope and setting was generated in the site. To check if the current user is allowed to preview the given scope the site had to make a request to the API and therefore needed the access token of the current user. With this setup the site has to be behind an authproxy. Now: the JWT is generated by the API and submitted to the site. This way the site can be public. ## BREAKING The API now requires the SITE_PREVIEW_SECRET environment variable. To make it explicit to add it this env now is also mandatory for local development.

fraxachun requested review from nsams, thomasdax98 and johnnyomair September 19, 2024 14:03

auto-assign bot assigned fraxachun Sep 19, 2024

fraxachun force-pushed the site-preview-not-authproxy branch from 0a35f45 to d504f9b Compare September 19, 2024 14:03

Create Site Preview JWT in the API

11eb1ed

Allows having the Site Preview unauthenticated

fraxachun force-pushed the site-preview-not-authproxy branch from d504f9b to 11eb1ed Compare September 19, 2024 14:12

fraxachun mentioned this pull request Sep 19, 2024

Allow setting base-path for admin vivid-planet/comet-starter#358

Closed

nsams reviewed Sep 20, 2024

View reviewed changes

thomasdax98 reviewed Sep 23, 2024

View reviewed changes

dkarnutsch mentioned this pull request Sep 23, 2024

Add DigitalOcean Deployment vivid-planet/comet-starter#368

Merged

3 tasks

johnnyomair reviewed Sep 25, 2024

View reviewed changes

fraxachun added 2 commits September 25, 2024 09:19

Merge remote-tracking branch 'origin/main' into site-preview-not-auth…

49f0995

…proxy

Show elaborate comment for breaking change

dea0a1d

fraxachun added 2 commits September 27, 2024 11:26

Merge remote-tracking branch 'origin/main' into site-preview-not-auth…

791b769

…proxy

Add changeset

5d8c99a

johnnyomair reviewed Sep 30, 2024

View reviewed changes

.changeset/cold-guests-train.md Outdated Show resolved Hide resolved

fraxachun added 2 commits October 2, 2024 11:02

Change text

df45f4d

Merge remote-tracking branch 'origin/main' into site-preview-not-auth…

ced0d11

…proxy

thomasdax98 approved these changes Oct 2, 2024

View reviewed changes

thomasdax98 merged commit 671e2b2 into main Oct 2, 2024
11 checks passed

thomasdax98 deleted the site-preview-not-authproxy branch October 2, 2024 14:46

johnnyomair mentioned this pull request Oct 3, 2024

Version Packages #2589

Merged

fraxachun added a commit to vivid-planet/comet-upgrade that referenced this pull request Oct 4, 2024

Add script for vivid-planet/comet#2554

ec57668

johnnyomair mentioned this pull request Oct 7, 2024

Add upgrade script to add the site preview secret to the API vivid-planet/comet-upgrade#22

Merged

johnnyomair pushed a commit to vivid-planet/comet-upgrade that referenced this pull request Oct 7, 2024

Add upgrade script to add the site preview secret to the API (#22)

0f303db

Script for the breaking change in vivid-planet/comet#2554.

fraxachun added a commit that referenced this pull request Oct 21, 2024

Use DAM preview urls only in block preview

19b19fd

Since #2554 the site preview isn't anymore behind an authproxy. As a consequence preview urls do not work anymore, but creating normal hashed urls should be sufficient

fraxachun mentioned this pull request Oct 21, 2024

Use non-preview DAM URLs for the site preview #2644

Merged

fraxachun mentioned this pull request Nov 4, 2024

Move CSP-Headers to middleware vivid-planet/comet-starter#415

Open

fraxachun mentioned this pull request Nov 11, 2024

Block preview credentials #2717

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create Site Preview JWT in the API #2554

Create Site Preview JWT in the API #2554

fraxachun commented Sep 19, 2024 •

edited

Loading

johnnyomair commented Sep 19, 2024

nsams Sep 20, 2024

thomasdax98 Sep 23, 2024

johnnyomair Sep 25, 2024

fraxachun Sep 25, 2024

nsams Sep 27, 2024

nsams commented Sep 20, 2024

thomasdax98 Sep 23, 2024

fraxachun Sep 25, 2024

fraxachun Sep 27, 2024

thomasdax98 Sep 23, 2024

johnnyomair Sep 25, 2024

johnnyomair Sep 25, 2024

fraxachun Sep 25, 2024

johnnyomair Sep 25, 2024

fraxachun Sep 26, 2024 •

edited

Loading

nsams commented Sep 27, 2024 •

edited

Loading

fraxachun commented Sep 27, 2024 •

edited

Loading

nsams commented Sep 27, 2024

fraxachun commented Sep 27, 2024

johnnyomair commented Sep 30, 2024 •

edited

Loading

Create Site Preview JWT in the API #2554

Create Site Preview JWT in the API #2554

Conversation

fraxachun commented Sep 19, 2024 • edited Loading

Description

BREAKING

Open TODOs/questions

johnnyomair commented Sep 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nsams commented Sep 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fraxachun Sep 26, 2024 • edited Loading

Choose a reason for hiding this comment

nsams commented Sep 27, 2024 • edited Loading

fraxachun commented Sep 27, 2024 • edited Loading

nsams commented Sep 27, 2024

fraxachun commented Sep 27, 2024

johnnyomair commented Sep 30, 2024 • edited Loading

fraxachun commented Sep 19, 2024 •

edited

Loading

fraxachun Sep 26, 2024 •

edited

Loading

nsams commented Sep 27, 2024 •

edited

Loading

fraxachun commented Sep 27, 2024 •

edited

Loading

johnnyomair commented Sep 30, 2024 •

edited

Loading