CVE Patch Review

GHSA-G72G-R7M4-9X4G: Root-Cause Fix for OAuth Token Persistence After Password Change in NocoDB

GHSA-G72G-R7M4-9X4G · Updated 2026-06-06 Root-cause

Summary

NocoDB's user credential-change flows (password change, password reset, forgot-password) did not revoke existing OAuth tokens, violating the principle of Insufficient Session Expiration (CWE-613). PR #13599 introduces OAuthToken.revokeAllByUser() and calls it at every credential-change code path. The fix correctly targets the root cause and is well-scoped, though reviewers should verify database-level revocation semantics and token introspection propagation.

Analysis

Vulnerability

Advisory GHSA-G72G-R7M4-9X4G documents an Insufficient Session Expiration weakness (CWE-613) in NocoDB. When a user changed their password, reset it via a reset link, or triggered a forgot-password flow, the application correctly invalidated the user's own session and refresh tokens — but it did not revoke any OAuth tokens that had been issued to third-party clients on behalf of that user.

The practical impact is significant: an attacker who has previously obtained a valid OAuth access token (e.g., through phishing, token leakage, or a compromised OAuth client) retains full API access even after the legitimate user changes their credentials to remediate the compromise. This directly undermines the security guarantee that a password change provides.

The vulnerable import line in users.service.ts shows that OAuthToken was simply absent from the credential-change code paths:

// VULNERABLE — OAuthToken not imported or called during password change/reset
import { PresignedUrl, User, UserRefreshToken } from '~/models';

Because no revocation call existed, all OAuth grants issued to a user persisted indefinitely across credential-change events, violating the security expectation that credential rotation invalidates all delegated access.

Patch

Pull request #13599 addresses the vulnerability across two files.

1. New model method — OAuthToken.revokeAllByUser()

A static helper is added to packages/nocodb/src/models/OAuthToken.ts that fetches all tokens belonging to a user and revokes each one individually:

// packages/nocodb/src/models/OAuthToken.ts  [PATCHED]
static async revokeAllByUser(userId: string, ncMeta = Noco.ncMeta) {
  const tokens = await this.listByUser(userId, ncMeta);
  if (tokens?.length) {
    await Promise.all(tokens.map((t) => this.revoke(t.id, ncMeta)));
  }
}

The method uses Promise.all for concurrent revocation, which is appropriate given that a user may hold multiple OAuth grants. The guard on tokens?.length prevents unnecessary work when no tokens exist.

2. Integration into credential-change flows — users.service.ts

OAuthToken is added to the model imports and revokeAllByUser is called at every credential-change code path (password change, password reset, forgot-password):

// packages/nocodb/src/services/users/users.service.ts  [PATCHED]
import { OAuthToken, PresignedUrl, User, UserRefreshToken } from '~/models';

// Called in: changePassword, resetPassword, forgotPassword handlers
await OAuthToken.revokeAllByUser(user.id);

3. Explicit non-revocation on sign-out (intentional design decision)

The patch also documents — via inline comment — that sign-out deliberately does not revoke OAuth tokens, because sign-out ends only the user's own browser session, not third-party client grants:

// OAuth tokens are not revoked on sign-out: sign-out ends the user's
// own session, not third-party OAuth client grants. OAuth tokens are
// revoked on password change/reset/forgot where credential compromise
// is assumed.

This is a correct and standards-aligned design choice consistent with OAuth 2.0 semantics (GHSA-G72G-R7M4-9X4G).

Review

Pros

  • Root-cause addressed: The fix targets the exact missing revocation calls rather than adding a compensating control elsewhere. All three credential-change flows (change, reset, forgot) are covered.
  • Minimal blast radius: The change is surgical — a new model method and import additions — with no refactoring of unrelated logic, reducing regression risk.
  • Concurrent revocation: Using Promise.all in revokeAllByUser avoids sequential database round-trips for users with many OAuth grants, which is a sound performance choice.
  • Intentional sign-out behavior documented: The inline comment clarifying why sign-out does not revoke OAuth tokens prevents future developers from incorrectly treating this as a bug and adding an undesired revocation there.
  • Consistent placement: Revocation is co-located with the existing UserRefreshToken invalidation logic, making the security invariant easy to audit: wherever refresh tokens are revoked, OAuth tokens are now also revoked.

Cons

  • No atomic transaction guarantee: Promise.all revokes tokens concurrently but not atomically. If one revocation fails mid-flight, some tokens may remain active. A transactional bulk-delete or a single DELETE WHERE userId = ? query would be more robust and performant.
  • Token introspection / caching not addressed: If any OAuth resource server or middleware caches token validity (e.g., in-memory or Redis), revocation in the database alone may not immediately invalidate active bearer tokens until the cache TTL expires. The patch does not address this layer.
  • No test coverage visible in the diff: The patch source digest does not include new unit or integration tests asserting that OAuth tokens are absent after a password change. Without tests, regression risk is elevated.
  • Error handling in revokeAllByUser: If this.revoke(t.id) throws for one token, Promise.all will reject and the calling service may surface an error to the user or silently swallow it depending on the caller's error handling, potentially leaving remaining tokens un-revoked.
  • Scope of OAuth grant revocation: The patch revokes tokens at the NocoDB data layer, but does not appear to notify any upstream OAuth authorization server (if one exists in the deployment). In federated setups this could leave dangling grants.

Verdict

Root-cause. The patch directly eliminates the missing revocation calls that constitute the vulnerability described in GHSA-G72G-R7M4-9X4G and tracked in the CVE report. The implementation is clean, well-scoped, and correctly models the security boundary between user session termination and OAuth grant revocation. The primary outstanding concerns are the lack of atomicity in bulk revocation, the absence of visible test coverage, and the potential for cached token validity to delay enforcement — all of which should be addressed in follow-up work before considering the vulnerability fully mitigated in production deployments that use token caching layers.

Sources