ECOPROJECT-4722 | fix: Add missing organization check to GetSourceDownloadURL endpoint

Signed-off-by: Ronen Avraham <ravraham@redhat.com>

Author: ronenav

api/v1alpha1/openapi.yaml

@@ -346,6 +346,12 @@ paths:
             application/json:
               schema:
                 $ref: "#/components/schemas/Error"
+        "500":
+          description: Internal error
+          content:
+            application/json:
+              schema:
+                $ref: "#/components/schemas/Error"
   /api/v1/sources/{id}/image:
     head:
       tags:

api/v1alpha1/spec.gen.go

@@ -234,15 +234,26 @@ func (s *ServiceHandler) HeadImage(ctx context.Context, request server.HeadImage
 
 // (GET /api/v1/sources/{id}/image-url)
 func (s *ServiceHandler) GetSourceDownloadURL(ctx context.Context, request server.GetSourceDownloadURLRequestObject) (server.GetSourceDownloadURLResponseObject, error) {
-	url, expireAt, err := s.sourceSrv.GetSourceDownloadURL(ctx, request.Id)
+	source, err := s.sourceSrv.GetSource(ctx, request.Id)
 	if err != nil {
 		switch err.(type) {
 		case *service.ErrResourceNotFound:
 			return server.GetSourceDownloadURL404JSONResponse{Message: err.Error()}, nil
 		default:
-			return server.GetSourceDownloadURL400JSONResponse{}, nil // FIX: should be 500
+			return server.GetSourceDownloadURL500JSONResponse{}, nil
 		}
 	}
+
+	user := auth.MustHaveUser(ctx)
+	if user.Username != source.Username || user.Organization != source.OrgID {
+		message := fmt.Sprintf("source %s not found", request.Id)
+		return server.GetSourceDownloadURL404JSONResponse{Message: message}, nil
+	}
+
+	url, expireAt, err := s.sourceSrv.GetSourceDownloadURL(ctx, request.Id)
+	if err != nil {
+		return server.GetSourceDownloadURL500JSONResponse{}, nil
+	}
 	return server.GetSourceDownloadURL200JSONResponse{Url: url, ExpiresAt: &expireAt}, nil
 }
 

internal/api/client/client.gen.go

@@ -1091,4 +1091,105 @@ var _ = Describe("source handler", Ordered, func() {
 			gormdb.Exec("DELETE FROM sources;")
 		})
 	})
+
+	Context("GetSourceDownloadURL", func() {
+		It("successfully returns image URL for owned source", func() {
+			sourceID := uuid.New()
+			tx := gormdb.Exec(fmt.Sprintf(insertSourceWithUsernameStm, sourceID, "admin", "admin"))
+			Expect(tx.Error).To(BeNil())
+
+			// Insert image_infra record (required for URL generation)
+			insertImageInfraStm := `INSERT INTO image_infras (source_id) VALUES ('%s');`
+			tx = gormdb.Exec(fmt.Sprintf(insertImageInfraStm, sourceID))
+			Expect(tx.Error).To(BeNil())
+
+			user := auth.User{
+				Username:     "admin",
+				Organization: "admin",
+				EmailDomain:  "admin.example.com",
+			}
+			ctx := auth.NewTokenContext(context.TODO(), user)
+
+			srv := handlers.NewServiceHandler(service.NewSourceService(s, nil), service.NewAssessmentService(s, nil, nil), nil, service.NewSizerService(nil, s), nil, nil, nil)
+			resp, err := srv.GetSourceDownloadURL(ctx, server.GetSourceDownloadURLRequestObject{Id: sourceID})
+			Expect(err).To(BeNil())
+			Expect(reflect.TypeOf(resp).String()).To(Equal(reflect.TypeOf(server.GetSourceDownloadURL200JSONResponse{}).String()))
+
+			result := resp.(server.GetSourceDownloadURL200JSONResponse)
+			Expect(result.Url).NotTo(BeEmpty())
+			Expect(result.ExpiresAt).NotTo(BeNil())
+		})
+
+		It("returns 404 when trying to access another org's source - SECURITY TEST", func() {
+			// Create source owned by "batman" org
+			victimSourceID := uuid.New()
+			tx := gormdb.Exec(fmt.Sprintf(insertSourceWithUsernameStm, victimSourceID, "batman", "batman"))
+			Expect(tx.Error).To(BeNil())
+
+			insertImageInfraStm := `INSERT INTO image_infras (source_id) VALUES ('%s');`
+			tx = gormdb.Exec(fmt.Sprintf(insertImageInfraStm, victimSourceID))
+			Expect(tx.Error).To(BeNil())
+
+			// Attempt to access with "joker" credentials
+			attackerUser := auth.User{
+				Username:     "joker",
+				Organization: "joker",
+				EmailDomain:  "joker.example.com",
+			}
+			ctx := auth.NewTokenContext(context.TODO(), attackerUser)
+
+			srv := handlers.NewServiceHandler(service.NewSourceService(s, nil), service.NewAssessmentService(s, nil, nil), nil, service.NewSizerService(nil, s), nil, nil, nil)
+			resp, err := srv.GetSourceDownloadURL(ctx, server.GetSourceDownloadURLRequestObject{Id: victimSourceID})
+
+			Expect(err).To(BeNil())
+			// CRITICAL: Must return 404 (not 403) to prevent existence oracle
+			Expect(reflect.TypeOf(resp).String()).To(Equal(reflect.TypeOf(server.GetSourceDownloadURL404JSONResponse{}).String()))
+		})
+
+		It("returns 404 when accessing with same username but different org - SECURITY TEST", func() {
+			// Create source owned by "batman" user in "batman-org"
+			victimSourceID := uuid.New()
+			tx := gormdb.Exec(fmt.Sprintf(insertSourceWithUsernameStm, victimSourceID, "batman", "batman-org"))
+			Expect(tx.Error).To(BeNil())
+
+			insertImageInfraStm := `INSERT INTO image_infras (source_id) VALUES ('%s');`
+			tx = gormdb.Exec(fmt.Sprintf(insertImageInfraStm, victimSourceID))
+			Expect(tx.Error).To(BeNil())
+
+			// Attempt to access with same username but different organization
+			attackerUser := auth.User{
+				Username:     "batman",
+				Organization: "evil-org",
+				EmailDomain:  "evil.example.com",
+			}
+			ctx := auth.NewTokenContext(context.TODO(), attackerUser)
+
+			srv := handlers.NewServiceHandler(service.NewSourceService(s, nil), service.NewAssessmentService(s, nil, nil), nil, service.NewSizerService(nil, s), nil, nil, nil)
+			resp, err := srv.GetSourceDownloadURL(ctx, server.GetSourceDownloadURLRequestObject{Id: victimSourceID})
+
+			Expect(err).To(BeNil())
+			// CRITICAL: Must return 404 to prove organization check is working
+			Expect(reflect.TypeOf(resp).String()).To(Equal(reflect.TypeOf(server.GetSourceDownloadURL404JSONResponse{}).String()))
+		})
+
+		It("returns 404 for non-existent source", func() {
+			user := auth.User{
+				Username:     "admin",
+				Organization: "admin",
+				EmailDomain:  "admin.example.com",
+			}
+			ctx := auth.NewTokenContext(context.TODO(), user)
+
+			srv := handlers.NewServiceHandler(service.NewSourceService(s, nil), service.NewAssessmentService(s, nil, nil), nil, service.NewSizerService(nil, s), nil, nil, nil)
+			resp, err := srv.GetSourceDownloadURL(ctx, server.GetSourceDownloadURLRequestObject{Id: uuid.New()})
+
+			Expect(err).To(BeNil())
+			Expect(reflect.TypeOf(resp).String()).To(Equal(reflect.TypeOf(server.GetSourceDownloadURL404JSONResponse{}).String()))
+		})
+
+		AfterEach(func() {
+			gormdb.Exec("DELETE FROM image_infras;")
+			gormdb.Exec("DELETE FROM sources;")
+		})
+	})
 })