From bd7888859cccd23e32a10099113d3c55d31ca57d Mon Sep 17 00:00:00 2001 From: Will Sargent Date: Tue, 14 Jan 2020 19:08:29 -0800 Subject: [PATCH] Simplify the logic --- .../ActivateAccountController.scala | 4 +- app/controllers/ApplicationController.scala | 4 +- .../ChangePasswordController.scala | 8 +- .../ForgotPasswordController.scala | 4 +- app/controllers/MyRequest.scala | 75 +++++++------------ app/controllers/ResetPasswordController.scala | 4 +- app/controllers/SignInController.scala | 4 +- app/controllers/SignUpController.scala | 4 +- app/controllers/SilhouetteController.scala | 53 +++++-------- app/controllers/SocialAuthController.scala | 2 +- app/controllers/TotpController.scala | 10 +-- app/controllers/TotpRecoveryController.scala | 4 +- 12 files changed, 72 insertions(+), 104 deletions(-) diff --git a/app/controllers/ActivateAccountController.scala b/app/controllers/ActivateAccountController.scala index 6dfb103..ebd14cb 100644 --- a/app/controllers/ActivateAccountController.scala +++ b/app/controllers/ActivateAccountController.scala @@ -26,7 +26,7 @@ class ActivateAccountController @Inject() ( * @param email The email address of the user to send the activation mail to. * @return The result to display. */ - def send(email: String) = MyUnsecuredAction.async { implicit request: MyRequest[AnyContent] => + def send(email: String) = UnsecuredAction.async { implicit request: MyRequest[AnyContent] => val decodedEmail = URLDecoder.decode(email, "UTF-8") val loginInfo = LoginInfo(CredentialsProvider.ID, decodedEmail) val result = Redirect(Calls.signin).flashing("info" -> Messages("activation.email.sent", decodedEmail)) @@ -55,7 +55,7 @@ class ActivateAccountController @Inject() ( * @param token The token to identify a user. * @return The result to display. */ - def activate(token: UUID) = MyUnsecuredAction.async { implicit request: MyRequest[AnyContent] => + def activate(token: UUID) = UnsecuredAction.async { implicit request: MyRequest[AnyContent] => authTokenService.validate(token).flatMap { case Some(authToken) => userService.retrieve(authToken.userID).flatMap { case Some(user) if user.loginInfo.providerID == CredentialsProvider.ID => diff --git a/app/controllers/ApplicationController.scala b/app/controllers/ApplicationController.scala index ac39b21..985ff92 100644 --- a/app/controllers/ApplicationController.scala +++ b/app/controllers/ApplicationController.scala @@ -22,7 +22,7 @@ class ApplicationController @Inject() ( * * @return The result to display. */ - def index = MySecuredAction.async { implicit request: MySecuredRequest[EnvType, AnyContent] => + def index = SecuredAction.async { implicit request: MySecuredRequest[AnyContent] => authInfoRepository.find[GoogleTotpInfo](request.identity.loginInfo).map { totpInfoOpt => Ok(home(request.identity, totpInfoOpt)) } @@ -33,7 +33,7 @@ class ApplicationController @Inject() ( * * @return The result to display. */ - def signOut = SecuredAction.async { implicit request: SecuredRequest[EnvType, AnyContent] => + def signOut = SecuredAction.async { implicit request: MySecuredRequest[AnyContent] => val result = Redirect(Calls.home) eventBus.publish(LogoutEvent(request.identity, request)) authenticatorService.discard(request.authenticator, result) diff --git a/app/controllers/ChangePasswordController.scala b/app/controllers/ChangePasswordController.scala index dfd0890..797436d 100644 --- a/app/controllers/ChangePasswordController.scala +++ b/app/controllers/ChangePasswordController.scala @@ -25,8 +25,8 @@ class ChangePasswordController @Inject() ( * * @return The result to display. */ - def view = MySecuredAction(WithProvider[AuthType](CredentialsProvider.ID)) { - implicit request: MySecuredRequest[DefaultEnv, AnyContent] => + def view = SecuredAction(WithProvider[AuthType](CredentialsProvider.ID)) { + implicit request => Ok(changePassword(ChangePasswordForm.form, request.identity)) } @@ -35,8 +35,8 @@ class ChangePasswordController @Inject() ( * * @return The result to display. */ - def submit = MySecuredAction(WithProvider[AuthType](CredentialsProvider.ID)).async { - implicit request: MySecuredEnvRequest[AnyContent] => + def submit = SecuredAction(WithProvider[AuthType](CredentialsProvider.ID)).async { + implicit request => ChangePasswordForm.form.bindFromRequest.fold( form => Future.successful(BadRequest(changePassword(form, request.identity))), password => { diff --git a/app/controllers/ForgotPasswordController.scala b/app/controllers/ForgotPasswordController.scala index 0f6f6c0..e648a98 100644 --- a/app/controllers/ForgotPasswordController.scala +++ b/app/controllers/ForgotPasswordController.scala @@ -24,7 +24,7 @@ class ForgotPasswordController @Inject() ( * * @return The result to display. */ - def view = MyUnsecuredAction.async { implicit request: MyRequest[AnyContent] => + def view = UnsecuredAction.async { implicit request: MyRequest[AnyContent] => Future.successful(Ok(forgotPassword(ForgotPasswordForm.form))) } @@ -36,7 +36,7 @@ class ForgotPasswordController @Inject() ( * * @return The result to display. */ - def submit = MyUnsecuredAction.async { implicit request: MyRequest[AnyContent] => + def submit = SecuredAction.async { implicit request: MyRequest[AnyContent] => ForgotPasswordForm.form.bindFromRequest.fold( form => Future.successful(BadRequest(forgotPassword(form))), email => { diff --git a/app/controllers/MyRequest.scala b/app/controllers/MyRequest.scala index 189fecd..e04c61f 100644 --- a/app/controllers/MyRequest.scala +++ b/app/controllers/MyRequest.scala @@ -1,64 +1,45 @@ package controllers -import com.mohiva.play.silhouette.api.Env +import org.slf4j.Marker +import play.api.MarkerContext import play.api.i18n.MessagesApi import play.api.mvc._ +import utils.auth.DefaultEnv -trait MyStuff - -case object ReallyMyStuff extends MyStuff - -trait MyRequestHeader extends MessagesRequestHeader with PreferredMessagesProvider { - def myStuff: MyStuff -} +// Allow this request to be used as a marker context +trait MyRequestHeader extends MessagesRequestHeader with PreferredMessagesProvider with MarkerContext class MyRequest[B]( - val myStuff: MyStuff, request: Request[B], - messagesApi: MessagesApi, -) extends MessagesRequest[B](request, messagesApi) with MyRequestHeader + messagesApi: MessagesApi +) extends MessagesRequest[B](request, messagesApi) with MyRequestHeader { + // Stubbed out here, but see marker context docs + // https://www.playframework.com/documentation/2.8.x/ScalaLogging#Using-Markers-and-Marker-Contexts + def marker: Option[Marker] = None +} -// Should be OOTB here -trait MySecuredRequestHeader[E <: Env] extends RequestHeader { - def identity: E#I - def authenticator: E#A +// XXX this should extend SecuredRequestHeader[DefaultEnv] here but not OOTB +trait MySecuredRequestHeader extends RequestHeader { + def identity: DefaultEnv#I + def authenticator: DefaultEnv#A } -class MySecuredRequest [E <: Env, B]( +class MySecuredRequest[B]( request: Request[B], - myStuff: MyStuff, messagesApi: MessagesApi, - val identity: E#I, - val authenticator: E#A -) extends MyRequest(myStuff, request, messagesApi) with MySecuredRequestHeader[E] - -// Should be able to use an OOTB user aware request header -trait MyUserAwareRequestHeader[E <: Env] extends RequestHeader { - def identity: Option[E#I] - def authenticator: Option[E#A] + val identity: DefaultEnv#I, + val authenticator: DefaultEnv#A, +) extends MyRequest(request, messagesApi) with MySecuredRequestHeader + +// XXX this should extend UserAwareRequestHeader[DefaultEnv] here but not OOTB +trait MyUserAwareRequestHeader extends RequestHeader { + def identity: Option[DefaultEnv#I] + def authenticator: Option[DefaultEnv#A] } -class MyUserAwareRequest[E <: Env, B]( +class MyUserAwareRequest[B]( request: Request[B], - myStuff: MyStuff, messagesApi: MessagesApi, - val identity: Option[E#I], - val authenticator: Option[E#A] -) extends MyRequest(myStuff, request, messagesApi) with MyUserAwareRequestHeader[E] - - -object Foo { - import com.mohiva.play.silhouette.api.actions.{SecuredRequest => OrigSecuredRequest} - - // Can't treat MyRequest as instance of SecuredRequest even though they both - // return identity & authentiator, must lose info and instantiate new request - // Note also that we're returning instance and there is no interface - // so can't do this: - // - // def createInstance[E <: Env, A](request: MySecuredRequest[E, A]): SecuredRequest[E, A] = - // request.asInstanceOf[SecuredRequest[E,A]] - // - def createInstance[E <: Env, A](request: MySecuredRequest[E, A]): OrigSecuredRequest[E, A] = { - OrigSecuredRequest(request.identity, request.authenticator, request) - } -} \ No newline at end of file + val identity: Option[DefaultEnv#I], + val authenticator: Option[DefaultEnv#A] +) extends MyRequest(request, messagesApi) with MyUserAwareRequestHeader diff --git a/app/controllers/ResetPasswordController.scala b/app/controllers/ResetPasswordController.scala index 8e9800b..34883e7 100644 --- a/app/controllers/ResetPasswordController.scala +++ b/app/controllers/ResetPasswordController.scala @@ -26,7 +26,7 @@ class ResetPasswordController @Inject() ( * @param token The token to identify a user. * @return The result to display. */ - def view(token: UUID) = MyUnsecuredAction.async { implicit request: MyRequest[AnyContent] => + def view(token: UUID) = UnsecuredAction.async { implicit request: MyRequest[AnyContent] => authTokenService.validate(token).map { case Some(_) => Ok(resetPassword(ResetPasswordForm.form, token)) case None => Redirect(Calls.signin).flashing("error" -> Messages("invalid.reset.link")) @@ -39,7 +39,7 @@ class ResetPasswordController @Inject() ( * @param token The token to identify a user. * @return The result to display. */ - def submit(token: UUID) = MyUnsecuredAction.async { implicit request: MyRequest[AnyContent] => + def submit(token: UUID) = UnsecuredAction.async { implicit request: MyRequest[AnyContent] => authTokenService.validate(token).flatMap { case Some(authToken) => ResetPasswordForm.form.bindFromRequest.fold( diff --git a/app/controllers/SignInController.scala b/app/controllers/SignInController.scala index 4ff7808..6943482 100644 --- a/app/controllers/SignInController.scala +++ b/app/controllers/SignInController.scala @@ -27,7 +27,7 @@ class SignInController @Inject() ( * * @return The result to display. */ - def view = MyUnsecuredAction.async { implicit request: MyRequest[AnyContent] => + def view = UnsecuredAction.async { implicit request: MyRequest[AnyContent] => Future.successful(Ok(signIn(SignInForm.form, socialProviderRegistry))) } @@ -36,7 +36,7 @@ class SignInController @Inject() ( * * @return The result to display. */ - def submit = MyUnsecuredAction.async { implicit request: MyRequest[AnyContent] => + def submit = UnsecuredAction.async { implicit request: MyRequest[AnyContent] => SignInForm.form.bindFromRequest.fold( form => Future.successful(BadRequest(signIn(form, socialProviderRegistry))), data => { diff --git a/app/controllers/SignUpController.scala b/app/controllers/SignUpController.scala index 86c3bbc..b87fd7c 100644 --- a/app/controllers/SignUpController.scala +++ b/app/controllers/SignUpController.scala @@ -27,7 +27,7 @@ class SignUpController @Inject() ( * * @return The result to display. */ - def view = MyUnsecuredAction.async { implicit request: MyRequest[AnyContent] => + def view = UnsecuredAction.async { implicit request: MyRequest[AnyContent] => Future.successful(Ok(signUp(SignUpForm.form))) } @@ -36,7 +36,7 @@ class SignUpController @Inject() ( * * @return The result to display. */ - def submit = MyUnsecuredAction.async { implicit request: MyRequest[AnyContent] => + def submit = UnsecuredAction.async { implicit request: MyRequest[AnyContent] => SignUpForm.form.bindFromRequest.fold( form => Future.successful(BadRequest(signUp(form))), data => { diff --git a/app/controllers/SilhouetteController.scala b/app/controllers/SilhouetteController.scala index 13b2a7d..ea1f385 100644 --- a/app/controllers/SilhouetteController.scala +++ b/app/controllers/SilhouetteController.scala @@ -21,18 +21,14 @@ import scala.concurrent.{ ExecutionContext, Future } abstract class SilhouetteController(override protected val controllerComponents: SilhouetteControllerComponents) extends MessagesAbstractController(controllerComponents) with SilhouetteComponents with Logging { - type SecuredEnvRequest[A] = SecuredRequest[EnvType, A] - type MySecuredEnvRequest[A] = MySecuredRequest[EnvType, A] - - type UserAwareEnvRequest[A] = UserAwareRequest[EnvType, A] - type MyUserAwareEnvRequest[A] = MyUserAwareRequest[EnvType, A] + type SecuredEnvRequest[A] = SecuredRequest[DefaultEnv, A] + type UserAwareEnvRequest[A] = UserAwareRequest[DefaultEnv, A] class MyActionFunction extends ActionFunction[Request, MyRequest] { def invokeBlock[A]( request: Request[A], block: MyRequest[A] => Future[Result]): Future[Result] = { val myRequest = new MyRequest[A]( - myStuff = ReallyMyStuff, messagesApi = messagesApi, request = request ) @@ -43,12 +39,11 @@ abstract class SilhouetteController(override protected val controllerComponents: controllerComponents.executionContext } - class MySecuredActionFunction extends ActionFunction[SecuredEnvRequest, MySecuredEnvRequest] { + class MySecuredActionFunction extends ActionFunction[SecuredEnvRequest, MySecuredRequest] { def invokeBlock[A]( request: SecuredEnvRequest[A], - block: MySecuredEnvRequest[A] => Future[Result]): Future[Result] = { - val myRequest = new MySecuredEnvRequest[A]( - myStuff = ReallyMyStuff, + block: MySecuredRequest[A] => Future[Result]): Future[Result] = { + val myRequest = new MySecuredRequest[A]( messagesApi = messagesApi, identity = request.identity, authenticator = request.authenticator, @@ -61,12 +56,11 @@ abstract class SilhouetteController(override protected val controllerComponents: controllerComponents.executionContext } - class MyUserAwareActionFunction extends ActionFunction[UserAwareEnvRequest, MyUserAwareEnvRequest] { + class MyUserAwareActionFunction extends ActionFunction[UserAwareEnvRequest, MyUserAwareRequest] { def invokeBlock[A]( request: UserAwareEnvRequest[A], - block: MyUserAwareEnvRequest[A] => Future[Result]): Future[Result] = { - val myRequest = new MyUserAwareEnvRequest[A]( - myStuff = ReallyMyStuff, + block: MyUserAwareRequest[A] => Future[Result]): Future[Result] = { + val myRequest = new MyUserAwareRequest[A]( messagesApi = messagesApi, identity = request.identity, authenticator = request.authenticator, @@ -79,7 +73,7 @@ abstract class SilhouetteController(override protected val controllerComponents: controllerComponents.executionContext } - def MyUnsecuredAction: ActionBuilder[MyRequest, AnyContent] = UnsecuredAction.andThen(new MyActionFunction) + def UnsecuredAction: ActionBuilder[MyRequest, AnyContent] = controllerComponents.silhouette.UnsecuredAction.andThen(new MyActionFunction) // Can't use ActionFunction here, there's no generic SecuredActionBuilder, i.e. // no out of the box extension for actionbuilder with apply for authorizations... @@ -88,28 +82,22 @@ abstract class SilhouetteController(override protected val controllerComponents: // def apply(errorHandler: SecuredErrorHandler): MySecuredActionBuilder[P] // def apply(authorization: Authorization[EnvType#I, EnvType#A]): MySecuredActionBuilder[ P] // } - // - // def MySecuredAction: MySecuredActionBuilder[AnyContent] = SecuredAction.andThen(new MySecuredActionFunction) - def MySecuredAction: ActionBuilder[MySecuredEnvRequest, AnyContent] = { - SecuredAction.andThen(new MySecuredActionFunction) + def SecuredAction: ActionBuilder[MySecuredRequest, AnyContent] = { + controllerComponents.silhouette.SecuredAction.andThen(new MySecuredActionFunction) } // But! We CAN do this though - def MySecuredAction(errorHandler: SecuredErrorHandler): ActionBuilder[MySecuredEnvRequest, AnyContent] = { - SecuredAction(errorHandler).andThen(new MySecuredActionFunction) + def SecuredAction(errorHandler: SecuredErrorHandler): ActionBuilder[MySecuredRequest, AnyContent] = { + controllerComponents.silhouette.SecuredAction(errorHandler).andThen(new MySecuredActionFunction) } // Or this! - def MySecuredAction(authorization: Authorization[EnvType#I, EnvType#A]): ActionBuilder[MySecuredEnvRequest, AnyContent] = { - SecuredAction(authorization).andThen(new MySecuredActionFunction) + def SecuredAction(authorization: Authorization[DefaultEnv#I, DefaultEnv#A]): ActionBuilder[MySecuredRequest, AnyContent] = { + controllerComponents.silhouette.SecuredAction(authorization).andThen(new MySecuredActionFunction) } - def MyUserAwareAction: ActionBuilder[MyUserAwareEnvRequest, AnyContent] = UserAwareAction.andThen(new MyUserAwareActionFunction) - - def SecuredAction: SecuredActionBuilder[EnvType, AnyContent] = controllerComponents.silhouette.SecuredAction - def UnsecuredAction: UnsecuredActionBuilder[EnvType, AnyContent] = controllerComponents.silhouette.UnsecuredAction - def UserAwareAction: UserAwareActionBuilder[EnvType, AnyContent] = controllerComponents.silhouette.UserAwareAction + def UserAwareAction: ActionBuilder[MyUserAwareRequest, AnyContent] = controllerComponents.silhouette.UserAwareAction.andThen(new MyUserAwareActionFunction) def userService: UserService = controllerComponents.userService def authInfoRepository: AuthInfoRepository = controllerComponents.authInfoRepository @@ -123,15 +111,14 @@ abstract class SilhouetteController(override protected val controllerComponents: def totpProvider: GoogleTotpProvider = controllerComponents.totpProvider def avatarService: AvatarService = controllerComponents.avatarService - def silhouette: Silhouette[EnvType] = controllerComponents.silhouette + def silhouette: Silhouette[DefaultEnv] = controllerComponents.silhouette def authenticatorService: AuthenticatorService[AuthType] = silhouette.env.authenticatorService def eventBus: EventBus = silhouette.env.eventBus } trait SilhouetteComponents { - type EnvType = DefaultEnv - type AuthType = EnvType#A - type IdentityType = EnvType#I + type AuthType = DefaultEnv#A + type IdentityType = DefaultEnv#I def userService: UserService def authInfoRepository: AuthInfoRepository @@ -145,7 +132,7 @@ trait SilhouetteComponents { def totpProvider: GoogleTotpProvider def avatarService: AvatarService - def silhouette: Silhouette[EnvType] + def silhouette: Silhouette[DefaultEnv] } trait SilhouetteControllerComponents extends MessagesControllerComponents with SilhouetteComponents diff --git a/app/controllers/SocialAuthController.scala b/app/controllers/SocialAuthController.scala index 0414ceb..04f1981 100644 --- a/app/controllers/SocialAuthController.scala +++ b/app/controllers/SocialAuthController.scala @@ -23,7 +23,7 @@ class SocialAuthController @Inject() ( * @param provider The ID of the provider to authenticate against. * @return The result to display. */ - def authenticate(provider: String) = MyUnsecuredAction.async { implicit request: MyRequest[AnyContent] => + def authenticate(provider: String) = UnsecuredAction.async { implicit request: MyRequest[AnyContent] => (socialProviderRegistry.get[SocialProvider](provider) match { case Some(p: SocialProvider with CommonSocialProfileBuilder) => p.authenticate().flatMap { diff --git a/app/controllers/TotpController.scala b/app/controllers/TotpController.scala index f114d0c..758451f 100644 --- a/app/controllers/TotpController.scala +++ b/app/controllers/TotpController.scala @@ -24,7 +24,7 @@ class TotpController @Inject() ( * Views the `TOTP` page. * @return The result to display. */ - def view(userId: java.util.UUID, sharedKey: String, rememberMe: Boolean) = MyUnsecuredAction.async { implicit request => + def view(userId: java.util.UUID, sharedKey: String, rememberMe: Boolean) = UnsecuredAction.async { implicit request => Future.successful(Ok(totp(TotpForm.form.fill(TotpForm.Data(userId, sharedKey, rememberMe))))) } @@ -32,7 +32,7 @@ class TotpController @Inject() ( * Enable TOTP. * @return The result to display. */ - def enableTotp = MySecuredAction.async { implicit request => + def enableTotp = SecuredAction.async { implicit request => val user = request.identity val credentials = totpProvider.createCredentials(user.email.get) val totpInfo = credentials.totpInfo @@ -46,7 +46,7 @@ class TotpController @Inject() ( * Disable TOTP. * @return The result to display. */ - def disableTotp = MySecuredAction.async { implicit request => + def disableTotp = SecuredAction.async { implicit request => val user = request.identity authInfoRepository.remove[GoogleTotpInfo](user.loginInfo) Future(Redirect(Calls.home).flashing("info" -> Messages("totp.disabling.info"))) @@ -56,7 +56,7 @@ class TotpController @Inject() ( * Handles the submitted form with TOTP initial data. * @return The result to display. */ - def enableTotpSubmit = MySecuredAction.async { implicit request => + def enableTotpSubmit = SecuredAction.async { implicit request => val user = request.identity TotpSetupForm.form.bindFromRequest.fold( form => authInfoRepository.find[GoogleTotpInfo](request.identity.loginInfo).map { totpInfoOpt => @@ -81,7 +81,7 @@ class TotpController @Inject() ( * Handles the submitted form with TOTP verification key. * @return The result to display. */ - def submit = MyUnsecuredAction.async { implicit request => + def submit = UnsecuredAction.async { implicit request => TotpForm.form.bindFromRequest.fold( form => Future.successful(BadRequest(totp(form))), data => { diff --git a/app/controllers/TotpRecoveryController.scala b/app/controllers/TotpRecoveryController.scala index c52a66b..be57d38 100644 --- a/app/controllers/TotpRecoveryController.scala +++ b/app/controllers/TotpRecoveryController.scala @@ -27,7 +27,7 @@ class TotpRecoveryController @Inject() ( * @param rememberMe the remember me flag. * @return The result to display. */ - def view(userID: UUID, sharedKey: String, rememberMe: Boolean) = MyUnsecuredAction.async { implicit request => + def view(userID: UUID, sharedKey: String, rememberMe: Boolean) = UnsecuredAction.async { implicit request => Future.successful(Ok(totpRecovery(TotpRecoveryForm.form.fill(TotpRecoveryForm.Data(userID, sharedKey, rememberMe))))) } @@ -35,7 +35,7 @@ class TotpRecoveryController @Inject() ( * Handles the submitted form with TOTP verification key. * @return The result to display. */ - def submit = MyUnsecuredAction.async { implicit request => + def submit = UnsecuredAction.async { implicit request => TotpRecoveryForm.form.bindFromRequest.fold( form => Future.successful(BadRequest(totpRecovery(form))), data => {