nuxt-community / auth-module

Zero-boilerplate authentication support for Nuxt 2
https://auth.nuxtjs.org
MIT License
1.93k stars 924 forks source link

fix(openIDConnect scheme): Check token expiration before id_token #1684

Closed tobyreid closed 2 years ago

tobyreid commented 2 years ago

(Creating this pull request as it appears that it's stalled awaiting a PR into a another fork, rather than the dev branch)

Incorporates both changes from dygeenen and shollingsworth for this PR https://github.com/nuxt-community/auth-module/pull/1417

(thanks both)

This ordering is required, because it is the access_token is sent in the Authorization header when making an axios request. If the access_token is expired, we need to refresh that token before continuing.

Checking whether the id_token has expired doesn't guarantee a successful call to the (a) back end and typically the id_token has a different (shorter) expiry schedule to the access_token.

tobyreid commented 2 years ago

@Intevel is this something I can bring your attention to?

Intevel commented 2 years ago

This looks clear to me. Good job! 💚

OfficialBAMM commented 2 years ago

Can confirm this fixes the issue.

Using version: "5.0.0-1643365392.dae6516" with Keycloak

m-brgs commented 2 years ago

@Intevel

Hi Can we get this merged et released please ?

Intevel commented 2 years ago

I am not a maintainer, I am not authorized to merge.

m-brgs commented 2 years ago

@bmulholland Hi ! Are you a maintainer or do you know someone who's able to merge this and trigger a release to npm ?

Intevel commented 2 years ago

He is able to merge this, but he does all this voluntarily. A ping is just annoying

bmulholland commented 2 years ago

Yep, agreed, thanks @Intevel

On the change itself: it needs a comment explaining why the ordering needs to be as it is. That way we'll know not to accidentally break it in future. Can you please put (an abbreviated version of) the PR description as a comment in the code?

shollingsworth commented 2 years ago

The PR has been updated with the requested comment.

bmulholland commented 2 years ago

Thanks for the fix, explanation, and for dealing with the annoying linter :)