Changed verification flow to support webhook mode behind labs flag

[aileen]

closes GVA-730

The verificationFlow feature flag now keeps the existing escalation email path as default while enabling a parallel webhook path through VerificationTrigger and verification-webhook-service.ts. This wires both strategies in members/service.js and adds tests to assert webhook payload behavior without regressing the legacy flow.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Adds a webhook-based escalation path for host email verification and centralizes email sending into a shared helper. Introduces VerificationWebhookService that posts JSON payloads with optional HMAC-SHA256 signing, timeout, and retries. VerificationTrigger now computes an effective threshold and method, prefers sending a verification webhook when enabled, catches webhook errors, and falls back to the legacy email flow. Unit and e2e tests were added/updated to cover webhook and lab-state behavior.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and concisely summarizes the main change: adding webhook mode support for the verification flow behind a labs feature flag.
Description check	✅ Passed	The description clearly explains the feature being implemented, references the related issue (GVA-730), and outlines the approach of keeping the default email path while enabling webhook via labs flag.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch verification-flow-behind-flag

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[ErisDS]

🤖 Velo CI Failure Analysis

Classification: 🟠 SOFT FAIL

• Workflow: CI
• Failed Step: E2E tests
• Run: View failed run
  What failed: Test assertion failed
  Why: The failure is caused by a code issue that needs to be fixed in this PR.
  Action:
  Check the error message and fix the issue in your code.

[ErisDS]

🤖 Velo CI Failure Analysis

Classification: 🟠 SOFT FAIL

• Workflow: CI
• Failed Step: Run yarn nx affected -t lint --base=f02388d5953e1cf9114b4b399be5e40852213978
• Run: View failed run
  What failed: Lint errors in code
  Why: The failure is caused by a code issue that needs to be fixed in this PR.
  Action:
  Check the error message and fix the issue in your code.

[ErisDS]

🤖 Velo CI Failure Analysis

Classification: 🟠 SOFT FAIL

• Workflow: CI
• Failed Step: Set up job
• Run: View failed run
  What failed: CI failure - likely code issue
  Why: The failure appears to be related to code changes. Check the error output for details.
  Action:
  Review the error logs and fix the issue in your code.

[coderabbitai]

🧹 Nitpick comments (1)

ghost/core/core/server/services/verification/verification-webhook-service.ts (1)

41-94: Consider adding error handling for the HTTP request.

The sendVerificationWebhook method awaits the request without error handling. If the webhook endpoint is unavailable or returns an error, this will propagate up and could potentially interfere with the verification flow completion (e.g., prevent setting email_verification_required).

The email path in sendVerificationEmail also doesn't have explicit error handling, so this may be intentional for consistency. However, the webhook retry logic (5 retries) could cause delays if the endpoint is down.

Consider whether:

1. The current behavior (letting errors propagate) is acceptable, OR
2. A try/catch with logging would be more appropriate to ensure the verification flag is always set

♻️ Optional: Add error handling with logging

         this.#logging.info(`Triggering verification webhook to "${webhookUrl}"`);
 
-        await this.#request(webhookUrl, requestOptions);
+        try {
+            await this.#request(webhookUrl, requestOptions);
+        } catch (error) {
+            this.#logging.error?.(`Failed to send verification webhook: ${error.message || error}`);
+            throw error; // Re-throw to maintain existing behavior if needed
+        }
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ghost/core/core/server/services/verification/verification-webhook-service.ts`
around lines 41 - 94, sendVerificationWebhook currently awaits this.#request
without handling failures, which can block verification flow; wrap the await
this.#request(webhookUrl, requestOptions) call in a try/catch, log failures
using this.#logging.error including webhookUrl,
requestOptions.headers['X-Ghost-Request-Timestamp'] (or timestamp) and the
caught error, and do not rethrow so verification completion isn't blocked;
optionally mirror the same pattern used by sendVerificationEmail for consistency
or make behavior configurable via a flag if propagation should be preserved in
some environments.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@ghost/core/core/server/services/verification/verification-webhook-service.ts`:
- Around line 41-94: sendVerificationWebhook currently awaits this.#request
without handling failures, which can block verification flow; wrap the await
this.#request(webhookUrl, requestOptions) call in a try/catch, log failures
using this.#logging.error including webhookUrl,
requestOptions.headers['X-Ghost-Request-Timestamp'] (or timestamp) and the
caught error, and do not rethrow so verification completion isn't blocked;
optionally mirror the same pattern used by sendVerificationEmail for consistency
or make behavior configurable via a flag if propagation should be preserved in
some environments.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 79161b5d-4cf8-4d0e-bf37-3ddcdef18c5d

📥 Commits

Reviewing files that changed from the base of the PR and between 3ad6d52 and d0a9f98.

📒 Files selected for processing (6)

• ghost/core/core/server/services/members/service.js
• ghost/core/core/server/services/verification-trigger.js
• ghost/core/core/server/services/verification/verification-webhook-service.ts
• ghost/core/test/e2e-api/admin/members.test.js
• ghost/core/test/legacy/api/admin/members-importer.test.js
• ghost/core/test/unit/server/services/verification-trigger.test.js

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@ghost/core/core/server/services/verification-trigger.js`:
- Around line 206-216: The site is marked as requiring verification before the
webhook is sent, which can leave the site stuck if webhook delivery fails;
change the flow so you only set email_verification_required after a confirmed
successful webhook send (or alternately gate the webhook branch on a positive
"webhook configured" check). Specifically, in verification-trigger.js, call
this._shouldUseWebhookFlow() and if true, await
this._sendVerificationWebhook({amountTriggered: amount, threshold: threshold ??
amount, method: method ?? source ?? 'import'}) and only when that promise
resolves successfully then call this._Settings.edit(...) to set key:
'email_verification_required'; on webhook failure do not change the setting and
surface/log the error so the legacy email path remains available; you can also
add an explicit "webhook configured" check used to decide whether to attempt the
webhook branch before touching the setting.

In
`@ghost/core/core/server/services/verification/verification-webhook-service.ts`:
- Around line 53-64: The code sets webhookType =
this.#config.get('hostSettings:emailVerification:webhookType') ||
'not_set_in_config', which masks a missing/empty config and prevents the
subsequent guard in verification-webhook-service.ts from bailing out; change the
logic in the webhookType retrieval (or the guard) so a missing or empty
webhookType is treated as unconfigured (e.g., remove the 'not_set_in_config'
default and/or check for falsy/empty string in the existing if (!webhookType ||
typeof webhookType !== 'string') return) so the service returns early instead of
emitting type: "not_set_in_config".
- Around line 89-102: The requestOptions object in
verification-webhook-service.ts is missing an explicit HTTP method; update the
requestOptions declaration (the one passed to this.#request) to include method:
'POST' so the webhook is sent as a POST with the provided body; ensure the
method property is added alongside body, headers, timeout, and retry before
calling this.#request(webhookUrl, requestOptions).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 879dcc8d-042e-4d33-82a0-5654f0eed488

📥 Commits

Reviewing files that changed from the base of the PR and between e3b6d4f and fb0eb46.

📒 Files selected for processing (6)

• ghost/core/core/server/services/members/service.js
• ghost/core/core/server/services/verification-trigger.js
• ghost/core/core/server/services/verification/verification-webhook-service.ts
• ghost/core/test/e2e-api/admin/members.test.js
• ghost/core/test/legacy/api/admin/members-importer.test.js
• ghost/core/test/unit/server/services/verification-trigger.test.js

[coderabbitai]

🧹 Nitpick comments (2)

ghost/core/test/unit/server/services/verification/verification-webhook-service.test.ts (2)

68-105: Good coverage of the happy path and signature verification.

The test correctly captures the request parameters, validates the payload structure, and recomputes the HMAC signature using the captured timestamp—ensuring the signature logic is verified end-to-end.

For completeness, consider adding:

1. A test for request failure (verify the error is thrown/propagated).
2. A test for empty webhookSecret (verify no X-Ghost-Signature header is set).

,

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@ghost/core/test/unit/server/services/verification/verification-webhook-service.test.ts`
around lines 68 - 105, Add two additional unit tests for createService(). First,
add a test that simulates the request function throwing an error (e.g., request
rejects) and assert that sendVerificationWebhook propagates/throws that error
(reference: createService and sendVerificationWebhook). Second, add a test where
the webhookSecret is empty/undefined in the service config, call
sendVerificationWebhook and assert that the request headers do not include
'X-Ghost-Signature' (reference: requestOptions.headers and the signature
generation flow in sendVerificationWebhook). Ensure both tests reuse the
existing pattern of injecting a custom request implementation and validating
requestUrl, requestOptions, and exceptions as appropriate.

---

33-66: Consider adding a parallel test for missing webhookUrl.

This test covers the webhookType missing branch, but the source code also has an early return when webhookUrl is missing or empty. Adding a similar test for that branch would improve coverage symmetry.

📝 Example test to add

it('returns false when webhookUrl is missing', async function () {
    let warnCalled = false;
    const service = createService({
        config: {
            get: (key: string) => {
                if (key === 'hostSettings:emailVerification:webhookUrl') {
                    return '';
                }
                if (key === 'hostSettings:emailVerification:webhookType') {
                    return 'member_import_limit_breach';
                }
                return null;
            }
        },
        logging: {
            info: () => {},
            warn: () => {
                warnCalled = true;
            },
            error: () => {}
        }
    });

    const result = await service.sendVerificationWebhook({
        amountTriggered: 1001,
        threshold: 1000,
        method: 'import'
    });

    assert.equal(result, false);
    assert.equal(warnCalled, true);
});

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@ghost/core/test/unit/server/services/verification/verification-webhook-service.test.ts`
around lines 33 - 66, Add a parallel unit test that mirrors the existing
"returns false when webhookType is missing" case but exercises the early-return
branch when the webhookUrl is empty: create a service via createService with
config.get returning '' for 'hostSettings:emailVerification:webhookUrl' and a
valid 'hostSettings:emailVerification:webhookType' (e.g.
'member_import_limit_breach'), stub logging.warn to set a flag, call
sendVerificationWebhook with the same payload, and assert the result is false
and that warn was called.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@ghost/core/test/unit/server/services/verification/verification-webhook-service.test.ts`:
- Around line 68-105: Add two additional unit tests for createService(). First,
add a test that simulates the request function throwing an error (e.g., request
rejects) and assert that sendVerificationWebhook propagates/throws that error
(reference: createService and sendVerificationWebhook). Second, add a test where
the webhookSecret is empty/undefined in the service config, call
sendVerificationWebhook and assert that the request headers do not include
'X-Ghost-Signature' (reference: requestOptions.headers and the signature
generation flow in sendVerificationWebhook). Ensure both tests reuse the
existing pattern of injecting a custom request implementation and validating
requestUrl, requestOptions, and exceptions as appropriate.
- Around line 33-66: Add a parallel unit test that mirrors the existing "returns
false when webhookType is missing" case but exercises the early-return branch
when the webhookUrl is empty: create a service via createService with config.get
returning '' for 'hostSettings:emailVerification:webhookUrl' and a valid
'hostSettings:emailVerification:webhookType' (e.g.
'member_import_limit_breach'), stub logging.warn to set a flag, call
sendVerificationWebhook with the same payload, and assert the result is false
and that warn was called.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2b5cd5eb-31d4-4510-8a88-00e1d56cc78f

📥 Commits

Reviewing files that changed from the base of the PR and between fb0eb46 and dbc9662.

📒 Files selected for processing (4)

• ghost/core/core/server/services/verification-trigger.js
• ghost/core/core/server/services/verification/verification-webhook-service.ts
• ghost/core/test/unit/server/services/verification-trigger.test.js
• ghost/core/test/unit/server/services/verification/verification-webhook-service.test.ts

✅ Files skipped from review due to trivial changes (1)

• ghost/core/test/unit/server/services/verification-trigger.test.js

🚧 Files skipped from review as they are similar to previous changes (2)

• ghost/core/core/server/services/verification-trigger.js
• ghost/core/core/server/services/verification/verification-webhook-service.ts

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@ghost/core/core/server/services/verification-trigger.js`:
- Around line 249-266: The code assumes _sendVerificationWebhook resolves false
on failure but that method can throw transport errors; wrap the call to
this._sendVerificationWebhook inside a try/catch so any exception is treated as
a non-delivery (set webhookWasSent = false), optionally log the caught error,
and then continue to call _startLegacyEmailVerificationProcess when
webhookWasSent is falsy; keep the existing flow that still marks verification
required and returns via _finishTrigger only when the webhook truly succeeded
(i.e., webhookWasSent === true), referencing the methods _shouldUseWebhookFlow,
_sendVerificationWebhook, _markVerificationRequired, _finishTrigger, and
_startLegacyEmailVerificationProcess.

In
`@ghost/core/core/server/services/verification/verification-webhook-service.ts`:
- Around line 103-110: The logs currently emit the raw webhookUrl (used in the
verification send flow) which can leak credentials; update the code around the
send in the verification webhook method to stop logging the full URL: replace
direct uses of webhookUrl in this.#logging.info and this.#logging.error with a
sanitized value (e.g. parse the URL and log only the origin/hostname or a masked
version that strips query, path, and credentials) by adding a small helper like
sanitizeWebhookUrl(...) and use that sanitized string when calling
this.#logging.info and this.#logging.error while leaving the actual call to
this.#request(webhookUrl, requestOptions) unchanged so delivery still uses the
full URL.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6b873e0c-f665-479b-8b99-b1fe3e915c5f

📥 Commits

Reviewing files that changed from the base of the PR and between dbc9662 and 0016a1a.

📒 Files selected for processing (7)

• ghost/core/core/server/services/members/service.js
• ghost/core/core/server/services/verification-trigger.js
• ghost/core/core/server/services/verification/verification-webhook-service.ts
• ghost/core/test/e2e-api/admin/members.test.js
• ghost/core/test/legacy/api/admin/members-importer.test.js
• ghost/core/test/unit/server/services/verification-trigger.test.js
• ghost/core/test/unit/server/services/verification/verification-webhook-service.test.ts

✅ Files skipped from review due to trivial changes (2)

• ghost/core/test/unit/server/services/verification-trigger.test.js
• ghost/core/test/unit/server/services/verification/verification-webhook-service.test.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• ghost/core/test/legacy/api/admin/members-importer.test.js

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

ghost/core/test/e2e-api/admin/members.test.js (1)

894-961: ⚠️ Potential issue | 🟠 Major

Always restore test-global state in finally.

These two cases mutate global config/settings and create members, but cleanup only runs on the happy path. If anything fails before Line 959 or Line 1057, later tests inherit the modified hostSettings, email_verification_required, and extra members.

♻️ Suggested structure

-        configUtils.set('hostSettings:emailVerification', {
-            ...
-        });
-
-        ...
-
-        // state cleanup
-        await agent.delete(`/members/${memberPassVerification.id}`);
-        await agent.delete(`/members/${memberFailVerification.id}`);
-
-        await configUtils.restore();
-        settingsCache.set('email_verification_required', {value: false});
+        let memberPassVerification;
+        let memberFailVerification;
+
+        try {
+            configUtils.set('hostSettings:emailVerification', {
+                ...
+            });
+
+            ...
+
+            memberPassVerification = passBody.members[0];
+            ...
+            memberFailVerification = failBody.members[0];
+            ...
+        } finally {
+            if (memberPassVerification) {
+                await agent.delete(`/members/${memberPassVerification.id}`);
+            }
+
+            if (memberFailVerification) {
+                await agent.delete(`/members/${memberFailVerification.id}`);
+            }
+
+            await configUtils.restore();
+            settingsCache.set('email_verification_required', {value: false});
+        }

Also applies to: 963-1059

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ghost/core/test/e2e-api/admin/members.test.js` around lines 894 - 961, The
test mutates global state and creates members without guaranteeing cleanup; wrap
the main test steps in a try/finally so configUtils.set(...),
mockManager.mockLabsDisabled('verificationFlow'), and created members (the
responses stored in memberPassVerification and memberFailVerification from
agent.post(`/members/`)) are always reverted: in finally call
configUtils.restore(), reset settingsCache.set('email_verification_required',
{value: false}) and delete any created members via
agent.delete(`/members/${id}`) if those variables are populated; keep assertions
(e.g., mockManager.assert.sentEmail) inside the try block and ensure
DomainEvents.allSettled() is awaited before cleanup.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@ghost/core/test/e2e-api/admin/members.test.js`:
- Around line 894-961: The test mutates global state and creates members without
guaranteeing cleanup; wrap the main test steps in a try/finally so
configUtils.set(...), mockManager.mockLabsDisabled('verificationFlow'), and
created members (the responses stored in memberPassVerification and
memberFailVerification from agent.post(`/members/`)) are always reverted: in
finally call configUtils.restore(), reset
settingsCache.set('email_verification_required', {value: false}) and delete any
created members via agent.delete(`/members/${id}`) if those variables are
populated; keep assertions (e.g., mockManager.assert.sentEmail) inside the try
block and ensure DomainEvents.allSettled() is awaited before cleanup.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e58083a6-713b-4966-85da-04805321bae8

📥 Commits

Reviewing files that changed from the base of the PR and between 0016a1a and 96e461e.

📒 Files selected for processing (7)

• ghost/core/core/server/services/members/service.js
• ghost/core/core/server/services/verification-trigger.js
• ghost/core/core/server/services/verification/verification-webhook-service.ts
• ghost/core/test/e2e-api/admin/members.test.js
• ghost/core/test/legacy/api/admin/members-importer.test.js
• ghost/core/test/unit/server/services/verification-trigger.test.js
• ghost/core/test/unit/server/services/verification/verification-webhook-service.test.ts

🚧 Files skipped from review as they are similar to previous changes (3)

• ghost/core/test/legacy/api/admin/members-importer.test.js
• ghost/core/test/unit/server/services/verification/verification-webhook-service.test.ts
• ghost/core/test/unit/server/services/verification-trigger.test.js