Skip to content

fix(auth): update code/token generation from CLI and redirect to successful page #518

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.

Sign up for GitHub

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

Open
eduardoRoth wants to merge 7 commits into
base: main
Choose a base branch
from
+103 −58
Open

fix(auth): update code/token generation from CLI and redirect to successful page #518

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e56eddf
fix(auth): env variables not used
eduardoRoth Mar 10, 2026
2c0a584
chore(auth): add new env variable for CLI login completed page
eduardoRoth Mar 18, 2026
db76111
feat(auth): refactor auth generation to wait for tokens and redirect …
eduardoRoth Mar 18, 2026
d78de5e
fix(auth): fix tests
eduardoRoth Mar 19, 2026
34cfe41
fix(auth): fix undefined during tests
eduardoRoth Mar 19, 2026
606208d
chore(auth): remove unused async
eduardoRoth Mar 19, 2026
49153e4
fix(auth): update EOL_LOG_IN_URL to right URL
eduardoRoth Mar 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .envrc.example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export GRAPHQL_HOST='https://api.nes.herodevs.com';
export GRAPHQL_PATH='/graphql';
export EOL_REPORT_URL='https://eol-report-card.apps.herodevs.com/reports';
export ANALYTICS_URL='https://eol-api.herodevs.com/track';
export EOL_LOG_IN_URL='https://apps.herodevs.com/eol/api/auth/cli-log-in';

# OAuth (for hd auth login)
export OAUTH_CONNECT_URL='';
Expand Down
27 changes: 19 additions & 8 deletions src/commands/auth/login.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { createInterface } from 'node:readline';
import { URL } from 'node:url';
import { Command } from '@oclif/core';
import { ensureUserSetup } from '../../api/user-setup.client.ts';
import { OAUTH_CALLBACK_ERROR_CODES } from '../../config/constants.ts';
import { EOL_LOG_IN_URL, OAUTH_CALLBACK_ERROR_CODES } from '../../config/constants.ts';
import { refreshIdentityFromStoredToken } from '../../service/analytics.svc.ts';
import { persistTokenResponse } from '../../service/auth.svc.ts';
import { getClientId, getRealmUrl } from '../../service/auth-config.svc.ts';
Expand Down Expand Up @@ -40,8 +40,7 @@ export default class AuthLogin extends Command {
`&code_challenge_method=S256` +
`&state=${state}`;

const code = await this.startServerAndAwaitCode(authUrl, state);
const token = await this.exchangeCodeForToken(code, codeVerifier);
const token = await this.startServerAndAwaitToken(authUrl, state, codeVerifier);

try {
await persistTokenResponse(token);
Expand All @@ -65,7 +64,11 @@ export default class AuthLogin extends Command {
this.log('\nLogin completed successfully.');
}

private startServerAndAwaitCode(authUrl: string, expectedState: string): Promise<string> {
private startServerAndAwaitToken(
authUrl: string,
expectedState: string,
codeVerifier: string,
): Promise<TokenResponse> {
return new Promise((resolve, reject) => {
this.server = http.createServer((req, res) => {
if (!req.url) {
Expand Down Expand Up @@ -138,10 +141,18 @@ export default class AuthLogin extends Command {
}

if (code) {
res.writeHead(200, { 'Content-Type': 'text/plain' });
res.end('Login successful. You can close this window.');
this.stopServer();
resolve(code);
this.exchangeCodeForToken(code, codeVerifier)
.then((token) => {
res.writeHead(302, {
Location: `${EOL_LOG_IN_URL}`,
});
res.end();
resolve(token);
})
.catch((error) => reject(error))
Copy link
Member

@marco-ippolito marco-ippolito Mar 19, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to refactor this function as its complexity is very high. Can we try to break it into smaller functions? Mixing async await and then catch syntax produces unexpected results. (for example the .catch is not catching rejections from an awaited promise), its better to avoid this pattern completely, given that we are also in another new promise.

.finally(() => {
this.stopServer();
});
} else {
res.writeHead(400, { 'Content-Type': 'text/plain' });
res.end('No authorization code returned. Please try again.');
Expand Down
9 changes: 5 additions & 4 deletions src/config/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
export const EOL_REPORT_URL = 'https://apps.herodevs.com/eol/reports';
export const GRAPHQL_HOST = 'https://gateway.prod.apps.herodevs.io';
export const GRAPHQL_PATH = '/graphql';
export const ANALYTICS_URL = 'https://apps.herodevs.com/api/eol/track';
export const EOL_REPORT_URL = process.env.EOL_REPORT_URL || 'https://apps.herodevs.com/eol/reports';
export const GRAPHQL_HOST = process.env.GRAPHQL_HOST || 'https://gateway.prod.apps.herodevs.io';
export const GRAPHQL_PATH = process.env.GRAPHQL_PATH || '/graphql';
export const ANALYTICS_URL = process.env.ANALYTICS_URL || 'https://apps.herodevs.com/api/eol/track';
export const EOL_LOG_IN_URL = process.env.EOL_LOG_IN_URL || 'https://apps.herodevs.com/eol/api/auth/cli-log-in';
export const CONCURRENT_PAGE_REQUESTS = 3;
export const PAGE_SIZE = 500;
export const GIT_OUTPUT_FORMAT = `"${['%h', '%an', '%ad'].join('|')}"`;
Expand Down
124 changes: 78 additions & 46 deletions test/commands/auth/login.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { ensureUserSetup } from '../../../src/api/user-setup.client.ts';
import AuthLogin from '../../../src/commands/auth/login.ts';
import { refreshIdentityFromStoredToken } from '../../../src/service/analytics.svc.ts';
import { persistTokenResponse } from '../../../src/service/auth.svc.ts';
import type { TokenResponse } from '../../../src/types/auth.js';
import { openInBrowser } from '../../../src/utils/open-in-browser.ts';

type ServerRequest = { url?: string };
Expand Down Expand Up @@ -167,22 +168,30 @@ describe('AuthLogin', () => {
persistTokenResponseMock.mockClear();
});

describe('startServerAndAwaitCode', () => {
describe('startServerAndAwaitToken', () => {
const authUrl = 'https://login.example/auth';
const basePort = 4900;

it('resolves with the authorization code when the callback is valid', async () => {
it('resolves with the token response when the callback is valid', async () => {
const command = createCommand(basePort);
const state = 'expected-state';
const pendingCode = (
command as unknown as { startServerAndAwaitCode: (url: string, state: string) => Promise<string> }
).startServerAndAwaitCode(authUrl, state);
const codeVerifier = 'verifier-123';

const commandWithInternals = command as unknown as {
startServerAndAwaitToken: (url: string, state: string, codeVerifier: string) => Promise<TokenResponse>;
exchangeCodeForToken: (...args: unknown[]) => Promise<TokenResponse>;
};
const tokenResponse = { access_token: 'access', refresh_token: 'refresh' };
vi.spyOn(commandWithInternals, 'exchangeCodeForToken').mockResolvedValue(tokenResponse);

const pendingCode = commandWithInternals.startServerAndAwaitToken(authUrl, state, codeVerifier);

const server = getLatestServer();

await flushAsync();
sendCallbackThroughStub({ code: 'test-code', state });

await expect(pendingCode).resolves.toBe('test-code');
await expect(pendingCode).resolves.toBe(tokenResponse);
expect(questionMock).toHaveBeenCalledWith(expect.stringContaining(authUrl), expect.any(Function));
expect(closeMock).toHaveBeenCalledTimes(1);
expect(openMock).toHaveBeenCalledWith(authUrl);
Expand All @@ -192,8 +201,10 @@ describe('AuthLogin', () => {
it('rejects when the callback is missing the state parameter', async () => {
const command = createCommand(basePort + 1);
const pendingCode = (
command as unknown as { startServerAndAwaitCode: (url: string, state: string) => Promise<string> }
).startServerAndAwaitCode(authUrl, 'expected-state');
command as unknown as {
startServerAndAwaitToken: (url: string, state: string, codeVerifier: string) => Promise<string>;
}
).startServerAndAwaitToken(authUrl, 'expected-state', 'code-verifier');
const server = getLatestServer();

await flushAsync();
Expand All @@ -206,8 +217,10 @@ describe('AuthLogin', () => {
it('rejects when the callback state does not match', async () => {
const command = createCommand(basePort + 2);
const pendingCode = (
command as unknown as { startServerAndAwaitCode: (url: string, state: string) => Promise<string> }
).startServerAndAwaitCode(authUrl, 'expected-state');
command as unknown as {
startServerAndAwaitToken: (url: string, state: string, codeVerifier: string) => Promise<string>;
}
).startServerAndAwaitToken(authUrl, 'expected-state', 'code-verifier');
const server = getLatestServer();

await flushAsync();
Expand All @@ -220,8 +233,10 @@ describe('AuthLogin', () => {
it('rejects with guidance when callback returns already_logged_in', async () => {
const command = createCommand(basePort + 3);
const pendingCode = (
command as unknown as { startServerAndAwaitCode: (url: string, state: string) => Promise<string> }
).startServerAndAwaitCode(authUrl, 'expected-state');
command as unknown as {
startServerAndAwaitToken: (url: string, state: string, codeVerifier: string) => Promise<string>;
}
).startServerAndAwaitToken(authUrl, 'expected-state', 'code-verifier');
const server = getLatestServer();

await flushAsync();
Expand All @@ -238,8 +253,10 @@ describe('AuthLogin', () => {
it('rejects when callback returns a generic OAuth error', async () => {
const command = createCommand(basePort + 4);
const pendingCode = (
command as unknown as { startServerAndAwaitCode: (url: string, state: string) => Promise<string> }
).startServerAndAwaitCode(authUrl, 'expected-state');
command as unknown as {
startServerAndAwaitToken: (url: string, state: string, codeVerifier: string) => Promise<string>;
}
).startServerAndAwaitToken(authUrl, 'expected-state', 'code-verifier');
const server = getLatestServer();

await flushAsync();
Expand All @@ -258,8 +275,10 @@ describe('AuthLogin', () => {
it('rejects with guidance when callback returns different_user_authenticated', async () => {
const command = createCommand(basePort + 5);
const pendingCode = (
command as unknown as { startServerAndAwaitCode: (url: string, state: string) => Promise<string> }
).startServerAndAwaitCode(authUrl, 'expected-state');
command as unknown as {
startServerAndAwaitToken: (url: string, state: string, codeVerifier: string) => Promise<string>;
}
).startServerAndAwaitToken(authUrl, 'expected-state', 'code-verifier');
const server = getLatestServer();

await flushAsync();
Expand All @@ -280,8 +299,10 @@ describe('AuthLogin', () => {
it('rejects when the callback omits the authorization code', async () => {
const command = createCommand(basePort + 6);
const pendingCode = (
command as unknown as { startServerAndAwaitCode: (url: string, state: string) => Promise<string> }
).startServerAndAwaitCode(authUrl, 'expected-state');
command as unknown as {
startServerAndAwaitToken: (url: string, state: string, codeVerifier: string) => Promise<string>;
}
).startServerAndAwaitToken(authUrl, 'expected-state', 'code-verifier');
const server = getLatestServer();

await flushAsync();
Expand All @@ -294,8 +315,10 @@ describe('AuthLogin', () => {
it('rejects when the callback URL is invalid', async () => {
const command = createCommand(basePort + 7);
const pendingCode = (
command as unknown as { startServerAndAwaitCode: (url: string, state: string) => Promise<string> }
).startServerAndAwaitCode(authUrl, 'expected-state');
command as unknown as {
startServerAndAwaitToken: (url: string, state: string, codeVerifier: string) => Promise<string>;
}
).startServerAndAwaitToken(authUrl, 'expected-state', 'code-verifier');
const server = getLatestServer();

await flushAsync();
Expand All @@ -310,8 +333,10 @@ describe('AuthLogin', () => {
it('returns a 400 response when the incoming request is missing a URL', async () => {
const command = createCommand(basePort + 8);
const pendingCode = (
command as unknown as { startServerAndAwaitCode: (url: string, state: string) => Promise<string> }
).startServerAndAwaitCode(authUrl, 'expected-state');
command as unknown as {
startServerAndAwaitToken: (url: string, state: string, codeVerifier: string) => Promise<string>;
}
).startServerAndAwaitToken(authUrl, 'expected-state', 'code-verifier');
const server = getLatestServer();

await flushAsync();
Expand All @@ -329,8 +354,10 @@ describe('AuthLogin', () => {
it('responds with not found for unrelated paths', async () => {
const command = createCommand(basePort + 9);
const pendingCode = (
command as unknown as { startServerAndAwaitCode: (url: string, state: string) => Promise<string> }
).startServerAndAwaitCode(authUrl, 'expected-state');
command as unknown as {
startServerAndAwaitToken: (url: string, state: string, codeVerifier: string) => Promise<string>;
}
).startServerAndAwaitToken(authUrl, 'expected-state', 'code-verifier');
const server = getLatestServer();

await flushAsync();
Expand All @@ -348,8 +375,10 @@ describe('AuthLogin', () => {
it('rejects when the local HTTP server emits an error', async () => {
const command = createCommand(basePort + 10);
const pendingCode = (
command as unknown as { startServerAndAwaitCode: (url: string, state: string) => Promise<string> }
).startServerAndAwaitCode(authUrl, 'expected-state');
command as unknown as {
startServerAndAwaitToken: (url: string, state: string, codeVerifier: string) => Promise<string>;
}
).startServerAndAwaitToken(authUrl, 'expected-state', 'code-verifier');
const server = getLatestServer();

await flushAsync();
Expand All @@ -369,17 +398,23 @@ describe('AuthLogin', () => {
const state = 'expected-state';

try {
const pendingCode = (
command as unknown as { startServerAndAwaitCode: (url: string, state: string) => Promise<string> }
).startServerAndAwaitCode(authUrl, state);
const commandWithInternals = command as unknown as {
startServerAndAwaitToken: (url: string, state: string, codeVerifier: string) => Promise<TokenResponse>;
exchangeCodeForToken: (...args: unknown[]) => Promise<TokenResponse>;
};
const tokenResponse = { access_token: 'access', refresh_token: 'refresh' };
vi.spyOn(commandWithInternals, 'exchangeCodeForToken').mockResolvedValue(tokenResponse);

const pendingCode = commandWithInternals.startServerAndAwaitToken(authUrl, 'expected-state', 'code-verifier');

const server = getLatestServer();

await flushAsync();

expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('Failed to open browser automatically'));

sendCallbackThroughStub({ code: 'manual-code', state });
await expect(pendingCode).resolves.toBe('manual-code');
await expect(pendingCode).resolves.toBe(tokenResponse);
expect(server.close).toHaveBeenCalledTimes(1);
} finally {
warnSpy.mockRestore();
Expand All @@ -389,9 +424,12 @@ describe('AuthLogin', () => {
it('deduplicates shutdown when callback success and server error race', async () => {
const command = createCommand(basePort + 12);
const state = 'expected-state';
const pendingCode = (
command as unknown as { startServerAndAwaitCode: (url: string, state: string) => Promise<string> }
).startServerAndAwaitCode(authUrl, state);

const commandWithInternals = command as unknown as {
startServerAndAwaitToken: (url: string, state: string, codeVerifier: string) => Promise<TokenResponse>;
};
const pendingCode = commandWithInternals.startServerAndAwaitToken(authUrl, 'expected-state', 'code-verifier');

const server = getLatestServer();
const warnSpy = vi
.spyOn(command as unknown as { warn: (...args: unknown[]) => unknown }, 'warn')
Expand All @@ -402,7 +440,7 @@ describe('AuthLogin', () => {
sendCallbackThroughStub({ code: 'race-code', state });
server.emitError(new Error('late listener error'));

await expect(pendingCode).resolves.toBe('race-code');
await expect(pendingCode).rejects.toThrow('late listener error');
expect(server.close).toHaveBeenCalledTimes(1);
expect(warnSpy).not.toHaveBeenCalledWith(expect.stringContaining('Failed to stop local OAuth callback server'));
} finally {
Expand Down Expand Up @@ -470,11 +508,9 @@ describe('AuthLogin', () => {
const command = createCommand(6000);
const tokenResponse = { access_token: 'access', refresh_token: 'refresh' };
const commandWithInternals = command as unknown as {
startServerAndAwaitCode: (...args: unknown[]) => Promise<string>;
exchangeCodeForToken: (...args: unknown[]) => Promise<unknown>;
startServerAndAwaitToken: (...args: unknown[]) => Promise<unknown>;
};
vi.spyOn(commandWithInternals, 'startServerAndAwaitCode').mockResolvedValue('code-123');
vi.spyOn(commandWithInternals, 'exchangeCodeForToken').mockResolvedValue(tokenResponse);
vi.spyOn(commandWithInternals, 'startServerAndAwaitToken').mockResolvedValue(tokenResponse);

await command.run();

Expand All @@ -488,11 +524,9 @@ describe('AuthLogin', () => {
const command = createCommand(6001);
const tokenResponse = { access_token: 'access', refresh_token: 'refresh' };
const commandWithInternals = command as unknown as {
startServerAndAwaitCode: (...args: unknown[]) => Promise<string>;
exchangeCodeForToken: (...args: unknown[]) => Promise<unknown>;
startServerAndAwaitToken: (...args: unknown[]) => Promise<unknown>;
};
vi.spyOn(commandWithInternals, 'startServerAndAwaitCode').mockResolvedValue('code-123');
vi.spyOn(commandWithInternals, 'exchangeCodeForToken').mockResolvedValue(tokenResponse);
vi.spyOn(commandWithInternals, 'startServerAndAwaitToken').mockResolvedValue(tokenResponse);

await command.run();

Expand All @@ -506,11 +540,9 @@ describe('AuthLogin', () => {
const command = createCommand(6002);
const tokenResponse = { access_token: 'access', refresh_token: 'refresh' };
const commandWithInternals = command as unknown as {
startServerAndAwaitCode: (...args: unknown[]) => Promise<string>;
exchangeCodeForToken: (...args: unknown[]) => Promise<unknown>;
startServerAndAwaitToken: (...args: unknown[]) => Promise<unknown>;
};
vi.spyOn(commandWithInternals, 'startServerAndAwaitCode').mockResolvedValue('code-123');
vi.spyOn(commandWithInternals, 'exchangeCodeForToken').mockResolvedValue(tokenResponse);
vi.spyOn(commandWithInternals, 'startServerAndAwaitToken').mockResolvedValue(tokenResponse);

await expect(command.run()).rejects.toThrow('User setup failed');
expect(refreshIdentityFromStoredTokenMock).not.toHaveBeenCalled();
Expand Down
Loading