fix: prevent token from being used more than once for account create

sstrigler · sstrigler · commit 8a265b853e15 · 2025-12-18T16:32:15.000+01:00
Since invite is held in state of connection, one could potentitally authorize
and unlimited amount of sessions and then create accounts. Now account creation
checks within a transaction or atomic update that token is still valid on
update.

Also, interface has been unified
diff --git a/src/mod_invites.erl b/src/mod_invites.erl
@@ -45,7 +45,7 @@
 %% helpers
 -export([create_account_allowed/2, get_invite/2, get_invites/2, get_max_invites/2, is_create_allowed/2,
          is_expired/1, is_reserved/3, is_token_valid/2, roster_add/2, send_presence/3,
-         set_invitee/3, update_invite/2, token_uri/1, xdata_field/3]).
+         set_invitee/3, set_invitee/5, token_uri/1, xdata_field/3]).
 
 %% ejabberd_http
 -export([process/2]).
@@ -76,8 +76,12 @@
 -callback is_token_valid(Host :: binary(), binary(), {binary(), binary()}) -> boolean().
 -callback list_invites(Host :: binary()) -> [tuple()].
 -callback remove_user(User :: binary(), Server :: binary()) -> any().
--callback set_invitee(Host :: binary(), Token :: binary(), Invitee :: binary()) -> ok.
--callback update_invite(Host :: binary(), Invite :: invite_token()) -> ok.
+-callback set_invitee(Fun :: fun(() -> {ok|error, Res}),
+                                Host :: binary(),
+                                Token :: binary(),
+                                Invitee :: binary(),
+                                AccountName :: binary()) -> Res
+ when Res :: any().
 
 %% @format-begin
 
@@ -101,9 +105,7 @@ mod_doc() ->
                      " create such invites. Furthermore it applies to 'roster invites' and allows "
                      "to do in-band registration (ibr) if the sending user is allowed by this rule. "
                      "Users from the 'admin' ACL are always allowed to create account invites."),
-              example =>
-                  ["mod_invites:",
-                   "  access_create_account: local"]}},
+              example => ["mod_invites:", "  access_create_account: local"]}},
            {db_type,
             #{value => "mnesia | sql",
               desc =>
@@ -623,14 +625,23 @@ is_token_valid(Host, Token) ->
 is_token_valid(Host, Token, Inviter) ->
     db_call(Host, is_token_valid, [Host, Token, Inviter]).
 
-set_invitee(Host, Token, InviteeJid) ->
-    %% This invalidates the invite token
-    db_call(Host,
-            set_invitee,
-            [Host,
-             Token,
-             jid:encode(
-                 jid:remove_resource(InviteeJid))]).
+-spec set_invitee(binary(), binary(), jid() | binary()) -> ok.
+set_invitee(Host, Token, #jid{} = InviteeJid) ->
+    set_invitee(Host,
+                Token,
+                jid:encode(
+                    jid:remove_resource(InviteeJid)),
+                <<>>);
+set_invitee(Host, Token, Invitee) ->
+    set_invitee(Host, Token, Invitee, <<>>).
+
+set_invitee(Host, Token, Invitee, AccountName) ->
+    set_invitee(fun() -> ok end, Host, Token, Invitee, AccountName).
+
+-spec set_invitee(binary(), binary(), binary(), binary()) -> ok.
+set_invitee(F, Host, Token, Invitee, AccountName) ->
+    %% This invalidates the invite token if Invitee isn't empty
+    db_call(Host, set_invitee, [F, Host, Token, Invitee, AccountName]).
 
 create_roster_invite(Host, Inviter) ->
     create_invite(roster_only, Host, Inviter, <<>>).
@@ -748,9 +759,6 @@ invite_token(Type, Host, Inviter, AccountName0) ->
                                     account_name = AccountName},
                       mod_invites_opt:token_expire_seconds(Host)).
 
-update_invite(Host, Invite) ->
-    db_call(Host, update_invite, [Host, Invite]).
-
 token_uri(#invite_token{type = Type,
                         token = Token,
                         account_name = AccountName,
diff --git a/src/mod_invites_http.erl b/src/mod_invites_http.erl
@@ -179,12 +179,8 @@ process_register_post(Invite,
          ensure_same(Token, proplists:get_value(<<"token">>, Q))}
     of
         {AppCtx, ok} ->
-            case mod_register:try_register(Username, Host, Password, Source, mod_invites, Lang) of
-                ok ->
-                    InviteeJid = jid:make(Username, Host),
-                    mod_invites:set_invitee(Host, Token, InviteeJid),
-                    UpdatedInvite = mod_invites:get_invite(Host, Token),
-                    mod_invites_register:maybe_create_mutual_subscription(UpdatedInvite),
+            case mod_invites_register:try_register(Invite, Username, Host, Password, Source, Lang) of
+                {ok, _UpdatedInvite} ->
                     Ctx = [{username, Username}, {password, Password} | AppCtx],
                     render_ok(Host, Lang, <<"register_success.html">>, Ctx);
                 {error, #stanza_error{text = Text, type = Type} = Error} ->
diff --git a/src/mod_invites_mnesia.erl b/src/mod_invites_mnesia.erl
@@ -29,7 +29,7 @@
 
 -export([cleanup_expired/1, create_invite/1, expire_tokens/2, get_invite/2, get_invites/2, init/2,
          is_reserved/3, is_token_valid/3, list_invites/1, remove_user/2,
-         set_invitee/3, update_invite/2]).
+         set_invitee/5]).
 
 -include("mod_invites.hrl").
 
@@ -113,14 +113,23 @@ remove_user(User, Server) ->
             <- mnesia:dirty_index_read(invite_token, Inviter, #invite_token.inviter)],
     ok.
 
-set_invitee(_Host, Token, Invitee) ->
+-spec set_invitee(fun(), binary(), binary(), binary(), binary()) -> ok.
+set_invitee(F, _Host, Token, Invitee, AccountName) ->
     Transaction =
         fun() ->
-           [Invite] = mnesia:read(invite_token, Token),
-           mnesia:write(Invite#invite_token{invitee = Invitee})
+           [#invite_token{invitee = <<>>, type = Type} = Invite] = mnesia:read(invite_token, Token),
+           case Type of
+               roster_only ->
+                   <<>> = Invite#invite_token.account_name;
+               _ ->
+                   ok
+           end,
+           case F() of
+               ok ->
+                   ok = mnesia:write(Invite#invite_token{invitee = Invitee, account_name = AccountName});
+               {error, _Res} = Error ->
+                   Error
+           end
         end,
-    {atomic, ok} = mnesia:transaction(Transaction),
-    ok.
-
-update_invite(_Host, Invite) ->
-    mnesia:dirty_write(Invite).
+    {atomic, Res} = mnesia:transaction(Transaction),
+    Res.
diff --git a/src/mod_invites_register.erl b/src/mod_invites_register.erl
@@ -27,7 +27,7 @@
 -author('stefan@strigler.de').
 
 -export([c2s_unauthenticated_packet/2, stream_feature_register/2]).
--export([maybe_create_mutual_subscription/1]).
+-export([try_register/6]).
 
 -import(mod_invites, [roster_add/2, send_presence/3, xdata_field/3]).
 -include("logger.hrl").
@@ -79,28 +79,21 @@ c2s_unauthenticated_packet(#{invite := Invite} = State,
                    {stop, ejabberd_c2s:send(State, ResIQ1)}
                 end);
 c2s_unauthenticated_packet(#{invite := Invite, server := Server} = State,
-                           #iq{type = set, sub_els = [_]} = IQ) ->
+                           #iq{type = set, sub_els = [_], lang = Lang} = IQ) ->
     %% Process registration request after processing token
     ?TRY_SUBTAG(IQ,
                 #register{},
                 fun(Register) ->
                    case check_captcha(mod_register_opt:captcha_protected(Server), Register, IQ) of
                        {ok, {Username, Password}} ->
-                           Token = Invite#invite_token.token,
                            #{ip := IP} = State,
                            {Address, _} = IP,
-                           case try_register(Token, Username, Server, Password, IQ, Address) of
-                               #iq{type = result} = ResIQ ->
-                                   NewInvite =
-                                       maybe_set_account_name(maybe_set_invitee(Invite,
-                                                                                jid:make(Username,
-                                                                                         Server)),
-                                                              Username),
-                                   ok = mod_invites:update_invite(Server, NewInvite),
-                                   ResState = State#{invite => NewInvite},
-                                   maybe_create_mutual_subscription(NewInvite),
-                                   {stop, ejabberd_c2s:send(ResState, ResIQ)};
-                               #iq{type = error} = ResIQ ->
+                           case try_register(Invite, Username, Server, Password, Address, Lang) of
+                               {ok, UpdatedInvite} ->
+                                   ResState = State#{invite => UpdatedInvite},
+                                   {stop, ejabberd_c2s:send(ResState, xmpp:make_iq_result(IQ))};
+                               {error, Err} ->
+                                   ResIQ =  make_stripped_error(IQ, #register{}, Err),
                                    {stop, ejabberd_c2s:send(State, ResIQ)}
                            end;
                        {error, ResIQ} ->
@@ -210,18 +203,23 @@ preauth_invalid(IQ, Lang) ->
     Text = ?T("The token provided is either invalid or expired."),
     make_stripped_error(IQ, #preauth{}, xmpp:err_item_not_found(Text, Lang)).
 
-try_register(Token, User, Server, Password, #iq{lang = Lang} = IQ, Source) ->
+try_register(Invite, User, Server, Password, Source, Lang) ->
+    #invite_token{token = Token} = Invite,
     case {jid:nodeprep(User), not mod_invites:is_reserved(Server, Token, User)} of
         {error, _} ->
-            Err = xmpp:err_jid_malformed(
-                      mod_register:format_error(invalid_jid), Lang),
-            make_stripped_error(IQ, #register{}, Err);
+            {error, xmpp:err_jid_malformed(mod_register:format_error(invalid_jid), Lang)};
         {_, true} ->
-            case mod_register:try_register(User, Server, Password, Source, mod_invites, Lang) of
+            RegF =
+                fun() -> mod_register:try_register(User, Server, Password, Source, mod_invites, Lang)
+                end,
+            NewInvite = #invite_token{invitee = Invitee, account_name = AccountName} =
+                maybe_set_account_name(maybe_set_invitee(Invite, jid:make(User, Server)), User),
+            case mod_invites:set_invitee(RegF, Server, Token, Invitee, AccountName) of
                 ok ->
-                    xmpp:make_iq_result(IQ);
-                {error, Error} ->
-                    make_stripped_error(IQ, #register{}, Error)
+                    maybe_create_mutual_subscription(NewInvite),
+                    {ok, NewInvite};
+                {error, _Reason} = Error ->
+                    Error
             end
     end.
 
diff --git a/src/mod_invites_sql.erl b/src/mod_invites_sql.erl
@@ -29,7 +29,7 @@
 
 -export([cleanup_expired/1, create_invite/1, expire_tokens/2, get_invite/2, get_invites/2, init/2,
          is_reserved/3, is_token_valid/3, list_invites/1, remove_user/2,
-         set_invitee/3, update_invite/2]).
+         set_invitee/5]).
 
 -export([sql_schemas/0]).
 
@@ -186,22 +186,20 @@ remove_user(User, Server) ->
     ejabberd_sql:sql_query(Server,
                            ?SQL("DELETE FROM invite_token WHERE username=%(User)s AND %(Server)H")).
 
-set_invitee(Host, Token, Invitee) ->
-    {updated, 1} =
-        ejabberd_sql:sql_query(Host,
-                               ?SQL("UPDATE invite_token SET invitee=%(Invitee)s WHERE token=%(Token)s")),
-    ok.
-
-update_invite(Host, Invite) ->
-    #invite_token{token = Token,
-                  invitee = Invitee,
-                  account_name = AccountName} =
-        Invite,
-    Query =
-        ?SQL("UPDATE invite_token SET invitee = %(Invitee)s, account_name = %(AccountName)s "
-             "WHERE %(Host)H AND token = %(Token)s"),
-    {updated, 1} = ejabberd_sql:sql_query(Host, Query),
-    ok.
+set_invitee(Fun, Host, Token, Invitee, AccountName) ->
+    Trans =
+        fun() ->
+           {updated, 1} =
+               ejabberd_sql:sql_query_t(?SQL("UPDATE invite_token SET invitee = %(Invitee)s, account_name = %(AccountName)s WHERE "
+                                             "%(Host)H AND token = %(Token)s AND invitee = '' AND (type != 'R' OR account_name = '')")),
+                ok = Fun()
+        end,
+    case ejabberd_sql:sql_transaction(Host, Trans) of
+        {atomic, Res} ->
+            Res;
+        {aborted, {badmatch, {error, _Res} = Error}} ->
+            Error
+    end.
 
 %%--------------------------------------------------------------------
 %%| helpers
diff --git a/test/invites_tests.erl b/test/invites_tests.erl
@@ -66,6 +66,7 @@ single_cases() ->
       single_test(ibr),
       single_test(ibr_reserved),
       single_test(ibr_subscription),
+      single_test(ibr_conflict),
       single_test(http)]}.
 
 %%%===================================================================
@@ -296,7 +297,9 @@ max_invites(Config0) ->
     ?match(#iq{type = error}, send_pars(Config, RosterInviteToken)),
 
     %% we can create more than 3 as an "admin" user
-    ?match(0, length([error || _ <- lists:seq(1, 4), element(1, mod_invites:gen_invite(Server)) == error])),
+    ?match(0,
+           length([error
+                   || _ <- lists:seq(1, 4), element(1, mod_invites:gen_invite(Server)) == error])),
 
     update_module_opts(Server, mod_invites, OldOpts),
     #invite_token{} = create_account_invite(Server, Inviter),
@@ -365,6 +368,7 @@ stream_feature(Config0) ->
 
 ibr(Config0) ->
     Server = ?config(server, Config0),
+    User = ?config(user, Config0),
     AccountName = <<"new_user">>,
 
     OldRegisterOpts = gen_mod:get_module_opts(Server, mod_register),
@@ -380,6 +384,10 @@ ibr(Config0) ->
     #invite_token{token = RosterToken} =
         mod_invites:create_roster_invite(Server, {<<"inviter">>, Server}),
     ?match(#iq{type = result}, send_pars(Config1, RosterToken)),
+    ?match(#iq{type = result}, send_iq_register(Config1, <<"roster_invite_user">>)),
+    %% roster tokens should still be valid because they will be used for roster pars later by the
+    %% client
+    ?match(true, mod_invites:is_token_valid(Server, RosterToken)),
 
     Config11 = reconnect(Config1),
 
@@ -401,7 +409,10 @@ ibr(Config0) ->
     ?match(#iq{type = result}, send_pars(Config2, Token2)),
     ?match(#iq{type = result, sub_els = [#register{username = <<"some_unfavorable_name">>}]},
            send_get_iq_register(Config2)),
+    ?match(#iq{type = error}, send_iq_register(Config2, User)),
+    ?match(#iq{type = error}, send_iq_register(Config2, <<"@invalid_user">>)),
     ?match(#iq{type = result}, send_iq_register(Config2, <<"some_much_better_name">>)),
+    ?match(#iq{type = error}, send_iq_register(Config2, <<"one_more_try">>)),
 
     Config3 = reconnect(Config2),
     #invite_token{token = Token3} = create_account_invite(Server, {<<>>, Server}),
@@ -538,6 +549,70 @@ receive_subscription_stanzas(Count,
           end,
     receive_subscription_stanzas(Count - 1, Res, ServerJID, UserFullJID, NewAccountFullJID).
 
+ibr_conflict(Config0) ->
+    Server = ?config(server, Config0),
+
+    OldRegisterOpts = gen_mod:get_module_opts(Server, mod_register),
+    NewRegisterOpts = gen_mod:set_opt(allow_modules, [mod_invites], OldRegisterOpts),
+    update_module_opts(Server, mod_register, NewRegisterOpts),
+
+    Config1 = reconnect(Config0),
+
+    #invite_token{token = Token} =
+        mod_invites:create_account_invite(Server, {<<>>, Server}, <<>>, false),
+    ?match(#iq{type = result}, send_pars(Config1, Token)),
+    Parent = self(),
+    Pid = spawn(fun() ->
+                   Config11 = connect(lists:keydelete(receiver, 1, Config1)),
+                   #iq{type = result} = send_pars(Config11, Token),
+                   #iq{type = Type} = send_iq_register(Config11, <<"ibr_conflict_1">>),
+                   Parent ! {self(), Type},
+                   disconnect(Config11)
+                end),
+    receive
+        {Pid, Result} ->
+            ?match(result, Result)
+    after 1000 ->
+        ct:fail(timeout)
+    end,
+
+    ?match(#iq{type = result}, send_get_iq_register(Config1)),
+    ?match(#iq{type = error}, send_iq_register(Config1, <<"ibr_conflict_2">>)),
+    ?match(false, ejabberd_auth:user_exists(<<"ibr_conflict_2">>, Server)),
+    ?match(false, mod_invites:is_token_valid(Server, Token)),
+
+    Config2 = reconnect(Config1),
+
+    #invite_token{token = RosterToken} =
+        mod_invites:create_roster_invite(Server, {<<"inviter">>, Server}),
+    ?match(#iq{type = result}, send_pars(Config2, RosterToken)),
+
+    Pid2 =
+        spawn(fun() ->
+                 Config21 = connect(lists:keydelete(receiver, 1, Config2)),
+                 #iq{type = result} = send_pars(Config21, RosterToken),
+                 #iq{type = Type} = send_iq_register(Config21, <<"ibr_conflict_3">>),
+                 Parent ! {self(), Type},
+                 disconnect(Config21)
+              end),
+    receive
+        {Pid2, Result2} ->
+            ?match(result, Result2)
+    after 1000 ->
+        ct:fail(timeout)
+    end,
+
+    ?match(#iq{type = result}, send_get_iq_register(Config2)),
+    ?match(#iq{type = error}, send_iq_register(Config2, <<"ibr_conflict_4">>)),
+    ?match(false, ejabberd_auth:user_exists(<<"ibr_conflict_4">>, Server)),
+    ?match(true, mod_invites:is_token_valid(Server, RosterToken)),
+
+    mod_invites:remove_user(<<"inviter">>, Server),
+    mod_invites:expire_tokens(<<>>, Server),
+    ?match(1, mod_invites:cleanup_expired()),
+    disconnect(Config2),
+    ok.
+
 http(Config) ->
     Server = ?config(server, Config),
     User = ?config(user, Config),
