ppy / osu-web

the browser-facing portion of osu!
https://osu.ppy.sh
GNU Affero General Public License v3.0
982 stars 384 forks source link

Calling api chat/new will throw a 403 error if a channel already exists for a PM conversation #6043

Closed peppy closed 4 years ago

peppy commented 4 years ago

Currently a 403 is returned the second time this is called for the same user. This should gracefully succeed for private messages. The lazer client is unaware of whether there's an old PM channel lurking in the database (that has since been "left" by the user).

The code can return the existing channel and allow this operation to continue.

AstralPhnx commented 4 years ago

I take it the channel factory is what currently handles the storage of open chat channels for each user correct? If that's the case then maybe just having lazer poll the database for a list of open chat channels for the specific user ID of the person you want to chat with then reopening a chat if it finds it's already there would be a good idea.

This is mostly a lazer issue from what I've experienced since the message button on the website itself functions as intended so there's something slipping in between the cracks in the middle.

nanaya commented 4 years ago

is it for /api/v2/chat/new? The only 403 path I see is if the user is not allowed to send message to target user...

nanaya commented 4 years ago

so the problem isn't sending the chat the second time but if the user closes the chat and then tries sending another message

peppy commented 4 years ago

The issue lies on this line:

https://github.com/ppy/osu-web/blob/e59aac8e9ef840b26c9b68eb9dcec4153b3fe2b8/app/Libraries/Chat.php#L87

The case is where a channel has been created, then the chat/new endpoint is used. It will lookup the existing channel but fail the permission check for some reason.

peppy commented 4 years ago

Yep, it's the case where the channel is closed between sends.

AstralPhnx commented 4 years ago

Worth noting is you can reopen chats using the website itself and it will work right, (you can even send messages in the reopened channels in lazer if you reopen them from your browser) just not the lazer client