fix(api): typed OpenAPI query params and not-found messages that hide private projects
- Query DTO properties with a default value (limit, offset, days, perColumn, assignedToMe, search, archived, requiresReason, force) were documented as `object` in openapi.json, which broke generated clients. They now declare their type explicitly. - Reading a private project, ticket, release or its comments without access answered "Project <internal id> not found", confirming the project exists. ReadAccess now reports what the caller asked for (e2e covered). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -21,7 +21,10 @@ export class GetCommentsQueryHandler
|
||||
'Issue',
|
||||
query.issueId,
|
||||
);
|
||||
await this.access.assertCanView(issue.projectId, query.viewerId);
|
||||
await this.access.assertCanView(issue.projectId, query.viewerId, {
|
||||
entity: 'Issue',
|
||||
ref: query.issueId,
|
||||
});
|
||||
return this.findComments.findByIssue(
|
||||
issue.id,
|
||||
Math.min(query.limit, 200),
|
||||
|
||||
@@ -18,7 +18,10 @@ export class GetIssueQueryHandler
|
||||
? await this.findIssues.findById(query.idOrKey)
|
||||
: await this.findIssues.findByKey(query.idOrKey.toUpperCase());
|
||||
const found = this.access.assertFound(issue, 'Issue', query.idOrKey);
|
||||
await this.access.assertCanView(found.projectId, query.viewerId);
|
||||
await this.access.assertCanView(found.projectId, query.viewerId, {
|
||||
entity: 'Issue',
|
||||
ref: query.idOrKey,
|
||||
});
|
||||
return found;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -18,7 +18,10 @@ export class GetProjectQueryHandler
|
||||
? await this.findProjects.findById(query.idOrKey)
|
||||
: await this.findProjects.findByKey(query.idOrKey.toUpperCase());
|
||||
const found = this.access.assertFound(project, 'Project', query.idOrKey);
|
||||
await this.access.assertCanView(found.id, query.viewerId);
|
||||
await this.access.assertCanView(found.id, query.viewerId, {
|
||||
entity: 'Project',
|
||||
ref: query.idOrKey,
|
||||
});
|
||||
return found;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -24,7 +24,10 @@ export class GetReleaseBurnupQueryHandler
|
||||
'Release',
|
||||
query.releaseId,
|
||||
);
|
||||
await this.access.assertCanView(release.projectId, query.viewerId);
|
||||
await this.access.assertCanView(release.projectId, query.viewerId, {
|
||||
entity: 'Release',
|
||||
ref: query.releaseId,
|
||||
});
|
||||
const [issues, timeline] = await Promise.all([
|
||||
this.findIssues.findByProject(release.projectId),
|
||||
this.findTimeline.findByProject(release.projectId),
|
||||
|
||||
@@ -23,7 +23,10 @@ export class GetReleaseProgressQueryHandler
|
||||
'Release',
|
||||
query.releaseId,
|
||||
);
|
||||
await this.access.assertCanView(release.projectId, query.viewerId);
|
||||
await this.access.assertCanView(release.projectId, query.viewerId, {
|
||||
entity: 'Release',
|
||||
ref: query.releaseId,
|
||||
});
|
||||
return this.findIssues.releaseProgress(release.id);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -19,7 +19,10 @@ export class GetReleaseQueryHandler
|
||||
'Release',
|
||||
query.releaseId,
|
||||
);
|
||||
await this.access.assertCanView(release.projectId, query.viewerId);
|
||||
await this.access.assertCanView(release.projectId, query.viewerId, {
|
||||
entity: 'Release',
|
||||
ref: query.releaseId,
|
||||
});
|
||||
return release;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -12,10 +12,18 @@ import { FindProjectsRepository } from '../ports/find-projects.repository';
|
||||
export class ReadAccess {
|
||||
constructor(private readonly findProjects: FindProjectsRepository) {}
|
||||
|
||||
/** The viewer's role, or null when they read a public project as an outsider. */
|
||||
/**
|
||||
* The viewer's role, or null when they read a public project as an outsider.
|
||||
* `asked` is what the caller looked for: the error names it, never the
|
||||
* project behind it (that would confirm a private project exists).
|
||||
*/
|
||||
async assertCanView(
|
||||
projectId: string,
|
||||
viewerId: string | null,
|
||||
asked: { entity: string; ref: string } = {
|
||||
entity: 'Project',
|
||||
ref: projectId,
|
||||
},
|
||||
): Promise<ProjectRole | null> {
|
||||
const role = viewerId
|
||||
? await this.findProjects.findRole(projectId, viewerId)
|
||||
@@ -23,7 +31,7 @@ export class ReadAccess {
|
||||
if (role) return role;
|
||||
const project = await this.findProjects.findById(projectId);
|
||||
// Same answer for "doesn't exist" and "not yours": don't leak projects.
|
||||
if (!project?.isPublic) throw new NotFoundError('Project', projectId);
|
||||
if (!project?.isPublic) throw new NotFoundError(asked.entity, asked.ref);
|
||||
return null;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user