Merge pull request #68 from ing-bank/fix/aws-key

Validate accessKey/sessionToken content + refactor JWT verification

src/main/scala/com/ing/wbaa/rokku/sts/api/AdminApi.scala

@@ -83,7 +83,7 @@ trait AdminApi extends LazyLogging with Encryption with JwtToken {
       path("service" / "npa") {
         formFields("npaAccount", "safeName", "awsAccessKey", "awsSecretKey") { (npaAccount, safeName, awsAccessKey, awsSecretKey) =>
           headerValueByName("Authorization") { bearerToken =>
-            if (verifyInternalToken(bearerToken)) {
+            verifyInternalToken(bearerToken) {
               val awsCredentials = AwsCredential(AwsAccessKey(awsAccessKey), AwsSecretKey(awsSecretKey))
               onComplete(insertAwsCredentials(Username(npaAccount), awsCredentials, isNpa = true)) {
                 case Success(true) =>
@@ -97,8 +97,6 @@ trait AdminApi extends LazyLogging with Encryption with JwtToken {
                   logger.error(s"NPA: $npaAccount create failed, " + ex.getMessage)
                   complete(ResponseMessage("NPA Create Failed", ex.getMessage, "NPA add"))
               }
-            } else {
-              reject(AuthorizationFailedRejection)
             }
           }
         }
@@ -170,13 +168,11 @@ trait AdminApi extends LazyLogging with Encryption with JwtToken {
       path("service" / "keycloak" / "user") {
         formFields((Symbol("username"))) { username =>
           headerValueByName("Authorization") { bearerToken =>
-            if (verifyInternalToken(bearerToken)) {
+            verifyInternalToken(bearerToken) {
               onComplete(insertUserToKeycloak(Username(username))) {
                 case Success(_)  => complete(ResponseMessage(s"Add user ok", s"$username added", "keycloak"))
                 case Failure(ex) => complete(ResponseMessage(s"Add user error", ex.getMessage, "keycloak"))
               }
-            } else {
-              reject(AuthorizationFailedRejection)
             }
           }
         }

src/main/scala/com/ing/wbaa/rokku/sts/api/UserApi.scala

@@ -6,7 +6,7 @@ import akka.http.scaladsl.server.Route
 import com.ing.wbaa.rokku.sts.data.aws.{ AwsAccessKey, AwsSessionToken }
 import com.ing.wbaa.rokku.sts.data.{ RequestId, STSUserInfo, UserAssumeRole, UserGroup }
 import com.ing.wbaa.rokku.sts.handler.LoggerHandlerWithId
-import com.ing.wbaa.rokku.sts.util.{ JwtToken, JwtTokenException }
+import com.ing.wbaa.rokku.sts.util.JwtToken
 import spray.json.RootJsonFormat
 
 import scala.concurrent.Future
@@ -26,6 +26,15 @@ trait UserApi extends JwtToken {
   implicit val userGroup: RootJsonFormat[UserGroup] = jsonFormat(UserGroup, "value")
   implicit val userInfoJsonFormat: RootJsonFormat[UserInfoToReturn] = jsonFormat5(UserInfoToReturn)
 
+  def containsOnlyAlphanumeric(value: String, errorMessage: String)(inner: Route)(implicit id: RequestId): Route = {
+    if (value.matches("""^[\w\d]*$""")) {
+      inner
+    } else {
+      logger.warn(errorMessage)
+      complete(StatusCodes.Forbidden, errorMessage)
+    }
+  }
+
   def isCredentialActive: Route = logRequestResult("debug") {
     path("isCredentialActive") {
       get {
@@ -36,34 +45,28 @@ trait UserApi extends JwtToken {
               case None    => RequestId("")
             }
 
-            try {
-              val isBearerTokenValid = verifyInternalToken(bearerToken)
-              if (isBearerTokenValid) {
-                parameters("accessKey", "sessionToken".?) { (accessKey, sessionToken) =>
-                  onSuccess(isCredentialActive(AwsAccessKey(accessKey), sessionToken.map(AwsSessionToken))) {
+            verifyInternalToken(bearerToken) {
+              parameters("accessKey", "sessionToken".?) { (accessKey, sessionToken) =>
+                containsOnlyAlphanumeric(accessKey, s"bad accessKey format=$accessKey") {
+                  containsOnlyAlphanumeric(sessionToken getOrElse "", s"bad sessionToken format=${sessionToken.get}") {
 
-                    case Some(userInfo) =>
-                      logger.info("isCredentialActive ok for accessKey={}, sessionToken={}", accessKey, sessionToken)
-                      complete((StatusCodes.OK, UserInfoToReturn(
-                        userInfo.userName.value,
-                        userInfo.userGroup.map(_.value),
-                        userInfo.awsAccessKey.value,
-                        userInfo.awsSecretKey.value,
-                        userInfo.userRole.getOrElse(UserAssumeRole("")).value)))
+                    onSuccess(isCredentialActive(AwsAccessKey(accessKey), sessionToken.map(AwsSessionToken))) {
+                      case Some(userInfo) =>
+                        logger.info("isCredentialActive ok for accessKey={}, sessionToken={}", accessKey, sessionToken)
+                        complete((StatusCodes.OK, UserInfoToReturn(
+                          userInfo.userName.value,
+                          userInfo.userGroup.map(_.value),
+                          userInfo.awsAccessKey.value,
+                          userInfo.awsSecretKey.value,
+                          userInfo.userRole.getOrElse(UserAssumeRole("")).value)))
 
-                    case None =>
-                      logger.warn("isCredentialActive forbidden for accessKey={}, sessionToken={}", accessKey, sessionToken)
-                      complete(StatusCodes.Forbidden)
+                      case None =>
+                        logger.warn("isCredentialActive forbidden for accessKey={}, sessionToken={}", accessKey, sessionToken)
+                        complete(StatusCodes.Forbidden)
+                    }
                   }
                 }
-              } else {
-                logger.warn("isCredentialActive not verified for token={}", bearerToken)
-                complete(StatusCodes.Forbidden)
               }
-            } catch {
-              case ex: JwtTokenException =>
-                logger.warn("isCredentialActive malformed token={}, $s", bearerToken, ex.getMessage)
-                complete(StatusCodes.BadRequest)
             }
           }
         }

src/main/scala/com/ing/wbaa/rokku/sts/handler/StsExceptionHandlers.scala

@@ -6,14 +6,11 @@ import akka.http.scaladsl.server.Directives._
 import akka.http.scaladsl.server.ExceptionHandler
 import com.ing.wbaa.rokku.sts.data.aws.{ AwsErrorCodes, AwsRoleArnException }
 import com.ing.wbaa.rokku.sts.keycloak.KeycloakException
-import com.ing.wbaa.rokku.sts.util.JwtTokenException
 
 object StsExceptionHandlers {
 
   val exceptionHandler: ExceptionHandler =
     ExceptionHandler {
-      case ex: JwtTokenException =>
-        complete(StatusCodes.Forbidden -> AwsErrorCodes.response(StatusCodes.Forbidden, message = Some(ex.getMessage)))
       case ex: KeycloakException =>
         complete(StatusCodes.Forbidden -> AwsErrorCodes.response(StatusCodes.Forbidden, message = Some(ex.getMessage)))
       case ex: AwsRoleArnException =>

src/main/scala/com/ing/wbaa/rokku/sts/util/JwtToken.scala

@@ -7,12 +7,13 @@ import com.ing.wbaa.rokku.sts.data.RequestId
 import com.ing.wbaa.rokku.sts.handler.LoggerHandlerWithId
 
 import scala.util.{ Failure, Success, Try }
+import akka.http.scaladsl.server.{ Route, AuthorizationFailedRejection, MalformedHeaderRejection, Directives }
 
 trait JwtToken {
   protected[this] def stsSettings: StsSettings
   private val logger = new LoggerHandlerWithId
 
-  def verifyInternalToken(bearerToken: String)(implicit id: RequestId): Boolean =
+  def verifyInternalToken(bearerToken: String)(inner: Route)(implicit id: RequestId): Route =
     Try {
       val algorithm = Algorithm.HMAC256(stsSettings.decodeSecret)
       val verifier = JWT.require(algorithm)
@@ -23,17 +24,15 @@ trait JwtToken {
       case Success(t) =>
         val serviceName = t.getClaim("service").asString()
         if (serviceName == "rokku") {
-          logger.debug(s"Successfully verified internal token for $serviceName")
-          true
+          logger.debug("Successfully verified internal token for {}", serviceName)
+          inner
         } else {
-          logger.debug(s"Failed to verify internal token")
-          false
+          logger.warn("Failed to verify internal token={}", bearerToken)
+          Directives.reject(AuthorizationFailedRejection)
         }
       case Failure(exception) =>
         logger.warn("jwt token exception - {}", exception.getMessage)
-        throw new JwtTokenException(exception.getMessage)
+        Directives.reject(MalformedHeaderRejection("bearer token", s"malformed token=$bearerToken"))
     }
 
 }
-
-class JwtTokenException(message: String) extends Exception(message)

src/test/scala/com/ing/wbaa/rokku/sts/api/AdminApiTest.scala

@@ -103,8 +103,8 @@ class AdminApiTest extends AnyWordSpec
       }
       "return Rejected if service token is not correct" in {
         Post("/admin/service/npa", FormData("npaAccount" -> "testNPA1", "safeName" -> "vault", "awsAccessKey" -> "SomeAccessKey", "awsSecretKey" -> "SomeSecretKey"))
-          .addHeader(RawHeader("Authorization", bearerToken("rokku1"))) ~> testRoute ~> check {
-            assert(status == StatusCodes.InternalServerError)
+          .addHeader(RawHeader("Authorization", bearerToken("rokku1"))) ~> Route.seal(testRoute) ~> check {
+            assert(status == StatusCodes.BadRequest)
           }
       }
       "return Rejected if user FormData is invalid" in {

src/test/scala/com/ing/wbaa/rokku/sts/api/UserApiTest.scala

@@ -3,7 +3,7 @@ package com.ing.wbaa.rokku.sts.api
 import akka.actor.ActorSystem
 import akka.http.scaladsl.model.StatusCodes
 import akka.http.scaladsl.model.headers.RawHeader
-import akka.http.scaladsl.server.{ MissingHeaderRejection, MissingQueryParamRejection, Route }
+import akka.http.scaladsl.server.{ MissingHeaderRejection, MalformedHeaderRejection, AuthorizationFailedRejection, MissingQueryParamRejection, Route }
 import akka.http.scaladsl.testkit.ScalatestRouteTest
 import com.auth0.jwt.JWT
 import com.auth0.jwt.algorithms.Algorithm
@@ -80,16 +80,39 @@ class UserApiTest extends AnyWordSpec
       "check credential and return status bad request because the bearerToken is not a valid JWT" in {
         Get(s"/isCredentialActive?accessKey=access&sessionToken=session")
           .addHeader(RawHeader("Authorization", "fakeToken")) ~> testRoute ~> check {
+            assert(rejection == MalformedHeaderRejection("bearer token", "malformed token=fakeToken"))
+          }
+        Get(s"/isCredentialActive?accessKey=access&sessionToken=session")
+          .addHeader(RawHeader("Authorization", "fakeToken")) ~> Route.seal(testRoute) ~> check {
             assert(status == StatusCodes.BadRequest)
           }
       }
 
       "check credential and return status forbidden because the bearerToken is invalid" in {
         Get(s"/isCredentialActive?accessKey=access&sessionToken=session")
           .addHeader(RawHeader("Authorization", generateBearerToken("invalid"))) ~> testRoute ~> check {
+            assert(rejection == AuthorizationFailedRejection)
+          }
+        Get(s"/isCredentialActive?accessKey=access&sessionToken=session")
+          .addHeader(RawHeader("Authorization", generateBearerToken("invalid"))) ~> Route.seal(testRoute) ~> check {
             assert(status == StatusCodes.Forbidden)
           }
       }
+
+      "check credential and return status bad request because the accessKey contains non-alphanumeric characters" in {
+        Get(s"/isCredentialActive?accessKey=access-key!with@special*characters&sessionToken=session")
+          .addHeader(RawHeader("Authorization", generateBearerToken())) ~> testRoute ~> check {
+            assert(status == StatusCodes.Forbidden)
+          }
+      }
+
+      "check credential and return status bad request because the sessionToken contains non-alphanumeric characters" in {
+        Get(s"/isCredentialActive?accessKey=access&sessionToken=session!with@special*characters")
+          .addHeader(RawHeader("Authorization", generateBearerToken())) ~> testRoute ~> check {
+            assert(status == StatusCodes.Forbidden)
+          }
+      }
+
     }
   }
 }