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

fix: authState getting wiped on page reload on static websites #785

Open
wants to merge 29 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
6dd86b2
set and reload data, rawToken from cookie
Vijayabhaskar96 Mar 14, 2024
8a36118
fix lint
Vijayabhaskar96 Mar 14, 2024
9b4b8b1
fix typecheck
Vijayabhaskar96 Mar 14, 2024
505c583
Merge branch 'main' into main
zoey-kaiser Apr 5, 2024
534557a
Merge branch 'main' into main
phoenix-ru Apr 18, 2024
d371950
set and reload data, rawToken from cookie
Vijayabhaskar96 Mar 14, 2024
5780a45
fix undefined sessionCookie case
Vijayabhaskar96 Jul 5, 2024
38cb342
Merge remote-tracking branch 'origin/main' into update-pull-request
Vijayabhaskar96 Jul 6, 2024
600d69c
Merge branch 'main' into update-pull-request
Vijayabhaskar96 Jul 6, 2024
d8f0461
fix lint
Vijayabhaskar96 Jul 6, 2024
d814048
fix undefined cases
Vijayabhaskar96 Jul 6, 2024
a43147c
Merge branch 'main' into update-pull-request
zoey-kaiser Jul 13, 2024
6d13eb4
Merge branch 'main' into update-pull-request
zoey-kaiser Jul 19, 2024
fb7a26a
Merge branch 'main' into update-pull-request
zoey-kaiser Aug 8, 2024
e1a122e
Merge branch 'main' into update-pull-request
zoey-kaiser Aug 8, 2024
0060358
Merge branch 'main' into update-pull-request
zoey-kaiser Aug 9, 2024
25ee10e
Merge branch 'main' into update-pull-request
zoey-kaiser Aug 20, 2024
e64a195
Merge branch 'main' into update-pull-request
zoey-kaiser Sep 18, 2024
b0ad984
fix: linting in useAuth
zoey-kaiser Sep 18, 2024
882061f
fix: lint in plugin
zoey-kaiser Sep 18, 2024
907ff2e
fix: trailing spaces
zoey-kaiser Sep 18, 2024
f812357
organize code
Vijayabhaskar96 Sep 19, 2024
e7163cc
Merge branch 'main' into update-pull-request
Vijayabhaskar96 Sep 19, 2024
86feff3
fix typo
Vijayabhaskar96 Sep 19, 2024
75d167e
Merge branch 'update-pull-request' of https://github.com/Vijayabhaska…
Vijayabhaskar96 Sep 19, 2024
0f7e51a
Merge branch 'main' into update-pull-request
Vijayabhaskar96 Oct 29, 2024
7d04329
Merge branch 'sidebase:main' into update-pull-request
Vijayabhaskar96 Dec 5, 2024
2be6b25
Merge branch 'main' into update-pull-request
phoenix-ru Dec 12, 2024
44d1291
Merge branch 'main' into update-pull-request
phoenix-ru Dec 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion src/runtime/composables/local/useAuth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { type UseAuthStateReturn, useAuthState } from './useAuthState'
import { callWithNuxt } from '#app/nuxt'
// @ts-expect-error - #auth not defined
import type { SessionData } from '#auth'
import { navigateTo, nextTick, useNuxtApp, useRoute, useRuntimeConfig } from '#imports'
import { navigateTo, nextTick, useCookie, useNuxtApp, useRoute, useRuntimeConfig } from '#imports'

type Credentials = { username?: string, email?: string, password?: string } & Record<string, any>

Expand Down Expand Up @@ -132,11 +132,22 @@ async function getSession(getSessionOptions?: GetSessionOptions): Promise<Sessio

const headers = new Headers(token ? { [config.token.headerName]: token } as HeadersInit : undefined)

const sessionCookie = useCookie<object | null>('auth:sessionCookie', {
default: () => null,
maxAge: config.token.maxAgeInSeconds,
sameSite: config.token.sameSiteAttribute
})

loading.value = true
try {
const result = await _fetch<any>(nuxt, path, { method, headers })
const { dataResponsePointer: sessionDataResponsePointer } = config.session
data.value = jsonPointerGet<SessionData>(result, sessionDataResponsePointer)

sessionCookie.value = {
lastRefreshedAt: lastRefreshedAt.value,
data: data.value
}
Comment on lines +147 to +150
Copy link
Collaborator

Choose a reason for hiding this comment

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

Storing user data in the cookie may lead to a GDPR violation if the getSession endpoint returns any personal information (which is often the case). It undoubtedly spawns the "protecting personal data" requirement for anyone using the module, even if they weren't affected by the SSG issue in the first place.

https://gdpr.eu/cookies/

I don't find it very attractive to introduce such a burden to NuxtAuth :/

Copy link

Choose a reason for hiding this comment

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

Maybe introducing an option to use SessionStorage nor LocalStorage would be great instead of using cookies ?

Copy link

Choose a reason for hiding this comment

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

If an attacker gains access to a token stored in a Cookie, Session, or LocalStorage, the primary concern is not just identifying the user, but also the ability to access personal data and perform actions the user is authorized to do. This presents a clear security risk.

From a GDPR perspective, the critical question is whether the token can be used to track the user or if it contains personal data. Regardless of where the token is stored, if it can uniquely identify a user or track their actions, it falls under GDPR regulations.

For example, JWTs often store a user identifier in the payload. Since JWTs are not opaque and can be decoded, they may be used to directly identify a user, making them subject to GDPR. Even if the token does not explicitly store "personal data" like an email address, it qualifies as personal data if it can uniquely identify a user (e.g., through a user ID).

In this case, you must inform users via a cookie notice or a similar consent mechanism, ensuring transparency about the use of the token and obtaining their consent where required. However, if the cookies are strictly necessary for the operation of the web application (e.g., to keep users logged in or enable certain features), explicit consent may not be required under GDPR. This generally applies to authentication cookies, afaik.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@thorge You are mostly right. However, auth cookies/jwts don't need to store emails or other identifying information. A website can use "shallow" JWTs storing only user id in it (this is a valid Access Token in NuxtAuth terms). As to whether an ID is "personal data" is very subjective.

However if a website uses such a "shallow" JWT (Access Token) and makes a call to a getSession endpoint to check its validity + get user data, then it's a different thing. For example, internally we optimize such a call to also return user email, name, permissions, etc. on top of a regular JWT validity check. I would prefer to not store this information in a cookie.

Please note that I am also not clearly understanding the usecase of static websites fully. If there is authentication, what is the issue with the session being refetched on page reload?

Copy link

Choose a reason for hiding this comment

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

@phoenix-ru There’s actually no need to store the session itself; we only need to store the access token. That’s what I thought this was about, but I must admit I didn’t examine the changes in detail. Sorry for my tldr answer above :-)

As @Geekimo and @Vijayabhaskar96 suggested, I’d prefer a solution where storing the access token in a cookie (or alternatively in SessionStorage/LocalStorage) is optional. If the session data isn’t available after a client page reload, the access token could be used to retrieve the session data as needed.

}
catch (err) {
if (!data.value && err instanceof Error) {
Expand All @@ -146,6 +157,7 @@ async function getSession(getSessionOptions?: GetSessionOptions): Promise<Sessio
// Clear all data: Request failed so we must not be authenticated
data.value = null
rawToken.value = null
sessionCookie.value = null
}
loading.value = false
lastRefreshedAt.value = new Date()
Expand Down
56 changes: 48 additions & 8 deletions src/runtime/plugin.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
import { getHeader } from 'h3'
import authMiddleware from './middleware/sidebase-auth'
import { getNitroRouteRules } from './utils/kit'
import type { ProviderLocal, SessionCookie } from './types'
import type { CookieRef } from '#app'

Check failure on line 5 in src/runtime/plugin.ts

View workflow job for this annotation

GitHub Actions / test-module

`#app` type import should occur after import of `./utils/url`
import { FetchConfigurationError } from './utils/fetch'
import { resolveApiBaseURL } from './utils/url'
import { _refreshHandler, addRouteMiddleware, defineNuxtPlugin, useAuth, useAuthState, useRuntimeConfig } from '#imports'
import { _refreshHandler, addRouteMiddleware, defineNuxtPlugin, useAuth, useAuthState, useCookie, useRuntimeConfig } from '#imports'

export default defineNuxtPlugin(async (nuxtApp) => {
// 1. Initialize authentication state, potentially fetch current session
const { data, lastRefreshedAt, loading } = useAuthState()
const { data, lastRefreshedAt, rawToken, loading } = useAuthState()
const { getSession } = useAuth()

// use runtimeConfig
Expand Down Expand Up @@ -48,14 +50,52 @@
&& !(isErrorUrl && requireAuthOnErrorPage)

if (shouldFetchSession) {
try {
await getSession()
if (runtimeConfig.provider.type === 'local') {
handleLocalAuth(runtimeConfig.provider)
}
catch (e) {
// Do not throw the configuration error as it can lead to infinite recursion
if (!(e instanceof FetchConfigurationError)) {
throw e

if (!data.value) {
try {
await getSession()
}
catch (e) {
// Do not throw the configuration error as it can lead to infinite recursion
if (!(e instanceof FetchConfigurationError)) {
throw e
}
}
}
}

function handleLocalAuth(config: ProviderLocal): void {
const sessionCookie = useCookie<SessionCookie | null>(
'auth:sessionCookie'
)
const cookieToken = useCookie<string | null>(
config.token?.cookieName ?? 'auth.token'
)

if (sessionCookie?.value && !rawToken?.value && cookieToken?.value) {
restoreSessionFromCookie(sessionCookie, cookieToken)
}
}

function restoreSessionFromCookie(
sessionCookie: CookieRef<SessionCookie | null>,
cookieToken: CookieRef<string | null>
): void {
try {
loading.value = true
const sessionData = sessionCookie.value
lastRefreshedAt.value = sessionData?.lastRefreshedAt
data.value = sessionData?.data
rawToken.value = cookieToken.value
}
catch (error) {
console.error('Failed to parse session data from cookie:', error)
}
finally {
loading.value = false
}
}

Expand Down
11 changes: 11 additions & 0 deletions src/runtime/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -629,3 +629,14 @@ export interface ModuleOptionsNormalized extends ModuleOptions {
globalAppMiddleware: NonNullable<ModuleOptions['globalAppMiddleware']>
originEnvKey: string
}
export interface SessionCookie {
lastRefreshedAt?: SessionLastRefreshedAt
data?: SessionDataObject
}

// Augment types
declare module 'nuxt/schema' {
interface PublicRuntimeConfig {
auth: ModuleOptionsNormalized
}
}
Comment on lines +637 to +642
Copy link
Collaborator

Choose a reason for hiding this comment

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

Question as to why was this added?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I don't remember, but I guess it was causing merge conflicts.

Loading