lahirujayathilake commented on code in PR #605:
URL: https://github.com/apache/airavata-custos/pull/605#discussion_r4215578249
##########
internal/server/server.go:
##########
@@ -78,8 +78,11 @@ func (s *Server) routes() {
s.router.RequireAuth("GET /projects", s.listProjects)
s.router.RequirePrivilege("POST /projects", models.ProjectsWrite,
s.createProject)
s.router.RequireScoped("GET /projects/{id}", s.canReadProject,
s.getProject)
+ s.router.RequirePrivilege("PUT /projects/{id}", models.ProjectsWrite,
s.updateProject)
s.router.RequirePrivilege("PUT /projects/{id}/status",
models.ProjectsWrite, s.updateProjectStatus)
+ s.router.RequirePrivilege("DELETE /projects/{id}",
models.ProjectsWrite, s.deleteProject)
s.router.RequirePrivilege("GET /projects/{id}/members",
models.ProjectsRead, s.listProjectMembers)
+ s.router.RequirePrivilege("PUT /projects/{id}/members/{userId}",
models.ProjectsWrite, s.updateProjectMember)
Review Comment:
I believe we can move these checks out of the service calls using
`s.router.RequireScoped(`. Move the existing methods 'requireEditableProject,
requireEditableAllocation, requireEditableMembership' into `route_policy.go`
renaming 'canWriteProject, canWriteAllocation, canWriteMembership and
canWriteUsage' and check the right privilege. Then all the current checks in
the service calls can be removed. Then this this restriction will only apply to
HTTP calls on external projects/allocations.
##########
pkg/models/project.go:
##########
@@ -39,6 +39,8 @@ const (
ProjectDeleted ProjectStatus = "DELETED"
)
+const ProjectOriginationCustos = "custos"
Review Comment:
How about 'internal' instead of 'custos'?
Better in perspective of the RP
##########
pkg/service/project.go:
##########
@@ -260,3 +293,51 @@ func (s *Service) DeleteProject(ctx context.Context, id
string) error {
}
return nil
}
+
+// requireEditable refuses HTTP callers' writes to externally managed projects;
+// connectors write without a caller.
+func requireEditable(ctx context.Context, p *models.Project) error {
+ if identity.CallerFromContext(ctx) != nil && p.Origination !=
models.ProjectOriginationCustos {
+ return ErrExternalManaged
+ }
+ return nil
+}
+
+func (s *Service) requireEditableProject(ctx context.Context, projectID
string) error {
+ if identity.CallerFromContext(ctx) == nil {
+ return nil
+ }
+ p, err := s.projs.FindByID(ctx, projectID)
+ if err != nil {
+ return fmt.Errorf("lookup project: %w", err)
+ }
+ if p == nil {
+ return ErrNotFound
+ }
+ return requireEditable(ctx, p)
+}
+
+func (s *Service) requireEditableAllocation(ctx context.Context, allocationID
string) error {
+ if identity.CallerFromContext(ctx) == nil {
+ return nil
+ }
+ a, err := s.allocs.FindByID(ctx, allocationID)
+ if err != nil {
+ return fmt.Errorf("lookup compute allocation: %w", err)
+ }
+ if a == nil { // usages outlive their allocation
+ return nil
Review Comment:
shouldn't this be an error?
Since the system already got a request to update an existing 'allocation'?
##########
pkg/service/project.go:
##########
@@ -260,3 +293,51 @@ func (s *Service) DeleteProject(ctx context.Context, id
string) error {
}
return nil
}
+
+// requireEditable refuses HTTP callers' writes to externally managed projects;
+// connectors write without a caller.
+func requireEditable(ctx context.Context, p *models.Project) error {
Review Comment:
check the comment -
https://github.com/apache/airavata-custos/pull/605/changes#r4215578249
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
