diff --git a/modules/api/src/main/scala/vinyldns/api/route/StatusRouting.scala b/modules/api/src/main/scala/vinyldns/api/route/StatusRouting.scala index f6e6fb6aa..e6fa509b8 100644 --- a/modules/api/src/main/scala/vinyldns/api/route/StatusRouting.scala +++ b/modules/api/src/main/scala/vinyldns/api/route/StatusRouting.scala @@ -84,15 +84,17 @@ class StatusRoute( (post & path("status")) { parameters("processingDisabled".as[Boolean]) { isProcessingDisabled => authenticateAndExecute(postStatus(isProcessingDisabled, _)){ isProcessingDisabled => - complete( - StatusCodes.OK, - CurrentStatus( - isProcessingDisabled, - serverConfig.color, - serverConfig.keyName, - serverConfig.version + onSuccess(processingDisabled.set(isProcessingDisabled).unsafeToFuture()) { + complete( + StatusCodes.OK, + CurrentStatus( + isProcessingDisabled, + serverConfig.color, + serverConfig.keyName, + serverConfig.version + ) ) - ) + } } } } diff --git a/modules/api/src/test/functional/tests/internal/status_test.py b/modules/api/src/test/functional/tests/internal/status_test.py index c8f87acbc..c6cfccc5a 100644 --- a/modules/api/src/test/functional/tests/internal/status_test.py +++ b/modules/api/src/test/functional/tests/internal/status_test.py @@ -18,6 +18,43 @@ def test_get_status_success(shared_zone_test_context): assert_that(result["version"], not_none()) +def test_post_status_fails_for_non_admin_users(shared_zone_test_context): + """ + Tests that the post request to status endpoint fails for non-admin users + """ + client = shared_zone_test_context.ok_vinyldns_client + result = client.post_status(True) + + assert_that(result, is_("Not authorized. User 'ok' cannot make the requested change.")) + + +def test_post_status_fails_for_non_users(shared_zone_test_context): + """ + Tests that the post request to status endpoint fails for non-users with fake access and secret key + """ + client = shared_zone_test_context.non_user_client + result = client.post_status(True) + + assert_that(result, is_("Authentication Failed: Account with accessKey not-exist-key specified was not found")) + + +def test_post_status_pass_for_admin_users(shared_zone_test_context): + """ + Tests that the post request to status endpoint pass for admin users + """ + client = shared_zone_test_context.support_user_client + + client.post_status(True) + + status = client.get_status() + assert_that(status["processingDisabled"], is_(True)) + + client.post_status(False) + + status = client.get_status() + assert_that(status["processingDisabled"], is_(False)) + + @pytest.mark.serial @pytest.mark.skip_production def test_toggle_processing(shared_zone_test_context): @@ -25,15 +62,16 @@ def test_toggle_processing(shared_zone_test_context): Test that updating a zone when processing is disabled does not happen """ client = shared_zone_test_context.ok_vinyldns_client + admin_client = shared_zone_test_context.support_user_client ok_zone = copy.deepcopy(shared_zone_test_context.ok_zone) # disable processing - client.post_status(True) + admin_client.post_status(True) status = client.get_status() assert_that(status["processingDisabled"], is_(True)) - client.post_status(False) + admin_client.post_status(False) status = client.get_status() assert_that(status["processingDisabled"], is_(False)) diff --git a/modules/api/src/test/functional/tests/shared_zone_test_context.py b/modules/api/src/test/functional/tests/shared_zone_test_context.py index 8d919cb23..614385773 100644 --- a/modules/api/src/test/functional/tests/shared_zone_test_context.py +++ b/modules/api/src/test/functional/tests/shared_zone_test_context.py @@ -31,8 +31,9 @@ class SharedZoneTestContext(object): self.unassociated_client = VinylDNSClient(VinylDNSTestContext.vinyldns_url, "listGroupAccessKey", "listGroupSecretKey") self.test_user_client = VinylDNSClient(VinylDNSTestContext.vinyldns_url, "testUserAccessKey", "testUserSecretKey") self.history_client = VinylDNSClient(VinylDNSTestContext.vinyldns_url, "history-key", "history-secret") + self.non_user_client = VinylDNSClient(VinylDNSTestContext.vinyldns_url, "not-exist-key", "not-exist-secret") self.clients = [self.ok_vinyldns_client, self.dummy_vinyldns_client, self.shared_zone_vinyldns_client, self.support_user_client, - self.unassociated_client, self.test_user_client, self.history_client] + self.unassociated_client, self.test_user_client, self.history_client, self.non_user_client] self.list_zones = ListZonesTestContext(partition_id) self.list_zones_client = self.list_zones.client self.list_records_context = ListRecordSetsTestContext(partition_id) diff --git a/modules/api/src/test/scala/vinyldns/api/VinylDNSTestHelpers.scala b/modules/api/src/test/scala/vinyldns/api/VinylDNSTestHelpers.scala index d071b846d..0b66e376a 100644 --- a/modules/api/src/test/scala/vinyldns/api/VinylDNSTestHelpers.scala +++ b/modules/api/src/test/scala/vinyldns/api/VinylDNSTestHelpers.scala @@ -16,9 +16,11 @@ package vinyldns.api +import cats.effect.{ContextShift, IO} import com.comcast.ip4s.IpAddress +import fs2.concurrent.SignallingRef import org.joda.time.DateTime -import vinyldns.api.config.{BatchChangeConfig, HighValueDomainConfig, LimitsConfig, ManualReviewConfig, ScheduledChangesConfig} +import vinyldns.api.config.{BatchChangeConfig, HighValueDomainConfig, LimitsConfig, ManualReviewConfig, ScheduledChangesConfig, ServerConfig} import vinyldns.api.domain.batch.V6DiscoveryNibbleBoundaries import vinyldns.core.domain.record._ import vinyldns.core.domain.zone._ @@ -27,6 +29,12 @@ import scala.util.matching.Regex trait VinylDNSTestHelpers { + private implicit val cs: ContextShift[IO] = + IO.contextShift(scala.concurrent.ExecutionContext.global) + + val processingDisabled: SignallingRef[IO, Boolean] = + fs2.concurrent.SignallingRef[IO, Boolean](false).unsafeRunSync() + val highValueDomainRegexList: List[Regex] = List(new Regex("high-value-domain.*")) val highValueDomainIpList: List[IpAddress] = (IpAddress("192.0.2.252") ++ IpAddress("192.0.2.253") ++ IpAddress( @@ -71,6 +79,9 @@ trait VinylDNSTestHelpers { val testLimitConfig: LimitsConfig = LimitsConfig(100,100,1000,1500,100,100,100) + val testServerConfig: ServerConfig = + ServerConfig(100, 100, 100, 100, true, approvedNameServers, "blue", "unset", "vinyldns.", false, true, true) + val batchChangeConfig: BatchChangeConfig = BatchChangeConfig(batchChangeLimit, sharedApprovedTypes, v6DiscoveryNibbleBoundaries) diff --git a/modules/api/src/test/scala/vinyldns/api/route/StatusRoutingSpec.scala b/modules/api/src/test/scala/vinyldns/api/route/StatusRoutingSpec.scala index adec84c2a..ceab8eb0c 100644 --- a/modules/api/src/test/scala/vinyldns/api/route/StatusRoutingSpec.scala +++ b/modules/api/src/test/scala/vinyldns/api/route/StatusRoutingSpec.scala @@ -18,35 +18,49 @@ package vinyldns.api.route import akka.actor.ActorSystem import akka.http.scaladsl.model.StatusCodes +import akka.http.scaladsl.server.Route import akka.http.scaladsl.testkit.ScalatestRouteTest -import cats.effect.{ContextShift, IO} -import fs2.concurrent.SignallingRef import org.scalatest._ import org.scalatest.matchers.should.Matchers import org.scalatest.wordspec.AnyWordSpec import org.scalatestplus.mockito.MockitoSugar +import vinyldns.api.VinylDNSTestHelpers +import vinyldns.api.VinylDNSTestHelpers.processingDisabled +import vinyldns.core.TestMembershipData.{notAuth, okAuth, superUserAuth} class StatusRoutingSpec extends AnyWordSpec with ScalatestRouteTest - with StatusRoute with OneInstancePerTest with VinylDNSJsonProtocol with BeforeAndAfterEach with MockitoSugar with Matchers { + val statusRoute: Route = + new StatusRoute( + VinylDNSTestHelpers.testServerConfig, + new TestVinylDNSAuthenticator(okAuth), + VinylDNSTestHelpers.processingDisabled + ).getRoutes + val notAuthRoute: Route = + new StatusRoute( + VinylDNSTestHelpers.testServerConfig, + new TestVinylDNSAuthenticator(notAuth), + VinylDNSTestHelpers.processingDisabled + ).getRoutes + val adminUserRoute: Route = + new StatusRoute( + VinylDNSTestHelpers.testServerConfig, + new TestVinylDNSAuthenticator(superUserAuth), + VinylDNSTestHelpers.processingDisabled + ).getRoutes + def actorRefFactory: ActorSystem = system - private implicit val cs: ContextShift[IO] = - IO.contextShift(scala.concurrent.ExecutionContext.global) - - val processingDisabled: SignallingRef[IO, Boolean] = - fs2.concurrent.SignallingRef[IO, Boolean](false).unsafeRunSync() - "GET /status" should { "return the current status of true" in { - Get("/status") ~> statusRoute("blue", "unset", "vinyldns.") ~> check { + Get("/status") ~> statusRoute ~> check { response.status shouldBe StatusCodes.OK val resultStatus = responseAs[CurrentStatus] resultStatus.processingDisabled shouldBe false @@ -58,16 +72,16 @@ class StatusRoutingSpec } "POST /status" should { - "disable processing" in { - Post("/status?processingDisabled=true") ~> statusRoute("blue", "unset", "vinyldns.") ~> check { + "disable processing when it's requested by admin user" in { + Post("/status?processingDisabled=true") ~> adminUserRoute ~> check { response.status shouldBe StatusCodes.OK val resultStatus = responseAs[CurrentStatus] resultStatus.processingDisabled shouldBe true } } - "enable processing" in { - Post("/status?processingDisabled=false") ~> statusRoute("blue", "unset", "vinyldns.") ~> check { + "enable processing when it's requested by admin user" in { + Post("/status?processingDisabled=false") ~> adminUserRoute ~> check { response.status shouldBe StatusCodes.OK val resultStatus = responseAs[CurrentStatus] resultStatus.processingDisabled shouldBe false @@ -76,5 +90,35 @@ class StatusRoutingSpec processingDisabled.get.unsafeRunSync() shouldBe false } } + + "not disable processing when it's requested by non-admin user" in { + Post("/status?processingDisabled=true") ~> statusRoute ~> check { + response.status shouldBe StatusCodes.Forbidden + } + } + + "not enable processing when it's requested by non-admin user" in { + Post("/status?processingDisabled=false") ~> statusRoute ~> check { + response.status shouldBe StatusCodes.Forbidden + + // remember, the signal is the opposite of intent + processingDisabled.get.unsafeRunSync() shouldBe false + } + } + + "not disable processing when it's requested by a non-user" in { + Post("/status?processingDisabled=true") ~> notAuthRoute ~> check { + response.status shouldBe StatusCodes.Forbidden + } + } + + "not enable processing when it's requested by a non-user" in { + Post("/status?processingDisabled=false") ~> notAuthRoute ~> check { + response.status shouldBe StatusCodes.Forbidden + + // remember, the signal is the opposite of intent + processingDisabled.get.unsafeRunSync() shouldBe false + } + } } }