From d71fa962d70f58c8cb95de63bdb5bf3da2a6bce6 Mon Sep 17 00:00:00 2001 From: Shane Rosenthal Date: Fri, 23 Oct 2026 12:55:28 -0400 Subject: [PATCH] Ask people to agree to the terms and privacy policy before they sign in 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 --- app/Devnull/Client.php | 17 +++++-- app/NativeComponents/Onboarding.php | 29 +++++++++-- resources/views/native/onboarding.blade.php | 26 +++++++++- tests/Feature/Screens/OnboardingAppleTest.php | 44 ++++++++--------- tests/Feature/Screens/OnboardingTest.php | 49 ++++++++++++++----- tests/Pest.php | 15 ++++++ 6 files changed, 138 insertions(+), 42 deletions(-) diff --git a/app/Devnull/Client.php b/app/Devnull/Client.php index c4da605..17aa2f2 100644 --- a/app/Devnull/Client.php +++ b/app/Devnull/Client.php @@ -38,6 +38,12 @@ */ public const string SignInPath = 'devices'; + /** + * The version of the terms and privacy policy the sign-in screen asks people to accept, which devnull records when + * the phone signs in. It's the date they took effect, and has to be the one devnull has in force. + */ + public const string TermsVersion = '2026-10-23'; + /** * Where the signed-in account's Apple ID is linked and unlinked. */ @@ -131,9 +137,13 @@ public static function pastelPath(string $id): string * phone knows, for the user it approved this phone for. It's signed like * every request, so the code only works for the key it was made for. */ - public function redeemSignIn(string $code, #[SensitiveParameter] string $verifier): User + public function redeemSignIn(string $code, #[SensitiveParameter] string $verifier, ?string $termsVersion = null): User { - return User::fromArray($this->data($this->send('POST', self::SignInPath, payload: ['code' => $code, 'code_verifier' => $verifier]))); + return User::fromArray($this->data($this->send('POST', self::SignInPath, payload: [ + 'code' => $code, + 'code_verifier' => $verifier, + ...($termsVersion === null ? [] : ['terms_version' => $termsVersion]), + ]))); } /** @@ -141,12 +151,13 @@ public function redeemSignIn(string $code, #[SensitiveParameter] string $verifie * an account, which approves this phone's key. Apple's identity token * carries sha256 of the nonce, and devnull checks it against the raw one. */ - public function signInWithApple(#[SensitiveParameter] string $identityToken, #[SensitiveParameter] string $nonce, string $deviceName): User + public function signInWithApple(#[SensitiveParameter] string $identityToken, #[SensitiveParameter] string $nonce, string $deviceName, ?string $termsVersion = null): User { return User::fromArray($this->data($this->send('POST', self::SignInPath, payload: [ 'apple_identity_token' => $identityToken, 'nonce' => $nonce, 'device_name' => $deviceName, + ...($termsVersion === null ? [] : ['terms_version' => $termsVersion]), ]))); } diff --git a/app/NativeComponents/Onboarding.php b/app/NativeComponents/Onboarding.php index 0b585aa..59e57d1 100644 --- a/app/NativeComponents/Onboarding.php +++ b/app/NativeComponents/Onboarding.php @@ -53,6 +53,12 @@ class Onboarding extends NativeComponent */ public ?string $appleError = null; + /** + * Whether the box for the terms and privacy policy is ticked. Neither way in works until it is, and devnull is told + * which version was accepted as the phone signs in. + */ + public bool $acceptedTerms = false; + private string $deviceName = ''; /** @@ -93,13 +99,28 @@ public function onResume(): void $this->finishSignIn(); } + public function toggleTerms(): void + { + $this->acceptedTerms = ! $this->acceptedTerms; + } + + public function openTerms(): void + { + Browser::inApp(rtrim((string) config('devnull.url'), '/').'/terms'); + } + + public function openPrivacy(): void + { + Browser::inApp(rtrim((string) config('devnull.url'), '/').'/privacy'); + } + /** * Opens dev-null.sh with a fresh verifier and state. Dismissing the sheet * sends nothing back, so the screen simply stays as it is. */ public function signIn(): void { - if ($this->fingerprint === '') { + if ($this->fingerprint === '' || ! $this->acceptedTerms) { return; } @@ -117,7 +138,7 @@ public function signIn(): void */ public function signInWithApple(): void { - if ($this->fingerprint === '' || ! AppleSignIn::isAvailable()) { + if ($this->fingerprint === '' || ! $this->acceptedTerms || ! AppleSignIn::isAvailable()) { return; } @@ -149,7 +170,7 @@ public function render(): View protected function useAppleToken(#[SensitiveParameter] string $identityToken, #[SensitiveParameter] string $nonce): void { try { - $user = app(Client::class)->signInWithApple($identityToken, $nonce, $this->deviceName()); + $user = app(Client::class)->signInWithApple($identityToken, $nonce, $this->deviceName(), $this->acceptedTerms ? Client::TermsVersion : null); } catch (ApiException $exception) { $this->appleError = $exception->firstError(); @@ -220,7 +241,7 @@ private function finishSignIn(): void } try { - $user = app(Client::class)->redeemSignIn($code, $attempt->verifier()); + $user = app(Client::class)->redeemSignIn($code, $attempt->verifier(), $this->acceptedTerms ? Client::TermsVersion : null); } catch (ApiException $exception) { $this->error = $exception->firstError(); diff --git a/resources/views/native/onboarding.blade.php b/resources/views/native/onboarding.blade.php index c20b669..c29a5fc 100644 --- a/resources/views/native/onboarding.blade.php +++ b/resources/views/native/onboarding.blade.php @@ -17,12 +17,34 @@ {{ $appleError }} @endif + {{-- What Apple asks of an app with user-generated content: agree to the terms before you're in. Neither button works until it's ticked. --}} + + + + @if ($acceptedTerms) + + @endif + + + + I agree to the terms of service and privacy policy. There's no tolerance for objectionable content or abusive users. + + + Terms of service + + + Privacy policy + + + + + {{-- Black with white text (white with black in dark mode, the theme's accent), large, with GitHub's mark. --}} - {{ $error && $fingerprint !== '' ? 'Try again' : 'Sign in with GitHub' }} + {{ $error && $fingerprint !== '' ? 'Try again' : 'Sign in with GitHub' }} {{-- Apple's button the same: the accent with its logo, as big as GitHub's, as Apple's guidelines ask. --}} @if ($offersApple) - Sign in with Apple + Sign in with Apple @endif diff --git a/tests/Feature/Screens/OnboardingAppleTest.php b/tests/Feature/Screens/OnboardingAppleTest.php index 04bfbf8..1f644bc 100644 --- a/tests/Feature/Screens/OnboardingAppleTest.php +++ b/tests/Feature/Screens/OnboardingAppleTest.php @@ -30,8 +30,8 @@ $buttons = findNodes($screen->tree(), fn (array $node): bool => ($node['type'] ?? null) === 'button'); expect(array_column($buttons, 'ref'))->toBe(['sign-in', 'sign-in-apple']) - ->and(childRefs($screen, 'actions'))->toBe(['sign-in', 'sign-in-apple']) - ->and(shownText($screen))->toBe([]) + ->and(childRefs($screen, 'actions'))->toBe(['terms', 'sign-in', 'sign-in-apple']) + ->and(shownText($screen))->toBe(termsText()) ->and($buttons[1]['props'])->toMatchArray(['label' => 'Sign in with Apple', 'variant' => 'accent', 'size' => 'lg', 'leading_icon' => 'apple.logo']) ->and($buttons[0]['props'])->toMatchArray(['variant' => 'accent', 'size' => 'lg', 'leading_icon' => 'custom:github']) ->and($buttons[1]['layout'])->toEqual($buttons[0]['layout'])->toEqual(['width' => 'fill']) @@ -49,7 +49,7 @@ expect(AppleSignIn::isAvailable())->toBeFalse() ->and((new NativeServiceProvider(app()))->plugins())->not->toContain(NativephpSocialAuthServiceProvider::class) - ->and(childRefs($screen, 'actions'))->toBe(['sign-in']); + ->and(childRefs($screen, 'actions'))->toBe(['terms', 'sign-in']); fakeApi(['GET /api/v1/me' => [['data' => userWithCountsJson(), 'unread_notifications_count' => 0, 'apple_linked' => false]]]); signIn(); @@ -64,14 +64,14 @@ $bridge = appleBridge(); $screen = Native::visit('/onboarding', platform: 'ios'); - $screen->tap('sign-in-apple')->assertNoNavigation(); + $screen->set('acceptedTerms', true)->tap('sign-in-apple')->assertNoNavigation(); $first = appleSheetParams($bridge); expect($first['nonce'])->toBe(heldAppleNonce())->toMatch('/^[0-9a-f]{64}$/') ->and($first['scopes'])->toBe([]) ->and($first['state'])->toBeNull(); - $screen->tap('sign-in-apple'); + $screen->set('acceptedTerms', true)->tap('sign-in-apple'); $second = appleSheetParams($bridge); expect($second['nonce'])->toBe(heldAppleNonce())->not->toBe($first['nonce']); @@ -86,7 +86,7 @@ 'POST /api/v1/devices' => [['data' => userWithCountsJson()]], 'GET /api/v1/users/local_dev' => [['data' => userWithCountsJson()]], ]); - $screen = Native::visit('/onboarding', platform: 'ios')->tap('sign-in-apple'); + $screen = Native::visit('/onboarding', platform: 'ios')->set('acceptedTerms', true)->tap('sign-in-apple'); $nonce = heldAppleNonce(); appleAnswers($screen, $nonce)->assertReplacedWith('/log'); @@ -99,7 +99,7 @@ Http::assertSentCount(2); Http::assertSent(fn (Request $request): bool => $request->method() === 'POST' && $request->url() === 'http://devnull.test/api/v1/'.Client::SignInPath - && $request->body() === json_encode(['apple_identity_token' => AppleIdentityToken, 'nonce' => $nonce, 'device_name' => 'devnull on iPhone']) + && $request->body() === json_encode(['apple_identity_token' => AppleIdentityToken, 'nonce' => $nonce, 'device_name' => 'devnull on iPhone', 'terms_version' => Client::TermsVersion]) && $request->hasHeader('X-SSH-Signature')); // Apple's token carries sha256(nonce) as hex, which is what devnull checks against the raw nonce. @@ -110,7 +110,7 @@ appleBridge(); $message = ErrorCode::AppleNotLinked->message(); fakeApi(['POST /api/v1/devices' => [['message' => 'No /dev/null account is linked to this Apple ID.', 'code' => 'apple_not_linked'], 404]]); - $screen = Native::visit('/onboarding', platform: 'ios')->tap('sign-in-apple'); + $screen = Native::visit('/onboarding', platform: 'ios')->set('acceptedTerms', true)->tap('sign-in-apple'); $screen = appleAnswers($screen, heldAppleNonce()) ->assertNoNavigation() @@ -119,8 +119,8 @@ ->assertSet('error', null) ->assertElement('button', fn (array $node): bool => ($node['ref'] ?? null) === 'sign-in' && $node['props']['label'] === 'Sign in with GitHub'); - expect(childRefs($screen, 'actions'))->toBe(['apple-error', 'sign-in', 'sign-in-apple']) - ->and(shownText($screen))->toBe([$message]); + expect(childRefs($screen, 'actions'))->toBe(['apple-error', 'terms', 'sign-in', 'sign-in-apple']) + ->and(shownText($screen))->toBe([$message, ...termsText()]); expect(app(Account::class)->user())->toBeNull(); }); @@ -128,7 +128,7 @@ it("says when devnull can't verify Apple's token, without turning GitHub into Try again", function () { appleBridge(); fakeApi(['POST /api/v1/devices' => [['message' => 'That Apple sign-in could not be verified.', 'code' => 'invalid_apple_token'], 422]]); - $screen = Native::visit('/onboarding', platform: 'ios')->tap('sign-in-apple'); + $screen = Native::visit('/onboarding', platform: 'ios')->set('acceptedTerms', true)->tap('sign-in-apple'); appleAnswers($screen, heldAppleNonce()) ->assertNoNavigation() @@ -142,7 +142,7 @@ it("returns quietly when Apple's sheet is closed", function () { appleBridge(); fakeApi([]); - $screen = Native::visit('/onboarding', platform: 'ios')->tap('sign-in-apple'); + $screen = Native::visit('/onboarding', platform: 'ios')->set('acceptedTerms', true)->tap('sign-in-apple'); $screen->emitNative(SignInFailed::class, ['provider' => 'apple', 'error' => 'Sign in with Apple was cancelled.', 'errorCode' => AppleSignIn::Cancelled]) ->assertNoNavigation() @@ -150,7 +150,7 @@ ->assertSet('error', null) ->assertDontSee('cancelled'); - expect(shownText($screen))->toBe([]); + expect(shownText($screen))->toBe(termsText()); expect(heldAppleNonce())->toBeNull(); Http::assertNothingSent(); @@ -159,7 +159,7 @@ it("says so when Apple's sign-in fails another way, and ignores Google's failures", function () { appleBridge(); fakeApi([]); - $screen = Native::visit('/onboarding', platform: 'ios')->tap('sign-in-apple'); + $screen = Native::visit('/onboarding', platform: 'ios')->set('acceptedTerms', true)->tap('sign-in-apple'); $screen->emitNative(SignInFailed::class, ['provider' => 'google', 'error' => 'Nope.', 'errorCode' => 'SIGN_IN_FAILED']) ->assertSet('appleError', null); @@ -176,7 +176,7 @@ it('rejects an answer whose nonce is not the one it sent, and sends nothing', function (?string $nonce) { appleBridge(); fakeApi([]); - $screen = Native::visit('/onboarding', platform: 'ios')->tap('sign-in-apple'); + $screen = Native::visit('/onboarding', platform: 'ios')->set('acceptedTerms', true)->tap('sign-in-apple'); appleAnswers($screen, '', ['nonce' => $nonce]) ->assertNoNavigation() @@ -201,7 +201,7 @@ it('spends an attempt on its first answer, so a replayed one is rejected', function () { appleBridge(); fakeApi(['POST /api/v1/devices' => [['message' => 'That Apple sign-in could not be verified.', 'code' => 'invalid_apple_token'], 422]]); - $screen = Native::visit('/onboarding', platform: 'ios')->tap('sign-in-apple'); + $screen = Native::visit('/onboarding', platform: 'ios')->set('acceptedTerms', true)->tap('sign-in-apple'); $nonce = heldAppleNonce(); appleAnswers($screen, $nonce); @@ -213,7 +213,7 @@ it('says so when Apple sends no identity token', function () { appleBridge(); fakeApi([]); - $screen = Native::visit('/onboarding', platform: 'ios')->tap('sign-in-apple'); + $screen = Native::visit('/onboarding', platform: 'ios')->set('acceptedTerms', true)->tap('sign-in-apple'); appleAnswers($screen, heldAppleNonce(), ['identityToken' => null])->assertSet('appleError', AppleSignIn::NoToken); @@ -224,7 +224,7 @@ fakeKeychain(testSeed())->respondTo('NativephpSocialAuth.AppleSignIn', null); Native::visit('/onboarding', platform: 'ios') - ->tap('sign-in-apple') + ->set('acceptedTerms', true)->tap('sign-in-apple') ->assertSee(AppleSignIn::CouldNotOpen); expect(heldAppleNonce())->toBeNull(); @@ -238,7 +238,7 @@ }); $bridge = appleBridge(); fakeApi(['POST /api/v1/devices' => [['data' => userWithCountsJson()]]]); - $screen = Native::visit('/onboarding', platform: 'ios')->tap('sign-in-apple'); + $screen = Native::visit('/onboarding', platform: 'ios')->set('acceptedTerms', true)->tap('sign-in-apple'); $nonce = heldAppleNonce(); expect(print_r(app(AppleSignIn::class), true))->not->toContain($nonce) @@ -267,7 +267,7 @@ Native::visit('/onboarding', platform: 'ios') ->assertElement('button', fn (array $node): bool => ($node['ref'] ?? null) === 'sign-in-apple' && ($node['props']['disabled'] ?? false) === true) - ->call('signInWithApple'); + ->set('acceptedTerms', true)->call('signInWithApple'); $bridge->assertNotCalled('NativephpSocialAuth.AppleSignIn'); }); @@ -277,7 +277,7 @@ $screen = Native::visit('/onboarding', platform: 'android') ->assertDontSee('Sign in with Apple') - ->call('signInWithApple'); + ->set('acceptedTerms', true)->call('signInWithApple'); expect(findNodes($screen->tree(), fn (array $node): bool => ($node['type'] ?? null) === 'button'))->toHaveCount(1); $bridge->assertNotCalled('NativephpSocialAuth.AppleSignIn'); @@ -286,7 +286,7 @@ it("asks whether the phone has an Apple Account when Apple's error says nothing more", function () { appleBridge(); fakeApi([]); - $screen = Native::visit('/onboarding', platform: 'ios')->tap('sign-in-apple'); + $screen = Native::visit('/onboarding', platform: 'ios')->set('acceptedTerms', true)->tap('sign-in-apple'); // What closing iOS's "Sign in to your Apple Account" alert sends back. $screen->emitNative(SignInFailed::class, ['provider' => 'apple', 'error' => 'The operation couldn’t be completed. (com.apple.AuthenticationServices.AuthorizationError error 1000.) (ASAuthorizationError.unknown)', 'errorCode' => 'SIGN_IN_FAILED']) diff --git a/tests/Feature/Screens/OnboardingTest.php b/tests/Feature/Screens/OnboardingTest.php index b418b36..be53cd8 100644 --- a/tests/Feature/Screens/OnboardingTest.php +++ b/tests/Feature/Screens/OnboardingTest.php @@ -60,7 +60,7 @@ function openedAuthQuery(FakeBridge $bridge): array */ function tapSignIn(TestableComponent $screen, FakeBridge $bridge): string { - $screen->tap('sign-in'); + $screen->set('acceptedTerms', true)->tap('sign-in'); return openedAuthQuery($bridge)['state']; } @@ -97,7 +97,7 @@ function sendBack(TestableComponent $screen, array $query): TestableComponent $screen = Native::visit('/onboarding'); - expect(shownText($screen))->toBe([]); + expect(shownText($screen))->toBe(termsText()); $screen->assertDontSee('This phone makes its own key') ->assertSet('fingerprint', app(DeviceKey::class)->current()->fingerprint()) @@ -128,7 +128,7 @@ function sendBack(TestableComponent $screen, array $query): TestableComponent ->and($space['layout'])->toMatchArray(['width' => 'fill', 'flex_grow' => 1, 'align_items' => AlignItems::Center->value, 'justify_content' => JustifyContent::Center->value]) ->and(array_column($screen->tree()['children'], 'ref'))->toBe(['logo-space', 'actions']) ->and($screen->tree()['layout'])->toMatchArray(['height' => 'fill', 'safe_area' => 1]) - ->and(childRefs($screen, 'actions'))->toBe(['sign-in']) + ->and(childRefs($screen, 'actions'))->toBe(['terms', 'sign-in']) // Drawn at up to 293pt on the widest iPhone, so it's rendered 300pt wide at @3x, at the lockup's own 15:4. ->and([$width, $height])->toBe([900, 240]) ->and(getimagesize(public_path(TabsLayout::DarkLogo))[0])->toBe(900); @@ -152,7 +152,7 @@ function sendBack(TestableComponent $screen, array $query): TestableComponent $bridge = signInBridge(testSeed()); $screen = Native::visit('/onboarding'); - $screen->tap('sign-in')->assertNoNavigation(); + $screen->set('acceptedTerms', true)->tap('sign-in')->assertNoNavigation(); $url = $bridge->callsTo('Browser.OpenAuth')[0]['params']['url']; $query = openedAuthQuery($bridge); @@ -276,7 +276,7 @@ function sendBack(TestableComponent $screen, array $query): TestableComponent ->assertNoNavigation() ->assertSet('error', null); - expect(shownText($screen))->toBe([]); + expect(shownText($screen))->toBe(termsText()); $screen ->assertElement('button', fn (array $node): bool => ($node['ref'] ?? null) === 'sign-in' && $node['props']['label'] === 'Sign in with GitHub'); @@ -295,8 +295,8 @@ function sendBack(TestableComponent $screen, array $query): TestableComponent ->assertElement('button', fn (array $node): bool => ($node['ref'] ?? null) === 'sign-in' && $node['props']['label'] === 'Try again'); // Just above the buttons, and the only text on the screen. - expect(childRefs($screen, 'actions'))->toBe(['error', 'sign-in']) - ->and(shownText($screen))->toBe(['Something broke on our side.']); + expect(childRefs($screen, 'actions'))->toBe(['error', 'terms', 'sign-in']) + ->and(shownText($screen))->toBe(['Something broke on our side.', ...termsText()]); $retry = tapSignIn($screen, $bridge); @@ -377,7 +377,7 @@ function sendBack(TestableComponent $screen, array $query): TestableComponent it('says so when the auth browser does not open', function () { fakeKeychain(testSeed())->respondTo('Browser.OpenAuth', ['success' => false]); - Native::visit('/onboarding')->tap('sign-in')->assertSee('fatal: could not open dev-null.sh to sign in.'); + Native::visit('/onboarding')->set('acceptedTerms', true)->tap('sign-in')->assertSee('fatal: could not open dev-null.sh to sign in.'); }); it("doesn't say why a 401 sent it here: the buttons are enough", function (string $message) { @@ -385,8 +385,8 @@ function sendBack(TestableComponent $screen, array $query): TestableComponent $screen = Native::visit('/onboarding', ['message' => $message])->assertDontSee($message); - expect(shownText($screen))->toBe([]) - ->and(childRefs($screen, 'actions'))->toBe(['sign-in']); + expect(shownText($screen))->toBe(termsText()) + ->and(childRefs($screen, 'actions'))->toBe(['terms', 'sign-in']); })->with([ 'a key devnull no longer takes' => ['This key is no longer accepted.'], 'a phone to approve' => [ErrorCode::DeviceNotApproved->message()], @@ -402,7 +402,7 @@ function sendBack(TestableComponent $screen, array $query): TestableComponent ->assertSee('fatal: could not read the device key from the Keychain. Device is locked.') ->assertSet('fingerprint', '') ->assertElement('button', fn (array $node): bool => ($node['ref'] ?? null) === 'sign-in' && ($node['props']['disabled'] ?? false) === true) - ->call('signIn'); + ->set('acceptedTerms', true)->call('signIn'); $bridge->assertNotCalled('SecureStorage.Set')->assertNotCalled('Browser.OpenAuth'); }); @@ -454,3 +454,30 @@ function sendBack(TestableComponent $screen, array $query): TestableComponent ->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'); }); + + +it('keeps both ways in shut until the terms and privacy policy are agreed to, and links to them', function () { + $bridge = signInBridge(); + $bridge->respondTo('Browser.OpenInApp', ['success' => true]); + + $screen = Native::visit('/onboarding'); + + $disabled = fn (string $ref): bool => (bool) ($screen->tree() && (nodeByRef($screen, $ref)['props']['disabled'] ?? false)); + + expect($disabled('sign-in'))->toBeTrue(); + + $screen->call('signIn'); + + expect($bridge->callsTo('Browser.OpenAuth'))->toBe([]); + + $screen->press('accept-terms')->assertSet('acceptedTerms', true)->call('signIn'); + + expect($bridge->callsTo('Browser.OpenAuth'))->toHaveCount(1); + + $screen->press('terms-link') + ->assertNativeCalled('Browser.OpenInApp', fn (array $params): bool => $params['url'] === 'http://devnull.test/terms') + ->press('privacy-link') + ->assertNativeCalled('Browser.OpenInApp', fn (array $params): bool => $params['url'] === 'http://devnull.test/privacy'); + + $screen->press('accept-terms')->assertSet('acceptedTerms', false); +}); diff --git a/tests/Pest.php b/tests/Pest.php index 9ea9bd6..e91bb66 100644 --- a/tests/Pest.php +++ b/tests/Pest.php @@ -951,3 +951,18 @@ function sentTo(string $method, string $path): array ->values() ->all(); } + + +/** + * What the sign-in screen says about the terms, above its buttons. + * + * @return list + */ +function termsText(): array +{ + return [ + 'I agree to the terms of service and privacy policy. There\'s no tolerance for objectionable content or abusive users.', + 'Terms of service', + 'Privacy policy', + ]; +}