-
Notifications
You must be signed in to change notification settings - Fork 1
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 #81
Conversation
d7da4a5
to
76a120e
Compare
4498ee2
to
cb88c40
Compare
23f4377
to
b1f7295
Compare
02039cd
to
153b647
Compare
715fddd
to
bc8b359
Compare
bc8b359
to
8b3a015
Compare
@@ -64,7 +64,7 @@ class DesktopLogin( | |||
|
|||
|
|||
if (validateUser(authedUserData)) { | |||
val token = CookieUtils.generateCookieData(authedUserData, panDomainSettings.settings.signingKeyPair.getPrivate) | |||
val token = CookieUtils.generateCookieData(authedUserData, panDomainSettings.settings.signingAndVerification) |
There was a problem hiding this comment.
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
.
@@ -30,27 +30,16 @@ class Emergency( | |||
val reissueTopic = "Your login session has not been extended" | |||
|
|||
(for { | |||
publicKey <- loginPublicSettings.publicKey |
There was a problem hiding this comment.
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.
} yield CookieUtils.parseCookieData(assymCookie.value, loginPublicSettings.verification).fold( | ||
tokenIntegrityFailure => | ||
unauthorised(s"Invalid existing session, could not log you in: $tokenIntegrityFailure", reissueTopic) | ||
, authenticatedUser => | ||
if (validateUser(authenticatedUser)) { | ||
val expires = (DateTime.now() + cookieLifetime).getMillis | ||
val newAuthUser = authenticatedUser.copy(expires = expires) | ||
val authCookie = generateCookie(newAuthUser) | ||
val authCookie = generateCookie(authenticatedUser.copy(expires = (DateTime.now() + cookieLifetime).getMillis)) | ||
Ok(views.html.emergency.reissueSuccess()).withCookies(authCookie) | ||
} else { | ||
unauthorised("Only Guardian email addresses with two-factor auth are supported.", reissueTopic) | ||
} | ||
} | ||
catch { | ||
case _: CookieSignatureInvalidException => | ||
unauthorised("Invalid existing session, could not log you in.", reissueTopic) | ||
case _: CookieParseException => | ||
unauthorised("Could not refresh existing session due to a corrupted cookie.", reissueTopic) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CookieUtils.generateCookieData()
now communicates errors with CookieResult
values containing a CookieIntegrityFailure
, rather than exceptions, thanks to guardian/pan-domain-authentication#152 in Panda v6.
This upgrades Panda from v5 to v7, allowing us to use key rotation as introduced with guardian/pan-domain-authentication#150. As login.gutools.co.uk is pretty special user of Panda the upgrade is slightly more involved than other upgrades (eg guardian/atom-workshop#361): * Panda v6: * guardian/pan-domain-authentication#152 `CookieUtils.generateCookieData()` now communicates errors with `CookieResult` values containing `CookieIntegrityFailure`, rather than exceptions. * Panda v7: * guardian/pan-domain-authentication#150 means that code shouldn't directly reference private or public keys anymore (eg do not reference `settings.signingKeyPair`). Instead, use `settings.signingAndVerification` or `publicSettings.verification`. Note also that `publicSettings.publicKey` was previously optional, and `publicSettings.verification` is not.
8b3a015
to
881deca
Compare
private lazy val panDomainSettings: PanDomainAuthSettingsRefresher = | ||
new PanDomainAuthSettingsRefresher( | ||
domain = config.domain, | ||
system = "login", | ||
bucketName = config.pandaAuthBucket, | ||
settingsFileKey = s"${config.domain}.settings", | ||
s3Client = aws.s3Client | ||
) | ||
|
||
private lazy val desktopPanDomainSettings: PanDomainAuthSettingsRefresher = { | ||
new PanDomainAuthSettingsRefresher( | ||
domain = config.desktopDomain, | ||
system = "login-desktop", | ||
bucketName = config.pandaAuthBucket, | ||
settingsFileKey = s"${config.desktopDomain}.settings", | ||
s3Client = aws.s3Client | ||
) | ||
} | ||
|
||
val loginPublicSettings = new PublicSettings( | ||
settingsFileKey = s"${config.domain}.settings.public", | ||
bucketName = config.pandaAuthBucket, | ||
s3Client = aws.s3Client | ||
private val s3BucketLoader: S3BucketLoader = forAwsSdkV1(aws.s3Client, "pan-domain-auth-settings") | ||
|
||
private lazy val panDomainSettings: PanDomainAuthSettingsRefresher = PanDomainAuthSettingsRefresher( | ||
domain = config.domain, | ||
system = "login", | ||
s3BucketLoader | ||
) | ||
|
||
private lazy val desktopPanDomainSettings: PanDomainAuthSettingsRefresher = PanDomainAuthSettingsRefresher( | ||
domain = config.desktopDomain, | ||
system = "login-desktop", | ||
s3BucketLoader | ||
) | ||
|
||
val loginPublicSettings: PublicSettings = PublicSettings( | ||
new Settings.Loader(s3BucketLoader, s"${config.domain}.settings.public") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR:
...introduced the new S3BucketLoader
abstraction, which simplifies constructing a PanDomainAuthSettingsRefresher
and means that Panda is no longer tied to AWS SDK v1 - an alternative AWS SDK v2 implementation of S3BucketLoader
could be introduced.
This upgrades Panda from v5 to v7, allowing us to perform key rotation.
As login.gutools.co.uk is pretty special user of Panda, the upgrade is slightly more involved than other upgrades (eg guardian/atom-workshop#361):
CookieUtils.generateCookieData()
method now communicates errors withCookieResult
values containingCookieIntegrityFailure
, rather than exceptions.S3BucketLoader
abstraction, which simplifies constructing aPanDomainAuthSettingsRefresher
and means that Panda is no longer tied to AWS SDK v1 - an alternative AWS SDK v2 implementation ofS3BucketLoader
could be introduced.settings.signingKeyPair
). Instead, usesettings.signingAndVerification
orpublicSettings.verification
. Note also thatpublicSettings.publicKey
was previously optional, andpublicSettings.verification
is not.This PR sits on top of: