Skip to content

Commit

Permalink
fix: issues with refresh token and auth (#329)
Browse files Browse the repository at this point in the history
Handle Auth issues
- Missing credentials file - reauth (added tests)
- Invalid credentials file - reauth
- Incomplete token set  - reauth
- Missing access_token - refresh token


Problem:
We had a lot of auth and refresh token-related issues
- #282 
- #253
- #321

Most of these issues were happening while refreshing tokens. When a
refresh token is missing or API fails to renew the token, it leaves the
user in an abrupt state needing to explicitly run `neonctl auth`.


Solution: 

This change handles different states of the `credentials` file as listed
above. Also added unit tests for all these scenarios.
  • Loading branch information
Shridhad committed Feb 14, 2025
1 parent b160383 commit da3c845
Show file tree
Hide file tree
Showing 2 changed files with 286 additions and 82 deletions.
208 changes: 177 additions & 31 deletions src/commands/auth.test.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,21 @@
import { Api } from '@neondatabase/api-client';
import axios from 'axios';
import { vi, beforeAll, describe, afterAll, expect } from 'vitest';
import {
existsSync,
mkdtempSync,
readFileSync,
rmSync,
writeFileSync,
} from 'node:fs';
import { AddressInfo } from 'node:net';
import { mkdtempSync, rmSync, readFileSync, writeFileSync } from 'node:fs';
import { join } from 'path';
import { TokenSet } from 'openid-client';
import { Api } from '@neondatabase/api-client';
import { join } from 'path';
import { afterAll, beforeAll, beforeEach, describe, expect, vi } from 'vitest';

import { startOauthServer } from '../test_utils/oauth_server';
import { OAuth2Server } from 'oauth2-mock-server';
import * as authModule from '../auth';
import { test } from '../test_utils/fixtures';
import { startOauthServer } from '../test_utils/oauth_server';
import { authFlow, ensureAuth } from './auth';

vi.mock('open', () => ({ default: vi.fn((url: string) => axios.get(url)) }));
Expand Down Expand Up @@ -52,59 +59,198 @@ describe('ensureAuth', () => {
let configDir = '';
let oauthServer: OAuth2Server;
let mockApiClient: Api<unknown>;
let authSpy: any;
let refreshTokenSpy: any;

beforeAll(async () => {
configDir = mkdtempSync('test-config');
oauthServer = await startOauthServer();
mockApiClient = {} as Api<unknown>;
authSpy = vi.spyOn(authModule, 'auth');
refreshTokenSpy = vi.spyOn(authModule, 'refreshToken');
});

afterAll(async () => {
rmSync(configDir, { recursive: true });
await oauthServer.stop();
vi.restoreAllMocks();
});

beforeEach(() => {
authSpy.mockClear();
refreshTokenSpy.mockClear();
});

const setupTestProps = (server: any) => ({
_: ['some-command'],
configDir,
oauthHost: `http://localhost:${oauthServer.address().port}`,
clientId: 'test-client-id',
forceAuth: true,
apiKey: '',
apiHost: `http://localhost:${(server.address() as AddressInfo).port}`,
help: false,
apiClient: mockApiClient,
});

test('should start new auth flow when refresh token fails', async ({
runMockServer,
}) => {
// Mock refresh token to fail
vi.mock('../auth.ts', async (importOriginal) => {
const actual = await importOriginal<object>();
return {
...actual,
refreshToken: vi.fn(() =>
Promise.reject(new Error('AUTH_REFRESH_FAILED')),
),
};
});
refreshTokenSpy.mockImplementationOnce(() =>
Promise.reject(new Error('AUTH_REFRESH_FAILED')),
);

authSpy.mockImplementationOnce(() =>
Promise.resolve(
new TokenSet({
access_token: 'new-auth-token',
refresh_token: 'new-refresh-token',
expires_at: Math.floor(Date.now() / 1000) + 3600,
}),
),
);

const server = await runMockServer('main');
// Setup expired token
const expiredTokenSet = new TokenSet({
access_token: 'expired-token',
refresh_token: 'refresh-token',
expires_at: Math.floor(Date.now() / 1000) - 3600, // 1 hour ago
});

writeFileSync(
join(configDir, 'credentials.json'),
JSON.stringify(expiredTokenSet),
{ mode: 0o700 },
);

const props = setupTestProps(server);
await ensureAuth(props);

expect(refreshTokenSpy).toHaveBeenCalledTimes(1);
expect(authSpy).toHaveBeenCalledTimes(1);
expect(props.apiKey).toBe('new-auth-token');
});

test('should trigger auth flow when credentials.json does not exist', async ({
runMockServer,
}) => {
const server = await runMockServer('main');

// Ensure the credentials file does not exist
const credentialsPath = join(configDir, 'credentials.json');
writeFileSync(credentialsPath, JSON.stringify(expiredTokenSet), {
mode: 0o700,
});
if (existsSync(credentialsPath)) {
rmSync(credentialsPath);
}

const props = {
_: ['some-command'],
configDir,
oauthHost: `http://localhost:${oauthServer.address().port}`,
clientId: 'test-client-id',
forceAuth: true,
apiKey: '',
apiHost: `http://localhost:${(server.address() as AddressInfo).port}`,
help: false,
apiClient: mockApiClient,
};
const props = setupTestProps(server);
await ensureAuth(props);

expect(authSpy).toHaveBeenCalledTimes(1);
expect(refreshTokenSpy).not.toHaveBeenCalled();
expect(props.apiKey).toEqual(expect.any(String));
});

test('should trigger auth flow when credentials.json is invalid', async ({
runMockServer,
}) => {
const server = await runMockServer('main');

// Write an empty credentials file
writeFileSync(join(configDir, 'credentials.json'), '', { mode: 0o700 });

const props = setupTestProps(server);
await ensureAuth(props);
expect(props.apiKey).not.toBe('expired-token');

expect(authSpy).toHaveBeenCalledTimes(1);
expect(refreshTokenSpy).not.toHaveBeenCalled();
expect(props.apiKey).toEqual(expect.any(String));
});

test('should try refresh when token is missing access_token but has refresh_token', async ({
runMockServer,
}) => {
const server = await runMockServer('main');
const tokenWithoutAccess = new TokenSet({
refresh_token: 'refresh-token',
});

writeFileSync(
join(configDir, 'credentials.json'),
JSON.stringify(tokenWithoutAccess),
{ mode: 0o700 },
);

refreshTokenSpy.mockImplementationOnce(() =>
Promise.resolve(
new TokenSet({
access_token: 'refreshed-token',
refresh_token: 'new-refresh-token',
expires_at: Math.floor(Date.now() / 1000) + 3600,
}),
),
);

const props = setupTestProps(server);
await ensureAuth(props);

expect(refreshTokenSpy).toHaveBeenCalledTimes(1);
expect(authSpy).not.toHaveBeenCalled();
expect(props.apiKey).toBe('refreshed-token');
});

test('should use existing valid token', async ({ runMockServer }) => {
const server = await runMockServer('main');
const validTokenSet = new TokenSet({
access_token: 'valid-token',
refresh_token: 'refresh-token',
expires_at: Math.floor(Date.now() / 1000) + 3600, // 1 hour from now
});

writeFileSync(
join(configDir, 'credentials.json'),
JSON.stringify(validTokenSet),
{ mode: 0o700 },
);

const props = setupTestProps(server);
await ensureAuth(props);

expect(authSpy).not.toHaveBeenCalled();
expect(refreshTokenSpy).not.toHaveBeenCalled();
expect(props.apiKey).toBe('valid-token');
});

test('should successfully refresh expired token', async ({
runMockServer,
}) => {
refreshTokenSpy.mockImplementationOnce(() =>
Promise.resolve(
new TokenSet({
access_token: 'new-token',
refresh_token: 'new-refresh-token',
expires_at: Math.floor(Date.now() / 1000) + 3600,
}),
),
);

const server = await runMockServer('main');
const expiredTokenSet = new TokenSet({
access_token: 'expired-token',
refresh_token: 'refresh-token',
expires_at: Math.floor(Date.now() / 1000) - 3600, // 1 hour ago
});

writeFileSync(
join(configDir, 'credentials.json'),
JSON.stringify(expiredTokenSet),
{ mode: 0o700 },
);

const props = setupTestProps(server);
await ensureAuth(props);

expect(refreshTokenSpy).toHaveBeenCalledTimes(1);
expect(authSpy).not.toHaveBeenCalled();
expect(props.apiKey).toBe('new-token');
});
});
Loading

0 comments on commit da3c845

Please sign in to comment.