Address remaining PR review comments
- Fix getAllInstances to handle empty instance array without throwing - Persist updated token expiry timestamps to disk after refresh - Mark internal httpJson helpers as @internal instead of @public Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com> Made-with: Cursor
This commit is contained in:
@@ -145,6 +145,10 @@ describe('CliAuth', () => {
|
||||
'refreshToken',
|
||||
'new-refresh-token',
|
||||
);
|
||||
expect(mockStorage.updateInstance).toHaveBeenCalledWith('production', {
|
||||
issuedAt: expect.any(Number),
|
||||
accessTokenExpiresAt: expect.any(Number),
|
||||
});
|
||||
});
|
||||
|
||||
it('throws when refresh token is missing and access token has expired', async () => {
|
||||
|
||||
@@ -19,6 +19,7 @@ import {
|
||||
getSelectedInstance,
|
||||
getInstanceMetadata,
|
||||
updateInstanceMetadata,
|
||||
updateInstance,
|
||||
accessTokenNeedsRefresh,
|
||||
} from './storage';
|
||||
import { getSecretStore, type SecretStore } from './secretStore';
|
||||
@@ -151,10 +152,12 @@ export class CliAuth {
|
||||
if (token.refresh_token) {
|
||||
await this.#secretStore.set(service, 'refreshToken', token.refresh_token);
|
||||
}
|
||||
this.#instance = {
|
||||
...this.#instance,
|
||||
issuedAt: Date.now(),
|
||||
accessTokenExpiresAt: Date.now() + token.expires_in * 1000,
|
||||
};
|
||||
const issuedAt = Date.now();
|
||||
const accessTokenExpiresAt = Date.now() + token.expires_in * 1000;
|
||||
this.#instance = { ...this.#instance, issuedAt, accessTokenExpiresAt };
|
||||
await updateInstance(this.#instance.name, {
|
||||
issuedAt,
|
||||
accessTokenExpiresAt,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
@@ -16,7 +16,7 @@
|
||||
|
||||
import { ResponseError } from '@backstage/errors';
|
||||
|
||||
/** @public */
|
||||
/** @internal */
|
||||
export type HttpInit = {
|
||||
headers?: Record<string, string>;
|
||||
method?: string;
|
||||
@@ -24,7 +24,7 @@ export type HttpInit = {
|
||||
signal?: AbortSignal;
|
||||
};
|
||||
|
||||
/** @public */
|
||||
/** @internal */
|
||||
export async function httpJson<T>(url: string, init?: HttpInit): Promise<T> {
|
||||
const res = await fetch(url, {
|
||||
...init,
|
||||
|
||||
@@ -124,6 +124,9 @@ export async function getAllInstances(): Promise<{
|
||||
selected: StoredInstance | undefined;
|
||||
}> {
|
||||
const { instances } = await readAll();
|
||||
if (instances.length === 0) {
|
||||
return { instances: [], selected: undefined };
|
||||
}
|
||||
const selected = instances.find(i => i.selected) ?? instances[0];
|
||||
return {
|
||||
instances: instances.map(i => ({
|
||||
@@ -189,6 +192,22 @@ export async function updateInstanceMetadata(
|
||||
});
|
||||
}
|
||||
|
||||
/** @internal */
|
||||
export async function updateInstance(
|
||||
instanceName: string,
|
||||
updates: Partial<Pick<StoredInstance, 'issuedAt' | 'accessTokenExpiresAt'>>,
|
||||
): Promise<void> {
|
||||
return withMetadataLock(async () => {
|
||||
const data = await readAll();
|
||||
const idx = data.instances.findIndex(i => i.name === instanceName);
|
||||
if (idx === -1) {
|
||||
throw new NotFoundError(`Instance '${instanceName}' not found`);
|
||||
}
|
||||
data.instances[idx] = { ...data.instances[idx], ...updates };
|
||||
await writeAll(data);
|
||||
});
|
||||
}
|
||||
|
||||
/** @internal */
|
||||
export function accessTokenNeedsRefresh(instance: StoredInstance): boolean {
|
||||
// 2 minutes before expiration
|
||||
|
||||
Reference in New Issue
Block a user