mvanduijker / laravel-mercure-broadcaster

Laravel broadcaster for Mercure
MIT License
133 stars 14 forks source link

Integrate the auth middleware into the library #9

Open Radiergummi opened 3 years ago

Radiergummi commented 3 years ago

It feels like the JWT middleware for private channels should be integrated in the library. I'm happy to contribute a PR, but I wanted to talk about the design first. I think most of the dynamic values could be configured in broadcasting.connections.mercure, with exception of the subscription URLs. What I'd aim for here is an integration of Laravel routing, so you can pass route IDs in addition to URI patterns:

Route::middleware('mercure.auth:user.notifications,id')->get('/user/{id}', [UserController::class, 'notifications']);
Route::middleware('mercure.auth:https://my.app/users/{id}/notifications')->get('/user/{id}', [UserController::class, 'notifications']);
Route::middleware('mercure.auth:/users/{id}/notifications')->get('/user/{id}', [UserController::class, 'notifications']);

The first middleware identifier, mercure.auth:user.notifications,id, passes the route name user.notifications and the request parameter id. By checking whether a route with that name exists and fetching the parameter from the request, we should be able to build the full subscription URL on behalf of the developer automatically.

In the second case, we receive a full URL with an inline parameter ({id}). Again, we can use the route parameters to replace the parameter with the actual value from the request (I've done this once for another package and it works well).

In the third case, if the first parameter starts with a slash, we can assume it's an URI, prepend current host and scheme in front of it, then continue as for case two.

All in all, this would make it really easy to use private channels and fit well into the Laravel ecosystem. What do you think?

mvanduijker commented 3 years ago

Thanks for creating an issue. After quickly reading, I don't really understand yet what problem we are exactly solving. Will read more thoroughly this week.

Radiergummi commented 3 years ago

Sorry, maybe I was unclear 🙂 The problem I'd like to see solved is the middleware currently available for copy-paste from the Readme. I would like to include a more refined version with the Laravel package, so users can use the middleware out of the box, without having to add it to their application code manually. The examples I added were suggestions on how this new middleware might be used.

mvanduijker commented 3 years ago

Yeah, that would definitely be a nice improvement. I gave it some thought a couple of weeks ago but couldn't come up with something good. If you have some ideas feel free to make a PR.