fix: fail-safe robustness nits in logout handler and require-auth#27
Draft
frederikprijck wants to merge 1 commit into
Draft
fix: fail-safe robustness nits in logout handler and require-auth#27frederikprijck wants to merge 1 commit into
frederikprijck wants to merge 1 commit into
Conversation
- backchannel-logout: read req.body?.logout_token inside try so a missing body parser yields the intended 400 instead of a TypeError/500. - require-auth: return next(err) instead of throwing, so a missing ApiClient reaches Express error middleware on Express 4 (no unhandled rejection). SECURITY: SDK-9 — fail-safe hardening (CWE-248).
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
1 participant
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.
Changes
req.body?.logout_tokeninside thetrywith optional chaining, so a missing body parser yields the intended 400 instead of aTypeError/500.return next(new Error(...))instead ofthrow, so a missing ApiClient reaches Express error middleware on Express 4 rather than becoming an unhandled rejection. Fails closed either way.Tests
New
require-auth.spec.tscovers the missing-ApiClient branch (assertsnext(Error)called, no response written). New backchannel test covers the no-body-parser → 400 path.