feat(dgs): added auth to queries and mutations (#1982)

Squashed commit of the following: commit 6f6420323765ad24f3c2c73d60c936d8dd32eadc Author: Rani <[email protected]> Date: Tue Aug 10 09:18:50 2021 -0700 fix(pr): skip schema validation commit e5e37c723ae9f9d33de48615a8953b26ab507c31 Author: Rani <[email protected]> Date: Mon Aug 9 15:56:51 2021 -0700 fix(dgs): application query auth commit 35aec6590744b97a05e464601d56e99277a5c23e Author: Rani <[email protected]> Date: Mon Aug 9 13:55:20 2021 -0700 feat(dgs): added auth to dismissNotification by adding the app name to the query commit cd17650f2c85befb9bce8b5357599687b7eefe84 Author: Rani <[email protected]> Date: Mon Aug 9 13:51:18 2021 -0700 feat(dgs): added auth to queries and mutations (cherry picked from commit 75b2a6fc1ed76c6c759a815a34e96f764b9c33a8) Co-authored-by: Rani Horev <[email protected]>
spinnaker · Aug 10, 2021 · 43bed86 · 43bed86
1 parent 7065b78
commit 43bed86
Show file tree

Hide file tree

Showing 7 changed files with 52 additions and 7 deletions.
diff --git a/...c/main/kotlin/com/netflix/spinnaker/keel/persistence/DismissibleNotificationRepository.kt b/...c/main/kotlin/com/netflix/spinnaker/keel/persistence/DismissibleNotificationRepository.kt
@@ -38,7 +38,7 @@ interface DismissibleNotificationRepository {
   /**
    * Sets [DismissibleNotification.isActive] to false in the corresponding database record.
    */
-  fun dismissNotificationById(notificationUid: UID, user: String): Boolean
+  fun dismissNotificationById(application: String, notificationUid: UID, user: String): Boolean
 
   /**
    * Sets [isActive] of a given [type] of [DismissibleNotification] to false for a given [application].

diff --git a/...ql/src/main/kotlin/com/netflix/spinnaker/keel/sql/SqlDismissibleNotificationRepository.kt b/...ql/src/main/kotlin/com/netflix/spinnaker/keel/sql/SqlDismissibleNotificationRepository.kt
@@ -84,13 +84,14 @@ class SqlDismissibleNotificationRepository(
     )
   }
 
-  override fun dismissNotificationById(notificationUid: UID, user: String): Boolean {
+  override fun dismissNotificationById(application: String, notificationUid: UID, user: String): Boolean {
     val updatedJson = generateDismissalJson(user)
     return sqlRetry.withRetry(WRITE) {
       jooq
         .update(DISMISSIBLE_NOTIFICATION)
         .set(DISMISSIBLE_NOTIFICATION.JSON, updatedJson)
         .where(DISMISSIBLE_NOTIFICATION.UID.eq(notificationUid.toString()))
+        .and(DISMISSIBLE_NOTIFICATION.APPLICATION.eq(application))
         .execute()
     } > 0
   }

diff --git a/...c/test/kotlin/com/netflix/spinnaker/keel/sql/SqlDismissibleNotificationRepositoryTests.kt b/...c/test/kotlin/com/netflix/spinnaker/keel/sql/SqlDismissibleNotificationRepositoryTests.kt
@@ -99,7 +99,7 @@ class SqlDismissibleNotificationRepositoryTests {
   @Test
   fun `correctly dismisses notification`() {
     val uid = notificationRepository.storeNotification(notification)
-    expectThat(notificationRepository.dismissNotificationById(uid, "[email protected]"))
+    expectThat(notificationRepository.dismissNotificationById(deliveryConfig.application, uid, "[email protected]"))
       .isTrue()
 
     val updatedNotification = notificationRepository.notificationHistory(deliveryConfig.application, limit = 1).first()
@@ -158,7 +158,7 @@ class SqlDismissibleNotificationRepositoryTests {
         notification.copy(triggeredAt = clock.tickMinutes(1))
       )
       if (it < 5) {
-        notificationRepository.dismissNotificationById(uid, "[email protected]")
+        notificationRepository.dismissNotificationById(deliveryConfig.application, uid, "[email protected]")
       }
     }
 

diff --git a/keel-web/src/main/kotlin/com/netflix/spinnaker/keel/rest/dgs/ApplicationFetcher.kt b/keel-web/src/main/kotlin/com/netflix/spinnaker/keel/rest/dgs/ApplicationFetcher.kt
@@ -9,6 +9,7 @@ import com.netflix.graphql.dgs.exceptions.DgsEntityNotFoundException
 import com.netflix.spinnaker.keel.api.Environment
 import com.netflix.spinnaker.keel.api.action.ActionType
 import com.netflix.spinnaker.keel.artifacts.ArtifactVersionLinks
+import com.netflix.spinnaker.keel.auth.AuthorizationSupport
 import com.netflix.spinnaker.keel.core.api.DependsOnConstraint
 import com.netflix.spinnaker.keel.events.EventLevel.ERROR
 import com.netflix.spinnaker.keel.events.EventLevel.WARNING
@@ -41,6 +42,7 @@ import com.netflix.spinnaker.keel.services.ResourceStatusService
 import graphql.execution.DataFetcherResult
 import graphql.schema.DataFetchingEnvironment
 import org.dataloader.DataLoader
+import org.springframework.security.access.prepost.PreAuthorize
 import java.util.concurrent.CompletableFuture
 
 /**
@@ -57,10 +59,12 @@ class ApplicationFetcher(
   private val artifactVersionLinks: ArtifactVersionLinks,
   private val applicationFetcherSupport: ApplicationFetcherSupport,
   private val notificationRepository: DismissibleNotificationRepository,
-  private val deliveryConfigImporter: DeliveryConfigImporter
+  private val deliveryConfigImporter: DeliveryConfigImporter,
+  private val authorizationSupport: AuthorizationSupport,
 ) {
 
   @DgsData(parentType = DgsConstants.QUERY.TYPE_NAME, field = DgsConstants.QUERY.Application)
+  @PreAuthorize("""@authorizationSupport.hasApplicationPermission('READ', 'APPLICATION', #appName)""")
   fun application(dfe: DataFetchingEnvironment, @InputArgument("appName") appName: String): MdApplication {
     val config = try {
       keelRepository.getDeliveryConfigForApplication(appName)

diff --git a/keel-web/src/main/kotlin/com/netflix/spinnaker/keel/rest/dgs/GitIntegration.kt b/keel-web/src/main/kotlin/com/netflix/spinnaker/keel/rest/dgs/GitIntegration.kt
@@ -4,6 +4,7 @@ import com.netflix.graphql.dgs.DgsComponent
 import com.netflix.graphql.dgs.DgsData
 import com.netflix.graphql.dgs.DgsDataFetchingEnvironment
 import com.netflix.graphql.dgs.InputArgument
+import com.netflix.spinnaker.keel.auth.AuthorizationSupport
 import com.netflix.spinnaker.keel.front50.Front50Service
 import com.netflix.spinnaker.keel.front50.model.Application
 import com.netflix.spinnaker.keel.front50.model.ManagedDeliveryConfig
@@ -14,6 +15,7 @@ import com.netflix.spinnaker.keel.graphql.types.MdUpdateGitIntegrationPayload
 import com.netflix.spinnaker.keel.igor.ScmService
 import com.netflix.spinnaker.keel.igor.getDefaultBranch
 import kotlinx.coroutines.runBlocking
+import org.springframework.security.access.prepost.PreAuthorize
 import org.springframework.web.bind.annotation.RequestHeader
 
 /**
@@ -23,6 +25,7 @@ import org.springframework.web.bind.annotation.RequestHeader
 class GitIntegration(
   private val front50Service: Front50Service,
   private val scmService: ScmService,
+  private val authorizationSupport: AuthorizationSupport,
 ) {
 
   @DgsData(parentType = DgsConstants.MDAPPLICATION.TYPE_NAME, field = DgsConstants.MDAPPLICATION.GitIntegration)
@@ -34,6 +37,10 @@ class GitIntegration(
   }
 
   @DgsData(parentType = DgsConstants.MUTATION.TYPE_NAME, field = DgsConstants.MUTATION.UpdateGitIntegration)
+  @PreAuthorize(
+    """@authorizationSupport.hasApplicationPermission('WRITE', 'APPLICATION', #payload.application)
+    and @authorizationSupport.hasServiceAccountAccess('APPLICATION', #payload.application)"""
+  )
   fun updateGitIntegration(
     @InputArgument payload: MdUpdateGitIntegrationPayload,
     @RequestHeader("X-SPINNAKER-USER") user: String

diff --git a/keel-web/src/main/kotlin/com/netflix/spinnaker/keel/rest/dgs/Mutations.kt b/keel-web/src/main/kotlin/com/netflix/spinnaker/keel/rest/dgs/Mutations.kt
@@ -7,6 +7,7 @@ import com.netflix.spinnaker.keel.api.ArtifactInEnvironmentContext
 import com.netflix.spinnaker.keel.api.action.ActionType
 import com.netflix.spinnaker.keel.api.constraints.ConstraintStatus
 import com.netflix.spinnaker.keel.api.constraints.UpdatedConstraintStatus
+import com.netflix.spinnaker.keel.auth.AuthorizationSupport
 import com.netflix.spinnaker.keel.core.api.EnvironmentArtifactPin
 import com.netflix.spinnaker.keel.core.api.EnvironmentArtifactVeto
 import com.netflix.spinnaker.keel.exceptions.InvalidConstraintException
@@ -26,6 +27,7 @@ import com.netflix.spinnaker.keel.services.ApplicationService
 import com.netflix.spinnaker.keel.veto.unhappy.UnhappyVeto
 import de.huxhorn.sulky.ulid.ULID
 import org.slf4j.LoggerFactory
+import org.springframework.security.access.prepost.PreAuthorize
 import org.springframework.web.bind.annotation.RequestHeader
 
 /**
@@ -37,7 +39,8 @@ class Mutations(
   private val actuationPauser: ActuationPauser,
   private val unhappyVeto: UnhappyVeto,
   private val deliveryConfigRepository: DeliveryConfigRepository,
-  private val notificationRepository: DismissibleNotificationRepository
+  private val notificationRepository: DismissibleNotificationRepository,
+  private val authorizationSupport: AuthorizationSupport,
 ) {
 
   companion object {
@@ -52,6 +55,10 @@ class Mutations(
   }
 
   @DgsData(parentType = DgsConstants.MUTATION.TYPE_NAME, field = DgsConstants.MUTATION.UpdateConstraintStatus)
+  @PreAuthorize(
+    """@authorizationSupport.hasApplicationPermission('WRITE', 'APPLICATION', #payload.application)
+    and @authorizationSupport.hasServiceAccountAccess('APPLICATION', #payload.application)"""
+  )
   fun updateConstraintStatus(
     @InputArgument payload: MdConstraintStatusPayload,
     @RequestHeader("X-SPINNAKER-USER") user: String
@@ -70,6 +77,7 @@ class Mutations(
   }
 
   @DgsData(parentType = DgsConstants.MUTATION.TYPE_NAME, field = DgsConstants.MUTATION.ToggleManagement)
+  @PreAuthorize("@authorizationSupport.hasApplicationPermission('WRITE', 'APPLICATION', #application)")
   fun toggleManagement(
     @InputArgument application: String,
     @InputArgument isPaused: Boolean,
@@ -85,6 +93,10 @@ class Mutations(
   }
 
   @DgsData(parentType = DgsConstants.MUTATION.TYPE_NAME, field = DgsConstants.MUTATION.PinArtifactVersion)
+  @PreAuthorize(
+    """@authorizationSupport.hasApplicationPermission('WRITE', 'APPLICATION', #payload.application)
+    and @authorizationSupport.hasServiceAccountAccess('APPLICATION', #payload.application)"""
+  )
   fun pinArtifactVersion(
     @InputArgument payload: MdArtifactVersionActionPayload,
     @RequestHeader("X-SPINNAKER-USER") user: String
@@ -94,6 +106,10 @@ class Mutations(
   }
 
   @DgsData(parentType = DgsConstants.MUTATION.TYPE_NAME, field = DgsConstants.MUTATION.UnpinArtifactVersion)
+  @PreAuthorize(
+    """@authorizationSupport.hasApplicationPermission('WRITE', 'APPLICATION', #payload.application)
+    and @authorizationSupport.hasServiceAccountAccess('APPLICATION', #payload.application)"""
+  )
   fun unpinArtifactVersion(
     @InputArgument payload: MdUnpinArtifactVersionPayload,
     @RequestHeader("X-SPINNAKER-USER") user: String
@@ -108,6 +124,10 @@ class Mutations(
   }
 
   @DgsData(parentType = DgsConstants.MUTATION.TYPE_NAME, field = DgsConstants.MUTATION.MarkArtifactVersionAsBad)
+  @PreAuthorize(
+    """@authorizationSupport.hasApplicationPermission('WRITE', 'APPLICATION', #payload.application)
+    and @authorizationSupport.hasServiceAccountAccess('APPLICATION', #payload.application)"""
+  )
   fun markArtifactVersionAsBad(
     @InputArgument payload: MdArtifactVersionActionPayload,
     @RequestHeader("X-SPINNAKER-USER") user: String
@@ -117,6 +137,10 @@ class Mutations(
   }
 
   @DgsData(parentType = DgsConstants.MUTATION.TYPE_NAME, field = DgsConstants.MUTATION.MarkArtifactVersionAsGood)
+  @PreAuthorize(
+    """@authorizationSupport.hasApplicationPermission('WRITE', 'APPLICATION', #payload.application)
+    and @authorizationSupport.hasServiceAccountAccess('APPLICATION', #payload.application)"""
+  )
   fun markArtifactVersionAsGood(
     @InputArgument payload: MdMarkArtifactVersionAsGoodPayload,
     @RequestHeader("X-SPINNAKER-USER") user: String
@@ -131,6 +155,10 @@ class Mutations(
   }
 
   @DgsData(parentType = DgsConstants.MUTATION.TYPE_NAME, field = DgsConstants.MUTATION.RetryArtifactVersionAction)
+  @PreAuthorize(
+    """@authorizationSupport.hasApplicationPermission('WRITE', 'APPLICATION', #payload.application)
+    and @authorizationSupport.hasServiceAccountAccess('APPLICATION', #payload.application)"""
+  )
   fun retryArtifactVersionAction(
     @InputArgument payload: MdRetryArtifactActionPayload,
     @RequestHeader("X-SPINNAKER-USER") user: String
@@ -165,12 +193,16 @@ class Mutations(
    * Dismisses a notification, given it's ID.
    */
   @DgsData(parentType = DgsConstants.MUTATION.TYPE_NAME, field = DgsConstants.MUTATION.DismissNotification)
+  @PreAuthorize(
+    """@authorizationSupport.hasApplicationPermission('WRITE', 'APPLICATION', #payload.application)
+    and @authorizationSupport.hasServiceAccountAccess('APPLICATION', #payload.application)"""
+  )
   fun dismissNotification(
     @InputArgument payload: MdDismissNotificationPayload,
     @RequestHeader("X-SPINNAKER-USER") user: String
   ): Boolean {
     log.debug("Dismissing notification with ID=${payload.id} (by user $user)")
-    return notificationRepository.dismissNotificationById(ULID.parseULID(payload.id), user)
+    return notificationRepository.dismissNotificationById(payload.application, ULID.parseULID(payload.id), user)
   }
 }
 

diff --git a/keel-web/src/main/resources/schema/schema.graphql b/keel-web/src/main/resources/schema/schema.graphql
@@ -333,5 +333,6 @@ enum MdEventLevel {
 }
 
 input MdDismissNotificationPayload {
+  application: String!
   id: String!
 }