From 6e11a68df89c24c3cc3df4d3b5135c57d067ea45 Mon Sep 17 00:00:00 2001
From: Roberto Tyley <52038+rtyley@users.noreply.github.com>
Date: Thu, 12 Sep 2024 15:41:22 +0100
Subject: [PATCH] Fix creation of superfluous `PanDomainAuthSettingsRefresher`
 instances

Having `def panDomainSettings: PanDomainAuthSettingsRefresher` in the `AuthActions`
trait as a `def` means that implementors of `AuthActions` are able to make the
mistake of also defining their implementation of that field as a `def`, meaning that
every reference to it can potentially create _another_ instance of the refresher,
as seen here:

https://github.com/guardian/atom-workshop/pull/361#issuecomment-2346317855

Surprisingly, this seems to have been a problem since at least as far back as
https://github.com/guardian/pan-domain-authentication/pull/41 in February 2018.

Changing the field type to `val` forces implementers to also use `val` for that
field, effectively making it a singleton, as we want.

Changing the abstract field of a trait to be `val` does open up another danger due
the initialization order of vals - the field could end up being evaluated as
`null` if the trait immediately evaluates the field:

See: https://docs.scala-lang.org/tutorials/FAQ/initialization-order.html

...consequently, I've made all `val` declarations in the `AuthActions` trait (that
evaluate `panDomainSettings` in some way) into `lazy val`s. This hopefully should
fix the problem.
---
 .../scala/com/gu/pandomainauth/action/Actions.scala  | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/pan-domain-auth-play/src/main/scala/com/gu/pandomainauth/action/Actions.scala b/pan-domain-auth-play/src/main/scala/com/gu/pandomainauth/action/Actions.scala
index 322cd27..0aa96f5 100644
--- a/pan-domain-auth-play/src/main/scala/com/gu/pandomainauth/action/Actions.scala
+++ b/pan-domain-auth-play/src/main/scala/com/gu/pandomainauth/action/Actions.scala
@@ -32,10 +32,10 @@ trait AuthActions {
     */
   def wsClient: WSClient
   def controllerComponents: ControllerComponents
-  def panDomainSettings: PanDomainAuthSettingsRefresher
+  val panDomainSettings: PanDomainAuthSettingsRefresher
 
-  private def system: String = panDomainSettings.system
-  private def domain: String = panDomainSettings.domain
+  private lazy val system: String = panDomainSettings.system
+  private lazy val domain: String = panDomainSettings.domain
   private def settings: PanDomainAuthSettings = panDomainSettings.settings
 
   private implicit val ec: ExecutionContext = controllerComponents.executionContext
@@ -81,14 +81,14 @@ trait AuthActions {
     */
   def authCallbackUrl: String
 
-  val OAuth = new OAuth(settings.oAuthSettings, system, authCallbackUrl)(ec, wsClient)
+  lazy val OAuth = new OAuth(settings.oAuthSettings, system, authCallbackUrl)(ec, wsClient)
 
   /**
     * Application name used for initialising Google API clients for directory group checking
     */
-  val applicationName: String = s"pan-domain-authentication-$system"
+  lazy val applicationName: String = s"pan-domain-authentication-$system"
 
-  val multifactorChecker: Option[Google2FAGroupChecker] = settings.google2FAGroupSettings.map {
+  lazy val multifactorChecker: Option[Google2FAGroupChecker] = settings.google2FAGroupSettings.map {
     new Google2FAGroupChecker(_, panDomainSettings.s3BucketLoader, applicationName)
   }