mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-17 13:39:44 +02:00
fix(API): Report the full route path in public API usage telemetry (no-changelog) (backport to release-candidate/2.39.x) (#38188)
Co-authored-by: Matt Kay <123335827+kayonaise@users.noreply.github.com> Co-authored-by: Matthias Kay <matthew.kay@n8n.io> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
f1d044cdfc
commit
cfeb4531be
|
|
@ -1,4 +1,5 @@
|
|||
import { Z } from '@n8n/api-types';
|
||||
import type { AuthenticatedRequest, User } from '@n8n/db';
|
||||
import {
|
||||
ApiResponse,
|
||||
Body,
|
||||
|
|
@ -28,6 +29,7 @@ describe('PublicApiControllerRegistry', () => {
|
|||
const authStrategyRegistry = mock<AuthStrategyRegistry>();
|
||||
const lastActiveAtService = mock<LastActiveAtService>();
|
||||
const eventService = mock<EventService>();
|
||||
const authenticatedUser = mock<User>({ id: 'user-1' });
|
||||
|
||||
function activate(): express.Express {
|
||||
const app = express();
|
||||
|
|
@ -44,13 +46,19 @@ describe('PublicApiControllerRegistry', () => {
|
|||
lastActiveAtService,
|
||||
eventService,
|
||||
).activate(router, 'v1');
|
||||
app.use(router);
|
||||
// mirrors the production mount, `apiController.use('/api/v1', ..., controllerRouter)`
|
||||
app.use('/api/v1', router);
|
||||
return app;
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
authStrategyRegistry.authenticate.mockResolvedValue(true);
|
||||
// mirrors the real strategies, which set `req.user` on success
|
||||
authStrategyRegistry.authenticate.mockImplementation(async (req: AuthenticatedRequest) => {
|
||||
req.user = authenticatedUser;
|
||||
return true;
|
||||
});
|
||||
lastActiveAtService.updateLastActiveIfStale.mockResolvedValue(undefined);
|
||||
Container.set(ControllerRegistryMetadata, new ControllerRegistryMetadata());
|
||||
});
|
||||
|
||||
|
|
@ -68,7 +76,7 @@ describe('PublicApiControllerRegistry', () => {
|
|||
}
|
||||
markPublicApiController(WidgetsPublicController as Controller, '/widgets');
|
||||
|
||||
const response = await request(activate()).get('/widgets').expect(200);
|
||||
const response = await request(activate()).get('/api/v1/widgets').expect(200);
|
||||
|
||||
expect(response.headers.deprecation).toBe(`@${Math.floor(since.getTime() / 1000)}`);
|
||||
});
|
||||
|
|
@ -88,7 +96,7 @@ describe('PublicApiControllerRegistry', () => {
|
|||
}
|
||||
markPublicApiController(WidgetsPublicController as Controller, '/widgets');
|
||||
|
||||
const response = await request(activate()).get('/widgets').expect(401);
|
||||
const response = await request(activate()).get('/api/v1/widgets').expect(401);
|
||||
|
||||
expect(response.headers.deprecation).toBe(`@${Math.floor(since.getTime() / 1000)}`);
|
||||
});
|
||||
|
|
@ -104,9 +112,32 @@ describe('PublicApiControllerRegistry', () => {
|
|||
}
|
||||
markPublicApiController(WidgetsPublicController as Controller, '/widgets');
|
||||
|
||||
const response = await request(activate()).get('/widgets').expect(200);
|
||||
const response = await request(activate()).get('/api/v1/widgets').expect(200);
|
||||
|
||||
expect(response.headers.deprecation).toBeUndefined();
|
||||
expect(eventService.emit).toHaveBeenCalledWith(
|
||||
'public-api-invoked',
|
||||
expect.objectContaining({ method: 'GET', path: '/widgets' }),
|
||||
);
|
||||
});
|
||||
|
||||
it('reports the version-less route path for a sub-path route', async () => {
|
||||
@Service()
|
||||
class WidgetsPublicController {
|
||||
@Get('/:widgetId')
|
||||
@ApiResponse(200)
|
||||
method() {
|
||||
return { ok: true };
|
||||
}
|
||||
}
|
||||
markPublicApiController(WidgetsPublicController as Controller, '/widgets');
|
||||
|
||||
await request(activate()).get('/api/v1/widgets/w-1').expect(200);
|
||||
|
||||
expect(eventService.emit).toHaveBeenCalledWith(
|
||||
'public-api-invoked',
|
||||
expect.objectContaining({ method: 'GET', path: '/widgets/w-1' }),
|
||||
);
|
||||
});
|
||||
|
||||
describe('validation failures', () => {
|
||||
|
|
@ -127,7 +158,7 @@ describe('PublicApiControllerRegistry', () => {
|
|||
markPublicApiController(WidgetsPublicController as Controller, '/widgets');
|
||||
|
||||
const response = await request(activate())
|
||||
.post('/widgets')
|
||||
.post('/api/v1/widgets')
|
||||
.send({ name: 'w', active: false })
|
||||
.expect(400);
|
||||
|
||||
|
|
@ -164,7 +195,7 @@ describe('PublicApiControllerRegistry', () => {
|
|||
registerBodyRoute();
|
||||
|
||||
await request(activate())
|
||||
.post('/widgets')
|
||||
.post('/api/v1/widgets')
|
||||
.set('Content-Type', 'application/json')
|
||||
.send({ name: 'a' })
|
||||
.expect(200);
|
||||
|
|
@ -174,7 +205,7 @@ describe('PublicApiControllerRegistry', () => {
|
|||
registerBodyRoute();
|
||||
|
||||
await request(activate())
|
||||
.post('/widgets')
|
||||
.post('/api/v1/widgets')
|
||||
.set('Content-Type', 'application/json; charset=utf-8')
|
||||
.send({ name: 'a' })
|
||||
.expect(200);
|
||||
|
|
@ -191,7 +222,7 @@ describe('PublicApiControllerRegistry', () => {
|
|||
registerBodyRoute();
|
||||
|
||||
const response = await request(activate())
|
||||
.post('/widgets')
|
||||
.post('/api/v1/widgets')
|
||||
.set('Content-Type', sent)
|
||||
.send('a')
|
||||
.expect(415);
|
||||
|
|
@ -203,7 +234,7 @@ describe('PublicApiControllerRegistry', () => {
|
|||
registerBodyRoute();
|
||||
|
||||
const response = await request(activate())
|
||||
.post('/widgets')
|
||||
.post('/api/v1/widgets')
|
||||
.set('Content-Type', 'application/x-www-form-urlencoded')
|
||||
.expect(415);
|
||||
|
||||
|
|
@ -219,7 +250,7 @@ describe('PublicApiControllerRegistry', () => {
|
|||
];
|
||||
|
||||
function postWithContentType(header: string | undefined) {
|
||||
const pending = request(activate()).post('/widgets');
|
||||
const pending = request(activate()).post('/api/v1/widgets');
|
||||
|
||||
return header === undefined ? pending : pending.set('Content-Type', header);
|
||||
}
|
||||
|
|
@ -245,7 +276,7 @@ describe('PublicApiControllerRegistry', () => {
|
|||
registerBodyRoute();
|
||||
|
||||
await request(activate())
|
||||
.post('/widgets')
|
||||
.post('/api/v1/widgets')
|
||||
.set('Content-Type', 'application/json; Foo=BAR')
|
||||
.send({ name: 'a' })
|
||||
.expect(200);
|
||||
|
|
|
|||
|
|
@ -25,6 +25,12 @@ import { sendPublicApiErrorResponse } from '@/public-api/v1/public-api-error-res
|
|||
import { AuthStrategyRegistry } from '@/services/auth-strategy.registry';
|
||||
import { LastActiveAtService } from '@/services/last-active-at.service';
|
||||
|
||||
// Match the legacy version-less route. req.path drops the prefix, req.baseUrl adds /api/v1
|
||||
function routePath(prefix: string, req: Request): string {
|
||||
const path = (prefix === '/' ? '' : prefix) + req.path;
|
||||
return path.length > 1 && path.endsWith('/') ? path.slice(0, -1) : path;
|
||||
}
|
||||
|
||||
@Service()
|
||||
export class PublicApiControllerRegistry {
|
||||
constructor(
|
||||
|
|
@ -105,7 +111,7 @@ export class PublicApiControllerRegistry {
|
|||
middlewares.push(deprecated(route.deprecated));
|
||||
}
|
||||
|
||||
middlewares.push(this.createAuthMiddleware(apiVersion));
|
||||
middlewares.push(this.createAuthMiddleware(apiVersion, prefix));
|
||||
|
||||
if (route.apiKeyScope) {
|
||||
middlewares.push(this.createApiKeyScopeMiddleware(route.apiKeyScope));
|
||||
|
|
@ -140,7 +146,7 @@ export class PublicApiControllerRegistry {
|
|||
}
|
||||
}
|
||||
|
||||
private createAuthMiddleware(apiVersion: string): RequestHandler {
|
||||
private createAuthMiddleware(apiVersion: string, prefix: string): RequestHandler {
|
||||
return async (req, res, next) => {
|
||||
const authenticated = await this.authStrategyRegistry.authenticate(
|
||||
req as AuthenticatedRequest,
|
||||
|
|
@ -156,7 +162,7 @@ export class PublicApiControllerRegistry {
|
|||
this.lastActiveAtService.updateLastActiveIfStale(userId).catch(() => undefined);
|
||||
this.eventService.emit('public-api-invoked', {
|
||||
userId,
|
||||
path: req.path,
|
||||
path: routePath(prefix, req),
|
||||
method: req.method,
|
||||
apiVersion,
|
||||
userAgent: req.headers['user-agent'],
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user