Repository navigation
fix(strategies): return 404 for unbound or unknown strategies - #288
Open
piyushsinghgaur1 wants to merge 3 commits into
Open
piyushsinghgaur1 wants to merge 3 commits into
piyushsinghgaur1 wants to merge 3 commits into
Conversation
- An unbound strategy factory or an unknown strategy name now rejects with HttpErrors.NotFound instead of a plain Error, so the client gets a 404, not a 500. - The body is a general message. The binding key no longer shows up in it. - Add AuthErrorKeys.StrategyNotAvailable for the message. - Add an integration test for an unbound Apple provider and an unknown strategy name. GH-287
|
Working fine now the api is giving 404 |
namandeep-s1ngh
approved these changes
Oct 22, 2026
namandeep-s1ngh
self-requested a review
October 22, 2026 14:08
Remove `async` from the process*Factory methods in AuthStrategyProvider. None of them awaits anything, so SonarCloud flagged them (S7503). They return the strategy directly now. value() is still async, so an unbound factory or an unknown strategy name still rejects with the 404. GH-287
namandeep-s1ngh
approved these changes
Oct 22, 2026
Run npm audit fix to update brace-expansion, fast-uri, handlebars, js-yaml, msgpack5 and proxy-addr, and move the undici override from 6.28.0 to 6.28.1. A trivy scan for HIGH and CRITICAL findings is clean afterwards. GH-287
|
Copilot stopped reviewing on behalf of
a-ganguly due to an error
October 22, 2026 14:44
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
An unbound strategy factory or an unknown strategy name threw a plain
Error, and the route turned that into a 500. Both cases now reject withHttpErrors.NotFound, so the client gets a 404.The response body is now a general message: "The requested resource is not available". It no longer shows the binding key or the strategy name.
The provider had 13 copies of the same
getSyncand throw block. They now go through one privategetFactoryhelper. The message lives in a newAuthErrorKeys.StrategyNotAvailablekey.Fixes #287
Type of change
How Has This Been Tested?
src/__tests__/integration/action-sequence/passport-apple-oauth2/apple-oauth2-unbound.integration.ts. It covers an unbound Apple provider and an unknown strategy name. Both return 404 with the general message and no binding key in the body.npm run build,npm run lintandnpm testpass. The suite runs 126 tests.Checklist: