simplifies conditions, minor changes

Author: timbastin

internal/core/asset/asset_controller.go

@@ -12,14 +12,16 @@ import (
 )
 
 type httpController struct {
-	assetRepository core.AssetRepository
-	assetService    core.AssetService
+	assetRepository       core.AssetRepository
+	assetService          core.AssetService
+	dependencyVulnService core.DependencyVulnService
 }
 
-func NewHttpController(repository core.AssetRepository, assetService core.AssetService) *httpController {
+func NewHttpController(repository core.AssetRepository, assetService core.AssetService, dependencyVulnService core.DependencyVulnService) *httpController {
 	return &httpController{
-		assetRepository: repository,
-		assetService:    assetService,
+		assetRepository:       repository,
+		assetService:          assetService,
+		dependencyVulnService: dependencyVulnService,
 	}
 }
 
@@ -151,7 +153,6 @@ func (c *httpController) Update(ctx core.Context) error {
 	enableTicketRangeUpdated := false
 
 	if patchRequest.EnableTicketRange {
-
 		if patchRequest.CVSSAutomaticTicketThreshold != nil {
 			if asset.CVSSAutomaticTicketThreshold != nil {
 				if !utils.CompareFirstTwoDecimals(*patchRequest.CVSSAutomaticTicketThreshold, *asset.CVSSAutomaticTicketThreshold) {
@@ -192,8 +193,8 @@ func (c *httpController) Update(ctx core.Context) error {
 		asset.RiskAutomaticTicketThreshold = nil
 	}
 
-	if enableTicketRangeUpdated {
-		err = c.assetService.UpdateAssetTickets(asset)
+	if enableTicketRangeUpdated || justification != "" {
+		err = c.dependencyVulnService.SyncTickets(asset)
 		if err != nil {
 			return fmt.Errorf("Error updating asset tickets: %v", err)
 		}
@@ -204,7 +205,7 @@ func (c *httpController) Update(ctx core.Context) error {
 		return echo.NewHTTPError(409, "assets with an empty name or an empty slug are not allowed").WithInternal(fmt.Errorf("assets with an empty name or an empty slug are not allowed"))
 	}
 
-	if updated {
+	if updated || enableTicketRangeUpdated {
 		err = c.assetRepository.Update(nil, &asset)
 		if err != nil {
 			return fmt.Errorf("error updating asset: %v", err)

internal/core/asset/asset_service.go

@@ -66,34 +66,6 @@ func (s *service) UpdateAssetRequirements(asset models.Asset, responsible string
 			return fmt.Errorf("could not update raw risk assessment: %v", err)
 		}
 
-		err = s.dependencyVulnService.SyncTickets(asset)
-		if err != nil {
-			slog.Info("error syncing tickets", "err", err)
-			return fmt.Errorf("could not sync tickets: %v", err)
-		}
-
-		return nil
-	})
-	if err != nil {
-		return fmt.Errorf("could not update asset: %v", err)
-	}
-
-	return nil
-}
-
-func (s *service) UpdateAssetTickets(asset models.Asset) error {
-	err := s.dependencyVulnRepository.Transaction(func(tx core.DB) error {
-		err := s.assetRepository.Save(tx, &asset)
-		if err != nil {
-			slog.Info("error saving asset", "err", err)
-			return fmt.Errorf("could not save asset: %v", err)
-		}
-
-		err = s.dependencyVulnService.SyncTickets(asset)
-		if err != nil {
-			slog.Info("error syncing tickets", "err", err)
-			return fmt.Errorf("could not sync tickets: %v", err)
-		}
 		return nil
 	})
 	if err != nil {

internal/core/common_interfaces.go

@@ -154,7 +154,6 @@ type OrganizationRepository interface {
 	ReadBySlug(slug string) (models.Org, error)
 	Update(tx DB, organization *models.Org) error
 	ContentTree(orgID uuid.UUID, projects []string) []common.ContentTreeElement
-	GetOrgByID(id uuid.UUID) (models.Org, error)
 }
 
 type InvitationRepository interface {

internal/core/dependency_vuln/dependency_vuln_service.go

@@ -278,7 +278,6 @@ func (s *service) SyncTicketsForAllAssets() error {
 }
 
 func (s *service) SyncTickets(asset models.Asset) error {
-
 	for _, assetVersion := range asset.AssetVersions {
 		if !s.ShouldCreateIssues(assetVersion) {
 			continue
@@ -318,9 +317,7 @@ func (s *service) SyncTickets(asset models.Asset) error {
 		}
 
 		errgroup := utils.ErrGroup[any](10)
-
 		for _, vulnerability := range vulnList {
-
 			if (cvssThreshold != nil && vulnerability.CVE.CVSS >= float32(*cvssThreshold)) || (riskThreshold != nil && *vulnerability.RawRiskAssessment >= *riskThreshold) {
 
 				if vulnerability.TicketID == nil {
@@ -332,14 +329,12 @@ func (s *service) SyncTickets(asset models.Asset) error {
 					}
 				} else {
 					// there is already a ticket,
-					//TODO: we need to update the ticket with the new information
 					errgroup.Go(func() (any, error) {
 						return nil, s.updateIssue(asset, vulnerability, repoID)
 					})
 				}
 				// check if the ticket should be closed
 			} else if (cvssThreshold != nil && vulnerability.CVE.CVSS < float32(*cvssThreshold)) || (riskThreshold != nil && *vulnerability.RawRiskAssessment < *riskThreshold) {
-
 				if vulnerability.TicketID != nil {
 					errgroup.Go(func() (any, error) {
 						return nil, s.closeIssue(vulnerability, repoID)

internal/core/integrations/github_integration.go

@@ -25,7 +25,6 @@ import (
 	"time"
 
 	"github.com/google/go-github/v62/github"
-	gitlab "gitlab.com/gitlab-org/api/client-go"
 
 	"github.com/l3montree-dev/devguard/internal/core"
 	"github.com/l3montree-dev/devguard/internal/core/org"
@@ -66,8 +65,7 @@ type githubIntegration struct {
 	assetRepository                 core.AssetRepository
 	assetVersionRepository          core.AssetVersionRepository
 
-	orgRepository core.OrganizationRepository
-
+	orgRepository       core.OrganizationRepository
 	projectRepository   core.ProjectRepository
 	githubClientFactory func(repoId string) (githubClientFacade, error)
 }
@@ -80,12 +78,9 @@ func NewGithubIntegration(db core.DB) *githubIntegration {
 	githubAppInstallationRepository := repositories.NewGithubAppInstallationRepository(db)
 
 	aggregatedVulnRepository := repositories.NewAggregatedVulnRepository(db)
-
 	dependencyVulnRepository := repositories.NewDependencyVulnRepository(db)
 	vulnEventRepository := repositories.NewVulnEventRepository(db)
-
 	projectRepository := repositories.NewProjectRepository(db)
-
 	orgRepository := repositories.NewOrgRepository(db)
 
 	frontendUrl := os.Getenv("FRONTEND_URL")
@@ -214,18 +209,18 @@ func (githubIntegration *githubIntegration) HandleWebhook(ctx core.Context) erro
 	}
 
 	switch event := event.(type) {
-	case *gitlab.IssueEvent:
+	case *github.IssueEvent:
 		// check if the issue is a devguard issue
-		issueNumber := event.ObjectAttributes.IID
-		issueID := event.ObjectAttributes.ID
+		issueNumber := event.Issue.GetNumber()
+		issueID := event.Issue.GetID()
 
 		// look for a vuln with such a github ticket id
 		vuln, err := githubIntegration.aggregatedVulnRepository.FindByTicketID(nil, fmt.Sprintf("github:%d/%d", issueID, issueNumber))
 		if err != nil {
 			slog.Debug("could not find vuln by ticket id", "err", err, "ticketId", fmt.Sprintf("github:%d/%d", issueID, issueNumber))
 			return nil
 		}
-		action := event.ObjectAttributes.Action
+		action := event.Action
 
 		// make sure to save the user - it might be a new user or it might have new values defined.
 		// we do not care about any error - and we want speed, thus do it on a goroutine
@@ -237,9 +232,9 @@ func (githubIntegration *githubIntegration) HandleWebhook(ctx core.Context) erro
 			}
 			// save the user in the database
 			user := models.ExternalUser{
-				ID:        fmt.Sprintf("github:%d", event.User.ID),
-				Username:  event.User.Username,
-				AvatarURL: event.User.AvatarURL,
+				ID:        fmt.Sprintf("github:%d", event.Actor.ID),
+				Username:  *event.Actor.Name,
+				AvatarURL: *event.Actor.AvatarURL,
 			}
 
 			err = githubIntegration.externalUserRepository.Save(nil, &user)
@@ -256,13 +251,7 @@ func (githubIntegration *githubIntegration) HandleWebhook(ctx core.Context) erro
 		switch action {
 		case "closed":
 			vulnDependencyVuln := vuln.(*models.DependencyVuln)
-
-			vulnDependencyVuln.SetTicketState(models.TicketStateClosed)
-			vuln.SetTicketState(models.TicketStateClosed)
-
-			var vulnEvent models.VulnEvent
-
-			vulnEvent = models.NewTicketClosedEvent(vuln.GetID(), fmt.Sprintf("github:%d", event.User.ID), fmt.Sprintf("This issue is closed by %s", event.User.Username))
+			vulnEvent := models.NewTicketClosedEvent(vuln.GetID(), fmt.Sprintf("github:%d", event.Actor.ID), fmt.Sprintf("This issue is closed by %s", *event.Actor.Name))
 
 			err := githubIntegration.dependencyVulnRepository.ApplyAndSave(nil, vulnDependencyVuln, &vulnEvent)
 			if err != nil {
@@ -271,12 +260,7 @@ func (githubIntegration *githubIntegration) HandleWebhook(ctx core.Context) erro
 
 		case "reopened":
 			vulnDependencyVuln := vuln.(*models.DependencyVuln)
-
-			vulnDependencyVuln.SetTicketState(models.TicketStateClosed)
-			vuln.SetTicketState(models.TicketStateClosed)
-
-			var vulnEvent models.VulnEvent
-			vulnEvent = models.NewReopenedEvent(vuln.GetID(), fmt.Sprintf("github:%d", event.User.ID), fmt.Sprintf("This issue is reopened by %s", event.User.Username))
+			vulnEvent := models.NewReopenedEvent(vuln.GetID(), fmt.Sprintf("github:%d", event.Actor.ID), fmt.Sprintf("This issue is reopened by %s", *event.Actor.Name))
 
 			err := githubIntegration.dependencyVulnRepository.ApplyAndSave(nil, vulnDependencyVuln, &vulnEvent)
 			if err != nil {
@@ -285,12 +269,7 @@ func (githubIntegration *githubIntegration) HandleWebhook(ctx core.Context) erro
 
 		case "deleted":
 			vulnDependencyVuln := vuln.(*models.DependencyVuln)
-
-			vulnDependencyVuln.SetTicketState(models.TicketStateDeleted)
-			vuln.SetTicketState(models.TicketStateDeleted)
-
-			var vulnEvent models.VulnEvent
-			vulnEvent = models.NewTicketDeletedEvent(vuln.GetID(), fmt.Sprintf("github:%d", event.User.ID), fmt.Sprintf("This issue is deleted by %s", event.User.Username))
+			vulnEvent := models.NewTicketDeletedEvent(vuln.GetID(), fmt.Sprintf("github:%d", event.Actor.ID), fmt.Sprintf("This issue is deleted by %s", *event.Actor.Name))
 
 			err := githubIntegration.dependencyVulnRepository.ApplyAndSave(nil, vulnDependencyVuln, &vulnEvent)
 			if err != nil {
@@ -733,27 +712,11 @@ func (g *githubIntegration) ReopenIssue(ctx context.Context, repoId string, depe
 }
 
 func (g *githubIntegration) UpdateIssue(ctx context.Context, asset models.Asset, repoId string, dependencyVuln models.DependencyVuln) error {
-
 	if !strings.HasPrefix(repoId, "github:") {
 		// this integration only handles github repositories.
 		return nil
 	}
 
-	// check if the dependencyVuln is open, if not we need to close the issue
-	if dependencyVuln.State != models.VulnStateOpen {
-		if dependencyVuln.TicketState == models.TicketStateOpen {
-			dependencyVuln.TicketState = models.TicketStateClosed
-			vulnEvent := models.NewTicketClosedEvent(dependencyVuln.ID, "system", "This issue is closed")
-
-			// save the event
-			err := g.dependencyVulnRepository.ApplyAndSave(nil, &dependencyVuln, &vulnEvent)
-			if err != nil {
-				slog.Error("could not save dependencyVuln and event", "err", err)
-			}
-			return nil
-		}
-	}
-
 	owner, repo, err := ownerAndRepoFromRepositoryID(repoId)
 	if err != nil {
 		return err
@@ -770,7 +733,7 @@ func (g *githubIntegration) UpdateIssue(ctx context.Context, asset models.Asset,
 		return err
 	}
 
-	org, err := g.orgRepository.GetOrgByID(project.OrganizationID)
+	org, err := g.orgRepository.Read(project.OrganizationID)
 	if err != nil {
 		slog.Error("could not get org by id", "err", err)
 		return err
@@ -794,8 +757,6 @@ func (g *githubIntegration) UpdateIssue(ctx context.Context, asset models.Asset,
 	if err != nil {
 		//check if err is 404 - if so, we can not reopen the issue
 		if err.Error() == "404 Not Found" {
-			// the issue was deleted - we need to set the ticket state to deleted
-			dependencyVuln.TicketState = models.TicketStateDeleted
 			// we can not reopen the issue - it is deleted
 			vulnEvent := models.NewTicketDeletedEvent(dependencyVuln.ID, "user", "This issue is deleted")
 			// save the event
@@ -808,39 +769,25 @@ func (g *githubIntegration) UpdateIssue(ctx context.Context, asset models.Asset,
 		return err
 	}
 
-	//check if the ticket state in devguard is different from the ticket state in gitlab, if so we need to update the ticket state in devguard
+	//check if the ticket state in devguard is different from the ticket state in github, if so we need to update the ticket state in devguard
 	ticketState := issue.State
 	devguardTicketState := dependencyVuln.TicketState
-	if *ticketState == "closed" {
-		if devguardTicketState == models.TicketStateOpen {
-			// the issue was closed - we need to set the ticket state to closed
-			dependencyVuln.TicketState = models.TicketStateClosed
-			// create a new event
-			vulnEvent := models.NewTicketClosedEvent(dependencyVuln.ID, "user", "This issue is closed")
-
-			// save the event
-			err := g.dependencyVulnRepository.ApplyAndSave(nil, &dependencyVuln, &vulnEvent)
-			if err != nil {
-				slog.Error("could not save dependencyVuln and event", "err", err)
-			}
-			return nil
+	if *ticketState == "closed" && devguardTicketState == models.TicketStateOpen {
+		// create a new event
+		vulnEvent := models.NewTicketClosedEvent(dependencyVuln.ID, "user", "This issue is closed")
 
+		// save the event
+		err := g.dependencyVulnRepository.ApplyAndSave(nil, &dependencyVuln, &vulnEvent)
+		if err != nil {
+			slog.Error("could not save dependencyVuln and event", "err", err)
 		}
-	}
-
-	if *ticketState == "opened" {
-		if devguardTicketState == models.TicketStateClosed {
-			// the issue was opened - we need to set the ticket state to open
-			dependencyVuln.TicketState = models.TicketStateOpen
-
-			// create a new event
-			vulnEvent := models.NewReopenedEvent(dependencyVuln.ID, "user", "This issue is reopened")
-			// save the event
-			err := g.dependencyVulnRepository.ApplyAndSave(nil, &dependencyVuln, &vulnEvent)
-			if err != nil {
-				slog.Error("could not save dependencyVuln and event", "err", err)
-			}
-			return nil
+	} else if *ticketState == "open" && devguardTicketState == models.TicketStateClosed {
+		// create a new event
+		vulnEvent := models.NewReopenedEvent(dependencyVuln.ID, "user", "This issue is reopened")
+		// save the event
+		err := g.dependencyVulnRepository.ApplyAndSave(nil, &dependencyVuln, &vulnEvent)
+		if err != nil {
+			slog.Error("could not save dependencyVuln and event", "err", err)
 		}
 	}