Harden private site access code checks#27775
Conversation
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThis pull request refactors the private blogging middleware authentication from a direct password comparison to a centralized access code mechanism. The implementation introduces helper functions to read and validate the configured access code from settings, updates session hash verification to use the access code with salt for SHA-256 computation, and rewrites the login handler to validate submitted codes using the new helpers. All test coverage is updated to match the new terminology and test scenarios, including negative cases for missing or empty access codes and validation of authenticated sessions when the configured access code is empty. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ghost/core/test/unit/frontend/apps/private-blogging/middleware.test.js (1)
251-254: ⚡ Quick winIsolate the stored-empty condition with a non-empty submitted code.
On Line 253, using
password: ''overlaps with the submitted-empty case (Line 262 onward), so this test does not uniquely prove the stored-empty guard.🔧 Minimal test tweak
it('doLoginToPrivateSite should return next if stored access code is empty', function () { settingsStub.withArgs('password').returns(''); - req.body = {password: ''}; + req.body = {password: 'rightpassword'};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ghost/core/test/unit/frontend/apps/private-blogging/middleware.test.js` around lines 251 - 254, The test for doLoginToPrivateSite currently sets both the stored access code and the submitted code to empty, so it doesn't isolate the "stored-empty" branch; change the submitted payload to a non-empty value (e.g., set req.body = {password: 'submitted-code'} or similar) while keeping settingsStub.withArgs('password').returns('') so the test uniquely verifies the stored-empty guard in doLoginToPrivateSite.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@ghost/core/test/unit/frontend/apps/private-blogging/middleware.test.js`:
- Around line 251-254: The test for doLoginToPrivateSite currently sets both the
stored access code and the submitted code to empty, so it doesn't isolate the
"stored-empty" branch; change the submitted payload to a non-empty value (e.g.,
set req.body = {password: 'submitted-code'} or similar) while keeping
settingsStub.withArgs('password').returns('') so the test uniquely verifies the
stored-empty guard in doLoginToPrivateSite.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e6344d5b-57fd-4682-814c-01d45d2fe76d
📒 Files selected for processing (2)
ghost/core/core/frontend/apps/private-blogging/lib/middleware.jsghost/core/test/unit/frontend/apps/private-blogging/middleware.test.js
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ghost/core/test/unit/frontend/apps/private-blogging/middleware.test.js`:
- Around line 251-260: Add explicit unit tests for the missing stored access
code cases in the private-blogging middleware: create new specs that call
privateBlogging.doLoginToPrivateSite with
settingsStub.withArgs('password').returns(undefined) (and another with
returns(null)) and assert the same failure path as the empty-string test — i.e.,
res.error exists and res.error.message === 'Incorrect access code.',
res.redirect is not called, and next is called; use the same req.body =
{password: 'rightpassword'} setup and sinon assertions to mirror the existing
test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 46d79ab3-0e58-49ad-8e93-5050bd9227a3
📒 Files selected for processing (1)
ghost/core/test/unit/frontend/apps/private-blogging/middleware.test.js
1 participant
Summary
Testing