Merge pull request #69 from toolsplus/bugfix/fix-asymmetric-installat…

…ion-lifecycle

Fix asymmetrically signed lifecycle methods

modules/core/app/io/toolsplus/atlassian/connect/play/actions/JwtAction.scala

@@ -0,0 +1,31 @@
+package io.toolsplus.atlassian.connect.play.actions
+
+import io.toolsplus.atlassian.connect.play.auth.jwt.JwtCredentials
+import play.api.mvc.Results.Unauthorized
+import play.api.mvc.{ActionRefiner, Request, Result, WrappedRequest}
+
+import javax.inject.Inject
+import scala.concurrent.{ExecutionContext, Future}
+
+case class JwtRequest[A](credentials: JwtCredentials, request: Request[A])
+  extends WrappedRequest[A](request)
+
+/**
+  * Play action refiner that extracts the JWT credentials from a request
+  *
+  * Note that this refiner will intercept the request and return an unauthorized
+  * result if no JWT credentials were found.
+  */
+class JwtActionRefiner @Inject()(
+                                  implicit val executionContext: ExecutionContext)
+  extends ActionRefiner[Request, JwtRequest] {
+
+  override def refine[A](
+                          request: Request[A]): Future[Either[Result, JwtRequest[A]]] =
+    JwtExtractor.extractJwt(request) match {
+      case Some(credentials) =>
+        Future.successful(Right(JwtRequest(credentials, request)))
+      case None =>
+        Future.successful(Left(Unauthorized("No authentication token found")))
+    }
+}

modules/core/app/io/toolsplus/atlassian/connect/play/actions/asymmetric/AsymmetricallySignedAtlassianHostUserAction.scala

@@ -0,0 +1,69 @@
+package io.toolsplus.atlassian.connect.play.actions.asymmetric
+
+import cats.implicits._
+import io.toolsplus.atlassian.connect.play.actions.{
+  JwtActionRefiner,
+  JwtRequest
+}
+import io.toolsplus.atlassian.connect.play.api.models.AtlassianHostUser
+import io.toolsplus.atlassian.connect.play.auth.jwt._
+import io.toolsplus.atlassian.connect.play.auth.jwt.asymmetric.AsymmetricJwtAuthenticationProvider
+import play.api.mvc.Results.Unauthorized
+import play.api.mvc._
+
+import javax.inject.Inject
+import scala.concurrent.{ExecutionContext, Future}
+
+case class MaybeAtlassianHostUserRequest[A](hostUser: Option[AtlassianHostUser],
+                                            request: JwtRequest[A])
+    extends WrappedRequest[A](request)
+
+case class AsymmetricallySignedAtlassianHostUserActionRefiner(
+    jwtAuthenticationProvider: AsymmetricJwtAuthenticationProvider,
+    qshProvider: QshProvider)(implicit val executionContext: ExecutionContext)
+    extends ActionRefiner[JwtRequest, MaybeAtlassianHostUserRequest] {
+  override def refine[A](request: JwtRequest[A])
+    : Future[Either[Result, MaybeAtlassianHostUserRequest[A]]] = {
+    val expectedQsh = qshProvider match {
+      case ContextQshProvider => ContextQshProvider.qsh
+      case CanonicalHttpRequestQshProvider =>
+        CanonicalHttpRequestQshProvider.qsh(
+          request.credentials.canonicalHttpRequest)
+    }
+    jwtAuthenticationProvider
+      .authenticate(request.credentials, expectedQsh)
+      .map(MaybeAtlassianHostUserRequest(_, request))
+      .leftMap(e => Unauthorized(s"JWT validation failed: ${e.getMessage}"))
+      .value
+  }
+}
+
+class AsymmetricallySignedAtlassianHostUserAction @Inject()(
+    bodyParser: BodyParsers.Default,
+    jwtActionRefiner: JwtActionRefiner,
+    asymmetricJwtAuthenticationProvider: AsymmetricJwtAuthenticationProvider)(
+    implicit executionCtx: ExecutionContext) {
+
+  /**
+    * Creates an action builder that validates asymmetrically signed JWT requests. Callers must specify
+    * how the query string hash claim should be verified.
+    *
+    * @param qshProvider Query string hash provider that specifies what kind of QSH the qsh claim contains
+    * @return Play action for asymmetrically signed JWT requests
+    */
+  def authenticateWith(qshProvider: QshProvider)
+    : ActionBuilder[MaybeAtlassianHostUserRequest, AnyContent] =
+    new ActionBuilder[MaybeAtlassianHostUserRequest, AnyContent] {
+      override val parser: BodyParsers.Default = bodyParser
+      override val executionContext: ExecutionContext = executionCtx
+      override def invokeBlock[A](
+          request: Request[A],
+          block: MaybeAtlassianHostUserRequest[A] => Future[Result])
+        : Future[Result] = {
+        (jwtActionRefiner andThen AsymmetricallySignedAtlassianHostUserActionRefiner(
+          asymmetricJwtAuthenticationProvider,
+          qshProvider))
+          .invokeBlock(request, block)
+      }
+    }
+}

modules/core/app/io/toolsplus/atlassian/connect/play/actions/AtlassianHostUserAction.scala → modules/core/app/io/toolsplus/atlassian/connect/play/actions/symmetric/SymmetricallySignedAtlassianHostUserAction.scala

@@ -1,43 +1,22 @@
-package io.toolsplus.atlassian.connect.play.actions
+package io.toolsplus.atlassian.connect.play.actions.symmetric
 
 import cats.implicits._
+import io.toolsplus.atlassian.connect.play.actions.{JwtActionRefiner, JwtRequest}
 import io.toolsplus.atlassian.connect.play.api.models.AtlassianHostUser
 import io.toolsplus.atlassian.connect.play.auth.jwt._
+import io.toolsplus.atlassian.connect.play.auth.jwt.symmetric.SymmetricJwtAuthenticationProvider
 import play.api.mvc.Results.Unauthorized
 import play.api.mvc._
 
 import javax.inject.Inject
 import scala.concurrent.{ExecutionContext, Future}
 
-case class JwtRequest[A](credentials: JwtCredentials, request: Request[A])
-    extends WrappedRequest[A](request)
-
-/**
-  * Play action refiner that extracts the JWT credentials from a request
-  *
-  * Note that this refiner will intercept the request and return an unauthorized
-  * result if no JWT credentials were found.
-  */
-class JwtActionRefiner @Inject()(
-    implicit val executionContext: ExecutionContext)
-    extends ActionRefiner[Request, JwtRequest] {
-
-  override def refine[A](
-      request: Request[A]): Future[Either[Result, JwtRequest[A]]] =
-    JwtExtractor.extractJwt(request) match {
-      case Some(credentials) =>
-        Future.successful(Right(JwtRequest(credentials, request)))
-      case None =>
-        Future.successful(Left(Unauthorized("No authentication token found")))
-    }
-}
-
 case class AtlassianHostUserRequest[A](hostUser: AtlassianHostUser,
                                        request: JwtRequest[A])
     extends WrappedRequest[A](request)
 
-case class AtlassianHostUserActionRefiner(
-    jwtAuthenticationProvider: AbstractJwtAuthenticationProvider,
+case class SymmetricallySignedAtlassianHostUserActionRefiner(
+    jwtAuthenticationProvider: SymmetricJwtAuthenticationProvider,
     qshProvider: QshProvider)(implicit val executionContext: ExecutionContext)
     extends ActionRefiner[JwtRequest, AtlassianHostUserRequest] {
   override def refine[A](request: JwtRequest[A])
@@ -56,21 +35,20 @@ case class AtlassianHostUserActionRefiner(
   }
 }
 
-class AtlassianHostUserAction @Inject()(
+class SymmetricallySignedAtlassianHostUserAction @Inject()(
     bodyParser: BodyParsers.Default,
-    jwtActionRefiner: JwtActionRefiner)(
+    jwtActionRefiner: JwtActionRefiner,
+    symmetricJwtAuthenticationProvider: SymmetricJwtAuthenticationProvider)(
     implicit executionCtx: ExecutionContext) {
 
   /**
-    * Creates an action builder that validates JWT authenticated requests. Callers must specify if they
-    * expect symmetrically or asymmetrically signed JWTs and how the query string hash claim should be verified.
+    * Creates an action builder that validates symmetrically signed JWT requests. Callers must specify
+    * how the query string hash claim should be verified.
     *
-    * @param jwtAuthenticationProvider JWT authentication provider that specifies if a symmetrically or asymmetrically
-    *                                  signed JWT is expected
     * @param qshProvider Query string hash provider that specifies what kind of QSH the qsh claim contains
-    * @return Play action for JWT validated requests
+    * @return Play action for symmetrically signed JWT requests
     */
-  def authenticateWith(jwtAuthenticationProvider: AbstractJwtAuthenticationProvider, qshProvider: QshProvider)
+  def authenticateWith(qshProvider: QshProvider)
     : ActionBuilder[AtlassianHostUserRequest, AnyContent] =
     new ActionBuilder[AtlassianHostUserRequest, AnyContent] {
       override val parser: BodyParsers.Default = bodyParser
@@ -79,8 +57,9 @@ class AtlassianHostUserAction @Inject()(
           request: Request[A],
           block: AtlassianHostUserRequest[A] => Future[Result])
         : Future[Result] = {
-        (jwtActionRefiner andThen AtlassianHostUserActionRefiner(jwtAuthenticationProvider, qshProvider))
-          .invokeBlock(request, block)
+        (jwtActionRefiner andThen SymmetricallySignedAtlassianHostUserActionRefiner(
+          symmetricJwtAuthenticationProvider,
+          qshProvider)).invokeBlock(request, block)
       }
     }
 
@@ -92,13 +71,12 @@ class AtlassianHostUserAction @Inject()(
       * Implicitly convert an instance of [[AtlassianHostUserRequest]] to an
       * instance of AtlassianHostUser.
       *
-      * @param r Atlassian host user request instance.
+      * @param request Atlassian host user request instance.
       * @return Atlassian host user instance extracted from request.
       */
     implicit def hostUserRequestToHostUser(
-        implicit r: AtlassianHostUserRequest[_]): AtlassianHostUser =
-      r.hostUser
+        implicit request: AtlassianHostUserRequest[_]): AtlassianHostUser =
+      request.hostUser
 
   }
-
 }

modules/core/app/io/toolsplus/atlassian/connect/play/auth/jwt/AbstractJwtAuthenticationProvider.scala

@@ -10,12 +10,14 @@ import play.api.Logger
 
 import scala.concurrent.{ExecutionContext, Future}
 
-abstract class AbstractJwtAuthenticationProvider(hostRepository: AtlassianHostRepository)(implicit executionContext: ExecutionContext) {
+abstract class AbstractJwtAuthenticationProvider[F[_]](
+    hostRepository: AtlassianHostRepository)(
+    implicit executionContext: ExecutionContext) {
 
-  private val logger = Logger(classOf[AbstractJwtAuthenticationProvider])
+  private val logger = Logger(classOf[AbstractJwtAuthenticationProvider[F]])
 
   def authenticate(jwtCredentials: JwtCredentials, qsh: String)
-  : EitherT[Future, JwtAuthenticationError, AtlassianHostUser]
+    : EitherT[Future, JwtAuthenticationError, F[AtlassianHostUser]]
 
   protected def parseJwt(rawJwt: String): Either[JwtAuthenticationError, Jwt] =
     JwtParser.parse(rawJwt).leftMap { e =>
@@ -30,12 +32,13 @@ abstract class AbstractJwtAuthenticationProvider(hostRepository: AtlassianHostRe
     * @return Client key or authentication error if no issue claim could be found
     */
   protected def extractClientKey(
-                                jwt: Jwt): Either[JwtAuthenticationError, String] = {
+      jwt: Jwt): Either[JwtAuthenticationError, String] = {
     Option(jwt.claims.getIssuer) match {
       case Some(clientKeyClaim) => Right(clientKeyClaim)
       case None =>
         Left(
-          JwtBadCredentialsError("Failed to extract client key due to missing issuer claim")
+          JwtBadCredentialsError(
+            "Failed to extract client key due to missing issuer claim")
         )
     }
   }
@@ -48,21 +51,18 @@ abstract class AbstractJwtAuthenticationProvider(hostRepository: AtlassianHostRe
     * @return Atlassian host user created from subject claim.
     */
   protected def hostUserFromSubjectClaim(
-                                        host: AtlassianHost,
-                                        verifiedClaims: JWTClaimsSet
-                                      ): AtlassianHostUser = {
+      host: AtlassianHost,
+      verifiedClaims: JWTClaimsSet
+  ): AtlassianHostUser = {
     DefaultAtlassianHostUser(host, Option(verifiedClaims.getSubject))
   }
 
   protected def fetchAtlassianHost(clientKey: String)
-  : EitherT[Future, JwtAuthenticationError, AtlassianHost] = {
+    : EitherT[Future, JwtAuthenticationError, AtlassianHost] = {
     EitherT(
       hostRepository.findByClientKey(clientKey).map {
         case Some(host) => Right(host)
-        case None =>
-          logger.error(
-            s"Could not find an installed host for the provided client key: $clientKey")
-          Left(UnknownJwtIssuerError(clientKey))
+        case None       => Left(UnknownJwtIssuerError(clientKey))
       }
     )
   }

modules/core/app/io/toolsplus/atlassian/connect/play/auth/jwt/asymmetric/AsymmetricJwtAuthenticationProvider.scala

@@ -21,11 +21,18 @@ import scala.concurrent.ExecutionContext.Implicits.global
 import scala.concurrent.Future
 import scala.util.Try
 
+/**
+  * Authentication provider that verifies asymmetrically signed JWTs.
+  *
+  * Note that for this authentication to succeed the Atlassian host indicated by the clientKey in the JWT does not
+  * have to be installed. As a result this authentication provider may or may not return the installed Atlassian host
+  * record.
+  */
 class AsymmetricJwtAuthenticationProvider @Inject()(
     appProperties: AppProperties,
     publicKeyProvider: PublicKeyProvider,
     hostRepository: AtlassianHostRepository)
-    extends AbstractJwtAuthenticationProvider(hostRepository) {
+    extends AbstractJwtAuthenticationProvider[Option](hostRepository) {
 
   private val logger = Logger(classOf[AsymmetricJwtAuthenticationProvider])
 
@@ -34,11 +41,11 @@ class AsymmetricJwtAuthenticationProvider @Inject()(
     *
     * @param jwtCredentials Untrusted JWT credentials
     * @param qsh Query string hash computed from the request the JWT credentials were attached to
-    * @return Atlassian host user associated with the given JWT credentials, or an authentication error.
+    * @return Atlassian host user associated with the given JWT credentials if it is installed, or an authentication error.
     * @see https://developer.atlassian.com/cloud/jira/platform/understanding-jwt-for-connect-apps/#verifying-a-asymmetric-jwt-token-for-install-callbacks
     */
   override def authenticate(jwtCredentials: JwtCredentials, qsh: String)
-    : EitherT[Future, JwtAuthenticationError, AtlassianHostUser] = {
+    : EitherT[Future, JwtAuthenticationError, Option[AtlassianHostUser]] = {
 
     for {
       jwt <- parseJwt(jwtCredentials.rawJwt).toEitherT[Future]
@@ -48,8 +55,8 @@ class AsymmetricJwtAuthenticationProvider @Inject()(
       verifiedToken <- verifyJwt(jwtCredentials, publicKey, qsh)
         .toEitherT[Future]
       clientKey <- extractClientKey(jwt).toEitherT[Future]
-      host <- fetchAtlassianHost(clientKey)
-    } yield hostUserFromSubjectClaim(host, verifiedToken.claims)
+      result <- maybeHostUser(clientKey, verifiedToken)
+    } yield result
   }
 
   private def extractPublicKeyId(
@@ -100,4 +107,19 @@ class AsymmetricJwtAuthenticationProvider @Inject()(
       }
   }
 
+  /**
+    * Tries to find an Atlassian host for the given client key and constructs a
+    * host user record if a host has been found.
+    *
+    * @param clientKey Client key of the Atlassian host to find
+    * @param verifiedToken Verified JWT claims to extract the subject claim from (subject claim == user account id)
+    * @return Atlassian host user if one has been found
+    */
+  private def maybeHostUser(clientKey: String, verifiedToken: Jwt)
+    : EitherT[Future, JwtAuthenticationError, Option[AtlassianHostUser]] =
+    fetchAtlassianHost(clientKey).transform {
+      case Right(host) => Right(Some(hostUserFromSubjectClaim(host, verifiedToken.claims)))
+      case Left(_) => Right(None)
+    }
+
 }