Admin REST API to remove an expired push token

[logicwonder]

Is your feature request related to a problem? Please describe.

The FCM push token published for a user account can become stale due to inactivity for a long period. According to FCM best practices, such tokens needs to be removed. This optimizes the push notification module by avoiding the need to maintain expired tokens and push messages with invalid tokens.

Describe the solution you'd like

The expiry of a push token would be detected in the Push client API implementation. On receiving an error from FCM, an eJabberd admin API may be called with the username and host to remove the push session from cache and SQL backend.

POST /api/delete_push_session
{
    "user": "user1",
    "host": "example.com"
}   
 HTTP/1.1 200 OK
""

Describe alternatives you've considered

I have tried deleting the push session from SQL backend but due to the caching policy, the push token still remains in the cache and gets used again to send an invalid push request.

Additional context

• Best practices for FCM token management
• APNS tokens do not expire and may change only on reinstallation or device reset.

[badlop]

I have tried deleting the push session from SQL backend

In what table is stored that?

The existing delete_old_push_sessions API is not suitable for that user case because... ?

[logicwonder]

The existing API is a bulk implementation where we assume all tokens older than n days are invalid. This delete could include both valid and invalid tokens.

We do receive unregistered error from FCM when we attempt using an invalid token. It seems to be a better if we are able to trigger a delete on this event.

Pls let me know if this approach is the right way to deal with the problem?

[logicwonder]

@badlop Any update on this? We are currently deleting the push token from push_session table when we are getting unregistered error from FCM.

[badlop]

I know almost nothing of that XEP and the module... maybe you mean something like this?

```diff diff --git a/src/mod_push.erl b/src/mod_push.erl index c911bb6ac..cbb4a206f 100644 --- a/src/mod_push.erl +++ b/src/mod_push.erl @@ -41,7 +41,8 @@ -export([process_iq/1]). %% ejabberd command. --export([get_commands_spec/0, delete_old_sessions/1]). +-export([get_commands_spec/0, delete_old_sessions/1, + delete_user_sessions/2]). %% API (used by mod_push_keepalive). -export([notify/3, notify/5, notify/7, is_incoming_chat_msg/1]). @@ -218,6 +219,13 @@ get_commands_spec() -> desc = "Remove push sessions older than DAYS", module = ?MODULE, function = delete_old_sessions, args = [{days, integer}], + result = {res, rescode}}, + #ejabberd_commands{name = delete_user_sessions, tags = [purge], + desc = "Remove push sessions of a user", + module = ?MODULE, function = delete_user_sessions, + args = [{user, binary}, {host, binary}], + args_desc = ["Username", "Server"], + args_example = [<<"bob">>, <<"example.com">>], result = {res, rescode}}]. -spec delete_old_sessions(non_neg_integer()) -> ok | any(). @@ -248,6 +256,20 @@ delete_old_sessions(Days) -> Reason end. +-spec delete_user_sessions(binary(), binary()) -> + {ok, binary()} | {error, binary()}. +delete_user_sessions(User, Server) -> + LUser = jid:nodeprep(User), + LServer = jid:nameprep(Server), + case remove_user(LUser, LServer) of + ok -> + {ok, <<"Push sessions of user were removed">>}; + {error, Bin} when is_binary(Bin) -> + {error, Bin}; + {error, _} -> + {error, <<"Db returned error">>} + end. + %%-------------------------------------------------------------------- %% Register/unregister hooks. %%-------------------------------------------------------------------- ```

[logicwonder]

Thanks @badlop. This is what I was looking for. I really wish to see this implemented as a REST API in a future release.

[badlop]

Did you test the patch? As I said, I know almost nothing of that XEP and the module, and consequently this would, at least, a confirmation that it really does what it should do, and nothing else.

[logicwonder]

Ok @badlop will test and give the confirmation.

On Mon, Jul 3, 2023 at 7:15 PM badlop @.***> wrote:

Did you test the patch? As I said, I know almost nothing of that XEP and the module, and consequently this would, at least a confirmation that it really does what it should do, and nothing else.

— Reply to this email directly, view it on GitHub https://github.com/processone/ejabberd/issues/4033#issuecomment-1618304411, or unsubscribe https://github.com/notifications/unsubscribe-auth/AASUPBKQND3DGEQELHOYGNTXOLEHZANCNFSM6AAAAAAYHLX4QQ . You are receiving this because you authored the thread.Message ID: @.***>

[logicwonder]

I have tested the feature and it is working functionally. The push session got deleted from the database. I could not test whether the push session was removed from the mod_push cache.

Test Case 1: Delete an existing push session using command ejabberd@localhost:~$ ejabberdctl delete_user_sessions test1 local.test.com Error: {ok,<<"Push sessions of user were removed">>}

Result: PASS (response message may be corrected)

Test Case 2: Delete an non existent push session using command ejabberd@localhost:~$ ejabberdctl delete_user_sessions test1 local.test.com Error: error Error: <<"Db returned error">>

Result: PASS (response message may be corrected)

Test Case 3: Delete an existing push session using REST API ejabberd@localhost:~$ curl --request POST 'http://127.0.0.1:5280/api/delete_user_sessions' \ --header 'Content-Type: application/json' \ --data-raw '{ "user": "test2", "host": "local.test.com" }' 1

Result: PASS (0 may used for success code)

Test Case 4: Delete an non existent push session using REST API ejabberd@localhost:~$ curl --request POST 'http://127.0.0.1:5280/api/delete_user_sessions' --header 'Content-Type: application/json' --data-raw '{ "user": "test2", "host": "local.test.com" }' "Db returned error"

Result: PASS (response may be changed to 1)

I am gald to test it again if required. Thanks again for implementing this feature.

[badlop]

Maybe this is a dumb question but: can an account have several sessions? If only one of them is obsolete, and you only desire to delete one... That command is not suitable, as it deletes all sessions associated with a user, right?

[logicwonder]

You are right. According to the push_session table definition, the unique key is defined as (username, server_host,timestamp). So technically it is possible to have multiple sessions for a single user (especially in a multi-device scenario). In my specific scenario, there is no requirement for multi device support. So the present implementation would suffice.

A more precise implementation could accept the push session ID as an additional parameter when deleting an expired push token.