Skip to content

Comments

Server side previewing#209

Closed
Benaiah wants to merge 41 commits intomainfrom
benaiah/server-side-previewing
Closed

Server side previewing#209
Benaiah wants to merge 41 commits intomainfrom
benaiah/server-side-previewing

Conversation

@Benaiah
Copy link
Contributor

@Benaiah Benaiah commented Jun 5, 2023

Jira ticket: LDAF-201

We want to allow content editors to view the site with unpublished changes applied.

Proposed changes

  • Centralizes Contentful client creation into a single place in a server hook called handleToken and uses locals to pass it to the server-side load functions.
  • In the handleToken server hook, if the preview query parameter is present and an access token is provided (either as a ldafUserToken cookie or as an Authorization: Bearer ... header), creates a Contentful client that uses the preview API token and automatically adds a preview variable with the value true to all GraphQL requests.
  • Uses GraphQL for all content requests; removes the Contentful REST client API.
  • Adds an OAuth login flow with Contentful allowing users to sign in to the site and have their token stored in a cookie to be used to authenticate with Contentful and authorize them to access unpublished content in the given space (see "Possible drawbacks" for some negative aspects of this approach).
  • Adds Content-Security-Policy headers that allow Contentful to embed the site (this doesn't work on preview deploys when Vercel deploy protection is turned on).
  • Adds a basic login/logout UI to the page header.
  • Adds authentication error handling.

Screenshots

Logged out:

image

Upon clicking the "Login" button:

image

After approving the login on Contentful's side and going thru the login flow:

image

Video of the login flow:

loginflow-1.mov

Acceptance criteria validation

  • Manually tested logging in and out and accessing unpublished versions of entries
  • Tests added to cover the change

Other details

Alternate solutions

  • We could store the user's management API token and "whether they have been authenticated or not" in a persistent cache like Redis. This would have the advantage of not exposing a Contentful Management API token in a JS-accessible cookie that has to use SameSite=none to work in the Contentful preview pane.

  • We could make the cookie inaccessible to JS by introducing another request on login whose response includes Set-Cookie with the HttpOnly flag.

Possible drawbacks

  • Contentful's preview pane embeds the site as an iframe, so the cookie returned by the OAuth login flow must be set to SameSite=None to include it in cross-origin requests (which includes requests made by an iframe embedded in another domain). On top of that, the current implementation uses a JS-accessible cookie. This is rather insecure, because we are sending the authorization cookie on requests to third-party domains, and any JS in the page has access to the API token. To make things worse, this is a Contentful Management API token (not a delivery token), so it has write access to Contentful equivalent to the user it was created for.

Requested feedback

  • This is a security sensitive PR, so close inspections of the security of the PR and suggestions to improve the security of the implementation are very much wanted.

Benaiah added 2 commits June 5, 2023 15:35
SQUASHME: client creation in Nav

rename contentful implementation file

remove unnecessary type declaration

don't override page data with the document; pass query to frontend

rename contentful/client to contentful/graphqlClient
@vercel
Copy link

vercel bot commented Jun 5, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
ldaf ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 13, 2023 1:36am

Benaiah added 4 commits June 6, 2023 15:57
ignore unused vars that start with _

improve server loading

enable previewing in nav graphql query

WIP

fix import

add management client to try block

delete cookie on logout

don't store the user token in localStorage; let it expire

fix oauth callback page

make sure user is activated

add some debug logging

decode the cookie values

fiddling with the cookie

move logout logic to an action

use sveltekit's fetch function instead of global fetch when possible

debugging

reduce token scope

can we use the user's token instead of a global one?

move codegen config; separate schema and schema type generation

switch default scalar type to "unknown" to avoid "any" pollution

update package scripts to generate schema types correctly

make preview authentication universal

let contentful embed the site

fix package scripts

throw preview authentication errors in layout not handler

fix broken oauth link

request correct token

WIP disable layout error

WIP catch preview auth errors with an actual layout

import component

remove unused preview client

add missing baseURL variable

stop throwing errors in the root layout

make errors more detailed

silly mistake

another silly mistake

set CSP in a hook in addition to vercel settings

try it again

try with x-frame-options

allow cookies to pass to frame

fix sameSite setting

WIP does this work

fix cookie samesite handling

tighten up CSP

improve CSP headers

create the server-side contentful client in a single place

add previewing to all gql queries
@Benaiah Benaiah marked this pull request as ready for review June 8, 2023 22:27
@Benaiah Benaiah requested review from LouisFettet and hinzed1127 June 8, 2023 22:27
@github-actions
Copy link

github-actions bot commented Jun 8, 2023

Coverage after merging benaiah/server-side-previewing into benaiah/graphql-contentful-client will be

89.98%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
src/lib/actions
   logout.ts47.83%0%0%52.38%10–19, 9
src/lib/components/Button
   Button.svelte100%100%100%100%
   buttonOptions.ts100%100%100%100%
   index.ts100%100%100%100%
src/lib/components/Card
   Card.svelte97.50%0%100%100%7
   index.ts100%100%100%100%
src/lib/components/ContentfulRichText
   ContentfulRichText.svelte100%100%100%100%
   headings.ts93.18%50%100%95.12%37–39
   predicates.ts97.87%100%93.33%98.46%63
src/lib/components/ContentfulRichText/nodes
   Blockquote.svelte100%100%100%100%
   Heading1.svelte100%100%100%100%
   Heading2.svelte100%100%100%100%
   Heading3.svelte100%100%100%100%
   Heading4.svelte100%100%100%100%
   Heading5.svelte100%100%100%100%
   Heading6.svelte100%100%100%100%
   Hr.svelte100%100%100%100%
   Hyperlink.svelte100%100%100%100%
   ListItem.svelte100%100%100%100%
   Node.svelte100%100%100%100%
   OrderedList.svelte100%100%100%100%
   Paragraph.svelte100%100%100%100%
   Table.svelte100%100%100%100%
   TableCell.svelte100%100%100%100%
   TableHeaderCell.svelte100%100%100%100%
   TableRow.svelte100%100%100%100%
   Text.svelte100%100%100%100%
   UnorderedList.svelte100%100%100%100%
   index.ts100%100%100%100%
src/lib/components/Header
   Header.svelte93.55%50%80%100%49, 49–50, 50, 53
src/lib/components/Header/Nav
   Nav.svelte97.06%83.33%100%100%19
   NavItem.svelte100%100%100%100%
   NavLink.svelte100%100%100%100%
   NavMenu.svelte98.25%83.33%100%100%59, 68
   index.ts100%100%100%100%
src/lib/components/Header/Title
   Title.svelte100%100%100%100%
   index.ts100%100%100%100%
src/lib/components/Header/User
   User.svelte84.62%0%100%100%13, 16, 16, 18
   index.ts100%100%100%100%
src/lib/components/Icon
   Icon.svelte93.33%33.33%100%100%19, 19
   index.ts100%100%100%100%
src/lib/components/Image
   BlurhashRenderer.svelte100%100%100%100%
   Image.svelte87.31%68.75%33.33%91.98%112, 117–121, 137, 137, 156–157, 17–22, 40, 69, 76–77, 89, 91–92
   generateSources.ts31.58%33.33%0%32.39%10–14, 19–40, 43–49, 5, 50–59, 6, 60–61, 8–9
   index.ts100%100%100%100%
   renderBlurhash.ts100%100%100%100%
src/lib/components/IntersectionObserver
   IntersectionObserver.svelte94.74%75%100%100%36–37, 39
   RootIntersectionObserver.svelte85.19%0%100%92%14–17
   index.ts100%100%100%100%
   key.ts100%100%100%100%
   observe.ts99.03%93.33%100%100%73
src/lib/components/Link
   Link.svelte100%100%100%100%
   index.ts100%100%100%100%
src/lib/components/LoginLink
   LoginLink.svelte100%100%100%100%
src/lib/components/Search
   Search.svelte90.63%33.33%100%96.49%32–34, 39, 39, 43
   index.ts100%100%100%100%
   options.ts100%100%100%100%
src/lib/constants
   support.ts35.71%0%100%40%12–14, 17–25, 4–9
src/lib/contexts
   currentUser.ts100%100%100%100%
src/lib/services/contentful
   graphqlClient.ts75%50%44.44%79.26%121, 123, 123, 50–51, 54–55, 58, 58–75, 80–83, 86–87, 97–98
src/lib/stores
   managementClient.ts50%0%100%52.63%10–17, 8–9
   userInfo.ts51.85%0%100%53.85%13–25
   userToken.ts72.73%0%100%80%6–8
src/lib/util
   classNames.ts100%100%100%100%
   cookie.ts38.10%100%0%43.24%10, 19–32, 35–37, 7–9
   cookieStore.ts26.32%100%0%27.78%10–18, 6–9
   localStorageStore.ts25%100%0%26.32%10–19, 6–9
   warn.ts100%100%100%100%
src/stories
   CardView.svelte99.09%50%100%100%74

@github-actions
Copy link

github-actions bot commented Jun 8, 2023

@Benaiah Benaiah force-pushed the benaiah/server-side-previewing branch from fd33529 to c840cf8 Compare June 12, 2023 23:48
@Benaiah Benaiah force-pushed the benaiah/graphql-contentful-client branch from ff5e920 to b63a6ff Compare June 26, 2023 19:24
Base automatically changed from benaiah/graphql-contentful-client to main June 26, 2023 19:40
@Benaiah
Copy link
Contributor Author

Benaiah commented Jul 1, 2023

Closing in favor of the rebased version (with a lot more added), #274.

@Benaiah Benaiah closed this Jul 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant