Open dbalduini opened 10 years ago
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
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.
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
}
}
I'm going to try this feature after release version 0.9.0 and 0.7.4.
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)
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.
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.
@tsuyoshizawa How is this going? Do you still have plan to support websocket with play2?
@lequangdzung Yes. I have plan to support this using WebSocket.tryAcceptWithActor
of play2.
It will just be simple wrapper. (I forgot this issue :dizzy_face: )
Do you want this as soon as possible? After I implemented this feature, I'm going to prepare release version 1.0.0.
Hi :smile:
Is there already any kind of support for WebSocket based actions?
I had made something like this to make it work here.
Controller:
If there isn't, do you plan to support it or we should just use the ProtectedResource class?