From 289e1c3692a72a1a69c8e3edd17f75313e4c986c Mon Sep 17 00:00:00 2001 From: Stephen Compall Date: Mon, 11 Apr 2022 14:47:57 -0400 Subject: [PATCH] use matchers for auth middleware test assertions (#13565) * use OptionValues instead of Option#get CHANGELOG_BEGIN CHANGELOG_END --- security-evidence.md | 40 ++--- triggers/service/auth/BUILD.bazel | 6 +- .../middleware/oauth2/TestMiddleware.scala | 163 ++++++++++-------- .../middleware/oauth2/TestRequestStore.scala | 25 +-- .../daml/auth/oauth2/test/server/Client.scala | 16 +- .../daml/auth/oauth2/test/server/Test.scala | 39 +++-- 6 files changed, 161 insertions(+), 128 deletions(-) diff --git a/security-evidence.md b/security-evidence.md index 3f3047f17c..501daa20a5 100644 --- a/security-evidence.md +++ b/security-evidence.md @@ -3,7 +3,7 @@ ## Authorization: - Updating the package service fails with insufficient authorization: [AuthorizationTest.scala](ledger-service/http-json/src/it/scala/http/AuthorizationTest.scala#L81) - Updating the package service succeeds with sufficient authorization: [AuthorizationTest.scala](ledger-service/http-json/src/it/scala/http/AuthorizationTest.scala#L89) -- accept user tokens: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L151) +- accept user tokens: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L152) - badly-authorized create is rejected: [AuthorizationSpec.scala](daml-lf/engine/src/test/scala/com/digitalasset/daml/lf/engine/AuthorizationSpec.scala#L60) - badly-authorized create is rejected: [AbstractHttpServiceIntegrationTest.scala](ledger-service/http-json/src/itlib/scala/http/AbstractHttpServiceIntegrationTest.scala#L1778) - badly-authorized exercise is rejected: [AuthorizationSpec.scala](daml-lf/engine/src/test/scala/com/digitalasset/daml/lf/engine/AuthorizationSpec.scala#L158) @@ -28,13 +28,13 @@ - refresh a token after expiry on the server side: [TriggerServiceTest.scala](triggers/service/src/test/scala/com/digitalasset/daml/lf/engine/trigger/TriggerServiceTest.scala#L718) - reject requests with missing auth header: [AbstractHttpServiceIntegrationTest.scala](ledger-service/http-json/src/itlib/scala/http/AbstractHttpServiceIntegrationTest.scala#L1272) - request a fresh token after expiry on user request: [TriggerServiceTest.scala](triggers/service/src/test/scala/com/digitalasset/daml/lf/engine/trigger/TriggerServiceTest.scala#L703) -- return the token from a cookie: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L95) -- return unauthorized on an expired token: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L138) -- return unauthorized on an invalid token: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L127) -- return unauthorized on insufficient app id claims: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L108) -- return unauthorized without cookie: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L86) -- the /login endpoint with an oauth server checking claims should not authorize disallowed admin claims: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L342) -- the /login endpoint with an oauth server checking claims should not authorize unauthorized parties: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L335) +- return the token from a cookie: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L96) +- return unauthorized on an expired token: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L139) +- return unauthorized on an invalid token: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L128) +- return unauthorized on insufficient app id claims: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L109) +- return unauthorized without cookie: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L87) +- the /login endpoint with an oauth server checking claims should not authorize disallowed admin claims: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L343) +- the /login endpoint with an oauth server checking claims should not authorize unauthorized parties: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L336) - websocket request with invalid protocol token should be denied: [AbstractWebsocketServiceIntegrationTest.scala](ledger-service/http-json/src/itlib/scala/http/AbstractWebsocketServiceIntegrationTest.scala#L91) - websocket request with valid protocol token should allow client subscribe to stream: [AbstractWebsocketServiceIntegrationTest.scala](ledger-service/http-json/src/itlib/scala/http/AbstractWebsocketServiceIntegrationTest.scala#L79) - websocket request without protocol token should be denied: [AbstractWebsocketServiceIntegrationTest.scala](ledger-service/http-json/src/itlib/scala/http/AbstractWebsocketServiceIntegrationTest.scala#L101) @@ -187,18 +187,18 @@ - restart triggers after shutdown: [TriggerServiceTest.scala](triggers/service/src/test/scala/com/digitalasset/daml/lf/engine/trigger/TriggerServiceTest.scala#L578) - restart triggers with initialization errors: [TriggerServiceTest.scala](triggers/service/src/test/scala/com/digitalasset/daml/lf/engine/trigger/TriggerServiceTest.scala#L466) - restart triggers with update errors: [TriggerServiceTest.scala](triggers/service/src/test/scala/com/digitalasset/daml/lf/engine/trigger/TriggerServiceTest.scala#L482) -- the /auth endpoint given claim token should return unauthorized on insufficient party claims: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L291) -- the /login endpoint should redirect and set the cookie: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L166) -- the /login endpoint should return OK and set cookie without redirectUri: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L195) -- the /login endpoint with an oauth server checking claims should redirect to the configured middleware callback URI: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L397) -- the /login endpoint with an oauth server checking claims should refuse requests when max capacity is reached: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L424) -- the /login endpoint with an oauth server checking claims should refuse requests when max capacity is reached: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L472) -- the /refresh endpoint should fail on an invalid refresh token: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L260) -- the /refresh endpoint should return a new access token: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L225) -- the TestMiddlewareClientAutoRedirectToLogin client should not redirect to /login for JSON request: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L634) -- the TestMiddlewareClientAutoRedirectToLogin client should redirect to /login for HTML request: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L616) -- the TestMiddlewareClientNoRedirectToLogin client should not redirect to /login: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L533) -- the TestMiddlewareClientYesRedirectToLogin client should redirect to /login: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L583) +- the /auth endpoint given claim token should return unauthorized on insufficient party claims: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L292) +- the /login endpoint should redirect and set the cookie: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L167) +- the /login endpoint should return OK and set cookie without redirectUri: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L196) +- the /login endpoint with an oauth server checking claims should redirect to the configured middleware callback URI: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L402) +- the /login endpoint with an oauth server checking claims should refuse requests when max capacity is reached: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L433) +- the /login endpoint with an oauth server checking claims should refuse requests when max capacity is reached: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L483) +- the /refresh endpoint should fail on an invalid refresh token: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L261) +- the /refresh endpoint should return a new access token: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L226) +- the TestMiddlewareClientAutoRedirectToLogin client should not redirect to /login for JSON request: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L645) +- the TestMiddlewareClientAutoRedirectToLogin client should redirect to /login for HTML request: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L627) +- the TestMiddlewareClientNoRedirectToLogin client should not redirect to /login: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L544) +- the TestMiddlewareClientYesRedirectToLogin client should redirect to /login: [TestMiddleware.scala](triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala#L593) ## Performance: - Tail call optimization: Tail recursion does not blow the scala JVM stack.: [TailCallTest.scala](daml-lf/interpreter/src/test/scala/com/digitalasset/daml/lf/speedy/TailCallTest.scala#L16) diff --git a/triggers/service/auth/BUILD.bazel b/triggers/service/auth/BUILD.bazel index a309a35183..d8b863d493 100644 --- a/triggers/service/auth/BUILD.bazel +++ b/triggers/service/auth/BUILD.bazel @@ -12,6 +12,8 @@ exports_files(["release/oauth2-middleware-logback.xml"]) scalacopts = [] +test_scalacopts = ["-P:wartremover:traverser:org.wartremover.warts.OptionPartial"] + da_scala_library( name = "oauth2-api", srcs = glob(["src/main/scala/com/daml/auth/oauth2/api/**/*.scala"]), @@ -154,7 +156,7 @@ da_scala_test( "@maven//:io_spray_spray_json", "@maven//:org_scalaz_scalaz_core", ], - scalacopts = scalacopts, + scalacopts = scalacopts + test_scalacopts, deps = [ ":oauth2-api", ":oauth2-test-server", @@ -193,7 +195,7 @@ da_scala_test( scala_runtime_deps = [ "@maven//:com_typesafe_akka_akka_stream_testkit", ], - scalacopts = scalacopts, + scalacopts = scalacopts + test_scalacopts, deps = [ ":middleware-api", ":oauth2-api", diff --git a/triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala b/triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala index 2ddebf1ddd..9ea2beb8eb 100644 --- a/triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala +++ b/triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestMiddleware.scala @@ -41,7 +41,8 @@ abstract class TestMiddleware extends AsyncWordSpec with TestFixture with SuiteResourceManagementAroundAll - with Matchers { + with Matchers + with OptionValues { protected[this] def makeJwt( claims: Request.Claims, expiresIn: Option[Duration], @@ -79,7 +80,7 @@ abstract class TestMiddleware source.close() } } - assert(bindingPort == filePort) + bindingPort should ===(filePort) } } "the /auth endpoint" should { @@ -89,7 +90,7 @@ abstract class TestMiddleware for { result <- middlewareClient.requestAuth(claims, Nil) } yield { - assert(result == None) + result should ===(None) } } // TEST_EVIDENCE: Authorization: return the token from a cookie @@ -99,10 +100,10 @@ abstract class TestMiddleware val cookieHeader = Cookie("daml-ledger-token", token.toCookieValue) for { result <- middlewareClient.requestAuth(claims, List(cookieHeader)) - auth = result.get + auth = result.value } yield { - assert(auth.accessToken == token.accessToken) - assert(auth.refreshToken == token.refreshToken) + auth.accessToken should ===(token.accessToken) + auth.refreshToken should ===(token.refreshToken) } } // TEST_EVIDENCE: Authorization: return unauthorized on insufficient app id claims @@ -121,7 +122,7 @@ abstract class TestMiddleware for { result <- middlewareClient.requestAuth(claims, List(cookieHeader)) } yield { - assert(result == None) + result should ===(None) } } // TEST_EVIDENCE: Authorization: return unauthorized on an invalid token @@ -132,7 +133,7 @@ abstract class TestMiddleware for { result <- middlewareClient.requestAuth(claims, List(cookieHeader)) } yield { - assert(result == None) + result should ===(None) } } // TEST_EVIDENCE: Authorization: return unauthorized on an expired token @@ -144,7 +145,7 @@ abstract class TestMiddleware for { result <- middlewareClient.requestAuth(claims, List(cookieHeader)) } yield { - assert(result == None) + result should ===(None) } } @@ -171,25 +172,25 @@ abstract class TestMiddleware resp <- Http().singleRequest(req) // Redirect to /authorize on authorization server resp <- { - assert(resp.status == StatusCodes.Found) - val req = HttpRequest(uri = resp.header[Location].get.uri) + resp.status should ===(StatusCodes.Found) + val req = HttpRequest(uri = resp.header[Location].value.uri) Http().singleRequest(req) } // Redirect to /cb on middleware resp <- { - assert(resp.status == StatusCodes.Found) - val req = HttpRequest(uri = resp.header[Location].get.uri) + resp.status should ===(StatusCodes.Found) + val req = HttpRequest(uri = resp.header[Location].value.uri) Http().singleRequest(req) } } yield { // Redirect to client callback - assert(resp.status == StatusCodes.Found) - assert(resp.header[Location].get.uri == middlewareClientCallbackUri) + resp.status should ===(StatusCodes.Found) + resp.header[Location].value.uri should ===(middlewareClientCallbackUri) // Store token in cookie - val cookie = resp.header[`Set-Cookie`].get.cookie - assert(cookie.name == "daml-ledger-token") - val token = OAuthResponse.Token.fromCookieValue(cookie.value).get - assert(token.tokenType == "bearer") + val cookie = resp.header[`Set-Cookie`].value.cookie + cookie.name should ===("daml-ledger-token") + val token = OAuthResponse.Token.fromCookieValue(cookie.value).value + token.tokenType should ===("bearer") } } // TEST_EVIDENCE: Semantics: the /login endpoint should return OK and set cookie without redirectUri @@ -200,24 +201,24 @@ abstract class TestMiddleware resp <- Http().singleRequest(req) // Redirect to /authorize on authorization server resp <- { - assert(resp.status == StatusCodes.Found) - val req = HttpRequest(uri = resp.header[Location].get.uri) + resp.status should ===(StatusCodes.Found) + val req = HttpRequest(uri = resp.header[Location].value.uri) Http().singleRequest(req) } // Redirect to /cb on middleware resp <- { - assert(resp.status == StatusCodes.Found) - val req = HttpRequest(uri = resp.header[Location].get.uri) + resp.status should ===(StatusCodes.Found) + val req = HttpRequest(uri = resp.header[Location].value.uri) Http().singleRequest(req) } } yield { // Return OK - assert(resp.status == StatusCodes.OK) + resp.status should ===(StatusCodes.OK) // Store token in cookie - val cookie = resp.header[`Set-Cookie`].get.cookie - assert(cookie.name == "daml-ledger-token") - val token = OAuthResponse.Token.fromCookieValue(cookie.value).get - assert(token.tokenType == "bearer") + val cookie = resp.header[`Set-Cookie`].value.cookie + cookie.name should ===("daml-ledger-token") + val token = OAuthResponse.Token.fromCookieValue(cookie.value).value + token.tokenType should ===("bearer") } } } @@ -230,21 +231,21 @@ abstract class TestMiddleware resp <- Http().singleRequest(loginReq) // Redirect to /authorize on authorization server resp <- { - assert(resp.status == StatusCodes.Found) - val req = HttpRequest(uri = resp.header[Location].get.uri) + resp.status should ===(StatusCodes.Found) + val req = HttpRequest(uri = resp.header[Location].value.uri) Http().singleRequest(req) } // Redirect to /cb on middleware resp <- { - assert(resp.status == StatusCodes.Found) - val req = HttpRequest(uri = resp.header[Location].get.uri) + resp.status should ===(StatusCodes.Found) + val req = HttpRequest(uri = resp.header[Location].value.uri) Http().singleRequest(req) } // Extract token from cookie (token1, refreshToken) = { - val cookie = resp.header[`Set-Cookie`].get.cookie - val token = OAuthResponse.Token.fromCookieValue(cookie.value).get - (AccessToken(token.accessToken), RefreshToken(token.refreshToken.get)) + val cookie = resp.header[`Set-Cookie`].value.cookie + val token = OAuthResponse.Token.fromCookieValue(cookie.value).value + (AccessToken(token.accessToken), RefreshToken(token.refreshToken.value)) } // Advance time _ = clock.fastForward(Duration.ofSeconds(1)) @@ -252,9 +253,9 @@ abstract class TestMiddleware authorize <- middlewareClient.requestRefresh(refreshToken) } yield { // Test that we got a new access token - assert(authorize.accessToken != token1) + authorize.accessToken should !==(token1) // Test that we got a new refresh token - assert(authorize.refreshToken.get != refreshToken) + authorize.refreshToken.value should !==(refreshToken) } } // TEST_EVIDENCE: Semantics: the /refresh endpoint should fail on an invalid refresh token @@ -265,7 +266,7 @@ abstract class TestMiddleware case value => fail(s"Expected failure with RefreshException but got $value") } } yield { - assert(exception.status.isInstanceOf[StatusCodes.ClientError]) + exception.status shouldBe a[StatusCodes.ClientError] } } } @@ -352,25 +353,27 @@ class TestMiddlewareClaimsToken extends TestMiddleware { resp <- Http().singleRequest(req) // Redirect to /authorize on authorization server resp <- { - assert(resp.status == StatusCodes.Found) - val req = HttpRequest(uri = resp.header[Location].get.uri) + resp.status should ===(StatusCodes.Found) + val req = HttpRequest(uri = resp.header[Location].value.uri) Http().singleRequest(req) } // Redirect to /cb on middleware resp <- { - assert(resp.status == StatusCodes.Found) - val req = HttpRequest(uri = resp.header[Location].get.uri) + resp.status should ===(StatusCodes.Found) + val req = HttpRequest(uri = resp.header[Location].value.uri) Http().singleRequest(req) } } yield { // Redirect to client callback - assert(resp.status == StatusCodes.Found) - assert(resp.header[Location].get.uri.withQuery(Uri.Query()) == middlewareClientCallbackUri) + resp.status should ===(StatusCodes.Found) + resp.header[Location].value.uri.withQuery(Uri.Query()) should ===( + middlewareClientCallbackUri + ) // with error parameter set - assert(resp.header[Location].get.uri.query().toMap.get("error") == Some("access_denied")) + resp.header[Location].value.uri.query().toMap.get("error") should ===(Some("access_denied")) // Without token in cookie val cookie = resp.header[`Set-Cookie`] - assert(cookie == None) + cookie should ===(None) } } } @@ -390,6 +393,8 @@ class TestMiddlewareUserToken extends TestMiddleware { class TestMiddlewareCallbackUriOverride extends AsyncWordSpec + with Matchers + with OptionValues with TestFixture with SuiteResourceManagementAroundAll { override protected val middlewareCallbackUri = Some(Uri("http://localhost/MIDDLEWARE_CALLBACK")) @@ -402,14 +407,16 @@ class TestMiddlewareCallbackUriOverride resp <- Http().singleRequest(req) // Redirect to /authorize on authorization server resp <- { - assert(resp.status == StatusCodes.Found) - val req = HttpRequest(uri = resp.header[Location].get.uri) + resp.status should ===(StatusCodes.Found) + val req = HttpRequest(uri = resp.header[Location].value.uri) Http().singleRequest(req) } } yield { // Redirect to configured callback URI on middleware - assert(resp.status == StatusCodes.Found) - assert(resp.header[Location].get.uri.withQuery(Uri.Query()) == middlewareCallbackUri.get) + resp.status should ===(StatusCodes.Found) + resp.header[Location].value.uri.withQuery(Uri.Query()) should ===( + middlewareCallbackUri.value + ) } } } @@ -417,6 +424,8 @@ class TestMiddlewareCallbackUriOverride class TestMiddlewareLimitedCallbackStore extends AsyncWordSpec + with Matchers + with OptionValues with TestFixture with SuiteResourceManagementAroundAll { override protected val maxMiddlewareLogins = 2 @@ -431,8 +440,8 @@ class TestMiddlewareLimitedCallbackStore } def followRedirect(resp: HttpResponse) = { - assert(resp.status == StatusCodes.Found) - val uri = resp.header[Location].get.uri + resp.status should ===(StatusCodes.Found) + val uri = resp.header[Location].value.uri val req = HttpRequest(uri = uri) Http().singleRequest(req) } @@ -445,10 +454,10 @@ class TestMiddlewareLimitedCallbackStore .flatMap(followRedirect) // The store should be full refusedCarol <- login(Party("Carol")) - _ = assert(refusedCarol.status == StatusCodes.ServiceUnavailable) + _ = refusedCarol.status should ===(StatusCodes.ServiceUnavailable) // Follow first redirect to middleware callback. resultAlice <- followRedirect(redirectAlice) - _ = assert(resultAlice.status == StatusCodes.Found) + _ = resultAlice.status should ===(StatusCodes.Found) // The store should have space again redirectCarol <- login(Party("Carol")) .flatMap(followRedirect) @@ -456,8 +465,8 @@ class TestMiddlewareLimitedCallbackStore resultBob <- followRedirect(redirectBob) resultCarol <- followRedirect(redirectCarol) } yield { - assert(resultBob.status == StatusCodes.Found) - assert(resultCarol.status == StatusCodes.Found) + resultBob.status should ===(StatusCodes.Found) + resultCarol.status should ===(StatusCodes.Found) } } } @@ -465,6 +474,8 @@ class TestMiddlewareLimitedCallbackStore class TestMiddlewareClientLimitedCallbackStore extends AsyncWordSpec + with Matchers + with OptionValues with TestFixture with SuiteResourceManagementAroundAll { override protected val maxClientAuthCallbacks = 2 @@ -484,8 +495,8 @@ class TestMiddlewareClientLimitedCallbackStore } def followRedirect(resp: HttpResponse) = { - assert(resp.status == StatusCodes.Found) - val uri = resp.header[Location].get.uri + resp.status should ===(StatusCodes.Found) + val uri = resp.header[Location].value.uri val req = HttpRequest(uri = uri) Http().singleRequest(req) } @@ -502,10 +513,10 @@ class TestMiddlewareClientLimitedCallbackStore .flatMap(followRedirect) // The store should be full refusedCarol <- login(Party("Carol")) - _ = assert(refusedCarol.status == StatusCodes.ServiceUnavailable) + _ = refusedCarol.status should ===(StatusCodes.ServiceUnavailable) // Follow first redirect to middleware client. resultAlice <- followRedirect(redirectAlice) - _ = assert(resultAlice.status == StatusCodes.OK) + _ = resultAlice.status should ===(StatusCodes.OK) // The store should have space again redirectCarol <- login(Party("Carol")) .flatMap(followRedirect) @@ -514,8 +525,8 @@ class TestMiddlewareClientLimitedCallbackStore resultBob <- followRedirect(redirectBob) resultCarol <- followRedirect(redirectCarol) } yield { - assert(resultBob.status == StatusCodes.OK) - assert(resultCarol.status == StatusCodes.OK) + resultBob.status should ===(StatusCodes.OK) + resultCarol.status should ===(StatusCodes.OK) } } } @@ -544,7 +555,7 @@ class TestMiddlewareClientNoRedirectToLogin for { resp <- Http().singleRequest(req) // Unauthorized with WWW-Authenticate header - _ = assert(resp.status == StatusCodes.Unauthorized) + _ = resp.status should ===(StatusCodes.Unauthorized) wwwAuthenticate = resp.header[headers.`WWW-Authenticate`].value challenge = wwwAuthenticate.challenges .find(_.scheme == Response.authenticateChallengeName) @@ -560,14 +571,12 @@ class TestMiddlewareClientNoRedirectToLogin // The body should include the same challenge bodyChallenge <- Unmarshal(resp).to[Response.AuthenticateChallenge] } yield { - assert(headerChallenge.auth == middlewareClient.authUri(claims)) - assert( - headerChallenge.login.withQuery(Uri.Query.Empty) == middlewareClientRoutes - .loginUri(claims) - .withQuery(Uri.Query.Empty) + headerChallenge.auth should ===(middlewareClient.authUri(claims)) + headerChallenge.login.withQuery(Uri.Query.Empty) should ===( + middlewareClientRoutes.loginUri(claims).withQuery(Uri.Query.Empty) ) - assert(headerChallenge.login.query().get("claims").value == claims.toQueryString()) - assert(bodyChallenge == headerChallenge) + headerChallenge.login.query().get("claims").value should ===(claims.toQueryString()) + bodyChallenge should ===(headerChallenge) } } } @@ -575,6 +584,7 @@ class TestMiddlewareClientNoRedirectToLogin class TestMiddlewareClientYesRedirectToLogin extends AsyncWordSpec + with Matchers with OptionValues with TestFixture with SuiteResourceManagementAroundAll { @@ -594,14 +604,14 @@ class TestMiddlewareClientYesRedirectToLogin resp <- Http().singleRequest(req) } yield { // Redirect to /login on middleware - assert(resp.status == StatusCodes.Found) + resp.status should ===(StatusCodes.Found) val loginUri = resp.header[headers.Location].value.uri - assert( - loginUri.withQuery(Uri.Query.Empty) == middlewareClientRoutes + loginUri.withQuery(Uri.Query.Empty) should ===( + middlewareClientRoutes .loginUri(claims) .withQuery(Uri.Query.Empty) ) - assert(loginUri.query().get("claims").value == claims.toQueryString()) + loginUri.query().get("claims").value should ===(claims.toQueryString()) } } } @@ -609,6 +619,7 @@ class TestMiddlewareClientYesRedirectToLogin class TestMiddlewareClientAutoRedirectToLogin extends AsyncWordSpec + with Matchers with TestFixture with SuiteResourceManagementAroundAll { override protected val redirectToLogin: Client.RedirectToLogin = Client.RedirectToLogin.Auto @@ -628,7 +639,7 @@ class TestMiddlewareClientAutoRedirectToLogin resp <- Http().singleRequest(req) } yield { // Redirect to /login on middleware - assert(resp.status == StatusCodes.Found) + resp.status should ===(StatusCodes.Found) } } // TEST_EVIDENCE: Semantics: the TestMiddlewareClientAutoRedirectToLogin client should not redirect to /login for JSON request @@ -646,7 +657,7 @@ class TestMiddlewareClientAutoRedirectToLogin resp <- Http().singleRequest(req) } yield { // Unauthorized with WWW-Authenticate header - assert(resp.status == StatusCodes.Unauthorized) + resp.status should ===(StatusCodes.Unauthorized) } } } diff --git a/triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestRequestStore.scala b/triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestRequestStore.scala index 763fc4410b..95c70486b5 100644 --- a/triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestRequestStore.scala +++ b/triggers/service/auth/src/test/scala/com/daml/auth/middleware/oauth2/TestRequestStore.scala @@ -4,27 +4,28 @@ package com.daml.auth.middleware.oauth2 import com.daml.auth.middleware.api.RequestStore +import org.scalatest.matchers.should.Matchers import org.scalatest.wordspec.AsyncWordSpec import scala.concurrent.duration._ -class TestRequestStore extends AsyncWordSpec { +class TestRequestStore extends AsyncWordSpec with Matchers { "return None on missing element" in { val store = new RequestStore[Int, String](1, 1.day) - assert(store.pop(0) == None) + store.pop(0) should ===(None) } "return previously put element" in { val store = new RequestStore[Int, String](1, 1.day) store.put(0, "zero") - assert(store.pop(0) == Some("zero")) + store.pop(0) should ===(Some("zero")) } "return None on previously popped element" in { val store = new RequestStore[Int, String](1, 1.day) store.put(0, "zero") store.pop(0) - assert(store.pop(0) == None) + store.pop(0) should ===(None) } "store multiple elements" in { @@ -32,9 +33,9 @@ class TestRequestStore extends AsyncWordSpec { store.put(0, "zero") store.put(1, "one") store.put(2, "two") - assert(store.pop(0) == Some("zero")) - assert(store.pop(1) == Some("one")) - assert(store.pop(2) == Some("two")) + store.pop(0) should ===(Some("zero")) + store.pop(1) should ===(Some("one")) + store.pop(2) should ===(Some("two")) } "store no more than max capacity" in { @@ -42,9 +43,9 @@ class TestRequestStore extends AsyncWordSpec { assert(store.put(0, "zero")) assert(store.put(1, "one")) assert(!store.put(2, "two")) - assert(store.pop(0) == Some("zero")) - assert(store.pop(1) == Some("one")) - assert(store.pop(2) == None) + store.pop(0) should ===(Some("zero")) + store.pop(1) should ===(Some("one")) + store.pop(2) should ===(None) } "return None on timed out element" in { @@ -52,7 +53,7 @@ class TestRequestStore extends AsyncWordSpec { val store = new RequestStore[Int, String](1, 1.day, () => time) store.put(0, "zero") time += 1.day.toNanos - assert(store.pop(0) == None) + store.pop(0) should ===(None) } "free capacity for timed out elements" in { @@ -62,6 +63,6 @@ class TestRequestStore extends AsyncWordSpec { assert(!store.put(1, "one")) time += 1.day.toNanos assert(store.put(2, "two")) - assert(store.pop(2) == Some("two")) + store.pop(2) should ===(Some("two")) } } diff --git a/triggers/service/auth/src/test/scala/com/daml/auth/oauth2/test/server/Client.scala b/triggers/service/auth/src/test/scala/com/daml/auth/oauth2/test/server/Client.scala index cc610e2cf2..a7a89a2411 100644 --- a/triggers/service/auth/src/test/scala/com/daml/auth/oauth2/test/server/Client.scala +++ b/triggers/service/auth/src/test/scala/com/daml/auth/oauth2/test/server/Client.scala @@ -126,7 +126,12 @@ object Client { // Now we have the access_token and potentially the refresh token. At this point, // we would start the trigger. complete( - AccessResponse(tokenResp.accessToken, tokenResp.refreshToken.get): Response + AccessResponse( + tokenResp.accessToken, + tokenResp.refreshToken.getOrElse( + sys.error("/token endpoint failed to return a refresh token") + ), + ): Response ) } } @@ -174,7 +179,14 @@ object Client { onSuccess(f) { tokenResp => // Now we have the access_token and potentially the refresh token. At this point, // we would start the trigger. - complete(AccessResponse(tokenResp.accessToken, tokenResp.refreshToken.get): Response) + complete( + AccessResponse( + tokenResp.accessToken, + tokenResp.refreshToken.getOrElse( + sys.error("/token endpoint failed to return a refresh token") + ), + ): Response + ) } } } diff --git a/triggers/service/auth/src/test/scala/com/daml/auth/oauth2/test/server/Test.scala b/triggers/service/auth/src/test/scala/com/daml/auth/oauth2/test/server/Test.scala index 3a74d75d60..0fc7ab19cc 100644 --- a/triggers/service/auth/src/test/scala/com/daml/auth/oauth2/test/server/Test.scala +++ b/triggers/service/auth/src/test/scala/com/daml/auth/oauth2/test/server/Test.scala @@ -19,6 +19,8 @@ import com.daml.ledger.api.auth.{ } import com.daml.ledger.api.refinements.ApiTypes.Party import com.daml.ledger.api.testing.utils.SuiteResourceManagementAroundAll +import org.scalatest.OptionValues +import org.scalatest.matchers.should.Matchers import org.scalatest.wordspec.AsyncWordSpec import spray.json._ @@ -26,7 +28,12 @@ import java.time.Instant import scala.concurrent.Future import scala.util.Try -abstract class Test extends AsyncWordSpec with TestFixture with SuiteResourceManagementAroundAll { +abstract class Test + extends AsyncWordSpec + with Matchers + with OptionValues + with TestFixture + with SuiteResourceManagementAroundAll { import Client.JsonProtocol._ import Test._ @@ -58,14 +65,14 @@ abstract class Test extends AsyncWordSpec with TestFixture with SuiteResourceMan resp <- Http().singleRequest(req) // Redirect to /authorize on authorization server (No automatic redirect handling in akka-http) resp <- { - assert(resp.status == StatusCodes.Found) - val req = HttpRequest(uri = resp.header[Location].get.uri) + resp.status should ===(StatusCodes.Found) + val req = HttpRequest(uri = resp.header[Location].value.uri) Http().singleRequest(req) } // Redirect to /cb on client. resp <- { - assert(resp.status == StatusCodes.Found) - val req = HttpRequest(uri = resp.header[Location].get.uri) + resp.status should ===(StatusCodes.Found) + val req = HttpRequest(uri = resp.header[Location].value.uri) Http().singleRequest(req) } // Actual token response (proxied from auth server to us via the client) @@ -152,22 +159,22 @@ abstract class Test extends AsyncWordSpec with TestFixture with SuiteResourceMan _ <- Future(clock.set((Tok exp token1) plusSeconds 1)) (token2, _) <- expectRefresh(refresh1) } yield { - assert((Tok exp token2) isAfter (Tok exp token1)) - assert((Tok withoutExp token1) == (Tok withoutExp token2)) + (Tok exp token2) should be > (Tok exp token1) + (Tok withoutExp token1) should ===((Tok withoutExp token2)) } } "return a token with the requested app id" in { for { (token, __) <- expectToken(Seq(), applicationId = Some("my-app-id")) } yield { - assert(Tok.userId(token) == Some("my-app-id")) + Tok.userId(token) should ===(Some("my-app-id")) } } "return a token with no app id if non is requested" in { for { (token, __) <- expectToken(Seq(), applicationId = None) } yield { - assert(Tok.userId(token) == None) + Tok.userId(token) should ===(None) } } @@ -182,7 +189,7 @@ class ClaimTokenTest extends Test { type Tok = CustomDamlJWTPayload override object Tok extends TokenCompat[Tok] { override def userId(t: Tok) = t.applicationId - override def exp(t: Tok) = t.exp.get + override def exp(t: Tok) = t.exp.value override def withoutExp(t: Tok) = t copy (exp = None) } @@ -199,21 +206,21 @@ class ClaimTokenTest extends Test { for { (token, _) <- expectToken(Seq()) } yield { - assert(token.actAs == Seq()) + token.actAs should ===(Seq()) } } "issue a token with 1 party" in { for { (token, _) <- expectToken(Seq("Alice")) } yield { - assert(token.actAs == Seq("Alice")) + token.actAs should ===(Seq("Alice")) } } "issue a token with multiple parties" in { for { (token, _) <- expectToken(Seq("Alice", "Bob")) } yield { - assert(token.actAs == Seq("Alice", "Bob")) + token.actAs should ===(Seq("Alice", "Bob")) } } "deny access to unauthorized parties" in { @@ -221,7 +228,7 @@ class ClaimTokenTest extends Test { for { error <- expectError(Seq("Alice", "Eve")) } yield { - assert(error == "access_denied") + error should ===("access_denied") } } "issue a token with admin access" in { @@ -236,7 +243,7 @@ class ClaimTokenTest extends Test { for { error <- expectError(Seq(), admin = true) } yield { - assert(error == "access_denied") + error should ===("access_denied") } } } @@ -250,7 +257,7 @@ class UserTokenTest extends Test { type Tok = StandardJWTPayload override object Tok extends TokenCompat[Tok] { override def userId(t: Tok) = Some(t.userId).filter(_.nonEmpty) - override def exp(t: Tok) = t.exp.get + override def exp(t: Tok) = t.exp.value override def withoutExp(t: Tok) = t copy (exp = None) }