Repository navigation
Ask people to agree to the terms and privacy policy before they sign in - #82
shanerbaner82 wants to merge 2 commits into
Conversation
Neither Sign in with GitHub nor Sign in with Apple works until the box is ticked, and links open the terms and privacy policy in the app's browser. The phone tells devnull which version it showed as it signs in (terms_version), which devnull records. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
Nice. Just waiting on the API PR to get merged first |
RaphaelFakhri
left a comment
There was a problem hiding this comment.
The GitHub sign-in drops terms_version whenever onboarding is the root screen. In that case AuthCallback replaces /onboarding with a fresh Onboarding, so $acceptedTerms is back to false when finishSignIn() runs from mount(), and redeemSignIn() posts {"code":"c","code_verifier":"..."} with no version. The pop-back path (onResume() on the same instance) does send "terms_version":"2026-10-23", so you only see the bug when the callback arrives as a replace. The existing test "follows the deep link back, redeems the code with the verifier and signs in" also locks this in: it still expects a body without terms_version. Since signIn() and signInWithApple() already refuse to start without the tick, you can send Client::TermsVersion unconditionally in finishSignIn() and useAppleToken(), or carry it on the SignInAttempt.
The branch also conflicts with main in resources/views/native/onboarding.blade.php after #92, and vendor/bin/pint --test flags the double blank lines this PR adds to tests/Pest.php and OnboardingTest.php. CONTRIBUTING.md asks for before and after screenshots on iOS and Android in light and dark mode for this screen.
tests/Feature/Screens/OnboardingTest.php line 209: This asserts the GitHub redeem body without terms_version, which contradicts "sends terms_version with the sign-in call for both ways in". Add 'terms_version' => Client::TermsVersion here once the replace path sends it.
|
|
||
| try { | ||
| $user = app(Client::class)->redeemSignIn($code, $attempt->verifier()); | ||
| $user = app(Client::class)->redeemSignIn($code, $attempt->verifier(), $this->acceptedTerms ? Client::TermsVersion : null); |
There was a problem hiding this comment.
When AuthCallback replaces onboarding, this runs from mount() on a new instance where $acceptedTerms is false, so the version is never sent. An attempt only exists after a ticked signIn(), so passing Client::TermsVersion here unconditionally is safe.
| ->and(file_get_contents(base_path('.env.example')))->not->toContain('DEVNULL_GITHUB_CLIENT_ID')->toContain("NATIVEPHP_DEEPLINK_SCHEME=devnull\n") | ||
| ->and(file_get_contents(base_path('README.md')))->not->toContain('DEVNULL_GITHUB_CLIENT_ID')->not->toContain('device flow'); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Pint (no_extra_blank_lines) flags the double blank line here, and again in tests/Pest.php at line 954.
What
The sign-in screen has a checkbox, "I agree to the terms of service and privacy policy. There's no tolerance for objectionable content or abusive users.", with links to both pages (dev-null.sh/terms and /privacy, in the app's browser). Sign in with GitHub and Sign in with Apple stay disabled until it's ticked. This is what App Review's guideline 1.2 asks of an app with user-generated content.
The phone sends
terms_version(Client::TermsVersion, 2026-10-23) with the sign-in call for both ways in, so devnull records what was accepted.Depends on
dev-null-sh/dev-null.sh#1042, which records the version and updates the terms and privacy wording. Until it's merged the server ignores
terms_version; sign-in still works.Client::TermsVersionhas to match the server'sconfig('legal.terms_version'), or the server answers 422.Not in this PR
People already signed in on a phone aren't asked. The server now says in
GET /mewhether they accepted the version in force (terms_accepted), so a gate for them can follow.Verified
Pest passes (1189, arch test excluded). Checked on the iPhone 17 Pro simulator: buttons grey until the box is ticked, then enabled.
🤖 Generated with Claude Code