Skip to content
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

[play-2.2.x] WebSocket authorization support #26

Open
dbalduini opened this issue Aug 25, 2014 · 9 comments
Open

[play-2.2.x] WebSocket authorization support #26

dbalduini opened this issue Aug 25, 2014 · 9 comments

Comments

@dbalduini
Copy link
Contributor

Hi 😄

Is there already any kind of support for WebSocket based actions?

I had made something like this to make it work here.

trait OAuth2AsyncProvider2 extends OAuth2AsyncProvider {

  def authorized[A, U](dataHandler: DataHandler[U])(implicit request: RequestHeader): Future[Either[OAuthError, AuthInfo[U]]] =
    ProtectedResource.handleRequest(request, dataHandler)

}

Controller:

  def liveMatch = WebSocket.async[JsValue] {
    implicit request =>
      authorized(MyDataHandler).flatMap {
        _ match {
          case Right(authInfo) => GameStream join authInfo.user
          case Left(error) =>
            // Just consume and ignore the input
            val in = Iteratee.foreach[JsValue](_ => {})
            // Send the error message and close
            val out = Enumerator[JsValue](
              JsObject(Seq("error" -> JsString(error.description)))
            ).andThen(Enumerator.enumInput(Input.EOF))
            Future.successful((in, out))
        }
      }
  }

If there isn't, do you plan to support it or we should just use the ProtectedResource class?

ProtectedResource.handleRequest(request, dataHandler)
@tsuyoshizawa
Copy link
Member

I think to support the authorization for WebSocket is good.

Play Websocket supports to return a future of either by tryAcceptWithActor
The method requires Future[Either[Result, HandlerProps]] to result of callback.
https://github.com/playframework/playframework/blob/2.3.3/framework/src/play/src/main/scala/play/api/mvc/WebSocket.scala#L175

If we support the authorization on OAuthProvider, the return signature would be Future[Either[Result, AuthInfo[U]]]. By doing so, user will be able to focus only normal processing.

We need to think the name for the new method. e.g. authorizeWithWebSocket, tryAuthorize

@dbalduini
Copy link
Contributor Author

Yep, that would be awesome.

It's sad that play 2.2.x doesn't have tryAccept and tryAcceptWithActor.
I think i will probably just upgrade my project to play 2.3.

I can work on this PR if you allow me. Nevertheless, do you think we should still add support for this feature to the play-2.2.x version?

For the name of the method i think tryAuthorize is fine.

@tsuyoshizawa
Copy link
Member

do you think we should still add support for this feature to the play-2.2.x version?

I don't think to add support except a critical bug to the play-2.2.x version.

For the name of the method i think tryAuthorize is fine.

I reconsidered that after a day, it is good to create another provider named OAuthWebSocketProvider.

Because, WebSocket of Playframework requires RequestHeader, not Request.
User will be confused by the interface when user uses it without knowing to not use body parameter.

We will support tryAccept and tryAcceptWithActor.
Then, we can support callback for new interface like existing OAuthProvider.

I hope your example will be changed to like the below.

  def liveMatch = WebSocket.tryAccept[JsValue] {
    implicit request =>
      authorize(MyDataHandler) { authInfo =>
        GameStream join authInfo.user
      }
  }
  def liveMatch = WebSocket.tryAcceptWithActor[JsValue, JsValue] {
    implicit request =>
      authorizeWithActor(MyDataHandler) { authInfo =>
        GameStream join authInfo.user
      }
  }

@tsuyoshizawa
Copy link
Member

I'm going to try this feature after release version 0.9.0 and 0.7.4.

@dbalduini
Copy link
Contributor Author

Yes, making another trait is better, but it should extends the Async trait because if not the controller cannot have protected actions that does not uses WebSockets.

Also, i think that it would be nice to provide a method that doesn't require a function body, instead just give to the client an Either if the user is or is not authenticated. This is inspired by the authorized method in Play2-Auth framework.

I took this is snippet from the issue 51 of the Play2-auth

object WSController extends Controller with AsyncAuth with AuthConfigImpl {

  def chat = WebSocket.async[JsValue] { implicit request  =>
    authorized(NormalUser).map {
      case Right(user) => ChatRoom.join(user.username)
      case Left(_)     => // return authentication/authorization failed response
    }
  }  

}

So, providing something like this would be great.

 def authorized[A, U](dataHandler: DataHandler[U])(implicit request: RequestHeader): Future[Either[OAuthError, AuthInfo[U]]] =
    ProtectedResource.handleRequest(request, dataHandler)

@dbalduini
Copy link
Contributor Author

I'm going to try this feature after release version 0.9.0 and 0.7.4

Ok

Besides, you can count on me if you need an extra hand to develop this feature.

@tsuyoshizawa
Copy link
Member

So, providing something like this would be great.

I think you directly use ProtectedResource in your controller for handling an error response is better.
Because it just calls ProtectedResource.handleRequest in the provider.

Existing OAuth2Provider provides callback and creating failed response interface.
That means most developer usually doesn't need to implement for error responses.
So, I think WebSocket provider also has a similar interface is good.

@lequangdzung
Copy link

@tsuyoshizawa How is this going? Do you still have plan to support websocket with play2?

@tsuyoshizawa
Copy link
Member

@lequangdzung Yes. I have plan to support this using WebSocket.tryAcceptWithActor of play2.
It will just be simple wrapper. (I forgot this issue 😵 )

Do you want this as soon as possible?
After I implemented this feature, I'm going to prepare release version 1.0.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants