-
Notifications
You must be signed in to change notification settings - Fork 4
Add latchkey prepare command and PKCE for Google OAuth
#92
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
Changes from 8 commits
d0e946e
367bf2b
051ed22
626370b
eb4d3ca
2b45bcc
7c8b89f
eddaf26
86cd21d
d1473dc
09fd7be
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 |
|---|---|---|
|
|
@@ -20,12 +20,15 @@ import { | |
| } from '../../playwrightUtils.js'; | ||
| import { | ||
| exchangeCodeForTokens, | ||
| generateCodeChallenge, | ||
| generateCodeVerifier, | ||
| refreshAccessToken, | ||
| startOAuthCallbackServer, | ||
| } from '../../oauthUtils.js'; | ||
| import { | ||
| Service, | ||
| BrowserFollowupServiceSession, | ||
| buildPreparedCredentials, | ||
| LoginFailedError, | ||
| LoginCancelledError, | ||
| isBrowserClosedError, | ||
|
|
@@ -773,13 +776,21 @@ class GoogleServiceSession extends BrowserFollowupServiceSession { | |
| ); | ||
| const redirectUri = `http://localhost:${port.toString()}/oauth2callback`; | ||
|
|
||
| // PKCE (RFC 7636): bind the authorization code to a one-time verifier so a | ||
| // stolen code cannot be redeemed without it. We keep sending the client | ||
| // secret too so this is confidential-client + PKCE, defense-in-depth. | ||
| const codeVerifier = generateCodeVerifier(); | ||
| const codeChallenge = generateCodeChallenge(codeVerifier); | ||
|
|
||
| const authUrl = new URL('https://accounts.google.com/o/oauth2/v2/auth'); | ||
| authUrl.searchParams.set('client_id', clientId); | ||
| authUrl.searchParams.set('redirect_uri', redirectUri); | ||
| authUrl.searchParams.set('response_type', 'code'); | ||
| authUrl.searchParams.set('scope', allScopes.join(' ')); | ||
| authUrl.searchParams.set('access_type', 'offline'); | ||
| authUrl.searchParams.set('prompt', 'consent'); | ||
| authUrl.searchParams.set('code_challenge', codeChallenge); | ||
| authUrl.searchParams.set('code_challenge_method', 'S256'); | ||
|
|
||
| await page.goto(authUrl.toString()); | ||
|
|
||
|
|
@@ -789,7 +800,8 @@ class GoogleServiceSession extends BrowserFollowupServiceSession { | |
| code, | ||
| clientId, | ||
| clientSecret, | ||
| redirectUri | ||
| redirectUri, | ||
| codeVerifier | ||
| ); | ||
| const accessTokenExpiresAt = new Date(Date.now() + tokens.expires_in * 1000).toISOString(); | ||
|
|
||
|
|
@@ -810,6 +822,20 @@ class GoogleServiceSession extends BrowserFollowupServiceSession { | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * JSON accepted by `latchkey auth prepare <google-service>`: the OAuth client | ||
| * credentials to use for that service. `.strict()` rejects unknown keys so | ||
| * typos are reported instead of silently ignored. | ||
| */ | ||
| export const GooglePrepareInputSchema = z | ||
| .object({ | ||
| clientId: z.string().min(1), | ||
| clientSecret: z.string().min(1), | ||
| }) | ||
| .strict(); | ||
|
|
||
| export type GooglePrepareInput = z.infer<typeof GooglePrepareInputSchema>; | ||
|
|
||
| /** | ||
| * Abstract base class for individual Google API services. | ||
| * | ||
|
|
@@ -822,6 +848,19 @@ export abstract class GoogleService extends Service { | |
|
|
||
| protected abstract readonly config: GoogleServiceConfig; | ||
|
|
||
| /** | ||
| * Google services accept an OAuth client's id/secret prepared | ||
| * in advance via `latchkey auth prepare`, stored as token-less OAuth credentials until login. | ||
| */ | ||
|
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. 🔴 Vet Issue The user request states 'Updated every Google service's info string (surfaced via services info) to recommend prepare first, with browser-prepare as the fallback' and 'Updated the README command overview.' However, the diff contains no changes to any service's info string nor to the README. The documentation updates described in the request are missing. |
||
| override prepareFromJson(parsedJson: unknown): ApiCredentials { | ||
| return buildPreparedCredentials( | ||
| this.name, | ||
| GooglePrepareInputSchema, | ||
| parsedJson, | ||
| ({ clientId, clientSecret }) => new OAuthCredentials(clientId, clientSecret) | ||
| ); | ||
| } | ||
|
|
||
| setCredentialsExample(serviceName: string): string { | ||
| return `latchkey auth set ${serviceName} -H "Authorization: Bearer <token>"`; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 Vet Issue
commit_message_mismatchseverity: 3/5, confidence: 0.80The request explicitly states 'Updated every Google service's info string (surfaced via services info) to recommend prepare first, with browser-prepare as the fallback' and 'Updated the README command overview.' Neither the README nor any service
infostrings are modified in the diff, leaving the documentation portion of the request unimplemented.