fix(authentication-oauth): Fix OAuth Callback Account Takeover (#3663)

daffl · web-flow · commit d6b0b5cfbaf6 · 2026-03-04T12:43:39.000-08:00
diff --git a/packages/authentication-oauth/src/service.ts b/packages/authentication-oauth/src/service.ts
@@ -1,6 +1,6 @@
 import { createDebug } from '@feathersjs/commons'
 import { HookContext, NextFunction, Params } from '@feathersjs/feathers'
-import { FeathersError, GeneralError } from '@feathersjs/errors'
+import { FeathersError, GeneralError, NotAuthenticated } from '@feathersjs/errors'
 // eslint-disable-next-line @typescript-eslint/ban-ts-comment
 //@ts-ignore
 import Grant from 'grant/lib/grant'
@@ -115,7 +115,13 @@ export class OAuthService {
       redirect
     }
 
-    const payload = grant?.response || result?.session?.response || result?.state?.response || params.query
+    const payload = grant?.response || result?.session?.response || result?.state?.response
+
+    if (!payload) {
+      throw new NotAuthenticated(
+        'No valid oAuth response. You must initiate the oAuth flow from the authorize endpoint.'
+      )
+    }
     const authentication = {
       strategy: name,
       ...payload
diff --git a/packages/authentication-oauth/test/service.test.ts b/packages/authentication-oauth/test/service.test.ts
@@ -90,6 +90,69 @@ describe('@feathersjs/authentication-oauth service security', () => {
   })
 })
 
+describe('Account Takeover via OAuth Callback Query Parameter Forgery', () => {
+  const port = 9781
+  const req = axios.create({
+    withCredentials: true,
+    maxRedirects: 0
+  })
+  let app: Awaited<ReturnType<typeof expressFixture>>
+
+  const fetchResponse = async (url: string, headers?: Record<string, string>): Promise<AxiosResponse> => {
+    try {
+      return await req.get(url, { headers })
+    } catch (error: any) {
+      return error.response
+    }
+  }
+
+  before(async () => {
+    app = await expressFixture(port, 5118)
+    // Create an existing user to demonstrate account takeover
+    await app.service('users').create({ email: 'admin@example.com' })
+  })
+
+  after(async () => {
+    await app.teardown()
+  })
+
+  it('should not authenticate when calling callback directly with forged query parameters', async () => {
+    const host = `http://localhost:${port}`
+    // Attack: directly hit the callback endpoint with a forged profile in query params
+    // Without a valid OAuth session, Grant returns no response, so the payload
+    // falls back to params.query which the attacker controls
+    const attackUrl = `${host}/oauth/github/callback?profile[foo]=bar&code=fake&state=fake`
+
+    const response = await fetchResponse(attackUrl)
+
+    // The forged request must NOT return a valid access token.
+    // Currently the vulnerable code falls back to params.query as the auth payload,
+    // allowing the attacker to forge authentication and receive a JWT.
+    const hasAccessToken =
+      response.data?.accessToken ||
+      (response.headers.location && response.headers.location.includes('access_token'))
+
+    assert.ok(!hasAccessToken, `Forged callback request should not return an access token but got one`)
+  })
+
+  it('should not authenticate when callback is called with a targeted profile id', async () => {
+    const host = `http://localhost:${port}`
+    // Attack: forge a specific profile ID to target a known user
+    const attackUrl = `${host}/oauth/github/callback?profile[id]=12345&code=fake&state=fake`
+
+    const response = await fetchResponse(attackUrl)
+
+    const hasAccessToken =
+      response.data?.accessToken ||
+      (response.headers.location && response.headers.location.includes('access_token'))
+
+    assert.ok(
+      !hasAccessToken,
+      `Forged callback request with targeted profile ID should not return an access token`
+    )
+  })
+})
+
 describe('@feathersjs/authentication-oauth service', () => {
   const port = 9778
   const req = axios.create({
