next-drupal icon indicating copy to clipboard operation
next-drupal copied to clipboard

Cached token

Open MontiMarco92 opened this issue 1 year ago • 3 comments

Package containing the bug

next-drupal (NPM package)

Describe the bug

A clear and concise description of what the bug is.

When having authentication enabled, fetch requests work at first, but then when the token expires, the getAccessToken function is triggered to refresh the token. However, it returns the same previous cached token causing subsequent requests to drupal to fail due to an Unauthorized 401 Error. From what I've seen, this is caused by the fact that Next caches everything by default and the fetch function for this token request does not have any cache or next: { revalidate } prop set.

With the revalidation features proposed on this PR to support NextJs revalidation options, I tried setting a cache: no-store or even a next: {revalidate: 0} prop to the fetch on the getAccessToken However, on production build I've faced a DYNAMIC_SERVER_USAGE error. The only way I worked around this is by actually setting a revalidate time to this. I've set a revalidate: 5. and now the token issue seems to solve, although I'm not really sure if this is the way to go.

Has someone else experienced this ?

  async getAccessToken(
    clientIdSecret?: NextDrupalAuthClientIdSecret
  ): Promise<AccessToken> {
    if (this.accessToken) {
      return this.accessToken
    }

    let auth: NextDrupalAuthClientIdSecret
    if (isClientIdSecretAuth(clientIdSecret)) {
      auth = {
        url: DEFAULT_AUTH_URL,
        ...clientIdSecret,
      }
    } else if (isClientIdSecretAuth(this.auth)) {
      auth = { ...this.auth }
    } else if (typeof this.auth === "undefined") {
      throw new Error(
        "auth is not configured. See https://next-drupal.org/docs/client/auth"
      )
    } else {
      throw new Error(
        `'clientId' and 'clientSecret' required. See https://next-drupal.org/docs/client/auth`
      )
    }

    const url = this.buildUrl(auth.url)

    // Ensure that the unexpired token was using the same scope and client
    // credentials as the current request before re-using it.
    if (
      this.token &&
      Date.now() < this._tokenExpiresOn &&
      this._tokenRequestDetails?.clientId === auth?.clientId &&
      this._tokenRequestDetails?.clientSecret === auth?.clientSecret &&
      this._tokenRequestDetails?.scope === auth?.scope
    ) {
      this.debug(`Using existing access token.`)
      return this.token
    }

    this.debug(`Fetching new access token.`)

    // Use BasicAuth to retrieve the access token.
    const clientCredentials: NextDrupalAuthUsernamePassword = {
      username: auth.clientId,
      password: auth.clientSecret,
    }
    const body = new URLSearchParams({ grant_type: "client_credentials" })

    if (auth?.scope) {
      body.set("scope", auth.scope)

      this.debug(`Using scope: ${auth.scope}`)
    }

    const response = await this.fetch(url.toString(), {
      method: "POST",
      headers: {
        Authorization: await this.getAuthorizationHeader(clientCredentials),
        Accept: "application/json",
        "Content-Type": "application/x-www-form-urlencoded",
      },
      body,
      next: { revalidate: 5 },
    })

    await this.throwIfJsonErrors(
      response,
      "Error while fetching new access token: "
    )

    const result: AccessToken = await response.json()

    this.token = result

    this._tokenRequestDetails = auth

    return result
  }

Expected behavior

Once the token expires, getAccessToken should actually refetch a fresh one, and not return the original cached one.

Steps to reproduce:

  1. First set up authentication on the NextDrupal client
  2. Then wait until token expires
  3. 😢 you get Unauthorized - 401 error, and If you log the used token, you can see is the old one.

Additional context

  • The path where I'm testing this is /[lang]/page.tsx with a generateStaticParams
  • I'm using ClientId + ClientSecret auth.
  • next-drupal npm package 2.0 beta version
  • Nextjs App router v 14.2.3

MontiMarco92 avatar Jul 16 '24 10:07 MontiMarco92

I also see this problem: If you do multiple concurrent authenticated requests with the DrupalClient you do a lot of requests against the oauth token endpoint:

Bildschirmfoto 2024-10-14 um 10 49 23

yobottehg avatar Oct 14 '24 08:10 yobottehg

Hello,

Does anyone know if this fix will be available soon ? If not, how can I properly fix this by knowing that I need to update the node_modules ?

tomzidani avatar Nov 08 '24 04:11 tomzidani

I have the same problem. Is there already a solution for this?

errauch avatar Apr 10 '25 16:04 errauch

I've got this issue on v2 also.

fiasco avatar Jul 27 '25 22:07 fiasco

I’ve encountered the same issue. I tried using a custom fetcher on the Drupal client and setting next with a revalidate value matching the access token expiration time on the associated consumer. However, this didn’t work as expected, and stale (expired) tokens were still being served.

Since the /oauth/token endpoint returns an uncacheable response, the revalidate property likely doesn't work as expected. Even if it did, the expiration value isn’t known upfront (unless manually checked in the UI), which makes this approach inflexible, especially when working across multiple environments and consumers.

In practice, the deprecated getAccessToken() method from next-drupal still seems like the best fit. If we would set cache: 'no-store' on the fetch call, the response wouldn't be cached by Next.js, but is instead stored in Node’s cache with the correct expires_in TTL.

That said, I’d prefer not to reintroduce a deprecated function without maintainer approval, so I haven’t submitted an PR. As a temporary workaround, I’ve copied getAccessToken(), applied cache: 'no-store', and set the function on the Drupal client.

See the following example:

const CACHE_KEY = "NEXT_DRUPAL_ACCESS_TOKEN"
async function getAccessToken(): Promise<AccessToken | null> {
  if (!process.env.DRUPAL_CLIENT_ID || !process.env.DRUPAL_CLIENT_SECRET) {
    return null
  }

  const cached = cache.get<AccessToken>(CACHE_KEY)
  if (cached?.access_token) {
    return cached
  }

  const basic = Buffer.from(
    `${process.env.DRUPAL_CLIENT_ID}:${process.env.DRUPAL_CLIENT_SECRET}`
  ).toString("base64")

  const response = await fetch(
    `${process.env.NEXT_PUBLIC_DRUPAL_BASE_URL}/oauth/token`,
    {
      method: "POST",
      headers: {
        Authorization: `Basic ${basic}`,
        "Content-Type": "application/x-www-form-urlencoded",
      },
      body: `grant_type=client_credentials`,
      cache: 'no-store',
    }
  )

  if (!response.ok) {
    throw new Error(response.statusText)
  }

  const result: AccessToken = await response.json()

  cache.set(CACHE_KEY, result, result.expires_in)

  return result
}

export const drupal = async () =>
  new DrupalClient(process.env.NEXT_PUBLIC_DRUPAL_BASE_URL as string, {
    auth: {
      clientId: process.env.DRUPAL_CLIENT_ID as string,
      clientSecret: process.env.DRUPAL_CLIENT_SECRET as string,
    },
    accessToken: await getAccessToken(),
  })

const drupalClient = await drupal()
const response = await drupalClient.getResourceCollection(...)

bojanbogdanovic avatar Sep 11 '25 11:09 bojanbogdanovic

I've also encountered this issue, got confused, and found that none of the solutions mentioned here solved the issue for me. Before I start it's good to note that my application hasn't hit production just yet, so the problem I've encountered was solely in the development environment.

What I found was that the /oauth/token request was actually cached at only one very specific moment in time. That moment was when a code change was made, and the next dev CLI triggered a restart of the application. When that happened I would get a previous response from /oauth/token which contained an non-new token. NextDrupal would then put that token in it's this.token and put set the this._tokenExpiresOn to the expires_at value from that response, while in reality the expires_at value will be lower because this token has existed for a longer time already. Then comes a point where the token actually expires, but NextDrupal still has it in it's Class state. In that case I would get the Unauthorized error.

I'm not sure this is what everybody here is experiencing, and I'm also not entirely sure this is an issue with NextDrupal as apposed to Next.js itself. However I did found a solution which solved my problem, and will likely solve yours:

diff --git a/dist/index.js b/dist/index.js
index a51ec24efd8d538af78db56c35155857e1ae2a58..a2bc9abb17d1eba1fdababfadba49abb1c04ed96 100644
--- a/dist/index.js
+++ b/dist/index.js
@@ -401,7 +401,7 @@ var NextDrupalBase = class {
         `'clientId' and 'clientSecret' required. See https://next-drupal.org/docs/client/auth`
       );
     }
-    const url = this.buildUrl(auth.url);
+    const url = this.buildUrl(auth.url, { t: Date.now() });
     if (this.token && Date.now() < this._tokenExpiresOn && this._tokenRequestDetails?.clientId === auth?.clientId && this._tokenRequestDetails?.clientSecret === auth?.clientSecret && this._tokenRequestDetails?.scope === auth?.scope) {
       this.debug(`Using existing access token.`);
       return this.token;

This will append a ?t= URL parameter with the current UNIX timestamp, preventing the Next.js caching layer (in any way) from returning a cached / old response for the /oauth/token endpoint ever.

I could draw up a PR if a maintainer can give it's approval on this. It's a fairly harsh solve for the problem, but as @bojanbogdanovic mentioned: "the /oauth/token endpoint returns an uncacheable response" so preventing it from ever being cached like this doesn't seem like such a bad solution.

boazpoolman avatar Sep 15 '25 15:09 boazpoolman

The no-store approach I previously suggested is a no-go because it breaks SSG. The suggestion from @boazpoolman to add a timestamp URL query parameter to the fetch seems like a good fix, but the downside is that it could increase the number of long-lived cached tokens. Alternatively, we could add a low-frequency, time-based revalidation to the request, but this would mean that all fetch requests without a specified revalidation frequency would fall back to this low value.

If you have multiple fetch requests in a statically rendered route, and each has a different revalidation frequency. The lowest time will be used for all requests. For dynamically rendered routes, each fetch request will be revalidated independently. https://nextjs.org/docs/14/app/building-your-application/data-fetching/fetching-caching-and-revalidating

bojanbogdanovic avatar Sep 19 '25 10:09 bojanbogdanovic

FYI: This issue occurs in Next.js 14 because all fetch calls are cached by default. Starting from Next.js 15, the default behavior has been updated, fetch calls are no longer automatically cached, making data fetching more predictable and better aligned with real-time use cases.

bojanbogdanovic avatar Sep 24 '25 12:09 bojanbogdanovic