mirror of
https://github.com/rowboatlabs/rowboat.git
synced 2026-04-30 19:06:23 +02:00
feat(oauth): switch Google OAuth from PKCE to authorization code flow with client secret
Previously, the Google OAuth integration used a PKCE-only flow (no client
secret). This switches to a standard authorization code flow where the user
provides both a Client ID and Client Secret from a "Web application" type
OAuth client in Google Cloud Console. PKCE is retained alongside the secret
for defense in depth.
Key changes:
- oauth-client.ts: discoverConfiguration() and createStaticConfiguration()
now accept an optional clientSecret param. When provided, uses
ClientSecretPost instead of None() for client authentication.
- oauth-handler.ts: connectProvider() takes a credentials object
({clientId, clientSecret}) instead of a bare clientId. Removed eager
persistence of clientId before flow completion — credentials are now
only saved after successful token exchange. Renamed resolveClientId to
resolveClientCredentials to return both values from a single repo read.
- google-client-factory.ts: same resolveClientId → resolveCredentials
rename. Passes clientSecret to OAuth2Client constructor and
discoverConfiguration for token refresh.
- repo.ts: added clientSecret to ProviderConnectionSchema. Not exposed
to renderer via ClientFacingConfigSchema (stays main-process only).
- IPC: added clientSecret to oauth:connect request schema. Handler builds
a credentials object and passes it through.
- UI: GoogleClientIdModal now collects both Client ID and Client Secret
(password field). Always shown on connect — no in-memory credential
caching. Renamed google-client-id-store to google-credentials-store
with a unified {clientId, clientSecret} object.
- google-setup.md: updated to instruct users to create a "Web application"
type OAuth client (instead of UWP), add the localhost redirect URI, and
copy both Client ID and Client Secret. Added credentials modal screenshot.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Ramnique Singh
parent
924e136505
commit
50bce6c1d6
15 changed files with 155 additions and 138 deletions
|
|
@ -37,9 +37,10 @@ function toOAuthTokens(response: client.TokenEndpointResponse): OAuthTokens {
|
|||
*/
|
||||
export async function discoverConfiguration(
|
||||
issuerUrl: string,
|
||||
clientId: string
|
||||
clientId: string,
|
||||
clientSecret?: string
|
||||
): Promise<client.Configuration> {
|
||||
const cacheKey = `${issuerUrl}:${clientId}`;
|
||||
const cacheKey = `${issuerUrl}:${clientId}:${clientSecret ? 'secret' : 'none'}`;
|
||||
|
||||
const cached = configCache.get(cacheKey);
|
||||
if (cached) {
|
||||
|
|
@ -50,8 +51,8 @@ export async function discoverConfiguration(
|
|||
const config = await client.discovery(
|
||||
new URL(issuerUrl),
|
||||
clientId,
|
||||
undefined, // no client_secret (PKCE flow)
|
||||
client.None(), // PKCE doesn't require client authentication
|
||||
clientSecret ?? undefined,
|
||||
clientSecret ? client.ClientSecretPost(clientSecret) : client.None(),
|
||||
{
|
||||
execute: [client.allowInsecureRequests],
|
||||
}
|
||||
|
|
@ -69,7 +70,8 @@ export function createStaticConfiguration(
|
|||
authorizationEndpoint: string,
|
||||
tokenEndpoint: string,
|
||||
clientId: string,
|
||||
revocationEndpoint?: string
|
||||
revocationEndpoint?: string,
|
||||
clientSecret?: string
|
||||
): client.Configuration {
|
||||
console.log(`[OAuth] Creating static configuration (no discovery)`);
|
||||
|
||||
|
|
@ -86,8 +88,8 @@ export function createStaticConfiguration(
|
|||
return new client.Configuration(
|
||||
serverMetadata,
|
||||
clientId,
|
||||
undefined, // no client_secret
|
||||
client.None() // PKCE auth
|
||||
clientSecret ?? undefined,
|
||||
clientSecret ? client.ClientSecretPost(clientSecret) : client.None()
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ import z from 'zod';
|
|||
const ProviderConnectionSchema = z.object({
|
||||
tokens: OAuthTokens.nullable().optional(),
|
||||
clientId: z.string().nullable().optional(),
|
||||
clientSecret: z.string().nullable().optional(),
|
||||
error: z.string().nullable().optional(),
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -18,21 +18,23 @@ export class GoogleClientFactory {
|
|||
client: OAuth2Client | null;
|
||||
tokens: OAuthTokens | null;
|
||||
clientId: string | null;
|
||||
clientSecret: string | null;
|
||||
} = {
|
||||
config: null,
|
||||
client: null,
|
||||
tokens: null,
|
||||
clientId: null,
|
||||
clientSecret: null,
|
||||
};
|
||||
|
||||
private static async resolveClientId(): Promise<string> {
|
||||
private static async resolveCredentials(): Promise<{ clientId: string; clientSecret?: string }> {
|
||||
const oauthRepo = container.resolve<IOAuthRepo>('oauthRepo');
|
||||
const { clientId } = await oauthRepo.read(this.PROVIDER_NAME);
|
||||
if (!clientId) {
|
||||
const connection = await oauthRepo.read(this.PROVIDER_NAME);
|
||||
if (!connection.clientId) {
|
||||
await oauthRepo.upsert(this.PROVIDER_NAME, { error: 'Google client ID missing. Please reconnect.' });
|
||||
throw new Error('Google client ID missing. Please reconnect.');
|
||||
}
|
||||
return clientId;
|
||||
return { clientId: connection.clientId, clientSecret: connection.clientSecret ?? undefined };
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -82,9 +84,11 @@ export class GoogleClientFactory {
|
|||
// Update cached tokens and recreate client
|
||||
this.cache.tokens = refreshedTokens;
|
||||
if (!this.cache.clientId) {
|
||||
this.cache.clientId = await this.resolveClientId();
|
||||
const creds = await this.resolveCredentials();
|
||||
this.cache.clientId = creds.clientId;
|
||||
this.cache.clientSecret = creds.clientSecret ?? null;
|
||||
}
|
||||
this.cache.client = this.createClientFromTokens(refreshedTokens, this.cache.clientId);
|
||||
this.cache.client = this.createClientFromTokens(refreshedTokens, this.cache.clientId, this.cache.clientSecret ?? undefined);
|
||||
console.log(`[OAuth] Token refreshed successfully`);
|
||||
return this.cache.client;
|
||||
} catch (error) {
|
||||
|
|
@ -105,9 +109,11 @@ export class GoogleClientFactory {
|
|||
console.log(`[OAuth] Creating new OAuth2Client instance`);
|
||||
this.cache.tokens = tokens;
|
||||
if (!this.cache.clientId) {
|
||||
this.cache.clientId = await this.resolveClientId();
|
||||
const creds = await this.resolveCredentials();
|
||||
this.cache.clientId = creds.clientId;
|
||||
this.cache.clientSecret = creds.clientSecret ?? null;
|
||||
}
|
||||
this.cache.client = this.createClientFromTokens(tokens, this.cache.clientId);
|
||||
this.cache.client = this.createClientFromTokens(tokens, this.cache.clientId, this.cache.clientSecret ?? undefined);
|
||||
return this.cache.client;
|
||||
}
|
||||
|
||||
|
|
@ -138,19 +144,20 @@ export class GoogleClientFactory {
|
|||
this.cache.client = null;
|
||||
this.cache.tokens = null;
|
||||
this.cache.clientId = null;
|
||||
this.cache.clientSecret = null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Initialize cached configuration (called once)
|
||||
*/
|
||||
private static async initializeConfigCache(): Promise<void> {
|
||||
const clientId = await this.resolveClientId();
|
||||
const { clientId, clientSecret } = await this.resolveCredentials();
|
||||
|
||||
if (this.cache.config && this.cache.clientId === clientId) {
|
||||
return; // Already initialized for this client ID
|
||||
if (this.cache.config && this.cache.clientId === clientId && this.cache.clientSecret === (clientSecret ?? null)) {
|
||||
return; // Already initialized for these credentials
|
||||
}
|
||||
|
||||
if (this.cache.clientId && this.cache.clientId !== clientId) {
|
||||
if (this.cache.clientId && (this.cache.clientId !== clientId || this.cache.clientSecret !== (clientSecret ?? null))) {
|
||||
this.clearCache();
|
||||
}
|
||||
|
||||
|
|
@ -163,18 +170,19 @@ export class GoogleClientFactory {
|
|||
console.log(`[OAuth] Discovery mode: issuer with static client ID`);
|
||||
this.cache.config = await oauthClient.discoverConfiguration(
|
||||
providerConfig.discovery.issuer,
|
||||
clientId
|
||||
clientId,
|
||||
clientSecret
|
||||
);
|
||||
} else {
|
||||
// DCR mode - need existing registration
|
||||
console.log(`[OAuth] Discovery mode: issuer with DCR`);
|
||||
const clientRepo = container.resolve<IClientRegistrationRepo>('clientRegistrationRepo');
|
||||
const existingRegistration = await clientRepo.getClientRegistration(this.PROVIDER_NAME);
|
||||
|
||||
|
||||
if (!existingRegistration) {
|
||||
throw new Error('Google client not registered. Please connect account first.');
|
||||
}
|
||||
|
||||
|
||||
this.cache.config = await oauthClient.discoverConfiguration(
|
||||
providerConfig.discovery.issuer,
|
||||
existingRegistration.client_id
|
||||
|
|
@ -185,28 +193,29 @@ export class GoogleClientFactory {
|
|||
if (providerConfig.client.mode !== 'static') {
|
||||
throw new Error('DCR requires discovery mode "issuer", not "static"');
|
||||
}
|
||||
|
||||
|
||||
console.log(`[OAuth] Using static endpoints (no discovery)`);
|
||||
this.cache.config = oauthClient.createStaticConfiguration(
|
||||
providerConfig.discovery.authorizationEndpoint,
|
||||
providerConfig.discovery.tokenEndpoint,
|
||||
clientId,
|
||||
providerConfig.discovery.revocationEndpoint
|
||||
providerConfig.discovery.revocationEndpoint,
|
||||
clientSecret
|
||||
);
|
||||
}
|
||||
|
||||
this.cache.clientId = clientId;
|
||||
this.cache.clientSecret = clientSecret ?? null;
|
||||
console.log(`[OAuth] Google OAuth configuration initialized`);
|
||||
}
|
||||
|
||||
/**
|
||||
* Create OAuth2Client from OAuthTokens
|
||||
*/
|
||||
private static createClientFromTokens(tokens: OAuthTokens, clientId: string): OAuth2Client {
|
||||
// Create OAuth2Client directly (PKCE flow doesn't use client secret)
|
||||
private static createClientFromTokens(tokens: OAuthTokens, clientId: string, clientSecret?: string): OAuth2Client {
|
||||
const client = new OAuth2Client(
|
||||
clientId,
|
||||
undefined, // client_secret not needed for PKCE
|
||||
clientSecret ?? undefined,
|
||||
undefined // redirect_uri not needed for token usage
|
||||
);
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue