feat: implement refresh schema by Danielwinkelmann · Pull Request #373 · sidebase/nuxt-auth

Danielwinkelmann · 2023-05-09T17:13:34Z

Implement the local "refresh" schema, it's based on the local schema and extends it.

I used as much code and types as possible from the 'local' schema.

Types are always extended and not rewritten, to maximize the compatibility between the schemas.

Checklist:

manually checked my feature (created playground)
testing not applicable
screenshot not applicable

BracketJohn · 2023-05-11T07:21:02Z

Thanks @Danielwinkelmann ❤️ 💯

This would be a great controbution, I will look into it in a bit (:

Note already: Tests are failing - can you check it out and resolve it?

Danielwinkelmann · 2023-05-11T07:33:53Z

@BracketJohn I updated the lock file

BracketJohn · 2023-05-11T15:58:33Z

Thanks - tests are still failing though, you can see why by clicking "Details: on the failing test. Here's the current state for convenience:

Danielwinkelmann · 2023-05-12T03:38:30Z

@BracketJohn I can't figure out the problem.
The Linter have a problem with the '#imports' in the playground.
I never touched the auth-js playground.
If i remove everything in the playground he shows the same issue on the local playground and so on.

It must be an dependency problem.

Maybe we should disable the Listing Rule "import/namespace"

BracketJohn · 2023-05-12T08:07:45Z

I figured it out! The problem is that you made extends of the root tsconfig an array which eslint-typescript cannot handle.

I:

fixed it,
added a CI for the refresh provider,
fixed a missing session-data from the main-merge I performed

I wanted to push the fix into your branch but don't have permissions on the fork. So for the time being here's the diff:

diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
index ab2ac7c..94bff65 100644
--- a/.github/workflows/ci.yaml
+++ b/.github/workflows/ci.yaml
@@ -67,8 +67,6 @@ jobs:
       # start prod-app and curl from it
       - run: "timeout 60 pnpm start & (sleep 45 && curl --fail localhost:3000)"
 
-
-
   test-playground-authjs:
     runs-on: ubuntu-latest
     defaults:
@@ -99,3 +97,34 @@ jobs:
         env:
           AUTH_ORIGIN: 'http://localhost:3001'
           PORT: 3001
+
+  test-playground-refresh:
+    runs-on: ubuntu-latest
+    defaults:
+      run:
+        working-directory: ./playground-refresh
+    steps:
+      - uses: actions/checkout@v3
+
+      - name: Use Node.js 16.14.2
+        uses: actions/setup-node@v3
+        with:
+          node-version: 16.14.2
+
+      - uses: pnpm/action-setup@v2
+        name: Install pnpm
+        id: pnpm-install
+        with:
+          version: 8
+
+      # Install deps
+      - run: pnpm i
+
+      # Check building
+      - run: pnpm build
+
+      # start prod-app and curl from it
+      - run: "timeout 60 pnpm start & (sleep 45 && curl --fail localhost:$PORT)"
+        env:
+          AUTH_ORIGIN: 'http://localhost:3002'
+          PORT: 3002
diff --git a/src/module.ts b/src/module.ts
index 72983f3..314d3e4 100644
--- a/src/module.ts
+++ b/src/module.ts
@@ -60,7 +60,8 @@ const defaultsByBackend: { [key in SupportedAuthProviders]: DeepRequired<Extract
     refreshToken: {
       signInResponseRefreshTokenPointer: '/refreshToken',
       maxAgeInSeconds: 60 * 60 * 24 * 7 // 7 days
-    }
+    },
+    sessionDataType: { id: 'string | number' }
   },
   authjs: {
     type: 'authjs',
diff --git a/tsconfig.json b/tsconfig.json
index 92f65fd..2a7b6fa 100644
--- a/tsconfig.json
+++ b/tsconfig.json
@@ -1,6 +1,6 @@
 {
   // Note: we need to explicitly extend one of the playgrounds here in order to have a nice and working typecheckk, otherwise `nuxt`-types are going to be unknown
-  "extends": ["./playground-local/.nuxt/tsconfig.json", "./playground-refresh/.nuxt/tsconfig.json"],
+  "extends": "./playground-refresh/.nuxt/tsconfig.json",
   "exclude": ["../docs"],
   "include": ["src/"]
 }

This reverts commit 1ecd9d8.

BracketJohn · 2023-05-12T08:45:32Z

Next problem:

This test basically checks whether the app launches and the / page is reachable without problems. Here it fails with a 403 for the refresh provider

BracketJohn

Can you please also extends the docs for this feature?

You'd only need to focus on the v0.6 docs. There I remember at least the following places that need an update:

docs/content/v0.6/1.getting-started/1.index.md -> update the feature table
docs/content/v0.6/1.getting-started/3.quick-start.md -> add a refresh specific section (can also be very small if there's nothing special to consider)
docs/content/v0.6/2.configuration/2.nuxt-config.md -> add provider specific config options
docs/content/v0.6/3.application-side/2.session-access-and-management.md -> add provider specific section

If in some cases refresh behaves the same as local provicder you can of course keep the sections small and reference the local provider

BracketJohn · 2023-05-14T09:34:18Z

src/runtime/server/plugins/refresh-token.server.ts

@@ -0,0 +1,39 @@
+import { _fetch } from '../../utils/fetch'


Here I have some points:

Do we need this plugin? Can we not rely on the server-side execution of the plugin.ts we already defined on the app to refresh the token on load?

The existance of this plugin forces the user to have a server, without it refresh could also work for fully-static apps. So if we keep this plugin (depends on the answer to my previous question) we should at least make it opt-in to ensure that static apps still work

BracketJohn · 2023-05-14T09:35:26Z

src/runtime/server/plugins/refresh-token.server.ts

+  name: 'refresh-token-plugin',
+  enforce: 'pre',
+  async setup (nuxtApp) {
+    // TODO: get rid of duplicate code


I think you can resolve this now?

You could add helpers into the refresh/ composable folder and import them here - I guess it's mostly about pointer extraction?

If you want to go a bit further you could add a more generic pointer extraction helper that can be used here, in your useAuth composable and in the local useAuth composable

zoey-kaiser · 2023-07-26T14:31:30Z

Hi @Danielwinkelmann 👋

I am going through some PRs and noticed yours is still open! Do you still want to continue with it or should we see that we take this over? 🤗

dommi10 · 2023-07-31T07:08:39Z

Hi @zoey-kaiser can you create a beta based on this pull request please! this is very helpfull, litteraly what the package need to support local provider

Danielwinkelmann · 2023-07-31T08:42:25Z

@zoey-kaiser Sorry for delay. Had a litte bit to much private stuff going on.
I will continue working on it this week.
The missing parts just documentation things and the (pre) server plugin.
@BracketJohn I thing we can let the server plugin exist, I will only need for SSR applications, to make sure to refresh the tokens, before making an api call from an other plugin

zoey-kaiser · 2023-07-31T14:08:27Z

The missing parts just documentation things and the (pre) server plugin.

Sounds good! Ill get around to another review by Wednesday and then I can release a pre-version, so @dommi10 can test it!

dommi10 · 2023-07-31T17:06:52Z

@zoey-kaiser i’m so excited to test

zoey-kaiser · 2023-08-02T14:22:36Z

Hi @dommi10

I released a test version of this branch in a new package, as there is no documentation yet and there are still some kinks to iron out. However, I did want to let you have a look at it and see if everything works in your projects:

https://www.npmjs.com/package/sidebase-auth-refresh

Feedback is super appreciated, maybe you can find some issues or have some improvement tips! Once we merge this and finish the documentation you should be able to switch to the official package! This one is just for temporary testing.

dommi10 · 2023-08-03T08:30:47Z

Hi @dommi10

I released a test version of this branch in a new package, as there is no documentation yet and there are still some kinks to iron out. However, I did want to let you have a look at it and see if everything works in your projects:

https://www.npmjs.com/package/sidebase-auth-refresh

Feedback is super appreciated, maybe you can find some issues or have some improvement tips! Once we merge this and finish the documentation you should be able to switch to the official package! This one is just for temporary testing.

Yes for sure

dommi10 · 2023-08-03T12:11:24Z

Hi @dommi10

I released a test version of this branch in a new package, as there is no documentation yet and there are still some kinks to iron out. However, I did want to let you have a look at it and see if everything works in your projects:

https://www.npmjs.com/package/sidebase-auth-refresh

Feedback is super appreciated, maybe you can find some issues or have some improvement tips! Once we merge this and finish the documentation you should be able to switch to the official package! This one is just for temporary testing.

Still have an issue when you select refresh as provider, the entire app crash

zoey-kaiser · 2023-08-09T15:19:20Z

Published a new test version:

https://www.npmjs.com/package/sidebase-auth-refresh

Should be available under 0.6.0-beta.5

dommi10 · 2023-08-09T16:19:12Z

Published a new test version:

https://www.npmjs.com/package/sidebase-auth-refresh

Should be available under 0.6.0-beta.5

Let’s us test again

zoey-kaiser · 2023-08-10T11:37:27Z

Let’s us test again

Let me know how it goes! If everything works well I want to aim to publish it properly next week!

dommi10 · 2023-08-10T11:39:10Z

Published a new test version:

https://www.npmjs.com/package/sidebase-auth-refresh

Should be available under 0.6.0-beta.5

An issues on logout redirect and issus when you try to refresh the token it doesn't add AUTORIZATION HEADER

dommi10 · 2023-08-10T11:45:53Z

An issues on logout redirect and issus when you try to refresh the token it doesn't add AUTORIZATION HEADER

An issues on logout redirect and issus when you try to refresh the token it doesn't add AUTORIZATION HEADER

zoey-kaiser · 2023-08-10T12:19:34Z

An issues on logout redirect and issus when you try to refresh the token it doesn't add AUTORIZATION HEADER

Okay good to know! Thank you for the initial testing 💪
Would you be so kind to send over a reproduction to the repo where this occured? Then I can look into the source code here and make some suggestions 🤗

dommi10 · 2023-08-10T12:39:00Z

An issues on logout redirect and issus when you try to refresh the token it doesn't add AUTORIZATION HEADER

Okay good to know! Thank you for the initial testing 💪 Would you be so kind to send over a reproduction to the repo where this occured? Then I can look into the source code here and make some suggestions 🤗

Error on Redirect after logout

No Autorization Header in refresh token request

dommi10 · 2023-08-14T12:56:35Z

An issues on logout redirect and issus when you try to refresh the token it doesn't add AUTORIZATION HEADER

Okay good to know! Thank you for the initial testing 💪 Would you be so kind to send over a reproduction to the repo where this occured? Then I can look into the source code here and make some suggestions 🤗

Error on Redirect after logout

No Autorization Header in refresh token request

@zoey-kaiser !!

MaxNvk · 2023-08-16T00:38:58Z

src/runtime/server/plugins/refresh-token.server.ts

+    // TODO: get rid of duplicate code
+    const { rawToken, rawRefreshToken, refreshToken, lastRefreshedAt } = useAuthState()
+    if (refreshToken.value) {
+      const config = nuxtApp.$config.auth


I think that you forgot to put .public. level for the path. But actually, you can use useRuntimeConfig().public.auth like in the /src/runtime/plugin.ts for code consistency.

clcoco · 2023-09-25T14:54:42Z

Any update on this?

akiot-b · 2023-10-23T04:11:29Z

I originally used nuxtjs/auth and the server side was running refresh token with express. is the sidebase/nuxt-auth refresh token going to work with third party servers such as express?

dommi10 · 2023-10-24T07:04:15Z

I originally used nuxtjs/auth and the server side was running refresh token with express. is the sidebase/nuxt-auth refresh token going to work with third party servers such as express?

This specific version can answer our questions

@zoey-kaiser any updates please ?? another beta please based on new modifications made by @MaxNvk

JeppePepp · 2023-11-20T18:45:52Z

this would be amazing to get asap

fix bugs on reference of runtime config add headers on refresh methode Refs: sidebase#373

zoey-kaiser · 2023-11-30T10:07:51Z

Taken over and merged in #581

Danielwinkelmann added 2 commits May 9, 2023 19:11

feat: implement refresh schema

bb927d1

fix: move initial refresh of the token to a server plugin

b99bc63

chore: update lockfile

e12c0cb

Merge branch 'main' into main

39da510

Merge branch 'main' into main

d55fe8b

Merge branch 'main' into main

093e3a5

Danielwinkelmann and others added 9 commits May 12, 2023 10:24

feat: add ci

1ecd9d8

Revert "feat: add ci"

908b640

This reverts commit 1ecd9d8.

Merge branch 'main' of https://github.com/Danielwinkelmann/nuxt-auth

a49e26a

feat(ci): add local-refresh

1dbc4e9

Merge branch 'main' into main

df01a51

fix: add sessionDataType to defaults

3a6f934

Merge branch 'main' of https://github.com/Danielwinkelmann/nuxt-auth

48bb760

fix: change ci version

024a4b6

fix: copy-pasta mistakes

6bcfc51

Danielwinkelmann and others added 2 commits May 12, 2023 10:52

fix: refresh times in sample and null check in the plugin

0597444

Merge branch 'main' into main

e3afb79

BracketJohn reviewed May 14, 2023

View reviewed changes

Danielwinkelmann added 2 commits May 14, 2023 15:43

Merge branch 'main' into main

614c9ce

Merge branch 'main' into main

69aa103

BracketJohn mentioned this pull request Jun 28, 2023

Provider: Implement crypto-wallet-based authentication #467

Closed

3 tasks

Merge branch 'main' into main

3576873

zoey-kaiser changed the base branch from main to Danielwinkelmannimplement-refresh-schema August 2, 2023 13:54

zoey-kaiser changed the base branch from Danielwinkelmannimplement-refresh-schema to main August 2, 2023 13:55

MaxNvk reviewed Aug 16, 2023

View reviewed changes

kingychiu mentioned this pull request Aug 27, 2023

Feature request about the incoming Local Auth Scheme #520

Closed

zoey-kaiser mentioned this pull request Oct 11, 2023

Is there an EXAMPLE of a refresh scheme? (Migrating from @nuxtjs/auth-next) #407

Closed

zoey-kaiser added the provider:new label Oct 11, 2023

dommi10 added a commit to dommi10/nuxt-auth that referenced this pull request Nov 21, 2023

feat: implement refresh provider schema

d60c0ca

fix bugs on reference of runtime config add headers on refresh methode Refs: sidebase#373

dommi10 mentioned this pull request Nov 21, 2023

feat: implement refresh provider schema #581

Merged

4 tasks

zoey-kaiser closed this Nov 30, 2023

Uh oh!

Conversation

Danielwinkelmann commented May 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BracketJohn commented May 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Danielwinkelmann commented May 11, 2023

Uh oh!

BracketJohn commented May 11, 2023

Uh oh!

Danielwinkelmann commented May 12, 2023

Uh oh!

BracketJohn commented May 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BracketJohn commented May 12, 2023

Uh oh!

BracketJohn left a comment

Choose a reason for hiding this comment

Uh oh!

BracketJohn May 14, 2023

Choose a reason for hiding this comment

Uh oh!

BracketJohn May 14, 2023

Choose a reason for hiding this comment

Uh oh!

zoey-kaiser commented Jul 26, 2023

Uh oh!

dommi10 commented Jul 31, 2023

Uh oh!

Danielwinkelmann commented Jul 31, 2023

Uh oh!

zoey-kaiser commented Jul 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dommi10 commented Jul 31, 2023

Uh oh!

zoey-kaiser commented Aug 2, 2023

Uh oh!

dommi10 commented Aug 3, 2023

Uh oh!

dommi10 commented Aug 3, 2023

Uh oh!

zoey-kaiser commented Aug 9, 2023

Uh oh!

dommi10 commented Aug 9, 2023

Uh oh!

zoey-kaiser commented Aug 10, 2023

Uh oh!

dommi10 commented Aug 10, 2023

Uh oh!

dommi10 commented Aug 10, 2023

Uh oh!

zoey-kaiser commented Aug 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dommi10 commented Aug 10, 2023

Error on Redirect after logout

No Autorization Header in refresh token request

Uh oh!

dommi10 commented Aug 14, 2023

Error on Redirect after logout

No Autorization Header in refresh token request

Uh oh!

MaxNvk Aug 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

clcoco commented Sep 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

akiot-b commented Oct 23, 2023

Uh oh!

dommi10 commented Oct 24, 2023

Uh oh!

JeppePepp commented Nov 20, 2023

Uh oh!

zoey-kaiser commented Nov 30, 2023

Danielwinkelmann commented May 9, 2023 •

edited

Loading

BracketJohn commented May 11, 2023 •

edited

Loading

BracketJohn commented May 12, 2023 •

edited

Loading

zoey-kaiser commented Jul 31, 2023 •

edited

Loading

zoey-kaiser commented Aug 10, 2023 •

edited

Loading

MaxNvk Aug 16, 2023 •

edited

Loading

clcoco commented Sep 25, 2023 •

edited

Loading