Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(@aws-amplify/auth): UniversalStorage for SSR #5710

Closed
wants to merge 8 commits into from

Conversation

ericclemmons
Copy link
Contributor

@ericclemmons ericclemmons commented May 7, 2020

Why

window.localStorage isn't available to server-side processes, unless credentials are explicitly sent via headers, query params, or POST body.

However, cookies are, which means that loading https://example.com/profile can render a user-specific page on the server without making a round-trip on the client as you would in a SPA.

What

Add support for Next.js (#5435) via a new UniversalStorage adapter that persists the minimum subset of credentials in cookies, so that they're available on the server for authenticated requests.

The following real, working examples come from my private sample: https://github.com/aws-amplify/amplify-js-samples-staging/pull/90

// /pages/index.tsx

import { CognitoUser } from '@aws-amplify/auth'
import { GRAPHQL_AUTH_MODE } from '@aws-amplify/api-graphql'
import { Amplify, API, Auth, UniversalStorage } from 'aws-amplify'
import awsconfig from '../src/aws-exports'

const storage = new UniversalStorage()
Amplify.configure({ ...awsconfig, storage })

type Props = { user?: CognitoUser }

export default function Home(props: Props) {
	...
}

export const getServerSideProps: GetServerSideProps = async (context) => {
  storage.setServerContext(context)

  let user = null

  try {
    user = await Auth.currentAuthenticatedUser({ bypassCache: false })
  } catch (error) {
    console.error(error)
  }

  return {
    props: {
      user: JSON.parse(JSON.stringify(user)),
    },
  }
}
// /pages/api/profile.tsx
import { Amplify, Auth, UniversalStorage } from 'aws-amplify'
import { NextApiRequest, NextApiResponse } from 'next'

import awsconfig from '../../src/aws-exports'

const storage = new UniversalStorage()
Amplify.configure({ ...awsconfig, storage })

export default async (req: NextApiRequest, res: NextApiResponse) => {
  storage.setServerContext({ req })

  try {
    const user = await Auth.currentAuthenticatedUser()

    return res.status(200).json({ user })
  } catch (error) {
    console.error(error)
    return res.status(500).json({ error })
  }
}

How

  • Use isomorphic-unfetch to support SSR fetch for Cognito
  • Add UniversalStorage from my sample to be exported from both aws-amplify and @aws-amplify/core.
    • Only authToken and idToken are persisted for the server.
    • All tokens persist in localStorage, as usual.
  • Determine if UniversalStorage is the default in the browser or not.
    • Opting not to do this, because UniversalStorage.setContext(ctx) is still required per-request.
  • Works with User Pools
  • Works with https://docs.aws.amazon.com/general/latest/gr/signature-version-4.html (Thanks you @elorzafe!)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@ericclemmons ericclemmons added this to the Support SSR (Auth + API) milestone May 7, 2020
@ericclemmons ericclemmons self-assigned this May 7, 2020
@codecov
Copy link

codecov bot commented May 7, 2020

Codecov Report

Merging #5710 into main will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #5710      +/-   ##
==========================================
+ Coverage   73.48%   73.51%   +0.02%     
==========================================
  Files         205      204       -1     
  Lines       12611    11915     -696     
  Branches     2457     2335     -122     
==========================================
- Hits         9267     8759     -508     
+ Misses       3153     2979     -174     
+ Partials      191      177      -14     
Impacted Files Coverage Δ
packages/core/src/Util/DateUtils.ts 66.66% <0.00%> (-15.69%) ⬇️
packages/datastore/src/datastore/datastore.ts 71.68% <0.00%> (-15.01%) ⬇️
packages/core/src/Logger/ConsoleLogger.ts 72.00% <0.00%> (-12.00%) ⬇️
packages/datastore/src/sync/merger.ts 28.57% <0.00%> (-6.73%) ⬇️
packages/datastore/src/storage/storage.ts 67.59% <0.00%> (-4.08%) ⬇️
packages/core/src/I18n/index.ts 65.85% <0.00%> (-2.44%) ⬇️
packages/datastore/src/util.ts 73.72% <0.00%> (-1.89%) ⬇️
packages/auth/src/OAuth/OAuth.ts 48.12% <0.00%> (-1.52%) ⬇️
packages/api-rest/src/RestAPI.ts 95.60% <0.00%> (-1.46%) ⬇️
packages/api-graphql/src/GraphQLAPI.ts 83.70% <0.00%> (-1.13%) ⬇️
... and 35 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a75e9c3...e4cd76d. Read the comment docs.

@ericclemmons ericclemmons requested a review from elorzafe May 19, 2020 22:33
@ericclemmons ericclemmons marked this pull request as ready for review May 19, 2020 22:33
@@ -48,6 +48,7 @@ export {
Signer,
I18n,
ServiceWorker,
UniversalStorage,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a breaking change that we would need to do a Major publish for?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be a minor.

@ericclemmons ericclemmons changed the title feat(@aws-amplify/auth): Support SSR feat(@aws-amplify/auth): UniversalStorage for SSR May 19, 2020
Comment on lines +35 to +36
: // @ts-ignore Argument of type 'Record<string, string>' is not assignable to parameter of type 'Pick<any, "req"> | { req: any; }'.
// Property 'req' is missing in type 'Record<string, string>' but required in type '{ req: any; }'.ts(2345)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few // @ts-ignore comments that exist because of nookies' forked behavior between server & client. On the server, context is { req, res }, while on the browser, it comes from document.cookies.

@amhinson amhinson changed the base branch from master to main June 18, 2020 18:49
@ericclemmons ericclemmons mentioned this pull request Jun 23, 2020
17 tasks
// @ts-ignore Argument of type 'Record<string, string>' is not assignable to parameter of type 'Pick<any, "res"> | { res: any; }'.
// Property 'res' is missing in type 'Record<string, string>' but required in type '{ res: any; }'.ts(2345)
nookies.set(this.store, key, value, {
maxAge: 30 * 24 * 60 * 60,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this so that it's session storage only.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Frameworks take care of XSRF.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Set HTTP-only (document.cookie) & Secure (HTTPS) for live environments.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't set it to the top-level domain.

@ericclemmons
Copy link
Contributor Author

Fixed via #6146

@github-actions
Copy link

github-actions bot commented Sep 3, 2021

This pull request has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server *-help channels or Discussions for those types of questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants