Added more fail safety for vulnerability export.

This should affect all exports when a vulnerability disappears.

app/controllers/Notifications.scala

@@ -6,7 +6,7 @@ import javax.inject.Inject
 import com.ysoft.concurrent.FutureLock._
 import com.ysoft.odc.statistics.{FailedProjects, LibDepStatistics}
 import com.ysoft.odc.{Absolutizer, ArtifactFile, ArtifactItem, SetDiff}
-import models.{EmailMessageId, ExportedVulnerability}
+import models.{EmailMessageId, ExportedVulnerability, StandardVulnerabilityOverview, VulnerabilityOverview}
 import modules.TemplateCustomization
 import play.api.i18n.MessagesApi
 import play.api.libs.Crypto
@@ -59,7 +59,7 @@ class Notifications @Inject()(
   )(
     reportVulnerability: (Vulnerability, Set[GroupedDependency]) => Future[ExportedVulnerability[T]]
   )(
-    reportChangedProjectsForVulnerability: (Vulnerability, SetDiff[String], T) => Future[Unit]
+    reportChangedProjectsForVulnerability: (VulnerabilityOverview, SetDiff[String], T) => Future[Unit]
   ) = {
     val vulnerabilitiesByName = lds.vulnerabilitiesToDependencies.map{case (v, deps) => (v.name, (v, deps))}
     for{
@@ -82,7 +82,7 @@ class Notifications @Inject()(
         if(diff.nonEmpty) {
           for{
             // Try to load vuln from memory; If the vuln has disappeared, we have to load it from DB.
-            vulnerability <- lds.vulnerabilitiesByName.get(vulnerabilityName).fold(odcService.getVulnerabilityDetails(vulnerabilityName).map(_.get))(Future(_))
+            vulnerability <- lds.vulnerabilitiesByName.get(vulnerabilityName).fold(odcService.getVulnerabilityDescription(vulnerabilityName))(x => Future(new StandardVulnerabilityOverview(x)))
             (_: Unit) <- reportChangedProjectsForVulnerability(vulnerability, diff, exportedVulnerability.ticket)
             (_: Unit) <- ep.changeProjects(ticketId, diff, projects)
           } yield Some(diff)

app/models/vulnerabilityOverviews.scala

@@ -0,0 +1,49 @@
+package models
+
+import com.ysoft.odc.CWE
+import controllers.Vulnerability
+
+/**
+  * Provides some overview about vulnerability. It might be either covered by fully-detailed vulnerability or represent a vulnerability we know little or nothing about.
+  */
+abstract sealed class VulnerabilityOverview {
+  def name: String
+  def descriptionAttempt: String
+  def isSureAboutDescription: Boolean
+  def cvssScore: Option[Double]
+  def cweOption: Option[CWE]
+}
+
+object VulnerabilityOverview{
+  def apply(name: String, v: Option[Vulnerability]): VulnerabilityOverview = v.fold(UnknownVulnerabilityOverview(name))(new StandardVulnerabilityOverview(_))
+}
+
+final class StandardVulnerabilityOverview(vulnerability: Vulnerability) extends VulnerabilityOverview {
+  override def name: String = vulnerability.name
+  override def descriptionAttempt: String = vulnerability.description
+  override def isSureAboutDescription = true
+  override def cvssScore: Option[Double] = vulnerability.cvssScore
+  override def cweOption = vulnerability.cweOption
+}
+
+private final class UnknownVulnerabilityOverview(override val name: String, link: String) extends VulnerabilityOverview {
+  override def descriptionAttempt: String = s"Unknown vulnerability. Try looking at the following address for more details: $link"
+  override def cvssScore: Option[Double] = None
+  override def isSureAboutDescription = false
+  override def cweOption = None
+}
+
+private final class TotallyUnknownVulnerabilityOverview(override val name: String) extends VulnerabilityOverview {
+  override def descriptionAttempt: String = s"Unknown vulnerability. Not even sure where to look for other details. Maybe Googling the identifier will help."
+  override def cvssScore: Option[Double] = None
+  override def isSureAboutDescription = false
+  override def cweOption = None
+}
+
+private object UnknownVulnerabilityOverview {
+  def apply(name: String): VulnerabilityOverview = name match {
+    case cveId if name startsWith "CVE-" => new UnknownVulnerabilityOverview(name, s"https://nvd.nist.gov/vuln/detail/$cveId")
+    case ossIndexId if name startsWith "OSSINDEX-" => new UnknownVulnerabilityOverview(name, s"https://ossindex.net/resource/vulnerability/$ossIndexId")
+    case other => new TotallyUnknownVulnerabilityOverview(other)
+  }
+}

app/services/EmailExportService.scala

@@ -8,7 +8,7 @@ import com.ysoft.html.HtmlWithText._
 import com.ysoft.odc.{Absolutizer, SetDiff}
 import controllers._
 import models.Change.Direction
-import models.{Change, EmailMessageId}
+import models.{Change, EmailMessageId, VulnerabilityOverview}
 import play.api.libs.mailer.{Email, MailerClient}
 import play.twirl.api.{Html, HtmlFormat}
 
@@ -19,50 +19,11 @@ object EmailExportType extends Enumeration {
   val Digest = Value("digest")
 
 
-}
-object EmailExportService {
-
-  private object VulnerabilityDescription{
-    def apply(name: String, v: Option[Vulnerability]): VulnerabilityDescription = v.fold(UnknownVulnerabilityDescription(name))(new StandardVulnerabilityDescription(_))
-  }
-
-  private abstract class VulnerabilityDescription {
-    def name: String
-    def description: String
-    def cvssScore: Option[Double]
-  }
-
-  private final class StandardVulnerabilityDescription(vulnerability: Vulnerability) extends VulnerabilityDescription {
-    override def name: String = vulnerability.name
-    override def description: String = vulnerability.description
-    override def cvssScore: Option[Double] = vulnerability.cvssScore
-  }
-
-  private final class UnknownVulnerabilityDescription(override val name: String, link: String) extends VulnerabilityDescription {
-    override def description: String = s"Unknown vulnerability. Try looking at the following address for more details: $link"
-    override def cvssScore: Option[Double] = None
-  }
-
-  private final class TotallyUnknownVulnerabilityDescription(override val name: String) extends VulnerabilityDescription {
-    override def description: String = s"Unknown vulnerability. Not even sure where to look for other details. Maybe Googling the identifier will help."
-    override def cvssScore: Option[Double] = None
-  }
-
-  private object UnknownVulnerabilityDescription {
-    def apply(name: String): VulnerabilityDescription = name match {
-      case cveId if name startsWith "CVE-" => new UnknownVulnerabilityDescription(name, s"https://nvd.nist.gov/vuln/detail/$cveId")
-      case ossIndexId if name startsWith "OSSINDEX-" => new UnknownVulnerabilityDescription(name, s"https://ossindex.net/resource/vulnerability/$ossIndexId")
-      case other => new TotallyUnknownVulnerabilityDescription(other)
-    }
-  }
-
 }
 
 class EmailExportService(from: String, nobodyInterestedContact: String, val exportType: EmailExportType.Value, odcService: OdcDbService, mailerClient: MailerClient, notificationService: VulnerabilityNotificationService, emailSendingExecutionContext: ExecutionContext, absolutizer: Absolutizer)(implicit executionContext: ExecutionContext) {
   // Maybe it is not the best place for exportType, but I am not sure if we want this to be configurable. If no, then we can get rid of it. If yes, we should refactor it.
 
-  import EmailExportService.VulnerabilityDescription
-
   private def getEmail(loginInfo: LoginInfo) = loginInfo.providerKey // TODO: get the email in a cleaner way
 
   def recipientsForProjects(projects: Set[ReportInfo]) = for{
@@ -74,7 +35,7 @@ class EmailExportService(from: String, nobodyInterestedContact: String, val expo
     }
   }
 
-  def mailForVulnerabilityProjectsChange(vuln: Vulnerability, emailMessageId: EmailMessageId, diff: SetDiff[String], projects: ProjectsWithReports) = {
+  def mailForVulnerabilityProjectsChange(vuln: VulnerabilityOverview, emailMessageId: EmailMessageId, diff: SetDiff[String], projects: ProjectsWithReports) = {
     def showProjects(s: Set[String]) = s.map(p =>
       "* " + (try{
         friendlyProjectNameString(projects.parseUnfriendlyName(p))
@@ -119,7 +80,7 @@ class EmailExportService(from: String, nobodyInterestedContact: String, val expo
   def emailDigest(subscriber: LoginInfo, changes: Seq[Change], projects: ProjectsWithReports): Future[Email] = {
     val vulnNames = changes.map(_.vulnerabilityName).toSet
     for {
-      vulns <- Future.traverse(vulnNames.toSeq)(name => odcService.getVulnerabilityDetails(name).map(v => name -> VulnerabilityDescription(name, v))).map(_.toMap)
+      vulns <- Future.traverse(vulnNames.toSeq)(name => odcService.getVulnerabilityDetails(name).map(v => name -> VulnerabilityOverview(name, v))).map(_.toMap)
       groups = changes.groupBy(_.direction).withDefaultValue(Seq())
     } yield {
       val changesMarks = Map(Direction.Added -> "❢", Direction.Removed -> "☑")
@@ -131,9 +92,9 @@ class EmailExportService(from: String, nobodyInterestedContact: String, val expo
         text = "more info: "+link,
         html = Html("<a href=\""+HtmlFormat.escape(link)+"\">more info</a>")
       )
-      def vulnerabilityText(change: Change, vulnerability: VulnerabilityDescription): HtmlWithText = (
+      def vulnerabilityText(change: Change, vulnerability: VulnerabilityOverview): HtmlWithText = (
         heading(4)(s"${changesMarks(change.direction)} ${vulnerability.name}${vulnerability.cvssScore.fold("")(sev => s" (CVSS severity: $sev)")}")
-        + justHtml("<p>") + plainText(vulnerability.description) + justHtml("<br>") + justText("\n")
+        + justHtml("<p>") + plainText(vulnerability.descriptionAttempt) + justHtml("<br>") + justText("\n")
         + moreInfo(absolutizer.absolutize(routes.Statistics.vulnerability(vulnerability.name, None))) + justHtml("</p>")
       )
       def vulnChanges(changes: Seq[Change]): HtmlWithText =

app/services/IssueTrackerService.scala

@@ -2,7 +2,7 @@ package services
 
 import com.ysoft.odc.SetDiff
 import controllers.{ReportInfo, Vulnerability}
-import models.ExportedVulnerability
+import models.{ExportedVulnerability, VulnerabilityOverview}
 
 import scala.concurrent.Future
 
@@ -10,6 +10,6 @@ trait IssueTrackerService {
   def reportVulnerability(vulnerability: Vulnerability, projects: Set[ReportInfo]): Future[ExportedVulnerability[String]]
   def ticketLink(ticket: String): String
   def ticketLink(ticket: ExportedVulnerability[String]): String = ticketLink(ticket.ticket)
-  def updateVulnerability(vuln: Vulnerability, diff: SetDiff[ReportInfo], ticket: String): Future[Unit]
+  def updateVulnerability(vuln: VulnerabilityOverview, diff: SetDiff[ReportInfo], ticket: String): Future[Unit]
   //def migrateOldIssues(): Future[Unit]
 }

app/services/JiraIssueTrackerService.scala

@@ -5,7 +5,7 @@ import javax.inject.Inject
 import com.google.inject.name.Named
 import com.ysoft.odc.{Absolutizer, AtlassianAuthentication, SetDiff}
 import controllers.{ReportInfo, Vulnerability, friendlyProjectNameString, routes}
-import models.ExportedVulnerability
+import models.{ExportedVulnerability, StandardVulnerabilityOverview, VulnerabilityOverview}
 import play.api.Logger
 import play.api.libs.json.Json.JsValueWrapper
 import play.api.libs.json._
@@ -52,7 +52,7 @@ class JiraIssueTrackerService @Inject()(absolutizer: Absolutizer, @Named("jira-s
   private implicit val TransitionsFormats = Json.format[Transitions]
 
   override def reportVulnerability(vulnerability: Vulnerability, projects: Set[ReportInfo]): Future[ExportedVulnerability[String]] = throttler.throttle(api("issue").post(Json.obj(
-    "fields" -> (extractInitialFields(vulnerability) ++ extractManagedFields(vulnerability, projects))
+    "fields" -> (extractInitialFields(vulnerability) ++ extractManagedFields(new StandardVulnerabilityOverview(vulnerability), projects, requiresDescription = true))
   ))).map(response => // returns responses like {"id":"1234","key":"PROJ-6","self":"https://…/rest/api/2/issue/1234"}
     try{
       val issueInfo = Json.reads[JiraNewIssueResponse].reads(response.json).get
@@ -81,7 +81,7 @@ class JiraIssueTrackerService @Inject()(absolutizer: Absolutizer, @Named("jira-s
     }
   }
 
-  override def updateVulnerability(vuln: Vulnerability, diff: SetDiff[ReportInfo], ticket: String): Future[Unit] = {
+  override def updateVulnerability(vuln: VulnerabilityOverview, diff: SetDiff[ReportInfo], ticket: String): Future[Unit] = {
     val requiredTransitionOption = diff.whichNonEmpty match {
       case SetDiff.Selection.Old => noRelevantProjectAffectedTransitionNameOption
       case SetDiff.Selection.New | SetDiff.Selection.Both => newProjectAddedTransitionNameOption
@@ -96,7 +96,7 @@ class JiraIssueTrackerService @Inject()(absolutizer: Absolutizer, @Named("jira-s
       }
     }.getOrElse(Future.successful(None))
     val fieldsUpdateResult = throttler.throttle(api(s"issue/$ticket").put(obj(
-      "fields" -> extractManagedFields(vuln, diff.newSet)
+      "fields" -> extractManagedFields(vuln, diff.newSet, requiresDescription = false)
     ))).requireStatus(204).map{ resp => () }
     fieldsUpdateResult.flatMap { (_: Unit) =>
       transitionOptionFuture flatMap {
@@ -113,13 +113,13 @@ class JiraIssueTrackerService @Inject()(absolutizer: Absolutizer, @Named("jira-s
     "summary" -> s"${vulnerability.name} – ${vulnerability.cweOption.map(_ + ": ").getOrElse("")}${vulnerability.description.take(50).takeWhile(c => c != '\n' && c != '\r')}…"
   )
 
-  private def extractManagedFields(vulnerability: Vulnerability, projects: Set[ReportInfo]): JsObject = {
+  private def extractManagedFields(vulnerability: VulnerabilityOverview, projects: Set[ReportInfo], requiresDescription: Boolean): JsObject = {
     val base = Json.obj(
       "issuetype" -> Json.obj(
         "id" -> vulnerabilityIssueType.toString
-      ),
-      "description" -> extractDescription(vulnerability)
+      )
     )
+    val descriptionObj = if(requiresDescription || vulnerability.isSureAboutDescription) Json.obj("description" -> extractDescription(vulnerability)) else Json.obj()
     val additionalFields = Seq[Option[(String, JsValueWrapper)]](
       fields.cweId.map(id => id -> vulnerability.cweOption.fold("")(_.brief)),
       fields.linkId.map(id => id -> link(vulnerability)),
@@ -129,7 +129,7 @@ class JiraIssueTrackerService @Inject()(absolutizer: Absolutizer, @Named("jira-s
     )
     val additionalObj = Json.obj(additionalFields.flatten : _*)
     val constantObj = fields.constantFields.getOrElse(Json.obj())
-    base ++ additionalObj ++ constantObj
+    base ++ descriptionObj ++ additionalObj ++ constantObj
   }
 
 
@@ -137,9 +137,9 @@ class JiraIssueTrackerService @Inject()(absolutizer: Absolutizer, @Named("jira-s
 
   }*/
 
-  private def extractDescription(vulnerability: Vulnerability): String = vulnerability.description + "\n\n" + s"Details: ${link(vulnerability)}"
+  private def extractDescription(vulnerability: VulnerabilityOverview): String = vulnerability.descriptionAttempt + "\n\n" + s"Details: ${link(vulnerability)}"
 
-  private def link(vulnerability: Vulnerability): String = {
+  private def link(vulnerability: VulnerabilityOverview): String = {
     absolutizer.absolutize(routes.Statistics.vulnerability(vulnerability.name, None))
   }
 

app/services/OdcDbService.scala

@@ -10,6 +10,7 @@ import _root_.org.owasp.dependencycheck.utils.{DependencyVersion, DependencyVers
 import com.github.nscala_time.time.Imports._
 import com.google.inject.Inject
 import com.mockrunner.mock.jdbc.MockConnection
+import models.VulnerabilityOverview
 import models.odc.tables._
 import models.odc.{OdcProperty, Vulnerabilities}
 import play.api.Logger
@@ -36,6 +37,8 @@ class OdcDbService @Inject()(@NamedDatabase("odc") protected val dbConfigProvide
 
   def getVulnerabilityDetails(name: String): Future[Option[com.ysoft.odc.Vulnerability]] = getVulnerabilityDetails(_.cve === name)
 
+  def getVulnerabilityDescription(name: String): Future[VulnerabilityOverview] = getVulnerabilityDetails(name).map(VulnerabilityOverview(name, _))
+
   private def getVulnerabilityDetails(cond: Vulnerabilities => Rep[Boolean]): Future[Option[com.ysoft.odc.Vulnerability]] = {
     db.run(vulnerabilities.filter(cond).result).map(_.headOption) flatMap { bareVulnOption =>
       bareVulnOption.fold[Future[Option[com.ysoft.odc.Vulnerability]]](Future.successful(None)) { case (id, bareVuln) =>