t2v / play2-auth

Play2.x Authentication and Authorization module
Apache License 2.0
608 stars 138 forks source link

OAuth2Controller#OAuth2StateKey should be `protected val` #161

Closed tototoshi closed 8 years ago

tototoshi commented 9 years ago

https://github.com/t2v/play2-auth/blob/master/social/src/main/scala/jp/t2v/lab/play2/auth/social/core/OAuth2Controller.scala#L18

When I tried to override and slightly modify the implementation of OAuth2Controller#login, I couldn't use OAuth2StateKey in the subclass since it is private val. Should it be protected?

class MyOAuth2Controller extends OAuth2Controller {

  ...

  // want to add some optional parameters but currently not supported.
  override def login(scope: String, optionalParam: String /* 👈 */) = AsyncStack(ExecutionContextKey -> OAuthExecutionContext) { implicit request =>
    implicit val ec = StackActionExecutionContext

    doSomethingGood(optionalParam)

    loggedIn match {
      case Some(u) =>
        loginSucceeded(request)
      case None =>
        val state = UUID.randomUUID().toString
        Future.successful(
          Redirect(authenticator.getAuthorizationUrl(scope, state)).withSession(
            request.session + (OAuth2StateKey /* Oops, it's private ❗️ */ -> state)
          )
        )
    }
  }
gakuzzzz commented 8 years ago

I understood the motivation. I change it to protected

gakuzzzz commented 8 years ago

done, I'll publish it.