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(adapters): add Account mapping before database write #7369

Merged
merged 8 commits into from
Apr 30, 2023

Conversation

balazsorban44
Copy link
Member

@balazsorban44 balazsorban44 commented Apr 26, 2023

Continues #5461

Right now, we pass through all the values of a provider account to the database adapter. It causes confusion/unforeseen breaks when a provider decides to return a new property or deviates from the default documented Adapter schema.

To solve this, we can add an account property to the provider configuration, similar to the profile callback, to control what part of the account should be saved to the database.

In this PR, I restricted the default Account to only save access_token and id_token, anything else will need to be returned via the account() method.

refresh_token and expires_in are also removed from the default case, since we don't implement Refresh Token rotation out-of-the-box currently. See: https://authjs.dev/guides/basics/refresh-token-rotation

We could introduce it via an unstable_tokenRotation: true flag on a Provider's config later, changing the default account mapping to include the required properties.

Reverted the above in f00ac78 to introduce fewer breaking changes. We already save the values and have been for a while, so no point in asking the users to do a database migration.

Fixes #6538
Closes #4893

@vercel
Copy link

vercel bot commented Apr 26, 2023

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

Name Status Preview Comments Updated (UTC)
auth-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 30, 2023 11:16am
2 Ignored Deployments
Name Status Preview Comments Updated (UTC)
auth-docs-nextra ⬜️ Ignored (Inspect) Apr 30, 2023 11:16am
next-auth-docs ⬜️ Ignored (Inspect) Apr 30, 2023 11:16am

@github-actions github-actions bot added adapters Changes related to the core code concerning database adapters core Refers to `@auth/core` providers labels Apr 26, 2023
@@ -165,6 +166,11 @@ export async function handleOAuth(
}
}

if (tokens.expires_in) {
tokens.expires_at =
Math.floor(Date.now() / 1000) + Number(tokens.expires_in)
Copy link
Member Author

Choose a reason for hiding this comment

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

Since we don't use openid-client anymore, this is implemented here now.

Note, we do not save this by default yet, as refresh token rotation is not currently added, but we provide this value to ease migration/support user-land rotation.

@balazsorban44 balazsorban44 merged commit d739e8e into main Apr 30, 2023
7 of 8 checks passed
@balazsorban44 balazsorban44 deleted the feat/account-mapper branch April 30, 2023 11:25
balazsorban44 added a commit that referenced this pull request May 10, 2023
Reverts some changes on #7369 so DB migration won't be needed
balazsorban44 added a commit that referenced this pull request Jun 1, 2023
* feat: map Account before saving to database

* document `acconut()`, explain default behaviour

* generate `expires_at` based on `expires_in`

Fixes #6538

* rename

* strip undefined on `defaultProfile`

* don't forward defaults to account callback

* improve internal namings, types, docs
balazsorban44 added a commit that referenced this pull request Jun 1, 2023
Reverts some changes on #7369 so DB migration won't be needed
ThangHuuVu added a commit that referenced this pull request Sep 23, 2023
* feat(adapter-surrealdb): implemented with unit tests

* chore: update README

* Use stateless DB connection

* Update surrealdb-rest-ts

* chore: bump turbo and pnpm

* chore(docs): fix dynamodb typo (#7130)

fix: typo

* chore: bump pnpm

* chore: update lock file, bump dev dependencies

* chore: run `pnpm install --fix-lockfile`

* chore: re-run pnpm install

* chore: add missing dev dep

* revert lock

* update lock

* use surrealdb.js

* add rest test

* remove commented-out code

* update readme

* modularize repeated code

* fix(docs): fix default `maxAge` formula (#7406)

* feat(adapters): add Account mapping before database write (#7369)

* feat: map Account before saving to database

* document `acconut()`, explain default behaviour

* generate `expires_at` based on `expires_in`

Fixes #6538

* rename

* strip undefined on `defaultProfile`

* don't forward defaults to account callback

* improve internal namings, types, docs

* chore: improve errors, add more docs (#7415)

* JWT Token -> JWT

* document some errors

* improve errors, docs

* fix: loosen profile types

* chore: type fixes

* fix: allow handling OAuth callback error response

related #7407

* fix(docs): remove extra heading

Fixes #7426

* chore: use `@ts-ignore`

* chore: support release any package as experimental

* chore: separate manual release job

* chore: skip test for manual release

* chore: tweak

* chore: tweaks

* chore: tweak manual release version

* Use query instead of select to be able to use query params

* Fix lint errors

* Update surrealdb.js and remove surrealdb-rest-ts in favor of ExperimentalSurrealHTTP

* update pnpm-lock

* fix merge

* fix merge

* fix merge

* migrate surrealdb.js api

* fix queries

* update package.json

* fix types

* prepare for rest

* update readme

* chore: format PR

* Update README.md

* Update package.json

---------

Co-authored-by: Balázs Orbán <info@balazsorban.com>
Co-authored-by: jakzo <jack@jf.id.au>
Co-authored-by: Victor <saptefrativictor@gmail.com>
Co-authored-by: Thang Vu <hi@thvu.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
adapters Changes related to the core code concerning database adapters core Refers to `@auth/core` providers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

decide on expires_in vs. expires_at in @auth/core
2 participants