diff --git a/app/Devnull/SignInAttempt.php b/app/Devnull/SignInAttempt.php index 51f0d6b..1b01587 100644 --- a/app/Devnull/SignInAttempt.php +++ b/app/Devnull/SignInAttempt.php @@ -46,13 +46,17 @@ public function answers(string $state): bool /** * The page on dev-null.sh that signs you in with GitHub and approves this - * phone's key, then sends the browser back to the app with a code. + * phone's key, then sends the browser back to the app with a code. The + * app's name goes with the phone's, so Settings → Devices tells them apart, + * and its id, which stays when the name changes. */ - public function authorizeUrl(string $baseUrl, string $fingerprint, string $deviceName): string + public function authorizeUrl(string $baseUrl, string $fingerprint, string $deviceName, string $appName, string $appId): string { return rtrim($baseUrl, '/').'/mobile/authorize?'.http_build_query([ 'fingerprint' => $fingerprint, 'device_name' => $deviceName, + 'app_name' => $appName, + 'app_id' => $appId, 'code_challenge' => $this->challenge(), 'code_challenge_method' => 'S256', 'state' => $this->state, diff --git a/app/NativeComponents/Onboarding.php b/app/NativeComponents/Onboarding.php index c9b96b2..02b85b2 100644 --- a/app/NativeComponents/Onboarding.php +++ b/app/NativeComponents/Onboarding.php @@ -18,6 +18,7 @@ use Native\Mobile\Edge\NativeComponent; use Native\Mobile\Facades\Browser; use Native\Mobile\Facades\Device; +use Native\Mobile\Facades\System; use SensitiveParameter; /** @@ -107,7 +108,7 @@ public function signIn(): void $attempt = app(SignIn::class)->begin(); - if (! Browser::auth($attempt->authorizeUrl((string) config('devnull.url'), $this->fingerprint, $this->deviceName()))) { + if (! Browser::auth($attempt->authorizeUrl((string) config('devnull.url'), $this->fingerprint, $this->deviceName(), $this->appName(), $this->appId()))) { $this->error = 'fatal: could not open dev-null.sh to sign in.'; } } @@ -276,7 +277,8 @@ private function keepProfile(User $user): void } /** - * Names the phone on devnull after its model, so it's clear what to remove later. + * Names the phone on devnull after its model, so it's clear what to remove + * later. The app says its own name next to it, see appName(). */ private function deviceName(): string { @@ -284,9 +286,27 @@ private function deviceName(): string $info = json_decode((string) Device::getInfo(), true); $model = is_array($info) && is_string($info['model'] ?? null) ? trim($info['model']) : ''; - $this->deviceName = 'devnull on '.($model === '' ? 'a phone' : $model); + $this->deviceName = $model === '' ? 'a phone' : $model; } return $this->deviceName; } + + /** + * What devnull shows next to the phone's name, like "/dev/null for iOS on iPhone". + */ + private function appName(): string + { + return System::isIos() ? '/dev/null for iOS' : '/dev/null for Android'; + } + + /** + * The app's platform and bundle id, like "ios:com.nativephp.devnull": the + * iOS and Android apps share a bundle id, but they are different apps. A + * build whose .env doesn't carry the bundle id says the release one. + */ + private function appId(): string + { + return (System::isIos() ? 'ios:' : 'android:').(config('nativephp.app_id') ?: 'com.nativephp.devnull'); + } } diff --git a/tests/Feature/Screens/OnboardingAppleTest.php b/tests/Feature/Screens/OnboardingAppleTest.php index a8c1c37..f297ccf 100644 --- a/tests/Feature/Screens/OnboardingAppleTest.php +++ b/tests/Feature/Screens/OnboardingAppleTest.php @@ -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' => 'iPhone']) && $request->hasHeader('X-SSH-Signature')); // Apple's token carries sha256(nonce) as hex, which is what devnull checks against the raw nonce. diff --git a/tests/Feature/Screens/OnboardingTest.php b/tests/Feature/Screens/OnboardingTest.php index 4e66097..c358a3a 100644 --- a/tests/Feature/Screens/OnboardingTest.php +++ b/tests/Feature/Screens/OnboardingTest.php @@ -148,9 +148,10 @@ function sendBack(TestableComponent $screen, array $query): TestableComponent Native::visit('/onboarding')->assertSet('fingerprint', testKey()->fingerprint()); }); -it('opens dev-null.sh in the auth browser with the key, the device and an S256 challenge', function () { +it('opens dev-null.sh in the auth browser with the key, the device, the app and an S256 challenge', function () { + config(['nativephp.app_id' => 'com.nativephp.devnull']); $bridge = signInBridge(testSeed()); - $screen = Native::visit('/onboarding'); + $screen = Native::visit('/onboarding', platform: 'ios'); $screen->tap('sign-in')->assertNoNavigation(); @@ -159,15 +160,17 @@ function sendBack(TestableComponent $screen, array $query): TestableComponent $attempt = pendingAttempt(); expect($url)->toStartWith('http://devnull.test/mobile/authorize?') - ->and(array_keys($query))->toBe(['fingerprint', 'device_name', 'code_challenge', 'code_challenge_method', 'state', 'redirect_uri']) + ->and(array_keys($query))->toBe(['fingerprint', 'device_name', 'app_name', 'app_id', 'code_challenge', 'code_challenge_method', 'state', 'redirect_uri']) ->and($query['fingerprint'])->toBe(testKey()->fingerprint()) - ->and($query['device_name'])->toBe('devnull on iPhone') + ->and($query['device_name'])->toBe('iPhone') + ->and($query['app_name'])->toBe('/dev/null for iOS') + ->and($query['app_id'])->toBe('ios:com.nativephp.devnull') ->and($query['code_challenge_method'])->toBe('S256') ->and($query['redirect_uri'])->toBe('devnull://auth/callback') ->and($query['state'])->toBe($attempt->state)->toMatch('/^[A-Za-z0-9_-]{22}$/') ->and($attempt->verifier())->toMatch('/^[A-Za-z0-9_-]{43}$/') ->and($query['code_challenge'])->toBe(rtrim(strtr(base64_encode(hash('sha256', $attempt->verifier(), true)), '+/', '-_'), '=')) - ->and($url)->toContain('device_name=devnull%20on%20iPhone') + ->and($url)->toContain('device_name=iPhone&app_name=%2Fdev%2Fnull%20for%20iOS&app_id=ios%3Acom.nativephp.devnull') ->and($url)->toContain('redirect_uri=devnull%3A%2F%2Fauth%2Fcallback'); $bridge->assertOpenedAuth(); @@ -175,6 +178,24 @@ function sendBack(TestableComponent $screen, array $query): TestableComponent Http::assertNothingSent(); }); +it('names itself the Android app on Android, a different app from the iOS one', function () { + config(['nativephp.app_id' => 'com.nativephp.devnull']); + $bridge = signInBridge(testSeed()); + + Native::visit('/onboarding', platform: 'android')->tap('sign-in'); + + expect(openedAuthQuery($bridge))->toMatchArray(['app_name' => '/dev/null for Android', 'app_id' => 'android:com.nativephp.devnull']); +}); + +it('says the release bundle id when the build does not carry one', function () { + config(['nativephp.app_id' => null]); + $bridge = signInBridge(testSeed()); + + Native::visit('/onboarding', platform: 'ios')->tap('sign-in'); + + expect(openedAuthQuery($bridge)['app_id'])->toBe('ios:com.nativephp.devnull'); +}); + it('makes a fresh verifier and state for every attempt', function () { $bridge = signInBridge(testSeed()); $screen = Native::visit('/onboarding'); @@ -409,12 +430,14 @@ function sendBack(TestableComponent $screen, array $query): TestableComponent it('builds the authorize URL and checks state on its own', function () { $attempt = SignInAttempt::start(); - $url = $attempt->authorizeUrl('https://dev-null.sh/', 'SHA256:a+b/c', 'devnull on iPhone'); + $url = $attempt->authorizeUrl('https://dev-null.sh/', 'SHA256:a+b/c', 'iPhone', '/dev/null for iOS', 'ios:com.nativephp.devnull'); parse_str((string) parse_url($url, PHP_URL_QUERY), $query); expect($url)->toStartWith('https://dev-null.sh/mobile/authorize?') ->and($query['fingerprint'])->toBe('SHA256:a+b/c') + ->and($query['app_name'])->toBe('/dev/null for iOS') + ->and($query['app_id'])->toBe('ios:com.nativephp.devnull') ->and($query['code_challenge'])->toBe($attempt->challenge()) ->and($attempt->answers($attempt->state))->toBeTrue() ->and($attempt->answers(''))->toBeFalse()