test: fix keepalive assert check #535
Conversation
WalkthroughThe changes introduce a new endpoint Changes
Possibly related PRs
Suggested labels
Poem
Tip OpenAI O1 model for chat
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
commit: |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #535 +/- ##
==========================================
- Coverage 97.87% 97.47% -0.41%
==========================================
Files 10 7 -3
Lines 1600 990 -610
Branches 257 260 +3
==========================================
- Hits 1566 965 -601
+ Misses 30 21 -9
Partials 4 4 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- test/fixtures/server.ts (1 hunks)
- test/keep-alive-header.test.ts (2 hunks)
- test/options.digestAuth.test.ts (1 hunks)
Additional comments not posted (3)
test/options.digestAuth.test.ts (1)
54-63: LGTM!The new test case
should auth fail on x-www-authenticateis a valuable addition to the test suite. It covers the important scenario of handling authentication failures when invalid credentials are provided.The test case follows the AAA pattern, making it easy to understand and maintain. The assertions correctly verify the expected status code (401) and error message ("authorization invaild") for the failure scenario.
By testing the negative path, this test case improves the robustness of the authentication logic and ensures that the application behaves correctly when faced with invalid input.
Great job on enhancing the test coverage! 👍
test/keep-alive-header.test.ts (2)
45-49: LGTM!The conditional check is a good defensive programming practice to prevent potential runtime errors. The assertions seem reasonable and match the expected state after a response is received.
139-143: LGTM!The conditional check is a good defensive programming practice. The assertions seem reasonable and match the expected state after sleeping for the keepalive timeout duration.
| if (pathname === '/digestAuth2') { | ||
| const authorization = req.headers.authorization; | ||
| if (!authorization) { | ||
| res.setHeader('x-www-authenticate', 'Digest realm="[email protected]", qop="auth,auth-int", nonce="dcd98b7102dd2f0e8b11d0f600bfb0c093", opaque="5ccc069c403ebaf9f0171e9517f40e41"'); | ||
| res.statusCode = 401; | ||
| return res.end(JSON.stringify({ | ||
| error: 'authorization required', | ||
| })); | ||
| } | ||
| if (!authorization.includes('Digest username="user"')) { | ||
| res.setHeader('x-www-authenticate', 'Digest realm="[email protected]", qop="auth,auth-int", nonce="dcd98b7102dd2f0e8b11d0f600bfb0c093", opaque="5ccc069c403ebaf9f0171e9517f40e41"'); | ||
| res.statusCode = 401; | ||
| return res.end(JSON.stringify({ | ||
| error: 'authorization invaild', | ||
| })); | ||
| } | ||
| return res.end(JSON.stringify({ | ||
| authorization, | ||
| })); | ||
| } |
There was a problem hiding this comment.
LGTM! The new /digestAuth2 endpoint implementation looks good.
The Digest Authentication flow is correctly implemented, handling the cases when the Authorization header is missing or invalid, and responding with the appropriate status codes and headers.
Consider refactoring the common logic between the /digestAuth and /digestAuth2 endpoints to avoid code duplication. You can extract the shared logic into a separate function that takes the header name (www-authenticate or x-www-authenticate) as a parameter.
For example:
function handleDigestAuth(req: IncomingMessage, res: ServerResponse, headerName: string) {
const authorization = req.headers.authorization;
if (!authorization) {
res.setHeader(headerName, 'Digest realm="[email protected]", qop="auth,auth-int", nonce="dcd98b7102dd2f0e8b11d0f600bfb0c093", opaque="5ccc069c403ebaf9f0171e9517f40e41"');
res.statusCode = 401;
return res.end(JSON.stringify({
error: 'authorization required',
}));
}
if (!authorization.includes('Digest username="user"')) {
res.setHeader(headerName, 'Digest realm="[email protected]", qop="auth,auth-int", nonce="dcd98b7102dd2f0e8b11d0f600bfb0c093", opaque="5ccc069c403ebaf9f0171e9517f40e41"');
res.statusCode = 401;
return res.end(JSON.stringify({
error: 'authorization invalid',
}));
}
return res.end(JSON.stringify({
authorization,
}));
}
// Usage in the `/digestAuth` endpoint
if (pathname === '/digestAuth') {
return handleDigestAuth(req, res, 'www-authenticate');
}
// Usage in the `/digestAuth2` endpoint
if (pathname === '/digestAuth2') {
return handleDigestAuth(req, res, 'x-www-authenticate');
}This refactoring will make the code more maintainable and easier to update in the future if any changes are needed to the Digest Authentication logic.
Summary by CodeRabbit
New Features
/digestAuth2for enhanced security through Digest Authentication.Bug Fixes
Tests