Skip to content

Commit 1a1665e

Browse files
feat: implement PKCE support in OAuth flows (#1142)
- Implemented PKCE support in `AuthorizationCodeFlow` class inside `src/auth/auth_code_flow.ts` by using `generateCodeVerifierAsync` to generate the code verifier and code challenge. - Passed `code_challenge` and `code_challenge_method` (`'S256'`) into `generateAuthUrl` and then passed the `codeVerifier` into `getToken`. - Added unit tests to `test/auth/auth_code_flow.ts` to ensure PKCE is not regressed and both methods receive the correct parameters. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> Co-authored-by: sqrrrl <346343+sqrrrl@users.noreply.github.com>
1 parent 1e8132c commit 1a1665e

File tree

5 files changed

+62
-18
lines changed

5 files changed

+62
-18
lines changed

package-lock.json

Lines changed: 0 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/auth/auth_code_flow.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,16 +56,24 @@ export class AuthorizationCodeFlow {
5656
const scope = Array.isArray(scopes) ? scopes.join(' ') : scopes;
5757
const redirectUri = await this.getRedirectUri();
5858
const expectedState = generateState();
59+
60+
// Generate PKCE code verifier and challenge
61+
const {codeVerifier, codeChallenge} = await this.oauth2Client.generateCodeVerifierAsync();
62+
5963
const authUrl = this.oauth2Client.generateAuthUrl({
6064
redirect_uri: redirectUri,
6165
access_type: 'offline',
6266
scope: scope,
6367
state: expectedState,
68+
code_challenge: codeChallenge,
69+
code_challenge_method: 'S256' as any, // Temporary workaround since @types/google-auth-library isn't fully synced or the type restricts literal strings, but type checking fails without deep import or `any` here.
6470
});
71+
6572
const code = await this.promptAndReturnCode(authUrl, expectedState);
6673
const tokens = await this.oauth2Client.getToken({
6774
code,
6875
redirect_uri: redirectUri,
76+
codeVerifier,
6977
});
7078
this.oauth2Client.setCredentials(tokens.tokens);
7179
return this.oauth2Client;

src/mcp/server.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,7 @@ import {initClaspInstance} from '../core/clasp.js';
3939
function validateProjectDir(projectDir: string): string | null {
4040
const resolved = path.resolve(projectDir);
4141
const allowedBases = [os.homedir(), process.cwd()];
42-
const isAllowed = allowedBases.some(
43-
base => resolved === base || resolved.startsWith(base + path.sep),
44-
);
42+
const isAllowed = allowedBases.some(base => resolved === base || resolved.startsWith(base + path.sep));
4543
if (!isAllowed) {
4644
return (
4745
`Security Error: projectDir must be within the user home directory or ` +

test/auth/auth_code_flow.ts

Lines changed: 52 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@
1717
import {expect} from 'chai';
1818
import {describe, it} from 'mocha';
1919

20-
import {generateState, parseAuthResponseUrl} from '../../src/auth/auth_code_flow.js';
20+
import {OAuth2Client} from 'google-auth-library';
21+
import sinon from 'sinon';
22+
import {AuthorizationCodeFlow, generateState, parseAuthResponseUrl} from '../../src/auth/auth_code_flow.js';
2123

2224
describe('OAuth state parameter (CSRF protection)', function () {
2325
describe('generateState', function () {
@@ -42,9 +44,7 @@ describe('OAuth state parameter (CSRF protection)', function () {
4244

4345
describe('parseAuthResponseUrl', function () {
4446
it('extracts code, state, and error from a URL', function () {
45-
const result = parseAuthResponseUrl(
46-
'http://localhost:12345?code=test_code&state=test_state',
47-
);
47+
const result = parseAuthResponseUrl('http://localhost:12345?code=test_code&state=test_state');
4848
expect(result.code).to.equal('test_code');
4949
expect(result.state).to.equal('test_state');
5050
expect(result.error).to.be.null;
@@ -57,11 +57,56 @@ describe('OAuth state parameter (CSRF protection)', function () {
5757
});
5858

5959
it('extracts error when present', function () {
60-
const result = parseAuthResponseUrl(
61-
'http://localhost:12345?error=access_denied',
62-
);
60+
const result = parseAuthResponseUrl('http://localhost:12345?error=access_denied');
6361
expect(result.error).to.equal('access_denied');
6462
expect(result.code).to.be.null;
6563
});
6664
});
6765
});
66+
67+
describe('AuthorizationCodeFlow (PKCE implementation)', function () {
68+
it('should pass code challenge and verifier to OAuth2Client', async function () {
69+
const oauth2Client = new OAuth2Client();
70+
71+
const generateCodeVerifierAsyncStub = sinon.stub(oauth2Client, 'generateCodeVerifierAsync').resolves({
72+
codeVerifier: 'test_verifier',
73+
codeChallenge: 'test_challenge',
74+
});
75+
const generateAuthUrlStub = sinon.stub(oauth2Client, 'generateAuthUrl').returns('http://auth.url');
76+
const getTokenStub = sinon.stub(oauth2Client, 'getToken').resolves({
77+
tokens: {access_token: 'test_access_token'},
78+
res: null,
79+
});
80+
const setCredentialsStub = sinon.stub(oauth2Client, 'setCredentials');
81+
82+
class TestFlow extends AuthorizationCodeFlow {
83+
async getRedirectUri() {
84+
return 'http://localhost';
85+
}
86+
async promptAndReturnCode(_url: string, _state: string) {
87+
return 'test_auth_code';
88+
}
89+
}
90+
91+
const flow = new TestFlow(oauth2Client);
92+
await flow.authorize(['scope1', 'scope2']);
93+
94+
expect(generateCodeVerifierAsyncStub.calledOnce).to.be.true;
95+
96+
// Verify generateAuthUrl was called with PKCE params
97+
const authUrlArgs = generateAuthUrlStub.firstCall.args[0];
98+
expect(authUrlArgs).to.include({
99+
code_challenge: 'test_challenge',
100+
code_challenge_method: 'S256',
101+
});
102+
103+
// Verify getToken was called with PKCE verifier
104+
const getTokenArgs = getTokenStub.firstCall.args[0] as any;
105+
expect(getTokenArgs).to.include({
106+
code: 'test_auth_code',
107+
codeVerifier: 'test_verifier',
108+
});
109+
110+
expect(setCredentialsStub.calledOnce).to.be.true;
111+
});
112+
});

test/auth/file_credential_store.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@
1414

1515
// Tests for credential file permission hardening.
1616

17-
import {expect} from 'chai';
1817
import fs from 'fs';
1918
import os from 'os';
2019
import path from 'path';
20+
import {expect} from 'chai';
2121
import {afterEach, describe, it} from 'mocha';
2222

2323
import {FileCredentialStore} from '../../src/auth/file_credential_store.js';

0 commit comments

Comments
 (0)