Skip to content

Conversation

@emicovi
Copy link
Contributor

@emicovi emicovi commented Jun 18, 2025

Fixes #1326

Problem

The isMultiStrategy property in AuthenticationRoute was always set to false, regardless of whether an array of strategies was provided. This caused incorrect callback behavior when all strategies failed - multi-strategy setups received a single status instead of an array of statuses.

Solution

  • 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

Testing

  • Added comprehensive tests in test/multi-strategy-callback.test.ts to verify the fix works correctly for both single and multi-strategy scenarios
  • All existing tests continue to pass, ensuring no regressions (147 tests total, 144 passed)
  • Test coverage includes:
    • Single strategy failure callback receives single status
    • Multi-strategy failure callback receives array of statuses
    • Mixed status types (some with status, some without)
    • Success scenarios still work properly

Changes

  • src/AuthenticationRoute.ts: Fixed line 93 to set isMultiStrategy = true for array inputs
  • test/multi-strategy-callback.test.ts: Added comprehensive test coverage for the fix

Verification

The fix has been tested with the reproduction case from the linked issue and resolves the problem completely.

emicovi added 2 commits June 18, 2025 14:32
Fixes fastify#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
@Uzlopak Uzlopak requested a review from Copilot September 25, 2025 19:37
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Fixes a bug where the isMultiStrategy property was incorrectly set to false when an array of authentication strategies was provided, causing multi-strategy setups to receive single status values instead of arrays when all strategies failed.

  • Fixed AuthenticationRoute constructor to properly set isMultiStrategy = true for array inputs
  • Added comprehensive test coverage for both single and multi-strategy authentication scenarios
  • Ensures proper callback behavior with correct status parameter types

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/AuthenticationRoute.ts Fixed bug by changing isMultiStrategy from false to true when array of strategies is provided
test/multi-strategy-callback.test.ts Added comprehensive test suite covering single/multi-strategy scenarios and mixed status types

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Contributor

@Uzlopak Uzlopak left a comment

Choose a reason for hiding this comment

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

LGTM

@Uzlopak Uzlopak closed this Sep 25, 2025
@Uzlopak Uzlopak reopened this Sep 25, 2025
@Uzlopak Uzlopak merged commit 26eb067 into fastify:main Sep 25, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AuthenticationRoute.isMultiStrategy is always false

2 participants