ably-forks / laravel-echo

Laravel Echo library for beautiful Pusher and Ably integration.
https://laravel.com/docs/broadcasting#client-side-installation
MIT License
12 stars 4 forks source link

Doesn't work with Laravel Sanctum #26

Closed zoranlorkovic closed 11 months ago

zoranlorkovic commented 11 months ago

Echo Version

1.0.3

Laravel Version

10.10

PHP Version

8.2

NPM Version

10.2

Database Driver & Version

No response

Description

Hi,

To be able to work with Laravel Sanctum, we should be able to to set a custom authoriser (axis) which is predefine for a cross-domain requests, similar as we need for Pusher (please check: https://laravel.com/docs/10.x/sanctum#authorizing-private-broadcast-channels)

But this is not possible if using Laravel Echo from Ably as there isn't an option to do so and you're getting "CSRF token mismatch." error when you try to auth to private channel.

I've tried to set csrfToken and bearerToken but it didn't worked.

Steps To Reproduce

sync-by-unito[bot] commented 11 months ago

➤ Automation for Jira commented:

The link to the corresponding Jira issue is https://ably.atlassian.net/browse/SDK-3944

sacOO7 commented 11 months ago

@zoranlorkovic thanks for raising the issue. We will look into adding support for the same.

KalanaPerera commented 11 months ago

@sacOO7 do you have any planned date for the release? We are blocked with this error, any alternative solution? TIA

sacOO7 commented 11 months ago

@KalanaPerera let me check if there is a way to use this feature without making explicit changes to the code

sacOO7 commented 11 months ago

@zoranlorkovic @KalanaPerera It seems we already have a solution here, you need to pass requestTokenFn as an argument while creating Echo instance =>

echo = new Echo({
        broadcaster: 'ably',
        useTls: true,
        requestTokenFn: async (channelName: string, existingToken: string) => {
            let postData = { channel_name: channelName, token: existingToken };
            return await axios.post("/api/broadcasting/auth", postData);
        },
});

Basically this code is equivalent of https://github.com/ably-forks/laravel-echo/blob/85f4dbcc01f8f54d5127a067a2ca1dc2f509cc22/src/channel/ably/auth.ts#L34-L49 I think we don't need to provide extra headers in case of axios. I will update README for the same.

sacOO7 commented 11 months ago

@zoranlorkovic @KalanaPerera As you can see, we have created a PR to update the README -> https://github.com/ably-forks/laravel-echo/pull/27. Let us know if above solution works, we will merge the same 👍

zoranlorkovic commented 11 months ago

@sacOO7 I think we're getting somewhere. My Laravel endpoint is not returning "CSRF token mismatch" error anymore and it's returning "token" value which is correct.

However, it seems Echo is not subscribing to channel when using Echo.private('channelName'). And when I check a data sent to /api/broadcasting/auth, both channelName and token are null.

Do we need to specify these values when creating Echo instance?

The other thing I've noticed is warning message in my browser console window: Ably: Auth.requestToken(): token request signing call returned error; err = TypeError: undefined is not an object (evaluating 'jwtToken.split') with Echo sending request to /api/broadcasting/auth every couple of seconds (not sure if that's expected behaviour)

sacOO7 commented 11 months ago

@zoranlorkovic wait a minute, let me check on this 🙂

sacOO7 commented 11 months ago

@zoranlorkovic can you check or log the response for axios post request. It should return { token, info } as per

https://github.com/ably-forks/laravel-echo/blob/85f4dbcc01f8f54d5127a067a2ca1dc2f509cc22/src/channel/ably/token-request.ts#L12-L16

Seems you need to modify code as

        requestTokenFn: async (channelName: string, existingToken: string) => {
            let postData = { channel_name: channelName, token: existingToken };
            const res = await axios.post("/api/broadcasting/auth", postData);
            return res.data;
        },

Also, do not add try/catch inside the code, it's already handled internally. I think the code should work now. Make sure to post the working code here 👍

zoranlorkovic commented 11 months ago

@sacOO7 Response from axios post is

{
    "token": "someToken"
}

Another thing I've noticed is that when using pusher "channel_name" and "socket_id" are sent to "/broadcasting/auth". I've checked Laravel code for this endpoint and don't see that they are expecting "token" as mentioned in the comment above.

sacOO7 commented 11 months ago

We have customized endpoint api/broadcasting/auth in case of ably-laravel-broadcaster. So, it will be a difference response than pusher one. It does expects existing token and channelName. As far as socket_id is concerned, it's automatically set by axios interceptor as a header here -> https://github.com/ably-forks/laravel-echo/blob/85f4dbcc01f8f54d5127a067a2ca1dc2f509cc22/src/echo.ts#L157-L165

We will know it when at the top level everything works fine.

sacOO7 commented 11 months ago

@zoranlorkovic let me know if code is working as per changes mentioned in the comment -> https://github.com/ably-forks/laravel-echo/issues/26#issuecomment-1828234953

sacOO7 commented 11 months ago

Also, don't forget to star both repos https://github.com/ably/laravel-broadcaster and https://github.com/ably-forks/laravel-echo/. Ask your colleagues to do so too :P. We are looking for devs like you to raise issues like this with us : )

zoranlorkovic commented 11 months ago

Ah, you're right, sorry about that. I was looking at default Laravel api/boradcasting/auth endpoint. I've just checked yours and indeed it does expect channel_name and token.

And it seems it does sends correct channel name when subscribing as I've noticed that in first call it makes to api/broadcasting/auth it's sending data:

{"channel_name":"private:App.Models.User.1","token":null}

but on every other request which is making every 45s or so it's just sending this data:

{"channel_name":null}

sacOO7 commented 11 months ago

Okay, let me check that one too 👍 So, for first channel, we saying, everything is good right

sacOO7 commented 11 months ago

Hi @zoranlorkovic By any chance did you set token expiry as 1 minute as per https://github.com/ably/laravel-broadcaster/tree/main#configure-advanced-features ? If that's the case, it will try to renew the token every 1 minute. In such a case, no new channel is being authenticated, rather existing channels from existing_token will be re-authenticated

sacOO7 commented 11 months ago

For every new channel.private()(private channel) or channel.join()(presence channel), it will always send channel_name in the request. Let me know if you still have more questions

zoranlorkovic commented 11 months ago

"ABLY_TOKEN_EXPIRY" is set to 3600 (1h) so not sure why it's sending request to backend every 30s? (confirmed it's 30s)

The last thing I want to check before doing further testing, what could be this warning message in console window:

Ably: Auth.requestToken(): token request signing call returned error; err = TypeError: undefined is not an object (evaluating 'jwtToken.split')

sacOO7 commented 11 months ago

Okay, is this still happening after making this change

        requestTokenFn: async (channelName: string, existingToken: string) => {
            let postData = { channel_name: channelName, token: existingToken };
            const res = await axios.post("/api/broadcasting/auth", postData);
            return res.data;
        },

Note that, we have modified the code to return res.data instead of res

zoranlorkovic commented 11 months ago

This seems to resolve this issue. Now there are just two requests to api/broadcast/auth:

First one is with data:

{"channel_name":"private:App.Models.User.1","token":null}

result:

{
    "token": "returnedToken"
}

Second request:

{"channel_name":null,"token":"returnedTokenFromPreviousRequest"}

result:

{
    "token": "returnedTokenSameAsPrevious"
}

Is this excepted result?

Also that message: Ably: Auth.requestToken(): token request signing call returned error; err = TypeError: undefined is not an object (evaluating 'jwtToken.split') seems to be gone now.

sacOO7 commented 11 months ago

@zoranlorkovic ofc, it should work! This is expected result : ) I had to mention solution for the third time after mentioning it here in top comment itself https://github.com/ably-forks/laravel-echo/issues/26#issuecomment-1828234953. You should go through the whole thread again! Also, you gotta star repos as per -> https://github.com/ably-forks/laravel-echo/issues/26#issuecomment-1828276466

zoranlorkovic commented 11 months ago

Ah, so sorry, don't know how I missed that additional code you've mentioned in https://github.com/ably-forks/laravel-echo/issues/26#issuecomment-1828234953 !

Repos already starred!

sacOO7 commented 11 months ago

Thanks! Please feel free to raise more issues 👍. There are no bugs reported in the library so far since the first release. So, we are eagerly waiting for improvements that can be done to support new trends !!

zoranlorkovic commented 11 months ago

Thank you for your help and quick replies @sacOO7 !

I've almost call it a day and returned back to Pusher but your quick replies really pushed me to stay and try again. Even after you've sent the solution an hour ago :)

sacOO7 commented 11 months ago

Haha, no worries! You were destined to stay with Ably XD !!!!