Skip to content

feat: add logs for failed sms #3318

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Salazareo wants to merge 1 commit into
base: main
Choose a base branch
from
+143 −18
Open

feat: add logs for failed sms #3318

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions src/backend/controllers/auth/AuthController.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1864,6 +1864,48 @@ describe('AuthController.handleSendConfirmPhone validation', () => {
);
});

it('attaches a support error_id to a Prelude block', async () => {
const { actor } = await makeUserAndActor();
await withPrelude(
stubPrelude({
createVerification: vi.fn(async () => ({ status: 'blocked' })),
}),
async () => {
await expect(
controller.handleSendConfirmPhone(
makeReq({ phone: '+14155550123' }, { actor }),
makeRes(),
),
).rejects.toMatchObject({
statusCode: 429,
fields: { error_id: expect.any(String) },
});
},
);
});

it('attaches a support error_id when the Prelude request throws', async () => {
const { actor } = await makeUserAndActor();
await withPrelude(
stubPrelude({
createVerification: vi.fn(async () => {
throw new Error('network down');
}),
}),
async () => {
await expect(
controller.handleSendConfirmPhone(
makeReq({ phone: '+14155550123' }, { actor }),
makeRes(),
),
).rejects.toMatchObject({
statusCode: 502,
fields: { error_id: expect.any(String) },
});
},
);
});

it('forwards ip, device fingerprint, and user-agent to Prelude as signals', async () => {
const { actor } = await makeUserAndActor();
const createVerification = vi.fn(async () => ({ status: 'success' }));
Expand Down
104 changes: 90 additions & 14 deletions src/backend/controllers/auth/AuthController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { v4 as uuidv4 } from 'uuid';
import validator from 'validator';
import { Controller, Get, Post } from '../../core/http/decorators.js';
import { HttpError } from '../../core/http/HttpError.js';
import type { HttpErrorOptions } from '../../core/http/HttpError.js';
import { antiCsrf } from '../../core/http/middleware/antiCsrf.js';
import { generateCaptcha } from '../../core/http/middleware/captcha.js';
import { checkRateLimit } from '../../core/http/middleware/rateLimit.js';
Expand Down Expand Up @@ -1018,6 +1019,43 @@ export class AuthController extends PuterController {

// -- Phone verification (SMS via Prelude) ------------------------

/**
* Build the error thrown when a verification SMS can't be sent (a delivery
* failure, or a refused/blocked send). Mints a short `error_id`, writes a
* single greppable line tying that id to the real reason (so support can
* look it up in CloudWatch with the id the user quotes), and returns the
* `HttpError` with the id attached as `error_id` for the GUI to surface.
* The phone number is deliberately omitted from the log line (PII); the
* user + country are enough to correlate.
*/
private smsSendError(
statusCode: number,
clientMessage: string,
reason: string,
ctx: {
userId?: number;
userUid?: string;
country?: string;
detail?: unknown;
},
options: HttpErrorOptions = {},
): HttpError {
const errorId = uuidv4();
const detail =
ctx.detail instanceof Error ? ctx.detail.message : ctx.detail;
console.warn(
`[send-confirm-phone] send_failed error_id=${errorId} ` +
`reason=${reason} status=${statusCode} ` +
`user_id=${ctx.userId ?? ''} user_uid=${ctx.userUid ?? ''} ` +
`country=${ctx.country ?? ''}` +
(detail ? ` detail=${JSON.stringify(String(detail))}` : ''),
);
return new HttpError(statusCode, clientMessage, {
...options,
fields: { ...options.fields, error_id: errorId },
});
}

@Post('/send-confirm-phone', {
subdomain: ['api', ''],
requireUserActor: true,
Expand All @@ -1042,9 +1080,13 @@ export class AuthController extends PuterController {
legacyCode: 'account_suspended',
});
if (!this.clients.prelude?.isConfigured())
throw new HttpError(503, 'Phone verification is unavailable.', {
legacyCode: 'service_unavailable' as never,
});
throw this.smsSendError(
503,
'Phone verification is unavailable.',
'prelude_not_configured',
{ userId: user.id, userUid: user.uuid },
{ legacyCode: 'service_unavailable' as never },
);

// Parse to E.164 (Prelude's required form + the stored form) and the
// country, so we can apply the per-country cost cap.
Expand Down Expand Up @@ -1073,9 +1115,15 @@ export class AuthController extends PuterController {
// (see PreludeClient / countries.ts). Avoids paying exorbitant per-SMS
// rates in low-revenue, high-fraud geographies.
if (!this.clients.prelude.isCountrySupported(parsed.country))
throw new HttpError(
throw this.smsSendError(
400,
'Phone verification is not available for this country.',
'country_not_supported',
{
userId: user.id,
userUid: user.uuid,
country: parsed.country,
},
{ legacyCode: 'phone_country_not_supported' as never },
);

Expand Down Expand Up @@ -1110,9 +1158,15 @@ export class AuthController extends PuterController {
// its meaning lives in the extension (which sets it) and the GUI (which
// displays it), so no abuse semantics leak into the OSS repo.
if (abuseCheck.allowed === false)
throw new HttpError(
throw this.smsSendError(
429,
'Phone verification is unavailable for this number right now.',
`not_allowed:${abuseCheck.reason ?? 'unspecified'}`,
{
userId: user.id,
userUid: user.uuid,
country: parsed.country,
},
{
legacyCode: 'phone_verification_unavailable' as never,
fields: abuseCheck.reason
Expand All @@ -1135,10 +1189,18 @@ export class AuthController extends PuterController {
expireAt: Math.floor(Date.now() / 1000) + 60 * 60,
});
} catch (e) {
console.warn('[send-confirm-phone] pending-store failed:', e);
throw new HttpError(503, 'Could not start phone verification.', {
legacyCode: 'service_unavailable' as never,
});
throw this.smsSendError(
503,
'Could not start phone verification.',
'pending_store_failed',
{
userId: user.id,
userUid: user.uuid,
country: parsed.country,
detail: e,
},
{ legacyCode: 'service_unavailable' as never },
);
}

const ip = req.ip || req.socket?.remoteAddress || undefined;
Expand All @@ -1161,18 +1223,32 @@ export class AuthController extends PuterController {
result.status === 'blocked' ||
result.status === 'shadow_blocked'
) {
throw new HttpError(
throw this.smsSendError(
429,
'Phone verification is temporarily unavailable for this number.',
`prelude_${result.status}`,
{
userId: user.id,
userUid: user.uuid,
country: parsed.country,
},
{ legacyCode: 'too_many_requests' as never },
);
}
} catch (e) {
if (e instanceof HttpError) throw e;
console.warn('[send-confirm-phone] createVerification failed:', e);
throw new HttpError(502, 'Could not send verification code.', {
legacyCode: 'upstream_error' as never,
});
throw this.smsSendError(
502,
'Could not send verification code.',
'prelude_request_failed',
{
userId: user.id,
userUid: user.uuid,
country: parsed.country,
detail: e,
},
{ legacyCode: 'upstream_error' as never },
);
}

// Tell the abuse extension a code was actually sent, so it can bump its
Expand Down
14 changes: 10 additions & 4 deletions src/gui/src/UI/UIWindowPhoneVerificationRequired.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -763,11 +763,17 @@ function UIWindowPhoneVerificationRequired(options) {
},
error: function (xhr) {
const reason = xhr.responseJSON?.reason;
showError(
let msg =
SEND_REASON_MESSAGES[reason] ??
xhr.responseJSON?.error ??
T.could_not_send,
);
xhr.responseJSON?.error ??
T.could_not_send;
// Append the support reference id the backend minted for
// this failure (it logged the real reason against it), so
// the user can quote it when they email support.
const errorId = xhr.responseJSON?.error_id;
if (errorId)
msg += ' ' + i18n('phone_error_reference', { id: errorId });
showError(msg);
},
complete: function () {
is_sending = false;
Expand Down
1 change: 1 addition & 0 deletions src/gui/src/i18n/translations/en.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@ const en = {
phone_enter_valid: 'Please enter a valid phone number.',
phone_invalid_code: 'Invalid verification code.',
phone_could_not_send: 'Could not send a code to that number.',
phone_error_reference: 'If this keeps happening, email support@puter.com and include this code: {{id}}',
phone_could_not_verify: 'Could not verify code.',
phone_search_countries: 'Search countries',
phone_no_matches: 'No matches',
Expand Down
Loading