Skip to content

Commit 26eb067

Browse files
authored
fix: set isMultiStrategy to true when array of strategies is provided (#1327)
* fix: set isMultiStrategy to true when array of strategies is provided Fixes #1326 - Fixed AuthenticationRoute constructor to properly set isMultiStrategy = true when Array.isArray(strategyOrStrategies) is true - This ensures that multi-strategy callbacks receive an array of statuses instead of a single status when all strategies fail - Added comprehensive tests to verify the fix works correctly for both single and multi-strategy scenarios - All existing tests continue to pass, ensuring no regressions * fix: resolve linting issues in multi-strategy-callback test
1 parent 8485f2e commit 26eb067

File tree

2 files changed

+158
-1
lines changed

2 files changed

+158
-1
lines changed

src/AuthenticationRoute.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ export class AuthenticationRoute<StrategyOrStrategies extends string | Strategy
9090
// This is typically used on API endpoints to allow clients to authenticate using their preferred choice of Basic, Digest, token-based schemes, etc. It is not feasible to construct a chain of multiple strategies that involve redirection (for example both Facebook and Twitter), since the first one to redirect will halt the chain.
9191
if (Array.isArray(strategyOrStrategies)) {
9292
this.strategies = strategyOrStrategies
93-
this.isMultiStrategy = false
93+
this.isMultiStrategy = true
9494
} else {
9595
this.strategies = [strategyOrStrategies]
9696
this.isMultiStrategy = false
Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
import { test, describe } from 'node:test'
2+
import assert from 'node:assert'
3+
import { getRegisteredTestServer } from './helpers'
4+
import { Strategy } from '../src/strategies'
5+
6+
// Strategy that always fails with a specific status
7+
class FailingStrategy extends Strategy {
8+
constructor (name: string, private status: number = 401) {
9+
super(name)
10+
}
11+
12+
authenticate (_request: any, _options?: { pauseStream?: boolean }) {
13+
this.fail(this.status)
14+
}
15+
}
16+
17+
const testSuite = (sessionPluginName: string) => {
18+
describe(`${sessionPluginName} tests`, () => {
19+
test('should call callback with single status for single strategy failure', async () => {
20+
const { server, fastifyPassport } = getRegisteredTestServer()
21+
22+
let callbackCalled = false
23+
let receivedStatus: number | undefined
24+
let receivedStatuses: (number | undefined)[] | undefined
25+
26+
server.get(
27+
'/',
28+
{
29+
preValidation: fastifyPassport.authenticate(
30+
new FailingStrategy('single-fail', 403),
31+
{ authInfo: false },
32+
async (request, reply, err, user, info, status) => {
33+
callbackCalled = true
34+
receivedStatus = status
35+
receivedStatuses = undefined // Should not be called with array for single strategy
36+
reply.code(401).send('Authentication failed')
37+
}
38+
)
39+
},
40+
async () => 'should not reach here'
41+
)
42+
43+
const response = await server.inject({ method: 'GET', url: '/' })
44+
45+
assert.strictEqual(response.statusCode, 401)
46+
assert.strictEqual(callbackCalled, true)
47+
assert.strictEqual(receivedStatus, 403)
48+
assert.strictEqual(receivedStatuses, undefined)
49+
})
50+
51+
test('should call callback with array of statuses for multi-strategy failure', async () => {
52+
const { server, fastifyPassport } = getRegisteredTestServer()
53+
54+
let callbackCalled = false
55+
let receivedStatus: number | undefined
56+
let receivedStatuses: (number | undefined)[] | undefined
57+
58+
server.get(
59+
'/',
60+
{
61+
preValidation: fastifyPassport.authenticate(
62+
[new FailingStrategy('multi-fail-1', 401), new FailingStrategy('multi-fail-2', 403)],
63+
{ authInfo: false },
64+
async (request, reply, err, user, info, statuses) => {
65+
callbackCalled = true
66+
receivedStatus = undefined // Should not be called with single status for multi-strategy
67+
receivedStatuses = statuses
68+
reply.code(401).send('Authentication failed')
69+
}
70+
)
71+
},
72+
async () => 'should not reach here'
73+
)
74+
75+
const response = await server.inject({ method: 'GET', url: '/' })
76+
77+
assert.strictEqual(response.statusCode, 401)
78+
assert.strictEqual(callbackCalled, true)
79+
assert.strictEqual(receivedStatus, undefined)
80+
assert.deepStrictEqual(receivedStatuses, [401, 403])
81+
})
82+
83+
test('should call callback with array of statuses for multi-strategy with mixed status types', async () => {
84+
const { server, fastifyPassport } = getRegisteredTestServer()
85+
86+
let callbackCalled = false
87+
let receivedStatuses: (number | undefined)[] | undefined
88+
89+
// One strategy fails with status, another fails without status (undefined)
90+
class FailingWithoutStatusStrategy extends Strategy {
91+
authenticate (_request: any, _options?: { pauseStream?: boolean }) {
92+
this.fail() // No status provided, should be undefined
93+
}
94+
}
95+
96+
server.get(
97+
'/',
98+
{
99+
preValidation: fastifyPassport.authenticate(
100+
[new FailingStrategy('multi-fail-1', 402), new FailingWithoutStatusStrategy('multi-fail-2')],
101+
{ authInfo: false },
102+
async (request, reply, err, user, info, statuses) => {
103+
callbackCalled = true
104+
receivedStatuses = statuses
105+
reply.code(401).send('Authentication failed')
106+
}
107+
)
108+
},
109+
async () => 'should not reach here'
110+
)
111+
112+
const response = await server.inject({ method: 'GET', url: '/' })
113+
114+
assert.strictEqual(response.statusCode, 401)
115+
assert.strictEqual(callbackCalled, true)
116+
assert.deepStrictEqual(receivedStatuses, [402, undefined])
117+
})
118+
119+
test('should work correctly when first strategy succeeds in multi-strategy setup', async () => {
120+
const { server, fastifyPassport } = getRegisteredTestServer()
121+
122+
let callbackCalled = false
123+
124+
// Strategy that always succeeds
125+
class SucceedingStrategy extends Strategy {
126+
authenticate (_request: any, _options?: { pauseStream?: boolean }) {
127+
this.success({ id: 'test-user', name: 'Test User' })
128+
}
129+
}
130+
131+
server.get(
132+
'/',
133+
{
134+
preValidation: fastifyPassport.authenticate(
135+
[new SucceedingStrategy('multi-success'), new FailingStrategy('multi-fail', 403)],
136+
{ authInfo: false },
137+
async (request, reply, err, user, info) => {
138+
callbackCalled = true
139+
// Success callback should not have status/statuses parameter
140+
reply.send('Authentication succeeded')
141+
}
142+
)
143+
},
144+
async () => 'should not reach here'
145+
)
146+
147+
const response = await server.inject({ method: 'GET', url: '/' })
148+
149+
assert.strictEqual(response.statusCode, 200)
150+
assert.strictEqual(response.body, 'Authentication succeeded')
151+
assert.strictEqual(callbackCalled, true)
152+
})
153+
})
154+
}
155+
156+
testSuite('@fastify/session')
157+
testSuite('@fastify/secure-session')

0 commit comments

Comments
 (0)