Skip to content

Phone OTP guest verification fails: send_sms rejects the Guest role #452

Description

@harshtandiya

Summary

Guest booking with guest_verification_method = "Phone OTP" cannot deliver a code. send_booking_otp calls the whitelisted send_sms, which permission-checks its caller, and the caller in this flow is Guest.

Where

buzz/api/booking/guests.py:8 imports the whitelisted wrapper:

from frappe.core.doctype.sms_settings.sms_settings import send_sms

send_sms runs check_sms_permission() (frappe/core/doctype/sms_settings/sms_settings.py:73-87), which passes only for System Manager or a role listed in SMS Settings allowed_roles.

buzz.api.booking.send_guest_booking_otp is @frappe.whitelist(allow_guest=True), so frappe.session.user is Guest with roles ['Guest'].

Reproduction

Verified on buzz.localhost, where allowed_roles is ['System Manager']:

frappe.set_user("Guest")
check_sms_permission()
# RAISED PermissionError: You are not permitted to send SMS

deliver_otp catches this in its bare except Exception and re-throws as "Failed to send verification code. Please try again.", so the guest sees a generic failure with no way forward.

Second failure mode: silent

If sms_gateway_url is unset, _send_sms calls msgprint(_("Please Update SMS Settings")) and returns — no exception. deliver_otp therefore succeeds, the OTP secret is written to frappe.cache, and the guest waits on an OTP screen for a code that was never sent.

Suggested fix

Import the internal _send_sms instead. Frappe's own 2FA path does the equivalent — send_token_via_sms (frappe/twofactor.py:298-337) calls send_request directly and never touches the permission check. The permission gate exists to guard the HTTP surface of send_sms; this call is server-side from an endpoint that has already validated the event and rate-limited the identifier.

Worth deciding separately whether a missing gateway should raise rather than msgprint, so deliver_otp fails loudly instead of caching a secret nobody can use.

Current mitigation

phone_otp_available() (added in #451) requires both a gateway URL and Guest in allowed_roles, so the dashboard reports Phone OTP as unavailable and validate_guest_verification_config refuses to save it while this bug stands. An admin who adds the Guest role to allowed_roles works around it today.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions