docs: correct API URLs, examples, and security claims - #145
Open
CuriousLearner wants to merge 1 commit into
Open
docs: correct API URLs, examples, and security claims#145CuriousLearner wants to merge 1 commit into
CuriousLearner wants to merge 1 commit into
Conversation
The router in phone_verify.urls registers the `phone` prefix itself and is built with `trailing_slash=False`, so the documented mount point and endpoint URLs were wrong in both directions. Mount at `api/` and drop the trailing slash: `POST /api/phone/register` and `POST /api/phone/verify`. README response bodies now match what the viewset actually returns. Rewrite the non-DRF integration example, which called a `send_verification()` overload and a `verify()` method that do not exist, around `send_security_code_and_generate_session_token()` and `verify_security_code()`, carrying the session token through the Django session. Apply the same fix to the Celery and OAuth snippets in the FAQ, and correct the sandbox example there to use `self._token` rather than a nonexistent `options` attribute. Replace the sandbox recipe in the customization guide. It overrode `validate_security_code()` to return valid unconditionally, which also discards the expiry, one-time-use and brute-force checks that live in that method. Custom backends now subclass their production backend and override `generate_security_code()` and `_should_bypass_code_check()`, matching the shipped Twilio and Nexmo backends. Custom backend examples set `exception_class` and explain what it narrows, and redundant `send_bulk_sms` overrides are dropped now that the base implementation is concrete. Correct the architecture guide's session token claims. The package only ever calls `jwt.encode`; there is no decode, no signature check, and no `iat`/`exp` in the payload. The token is an opaque bearer value used as a lookup key, and expiry is governed by the verification record. The step-1 diagram also had the SMS send before the database write. Document `MAX_FAILED_ATTEMPTS` in the security guide, FAQ and security policy, framed as per-record protection that does not replace request rate limiting. Replace the hand-rolled cleanup snippets with the shipped `cleanup_phone_verifications` command and `RECORD_RETENTION_DAYS`, add the `language` parameter to the service signatures in the API reference, correct the claim that useful indexes ship in the migrations, and refresh the supported versions table.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Every documented API URL was wrong in two ways. The router already registers the
phoneprefix and is created withtrailing_slash=False, but the docs told users to mount atapi/phone/and then call/api/phone/register/, which produces a doubled prefix and a trailing slash that does not exist. Verified by resolving against a real URLconf:/phone/register/raisesResolver404. The mount is nowpath("api/", include("phone_verify.urls")), givingPOST /api/phone/registerandPOST /api/phone/verify.docs/integration.rstshowed a non-DRF flow callingsend_verification()with no arguments and averify()method the service has never had. It is rewritten aroundsend_security_code_and_generate_session_token()andverify_security_code().docs/architecture.rstdescribed session tokens as validated JWTs, claiming the signature is checked, that the payload carriesiatandexp, and that tampering is therefore prevented. The package only ever callsjwt.encode; there is nojwt.decodeanywhere, and the token is used as an opaque database lookup key. The section now describes what actually happens, including that a tampered token fails by matching no row rather than by failing a signature check, and that the token is a bearer value with no independent expiry. The step-1 flow is also reordered, since the record is written before the SMS is sent.The sandbox recipe in
docs/customization.rsttold users to overridevalidate_security_code()to return valid unconditionally. That method is where expiry, one-time use, and the brute-force lockout are enforced, so following it silently disabled all three. It now mirrors the shipped backends, which subclass the production backend and overridegenerate_security_code()and_should_bypass_code_check().The built-in brute-force lockout was absent from every security-facing document, so
security.rstand the FAQ still told readers this was entirely their responsibility. It is now documented, framed as per-record protection that does not replace request rate limiting. Likewise the shippedcleanup_phone_verificationscommand replaces three copies of a hand-rolled retention snippet.Smaller corrections: examples now set
exception_class; redundantsend_bulk_smsoverrides are dropped now that it is concrete onBaseBackend; a sandbox example using a nonexistentself.optionsattribute usesself._token; thelanguageparameter is documented; README response bodies match what the API returns; the claim that migrations ship appropriate indexes is corrected, since the only index is the unique constraint whose leading column cannot serve the lookup path; and the security policy's supported-versions table covers the current release.Test plan
sqlmigrate.