feat(server-nestjs): secure deployment API with project permission guards#2288
feat(server-nestjs): secure deployment API with project permission guards#2288KepoParis wants to merge 1 commit into
Conversation
|
🤖 Hey ! The security scan report for the current pull request is available here. |
|
0 New Issues
5 Fixed Issues
0 Accepted Issues
|
|
||
| @OnEvent('project.argocd.update') | ||
| @StartActiveSpan() | ||
| async handleArgoCDProjectUpdate(project: ProjectWithDetails): Promise<RequiredServiceResult<'argocd'>> { |
There was a problem hiding this comment.
Better just using project.upsert, its designed to run on any changes in a GitOps style.
| id: projectId, | ||
| name: 'Test Project', | ||
| } | ||
| const projectCtx: ProjectExecutionContext = { projectId, userId, requestId } |
There was a problem hiding this comment.
Clever, I'll steal the design.
|
|
||
| await this.upsertProject(projectId) | ||
| // TODO handle result and add logs | ||
| await this.updateArgoCDProject({ |
There was a problem hiding this comment.
emitAsync can be wait if it was the use case. The core design of the "reconciler" is take a database as a spec and reconcile the state with the target resource. Action shouldn't exists by itself from the outside of ArgoCD service.
| }) | ||
|
|
||
| if (failed.length > 0) { | ||
| this.logger.error(`project.argocd.update failed (projectId=${projectId})`) |
There was a problem hiding this comment.
Instead of returning the result of the creation, I have been thinking about letting the reconciliation in async and use logService.addLog to return an error in a GitOps style
| requestId: string | ||
| } | ||
|
|
||
| export const ProjectContext = createParamDecorator( |
There was a problem hiding this comment.
We now have the ProjectContext and AuthUser decorators, it should have all we need
| import type { FastifyRequest } from 'fastify' | ||
| import { createParamDecorator } from '@nestjs/common' | ||
|
|
||
| export const ProjectId = createParamDecorator( |
There was a problem hiding this comment.
Project should already have everything
2 participants
Issues liées
Issues numéro:
Quel est le comportement actuel ?
Les endpoints de déploiement sont exposés sous
/api/v1/deploymentsavec leprojectIdpassé en query/body, sans garde de permission (un TODOadd auth and project perms guardétait présent). N'importe quel utilisateur authentifié peut donc lister, créer, modifier ou supprimer des déploiements sans vérification des droits sur le projet.Quel est le nouveau comportement ?
Sécurisation de l'API de déploiement via les permissions projet :
/api/v1/projects/:projectId/deploymentset application duProjectGuard.ListDeployments/ManageDeployments(RequireProjectPermission) etRequireAdminPermissionsur la liste.ProjectContextetProjectIdpour résoudre projet/utilisateur depuis la requête, et propagation d'unProjectExecutionContextjusqu'à la couche service.ServiceResults,mergeServiceResults,getFailedServices).longestDeploymentNameet export deDeploymentSourceSchema.Cette PR introduit-elle un breaking change ?
Oui. Les URLs des endpoints de déploiement changent : le
projectIdpasse de query/body à un segment de path (/api/v1/projects/:projectId/deployments). Les clients appelant l'ancienne route/api/v1/deploymentsdoivent être mis à jour.Autres informations