Repository navigation
fix: refresh ssh keys while you are signed in #1244
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: production
Are you sure you want to change the base?
Changes from all commits
feccac7
8759e00
0f2f862
ffa867e
6185539
513bf9b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace App\Http\Middleware; | ||
|
|
||
| use App\Jobs\SyncGitHubSshKeysJob; | ||
| use App\Models\User; | ||
| use Closure; | ||
| use Illuminate\Http\Request; | ||
| use Illuminate\Support\Facades\Cache; | ||
| use Symfony\Component\HttpFoundation\JsonResponse; | ||
| use Symfony\Component\HttpFoundation\Response; | ||
|
|
||
| final readonly class QueueGitHubSshKeySync | ||
| { | ||
| /** | ||
| * A signed-in page visit queues a refresh of that person's GitHub SSH keys, so a key added on GitHub shows up | ||
| * without signing out. One refresh costs three GitHub requests on the shared hourly quota, so only a real page | ||
| * load counts. A star or another action does not, nor does polling, a partial reload (including one that only | ||
| * names props to leave out), a prefetch, or JSON. A full Inertia visit does, and a terminal request does not. | ||
| * A visit skips the queue when a sync just finished, and otherwise only the first visit in an hour queues one. | ||
| * Sync from GitHub on the profile still refreshes the keys right away. A failure is reported, never thrown: | ||
| * refreshing keys must not break the page. | ||
| * | ||
| * @param Closure(Request): (Response) $next | ||
| */ | ||
| public function handle(Request $request, Closure $next): Response | ||
| { | ||
| $response = $next($request); | ||
| $user = $request->user(); | ||
|
|
||
| $isPageVisit = $request->isMethod('GET') | ||
| && ! $request->prefetch() | ||
| && ! $request->hasHeader('X-Inertia-Partial-Data') | ||
| && ! $request->hasHeader('X-Inertia-Partial-Except') | ||
| && ($request->hasHeader('X-Inertia') || (! $request->expectsJson() && ! $response instanceof JsonResponse)); | ||
|
|
||
| if ($user instanceof User && $response->isSuccessful() && $isPageVisit && ! $request->is('api/cli/*')) { | ||
| rescue(function () use ($user): void { | ||
| if (Cache::has("ssh-keys-synced:{$user->id}")) { | ||
| return; | ||
| } | ||
|
|
||
| if (! Cache::add("ssh-keys-sync-queued:{$user->id}", true, now()->addHour())) { | ||
| return; | ||
| } | ||
|
|
||
| dispatch(new SyncGitHubSshKeysJob($user)); | ||
| }); | ||
| } | ||
|
|
||
| return $response; | ||
| } | ||
| } | ||
|
Comment on lines
+35
to
+55
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,166 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| use App\Enums\Log; | ||
| use App\Http\Middleware\HandleInertiaRequests; | ||
| use App\Jobs\SyncGitHubSshKeysJob; | ||
| use App\Models\Commit; | ||
| use App\Models\SshKey; | ||
| use App\Models\User; | ||
| use Illuminate\Database\QueryException; | ||
| use Illuminate\Support\Facades\Cache; | ||
| use Illuminate\Support\Facades\Exceptions; | ||
| use Illuminate\Support\Facades\Http; | ||
| use Illuminate\Support\Facades\Queue; | ||
| use Illuminate\Support\Facades\Schema; | ||
| use Tests\Support\SshKeyPair; | ||
|
|
||
| beforeEach(function (): void { | ||
| $this->user = User::factory()->create(); | ||
| }); | ||
|
|
||
| it('queues a sync of the github ssh keys when a signed in user opens a page', function (): void { | ||
| Queue::fake(); | ||
| Http::fake(); | ||
|
|
||
| $this->actingAs($this->user)->get(route('log.show', Log::Following))->assertOk(); | ||
|
|
||
| Queue::assertPushed(SyncGitHubSshKeysJob::class, fn (SyncGitHubSshKeysJob $job): bool => $job->user->is($this->user)); | ||
| Http::assertNothingSent(); | ||
| }); | ||
|
|
||
| it('does not queue another sync when the keys were just synced', function (): void { | ||
| Queue::fake(); | ||
| Cache::put("ssh-keys-synced:{$this->user->id}", true, now()->addMinutes(5)); | ||
|
|
||
| $this->actingAs($this->user)->get(route('log.show', Log::Following))->assertOk(); | ||
|
|
||
| Queue::assertNotPushed(SyncGitHubSshKeysJob::class); | ||
| }); | ||
|
|
||
| it('does not queue another sync while one is already queued', function (): void { | ||
| Queue::fake(); | ||
|
|
||
| $this->actingAs($this->user)->get(route('log.show', Log::Following))->assertOk(); | ||
| $this->actingAs($this->user)->get(route('log.show', Log::Following))->assertOk(); | ||
|
|
||
| Queue::assertPushed(SyncGitHubSshKeysJob::class, 1); | ||
| }); | ||
|
|
||
| it('queues the sync again after an hour', function (): void { | ||
| Queue::fake(); | ||
|
|
||
| $this->actingAs($this->user)->get(route('log.show', Log::Following))->assertOk(); | ||
|
|
||
| $this->travel(61)->seconds(); | ||
|
|
||
| $this->actingAs($this->user)->get(route('log.show', Log::Following))->assertOk(); | ||
|
|
||
| Queue::assertPushed(SyncGitHubSshKeysJob::class, 1); | ||
|
|
||
| $this->travel(1)->hour(); | ||
|
|
||
| $this->actingAs($this->user)->get(route('log.show', Log::Following))->assertOk(); | ||
|
|
||
| Queue::assertPushed(SyncGitHubSshKeysJob::class, 2); | ||
| }); | ||
|
|
||
| it('does not queue a sync for what a page requests on its own', function (array $headers): void { | ||
| Queue::fake(); | ||
|
|
||
| if (array_key_exists('X-Inertia', $headers)) { | ||
| $headers['X-Inertia-Version'] = (string) new HandleInertiaRequests()->version(request()); | ||
| } | ||
|
|
||
| $this->actingAs($this->user)->get(route('log.show', Log::Following), $headers)->assertOk(); | ||
|
|
||
| Queue::assertNotPushed(SyncGitHubSshKeysJob::class); | ||
| })->with([ | ||
| 'a partial reload' => [['X-Inertia' => 'true', 'X-Inertia-Partial-Component' => 'log/show', 'X-Inertia-Partial-Data' => 'commits']], | ||
| 'an except-only partial reload' => [['X-Inertia' => 'true', 'X-Inertia-Partial-Component' => 'log/show', 'X-Inertia-Partial-Except' => 'commits']], | ||
| 'a prefetch' => [['Purpose' => 'prefetch']], | ||
| 'a json request' => [['Accept' => 'application/json']], | ||
| ]); | ||
|
|
||
| it('does not queue a sync for a request that fails', function (): void { | ||
| Queue::fake(); | ||
|
|
||
| $this->actingAs($this->user)->get(route('operations.show'))->assertNotFound(); | ||
|
|
||
| Queue::assertNotPushed(SyncGitHubSshKeysJob::class); | ||
| }); | ||
|
|
||
| it('queues a sync for a full inertia visit', function (): void { | ||
| Queue::fake(); | ||
|
|
||
| $this->actingAs($this->user)->get(route('log.show', Log::Following), [ | ||
| 'X-Inertia' => 'true', | ||
| 'X-Inertia-Version' => (string) new HandleInertiaRequests()->version(request()), | ||
| ])->assertOk(); | ||
|
|
||
| Queue::assertPushed(SyncGitHubSshKeysJob::class, 1); | ||
| }); | ||
|
|
||
| it('does not queue a sync for the commit status poll', function (): void { | ||
| Queue::fake(); | ||
|
|
||
| $this->actingAs($this->user) | ||
| ->get(route('commit-status.show', Commit::factory()->for($this->user)->create())) | ||
| ->assertOk(); | ||
|
|
||
| Queue::assertNotPushed(SyncGitHubSshKeysJob::class); | ||
| }); | ||
|
|
||
| it('does not queue a sync when a signed in user does something without opening a page', function (): void { | ||
| Queue::fake(); | ||
|
|
||
| $this->actingAs($this->user) | ||
| ->postJson(route('commit-star.store', Commit::factory()->create())) | ||
| ->assertOk(); | ||
|
|
||
| Queue::assertNotPushed(SyncGitHubSshKeysJob::class); | ||
| }); | ||
|
|
||
| it('does not queue a sync for a guest', function (): void { | ||
| Queue::fake(); | ||
|
|
||
| $this->get(route('log.show', Log::Trending))->assertOk(); | ||
|
|
||
| Queue::assertNotPushed(SyncGitHubSshKeysJob::class); | ||
| }); | ||
|
|
||
| it('does not call github when looking up an unknown key', function (): void { | ||
| Queue::fake(); | ||
| Http::fake(); | ||
|
|
||
| $this->call('POST', route('cli-key-lookup.store'), content: SshKeyPair::generate()->publicKey->toOpenSsh()) | ||
| ->assertNotFound(); | ||
|
|
||
| Http::assertNothingSent(); | ||
| Queue::assertNotPushed(SyncGitHubSshKeysJob::class); | ||
| }); | ||
|
|
||
| it('does not queue a sync for a terminal commit', function (): void { | ||
| Queue::fake(); | ||
| $keyPair = SshKeyPair::generate(); | ||
| SshKey::factory()->for($this->user)->withPublicKey($keyPair->publicKey)->create(); | ||
| fakeGitHubSshKeys($this->user, [$keyPair->publicKey]); | ||
| $commit = signedCommit($keyPair, 'It was DNS.'); | ||
|
|
||
| $this->call('POST', route('cli-commit.store'), server: [ | ||
| 'CONTENT_TYPE' => 'text/plain; charset=utf-8', | ||
| 'HTTP_X_SSH_SIGNATURE' => $commit['signature'], | ||
| ], content: $commit['payload'])->assertCreated(); | ||
|
|
||
| Queue::assertNotPushed(SyncGitHubSshKeysJob::class); | ||
| }); | ||
|
|
||
| it('reports a failure to queue the sync without breaking the page', function (): void { | ||
| Exceptions::fake(); | ||
| Schema::drop('jobs'); | ||
|
|
||
| $this->actingAs($this->user)->get(route('log.show', Log::Following))->assertOk(); | ||
|
|
||
| Exceptions::assertReported(QueryException::class); | ||
| }); |
Uh oh!
There was an error while loading. Please reload this page.