play-silhouette-seed
play-silhouette-seed copied to clipboard
Add MyRequest
This is a sample PR to show extending a custom request type on top of SecuredRequest.
Note that since MyRequest extends MessagesRequest it is automatically a MessagesProvider
so that's one fewer implicit to pass around
@akkie I think this is a good practice because people will want to customize the request, also see the XXX bits for where traits would be used.
Also see https://discourse.silhouette.rocks/t/extending-securedrequest-with-actionrefiner/167
@wsargent What's the advantage over using the action refiner? For me it looks like a lot of boilerplate. Could you assemble a really small example that can then be used in the documentation?
I did some refactoring and switched to ActionTransformer. Does this work?
abstract class SomeAbstractController @Inject() (controllerComponents: SilhouetteControllerComponents) {
type SecuredEnvRequest[A] = SecuredRequest[DefaultEnv, A]
protected abstract class AbstractActionTransformer[-R[_], +P[_]](cc: SilhouetteControllerComponents) extends ActionTransformer[R, P] {
override protected def executionContext: ExecutionContext =
cc.executionContext
}
class MySecuredActionTransformer(cc: SilhouetteControllerComponents) extends AbstractActionTransformer[SecuredEnvRequest, MySecuredRequest](cc) {
override protected def transform[A](request: SecuredEnvRequest[A]): Future[MySecuredRequest[A]] = {
Future.successful( new MySecuredRequest[A](
messagesApi = cc.messagesApi,
identity = request.identity,
authenticator = request.authenticator,
request = request
))
}
}
private val mySecuredActionTransformer = new MySecuredActionTransformer(controllerComponents)
def SecuredAction: ActionBuilder[MySecuredRequest, AnyContent] = {
controllerComponents.silhouette.SecuredAction.andThen(mySecuredActionTransformer)
}
}
Probably should split up into IdentityProvider
and AuthenticatorProvider
traits really
Okay, broken out some more.
@wsargent Thanks :+1: It looks much cleaner and more understandable now.
I've some notes to your implementation. Some are global some I will comment inline.
-
You have hardcoded the type of the environment. There are a lot of users which are use different environments. Maybe the environment could be a type parameter of the
SilhouetteComponents
trait. This makes it easier to switch the environment without rewriting a lot of code. -
Could you please move the auth related traits to the
utils.auth
package? -
I don't like the name
MyRequest
. MAybe we find an mane that is more descriptive? Maybe AppRequest?
@akkie updated with comment fixes
The type alias can be used by subclasses. The parametrized type can’t.
On Thu, Jan 16, 2020 at 11:07 PM Christian Kaps [email protected] wrote:
@akkie commented on this pull request.
In app/controllers/SilhouetteController.scala https://github.com/mohiva/play-silhouette-seed/pull/118#discussion_r367794773 :
def eventBus: EventBus = silhouette.env.eventBus }
-trait SilhouetteComponents { +trait SilhouetteComponents[E <: Env] {
- type EnvType = E
Why not using E directly?
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/mohiva/play-silhouette-seed/pull/118?email_source=notifications&email_token=AMUFROUVASV55WUOGKOWEUTQ6FKMXA5CNFSM4KBHG472YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCSDQWQY#pullrequestreview-344394563, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMUFROREMCSSXFB35AOMKVLQ6FKMXANCNFSM4KBHG47Q .
@akkie how does it look?
I thought we should wait for the next Silhouette release, so that you can depend on https://github.com/mohiva/play-silhouette/pull/574. But the next Silhouette release depends on the new Play 2.8.1 release because of https://github.com/mohiva/play-silhouette/pull/570
I’m fine with rebasing and merging any conflicts
On Sun, Jan 26, 2020 at 11:15 PM Christian Kaps [email protected] wrote:
I thought we should wait for the next Silhouette release, so that you can depend on mohiva/play-silhouette#574 https://github.com/mohiva/play-silhouette/pull/574. But the next Silhouette release depends on the new Play 2.8.1 release because of mohiva/play-silhouette#570 https://github.com/mohiva/play-silhouette/pull/570
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/mohiva/play-silhouette-seed/pull/118?email_source=notifications&email_token=AMUFROR6OU6OCOT5Q4RKFC3Q72CY7A5CNFSM4KBHG472YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJ6QQLI#issuecomment-578619437, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMUFROXHMED6RARB6RMRZX3Q72CY7ANCNFSM4KBHG47Q .
@akkie 2.8.1 is out now https://github.com/playframework/playframework/releases/tag/2.8.1
@wsargent I've released the final 7.0.0