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

Doesn't work with Laravel Sanctum #26

Closed
zoranlorkovic opened this issue Nov 10, 2023 · 25 comments · Fixed by #27
Closed

Doesn't work with Laravel Sanctum #26

zoranlorkovic opened this issue Nov 10, 2023 · 25 comments · Fixed by #27
Assignees

Comments

@zoranlorkovic
Copy link

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

  • Install new Laravel installation
  • Install Laravel Sanctum package
  • Install Laravel Echo from Ably
  • Follow instructions to setup Broadcast rules
Copy link

sync-by-unito bot commented Nov 10, 2023

➤ Automation for Jira commented:

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

@sacOO7
Copy link
Collaborator

sacOO7 commented Nov 11, 2023

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

@KalanaPerera
Copy link

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

@sacOO7
Copy link
Collaborator

sacOO7 commented Nov 26, 2023

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

@sacOO7
Copy link
Collaborator

sacOO7 commented Nov 26, 2023

@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

requestToken = async (channelName: string, existingToken: string) => {
let postData = JSON.stringify({ channel_name: channelName, token: existingToken });
let postOptions = {
uri: this.authEndpoint,
method: 'POST',
headers: {
Accept: 'application/json',
'Content-Type': 'application/json',
'Content-Length': postData.length,
...this.authHeaders,
},
body: postData,
};
return await httpRequestAsync(postOptions);
};

I think we don't need to provide extra headers in case of axios. I will update README for the same.

@sacOO7
Copy link
Collaborator

sacOO7 commented Nov 26, 2023

@zoranlorkovic @KalanaPerera
As you can see, we have created a PR to update the README -> #27.
Let us know if above solution works, we will merge the same 👍

@sacOO7 sacOO7 self-assigned this Nov 26, 2023
@zoranlorkovic
Copy link
Author

@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
Copy link
Collaborator

sacOO7 commented Nov 27, 2023

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

@sacOO7
Copy link
Collaborator

sacOO7 commented Nov 27, 2023

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

execute = (tokenRequestFn: Function): Promise<{ token: string; info: any }> =>
new Promise((resolve, reject) => {
this.queue.run(async () => {
try {
const { token, info } = await tokenRequestFn(this.cachedToken);

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
Copy link
Author

zoranlorkovic commented Nov 27, 2023

@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
Copy link
Collaborator

sacOO7 commented Nov 27, 2023

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 ->

laravel-echo/src/echo.ts

Lines 157 to 165 in 85f4dbc

registerAxiosRequestInterceptor(): void {
axios.interceptors.request.use((config) => {
if (this.socketId()) {
config.headers['X-Socket-Id'] = this.socketId();
}
return config;
});
}

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

@sacOO7
Copy link
Collaborator

sacOO7 commented Nov 27, 2023

@zoranlorkovic let me know if code is working as per changes mentioned in the comment -> #26 (comment)

@sacOO7
Copy link
Collaborator

sacOO7 commented Nov 27, 2023

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
Copy link
Author

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
Copy link
Collaborator

sacOO7 commented Nov 27, 2023

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

@sacOO7
Copy link
Collaborator

sacOO7 commented Nov 27, 2023

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
Copy link
Collaborator

sacOO7 commented Nov 27, 2023

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
Copy link
Author

zoranlorkovic commented Nov 27, 2023

"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
Copy link
Collaborator

sacOO7 commented Nov 27, 2023

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
Copy link
Author

zoranlorkovic commented Nov 27, 2023

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
Copy link
Collaborator

sacOO7 commented Nov 27, 2023

@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
#26 (comment).
You should go through the whole thread again!
Also, you gotta star repos as per -> #26 (comment)

@zoranlorkovic
Copy link
Author

Ah, so sorry, don't know how I missed that additional code you've mentioned in #26 (comment) !

Repos already starred!

@sacOO7
Copy link
Collaborator

sacOO7 commented Nov 27, 2023

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
Copy link
Author

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
Copy link
Collaborator

sacOO7 commented Nov 27, 2023

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

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

Successfully merging a pull request may close this issue.

3 participants