Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to Panda v7 - support key rotation #58

Merged
merged 1 commit into from
Sep 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 20 additions & 29 deletions app/lib/PanAuth.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ package lib


import com.gu.pandomainauth.model.{Authenticated, AuthenticatedUser, AuthenticationStatus, User}
import com.gu.pandomainauth.service.CryptoConf.Verification
import com.gu.pandomainauth.{PanDomain, PublicSettings}
import play.api.Logging
import play.api.mvc._

import java.security.PublicKey
import scala.concurrent.{ExecutionContext, Future}
import com.gu.permissions.PermissionsProvider
import com.gu.permissions.PermissionDefinition
Expand All @@ -28,46 +28,37 @@ trait PandaController extends BaseControllerHelpers with Logging {
Future.successful(Forbidden(views.html.unauthorised()(request)))
}

def authStatus(cookie: Cookie, publicKey: PublicKey): AuthenticationStatus = {
PanDomain.authStatus(
cookie.value,
publicKey,
PanDomain.guardianValidation,
apiGracePeriod = 0,
system = "s3-upload",
cacheValidation = false,
forceExpiry = false
)
}
def authStatus(cookie: Cookie, verification: Verification): AuthenticationStatus = PanDomain.authStatus(
cookie.value,
verification,
Comment on lines -31 to +33
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

guardian/pan-domain-authentication#150 means that code shouldn't directly reference private or public keys anymore (eg do not reference settings.signingKeyPair- there may be more accepted public keys than that field would have told you about!). Instead, we need to use settings.signingAndVerification or publicSettings.verification.

Comment on lines -31 to +33
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

guardian/pan-domain-authentication#150 means that code shouldn't directly reference private or public keys anymore (eg do not reference settings.signingKeyPair- there may be more accepted public keys than that field would have told you about!). Instead, we need to use settings.signingAndVerification or publicSettings.verification.

PanDomain.guardianValidation,
apiGracePeriod = 0,
system = "s3-upload",
cacheValidation = false,
forceExpiry = false
)

object AuthAction extends ActionBuilder[UserRequest, AnyContent] {
override def parser: BodyParser[AnyContent] = PandaController.this.controllerComponents.parsers.default
override protected def executionContext: ExecutionContext = PandaController.this.controllerComponents.executionContext

override def invokeBlock[A](request: Request[A], block: UserRequest[A] => Future[Result]): Future[Result] = {
publicSettings.publicKey match {
case Some(pk) =>
request.cookies.get("gutoolsAuth-assym") match {
case Some(cookie) =>
authStatus(cookie, pk) match {
case Authenticated(AuthenticatedUser(user, _, _, _, _)) =>
if (!permissions.hasPermission(S3UploaderAccess, user.email)) {
logger.warn(s"User ${user.email} does not have ${S3UploaderAccess.name} permission")
}
block(new UserRequest(user, request))

case other =>
logger.info(s"Login response $other")
unauthenticatedResponse(request)
request.cookies.get("gutoolsAuth-assym") match {
case Some(cookie) =>
authStatus(cookie, publicSettings.verification) match {
case Authenticated(AuthenticatedUser(user, _, _, _, _)) =>
if (!permissions.hasPermission(S3UploaderAccess, user.email)) {
logger.warn(s"User ${user.email} does not have ${S3UploaderAccess.name} permission")
}
block(new UserRequest(user, request))

case None =>
logger.warn("Panda cookie missing")
case other =>
logger.info(s"Login response $other")
unauthenticatedResponse(request)
}

case None =>
logger.error("Panda public key unavailable")
logger.warn("Panda cookie missing")
unauthenticatedResponse(request)
}
Comment on lines 59 to 72
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that publicSettings.publicKey was previously optional (although that ended with guardian/pan-domain-authentication#151 in Panda v6), and publicSettings.verification (introduced with guardian/pan-domain-authentication#150 in Panda v7) is not optional.

If the Panda settings refresher can not load valid settings on startup, it will simply throw an exception, and the service should terminate.

}
Expand Down
4 changes: 3 additions & 1 deletion build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@ scalacOptions := Seq(
libraryDependencies ++= Seq(
ws, filters,
"com.amazonaws" % "aws-java-sdk-s3" % "1.12.761",
"com.gu" %% "pan-domain-auth-verification" % "5.0.0",
"com.gu" %% "pan-domain-auth-verification" % "7.0.0",
"com.gu" %% "editorial-permissions-client" % "3.0.0"
)

resolvers ++= Resolver.sonatypeOssRepos("releases")

lazy val root = (project in file("."))
.enablePlugins(PlayScala, JDebPackaging, SystemdPlugin)
.settings(
Expand Down
Loading