humhub / app

20 stars 8 forks source link

Delete/Reset Push Token on Logout #102

Closed luke- closed 12 months ago

PrimozRatej commented 1 year ago

@luke- This can be handled the same way as registerFcmDevice before the actual logout we could trigger the JS channel with a DELETE url redirect.

luke- commented 1 year ago

@PrimozRatej I've added a new message similar to register, for unregister. Can you please implement this on app sidE? https://github.com/humhub/fcm-push/blob/master/helpers/MobileAppHelper.php#L55

PrimozRatej commented 1 year ago

Hey @luke- , can you please add the trigger for unregisterFcmDevice on the Mobile App Debug Page for testing purposes, also this should be triggered before logout and after showOpener so that we delete the token first and redirect to the opener second.

luke- commented 1 year ago

@PrimozRatej I've just added the trigger on the Debug Page.

Also I can confirm that unregister will be fired before showOpener. It this the correct order? image

PrimozRatej commented 1 year ago

Hey @luke- Yes, that's the correct order, I implemented the functionality it works on the debug page, while manually triggered. But there is still one problem. The Logout is called before unregisterFcmDevice or it's unawaited. The problem is that we Logout the user before it could trigger the unregisterFcmDevice, because we need a valid access token for that , the response is a redirect to the login page and not a token unregistered.

Maybe the best way to impl. this feature would be to use the App Detection functionality on the web side and just unregister it there. When the user logout, if the request came from the Mobile app.

image

luke- commented 1 year ago

@PrimozRatej If I am to implement this on the web side, I would need to know which token belongs to the current app request. Because the logged in user could have multiple mobiles or even PWAs. Not sure this is possible?

An alternative could be to combine the logout and unregister channel event? So we include the "unregisterFcm" url in the logout message... what do you mean?

PrimozRatej commented 1 year ago

I had headers in mind. So I could append the push token to request headers, or we could use the replyProxy to pass push token to web app https://inappwebview.dev/docs/webview/javascript/communication/#web-message-channels:~:text=replyProxy)%20%7B-,replyProxy.postMessage(%22Got%20it!%22)%3B,-%7D%2C%0A%20%20%20%20%20)

If I understand the alternative correctly, we want to combine the logout and unregisterFcmDevice, if the instance is running in the mobile app (this could be checked on the web) do the logout on mobile if else do it on the web. There would be an action logout on flutterChannel that would first unregister the user push token, then log him out and lastly redirect to the opener.

@luke- Do I understand it correctly?

luke- commented 1 year ago

@PrimozRatej I am not quite sure, my idea was that in the existing logout flutter channel message, we also include an attribute unregisterFcmDeviceUrl. (Instead of the own channel message), if this attribute is set, you could send a POST request with the device token (like to register process).

Is there a reason why this is not possible on the app side?

PrimozRatej commented 1 year ago

Currently, there is no logout channel message.

Current massages: enum ChannelAction { showOpener, hideOpener, registerFcmDevice, unregisterFcmDevice, updateNotificationCount, none }

https://github.com/humhub/app/assets/10835179/d2c5267a-b9b5-4b21-adac-4c3f51c6ec2f

I hope that the video explains it. The logout is called the _identity is removed and the session is changed. I assume we need this, to call POST for unregistered users' push tokens.

image

luke- commented 12 months ago

@PrimozRatej Ok understand. I have now removed the condition that a valid session is required when unregistering a push token. Does this solve our problem?

It is already rolled out the update on our community and somtest123456.

PrimozRatej commented 12 months ago

@luke- This now solves the problem. Tested it it looks fine.