Merge pull request #34142 from backstage/session/suspicious-heron-9w2p
feat(auth-backend): validate catalog user existence on refresh token usage
This commit is contained in:
Vendored
+11
@@ -133,6 +133,17 @@ export interface Config {
|
||||
* @visibility backend
|
||||
*/
|
||||
maxTokensPerUser?: number;
|
||||
/**
|
||||
* Disables the check that verifies the user's catalog entity still
|
||||
* exists when refreshing a token. This is an escape hatch for
|
||||
* Backstage instances that allow sign-in without a corresponding
|
||||
* catalog user entity. Without the check, refresh tokens for
|
||||
* removed or offboarded users remain valid until they naturally
|
||||
* expire.
|
||||
* @default false
|
||||
* @visibility backend
|
||||
*/
|
||||
dangerouslyDisableCatalogPresenceCheck?: boolean;
|
||||
};
|
||||
|
||||
/**
|
||||
|
||||
@@ -98,6 +98,8 @@ export const authPlugin = createBackendPlugin({
|
||||
database,
|
||||
logger,
|
||||
lifecycle,
|
||||
catalog,
|
||||
auth,
|
||||
})
|
||||
: undefined;
|
||||
|
||||
|
||||
@@ -15,6 +15,7 @@
|
||||
*/
|
||||
|
||||
import {
|
||||
AuthService,
|
||||
DatabaseService,
|
||||
LifecycleService,
|
||||
LoggerService,
|
||||
@@ -23,6 +24,7 @@ import {
|
||||
import { AuthenticationError } from '@backstage/errors';
|
||||
import { readDurationFromConfig } from '@backstage/config';
|
||||
import { durationToMilliseconds } from '@backstage/types';
|
||||
import { CatalogService } from '@backstage/plugin-catalog-node';
|
||||
import { randomUUID as uuid } from 'node:crypto';
|
||||
import { OfflineSessionDatabase } from '../database/OfflineSessionDatabase';
|
||||
import {
|
||||
@@ -39,12 +41,17 @@ import { TokenIssuer } from '../identity/types';
|
||||
export class OfflineAccessService {
|
||||
readonly #offlineSessionDb: OfflineSessionDatabase;
|
||||
readonly #logger: LoggerService;
|
||||
readonly #dangerouslyDisableCatalogPresenceCheck: boolean;
|
||||
readonly #catalog: CatalogService;
|
||||
readonly #auth: AuthService;
|
||||
|
||||
static async create(options: {
|
||||
config: RootConfigService;
|
||||
database: DatabaseService;
|
||||
logger: LoggerService;
|
||||
lifecycle: LifecycleService;
|
||||
catalog: CatalogService;
|
||||
auth: AuthService;
|
||||
}): Promise<OfflineAccessService> {
|
||||
const { config, database, logger, lifecycle } = options;
|
||||
|
||||
@@ -98,6 +105,11 @@ export class OfflineAccessService {
|
||||
);
|
||||
}
|
||||
|
||||
const dangerouslyDisableCatalogPresenceCheck =
|
||||
config.getOptionalBoolean(
|
||||
'auth.experimentalRefreshToken.dangerouslyDisableCatalogPresenceCheck',
|
||||
) ?? false;
|
||||
|
||||
const knex = await database.getClient();
|
||||
|
||||
if (
|
||||
@@ -135,15 +147,28 @@ export class OfflineAccessService {
|
||||
clearInterval(cleanupInterval);
|
||||
});
|
||||
|
||||
return new OfflineAccessService(offlineSessionDb, logger);
|
||||
return new OfflineAccessService(
|
||||
offlineSessionDb,
|
||||
logger,
|
||||
dangerouslyDisableCatalogPresenceCheck,
|
||||
options.catalog,
|
||||
options.auth,
|
||||
);
|
||||
}
|
||||
|
||||
private constructor(
|
||||
offlineSessionDb: OfflineSessionDatabase,
|
||||
logger: LoggerService,
|
||||
dangerouslyDisableCatalogPresenceCheck: boolean,
|
||||
catalog: CatalogService,
|
||||
auth: AuthService,
|
||||
) {
|
||||
this.#offlineSessionDb = offlineSessionDb;
|
||||
this.#logger = logger;
|
||||
this.#dangerouslyDisableCatalogPresenceCheck =
|
||||
dangerouslyDisableCatalogPresenceCheck;
|
||||
this.#catalog = catalog;
|
||||
this.#auth = auth;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -212,6 +237,33 @@ export class OfflineAccessService {
|
||||
throw new AuthenticationError('Invalid refresh token');
|
||||
}
|
||||
|
||||
if (!this.#dangerouslyDisableCatalogPresenceCheck) {
|
||||
try {
|
||||
const entity = await this.#catalog.getEntityByRef(
|
||||
session.userEntityRef,
|
||||
{ credentials: await this.#auth.getOwnServiceCredentials() },
|
||||
);
|
||||
if (!entity) {
|
||||
this.#logger.info(
|
||||
`Rejecting refresh for user ${session.userEntityRef} - catalog entity not found, revoking session ${sessionId}`,
|
||||
);
|
||||
await this.#offlineSessionDb.deleteSession(sessionId);
|
||||
throw new AuthenticationError(
|
||||
'User entity no longer exists in the catalog',
|
||||
);
|
||||
}
|
||||
} catch (error) {
|
||||
if (error.name === 'AuthenticationError') {
|
||||
throw error;
|
||||
}
|
||||
this.#logger.warn(
|
||||
`Failed to validate catalog user existence for ${session.userEntityRef}, rejecting refresh`,
|
||||
error,
|
||||
);
|
||||
throw new AuthenticationError('Unable to validate user existence');
|
||||
}
|
||||
}
|
||||
|
||||
const { token: newRefreshToken, hash: newHash } =
|
||||
await generateRefreshToken(sessionId);
|
||||
|
||||
|
||||
@@ -36,6 +36,7 @@ import { OidcService } from '../service/OidcService';
|
||||
import { TokenIssuer } from '../identity/types';
|
||||
import { OfflineAccessService } from './OfflineAccessService';
|
||||
import { CimdClientInfo, validateCimdUrl } from './CimdClient';
|
||||
import { catalogServiceMock } from '@backstage/plugin-catalog-node/testUtils';
|
||||
|
||||
jest.mock('./CimdClient', () => {
|
||||
const actual = jest.requireActual('./CimdClient');
|
||||
@@ -137,7 +138,10 @@ describe('OidcRouter', () => {
|
||||
};
|
||||
}
|
||||
|
||||
async function createRouterWithOfflineAccess(databaseId: TestDatabaseId) {
|
||||
async function createRouterWithOfflineAccess(
|
||||
databaseId: TestDatabaseId,
|
||||
refreshTokenConfig?: Record<string, unknown>,
|
||||
) {
|
||||
const knex = await databases.init(databaseId);
|
||||
|
||||
await knex.migrate.latest({
|
||||
@@ -166,6 +170,13 @@ describe('OidcRouter', () => {
|
||||
|
||||
const mockAuth = mockServices.auth.mock();
|
||||
const mockHttpAuth = mockServices.httpAuth.mock();
|
||||
const mockCatalog = catalogServiceMock.mock();
|
||||
mockCatalog.getEntityByRef.mockResolvedValue({
|
||||
apiVersion: 'backstage.io/v1alpha1',
|
||||
kind: 'User',
|
||||
metadata: { name: 'test-user', namespace: 'default' },
|
||||
spec: {},
|
||||
});
|
||||
const mockConfig = mockServices.rootConfig({
|
||||
data: {
|
||||
auth: {
|
||||
@@ -174,6 +185,7 @@ describe('OidcRouter', () => {
|
||||
},
|
||||
experimentalRefreshToken: {
|
||||
enabled: true,
|
||||
...refreshTokenConfig,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -186,6 +198,8 @@ describe('OidcRouter', () => {
|
||||
database: { getClient: async () => knex },
|
||||
logger: mockServices.logger.mock(),
|
||||
lifecycle: mockLifecycle,
|
||||
catalog: mockCatalog,
|
||||
auth: mockAuth,
|
||||
});
|
||||
|
||||
const oidcService = OidcService.create({
|
||||
@@ -221,6 +235,7 @@ describe('OidcRouter', () => {
|
||||
userInfo: userInfoDatabase,
|
||||
service: oidcService,
|
||||
tokenIssuer: mockTokenIssuer,
|
||||
catalog: mockCatalog,
|
||||
},
|
||||
};
|
||||
}
|
||||
@@ -922,8 +937,14 @@ describe('OidcRouter', () => {
|
||||
});
|
||||
|
||||
describe('refresh tokens', () => {
|
||||
async function doAuthFlowWithOfflineAccess(databaseId_: TestDatabaseId) {
|
||||
const result = await createRouterWithOfflineAccess(databaseId_);
|
||||
async function doAuthFlowWithOfflineAccess(
|
||||
databaseId_: TestDatabaseId,
|
||||
refreshTokenConfig?: Record<string, unknown>,
|
||||
) {
|
||||
const result = await createRouterWithOfflineAccess(
|
||||
databaseId_,
|
||||
refreshTokenConfig,
|
||||
);
|
||||
const {
|
||||
mocks: { auth, service, tokenIssuer, httpAuth },
|
||||
router,
|
||||
@@ -1121,6 +1142,136 @@ describe('OidcRouter', () => {
|
||||
})
|
||||
.expect(400);
|
||||
});
|
||||
|
||||
describe('catalog user validation', () => {
|
||||
it('should reject refresh when catalog user does not exist', async () => {
|
||||
const { server, tokenResponse, mocks } =
|
||||
await doAuthFlowWithOfflineAccess(databaseId);
|
||||
|
||||
mocks.catalog.getEntityByRef.mockResolvedValueOnce(undefined);
|
||||
|
||||
await request(server)
|
||||
.post('/api/auth/v1/token')
|
||||
.send({
|
||||
grant_type: 'refresh_token',
|
||||
refresh_token: tokenResponse.body.refresh_token,
|
||||
})
|
||||
.expect(400);
|
||||
});
|
||||
|
||||
it('should reject refresh when catalog is unavailable', async () => {
|
||||
const { server, tokenResponse, mocks } =
|
||||
await doAuthFlowWithOfflineAccess(databaseId);
|
||||
|
||||
mocks.catalog.getEntityByRef.mockRejectedValueOnce(
|
||||
new Error('Catalog unavailable'),
|
||||
);
|
||||
|
||||
await request(server)
|
||||
.post('/api/auth/v1/token')
|
||||
.send({
|
||||
grant_type: 'refresh_token',
|
||||
refresh_token: tokenResponse.body.refresh_token,
|
||||
})
|
||||
.expect(400);
|
||||
});
|
||||
|
||||
it('should allow retry after transient catalog failure', async () => {
|
||||
const { server, tokenResponse, mocks } =
|
||||
await doAuthFlowWithOfflineAccess(databaseId);
|
||||
|
||||
mocks.catalog.getEntityByRef.mockRejectedValueOnce(
|
||||
new Error('Catalog unavailable'),
|
||||
);
|
||||
|
||||
// First refresh fails due to catalog error
|
||||
await request(server)
|
||||
.post('/api/auth/v1/token')
|
||||
.send({
|
||||
grant_type: 'refresh_token',
|
||||
refresh_token: tokenResponse.body.refresh_token,
|
||||
})
|
||||
.expect(400);
|
||||
|
||||
// Retry with same token succeeds because session was preserved
|
||||
mocks.catalog.getEntityByRef.mockResolvedValueOnce({
|
||||
apiVersion: 'backstage.io/v1alpha1',
|
||||
kind: 'User',
|
||||
metadata: { name: 'test-user', namespace: 'default' },
|
||||
spec: {},
|
||||
});
|
||||
mocks.tokenIssuer.issueToken.mockResolvedValue({
|
||||
token: 'mock-refreshed-token',
|
||||
});
|
||||
|
||||
const retryResponse = await request(server)
|
||||
.post('/api/auth/v1/token')
|
||||
.send({
|
||||
grant_type: 'refresh_token',
|
||||
refresh_token: tokenResponse.body.refresh_token,
|
||||
})
|
||||
.expect(200);
|
||||
|
||||
expect(retryResponse.body.access_token).toBe('mock-refreshed-token');
|
||||
});
|
||||
|
||||
it('should not allow retry after user entity not found', async () => {
|
||||
const { server, tokenResponse, mocks } =
|
||||
await doAuthFlowWithOfflineAccess(databaseId);
|
||||
|
||||
mocks.catalog.getEntityByRef.mockResolvedValueOnce(undefined);
|
||||
|
||||
// First refresh fails and session is revoked
|
||||
await request(server)
|
||||
.post('/api/auth/v1/token')
|
||||
.send({
|
||||
grant_type: 'refresh_token',
|
||||
refresh_token: tokenResponse.body.refresh_token,
|
||||
})
|
||||
.expect(400);
|
||||
|
||||
// Retry fails because session was deleted
|
||||
mocks.catalog.getEntityByRef.mockResolvedValueOnce({
|
||||
apiVersion: 'backstage.io/v1alpha1',
|
||||
kind: 'User',
|
||||
metadata: { name: 'test-user', namespace: 'default' },
|
||||
spec: {},
|
||||
});
|
||||
|
||||
await request(server)
|
||||
.post('/api/auth/v1/token')
|
||||
.send({
|
||||
grant_type: 'refresh_token',
|
||||
refresh_token: tokenResponse.body.refresh_token,
|
||||
})
|
||||
.expect(400);
|
||||
});
|
||||
|
||||
it('should skip catalog check when dangerouslyDisableCatalogPresenceCheck is set', async () => {
|
||||
const { server, tokenResponse, mocks } =
|
||||
await doAuthFlowWithOfflineAccess(databaseId, {
|
||||
dangerouslyDisableCatalogPresenceCheck: true,
|
||||
});
|
||||
|
||||
mocks.catalog.getEntityByRef.mockResolvedValueOnce(undefined);
|
||||
mocks.tokenIssuer.issueToken.mockResolvedValue({
|
||||
token: 'mock-refreshed-token',
|
||||
});
|
||||
|
||||
const refreshResponse = await request(server)
|
||||
.post('/api/auth/v1/token')
|
||||
.send({
|
||||
grant_type: 'refresh_token',
|
||||
refresh_token: tokenResponse.body.refresh_token,
|
||||
})
|
||||
.expect(200);
|
||||
|
||||
expect(refreshResponse.body.access_token).toBe(
|
||||
'mock-refreshed-token',
|
||||
);
|
||||
expect(mocks.catalog.getEntityByRef).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('CIMD metadata endpoint', () => {
|
||||
|
||||
Reference in New Issue
Block a user