From 47d898bb8bb11a5a7e9528e0aa61a4c827cb6a87 Mon Sep 17 00:00:00 2001 From: Stefan Strigler Date: Tue, 13 Jan 2026 18:31:56 +0100 Subject: [PATCH] add support for roster pre-approval Implementation of RFC 6121 section 3.4: Pre-Approving a Subscription Request https://datatracker.ietf.org/doc/html/rfc6121#section-3.4 --- include/ejabberd_db_serialize.hrl | 1 + include/mod_roster.hrl | 1 + sql/lite.new.sql | 1 + sql/lite.sql | 1 + sql/mssql.new.sql | 1 + sql/mssql.sql | 1 + sql/mysql.new.sql | 1 + sql/mysql.sql | 1 + sql/pg.new.sql | 1 + sql/pg.sql | 1 + src/mod_roster.erl | 136 +++++++++++++++++---- src/mod_roster_mnesia.erl | 15 +++ src/mod_roster_sql.erl | 70 +++++++++-- test/ejabberd_SUITE.erl | 14 +-- test/roster_tests.erl | 194 +++++++++++++++++++----------- test/suite.erl | 3 + 16 files changed, 328 insertions(+), 114 deletions(-) diff --git a/include/ejabberd_db_serialize.hrl b/include/ejabberd_db_serialize.hrl index 2ef3304e9..504fa4094 100644 --- a/include/ejabberd_db_serialize.hrl +++ b/include/ejabberd_db_serialize.hrl @@ -44,6 +44,7 @@ binary(), [binary()], both | from | to | none, + boolean(), subscribe | unsubscribe | both | in | out | none, binary()}] }). diff --git a/include/mod_roster.hrl b/include/mod_roster.hrl index d8df208d8..7f63336cd 100644 --- a/include/mod_roster.hrl +++ b/include/mod_roster.hrl @@ -25,6 +25,7 @@ jid = {<<>>, <<>>, <<>>} :: jid:ljid(), name = <<>> :: binary() | '_', subscription = none :: subscription() | '_', + approved = false :: boolean() | '_', ask = none :: ask() | '_', groups = [] :: [binary()] | '_', askmessage = <<"">> :: binary() | '_', diff --git a/sql/lite.new.sql b/sql/lite.new.sql index c267f369e..d39590281 100644 --- a/sql/lite.new.sql +++ b/sql/lite.new.sql @@ -44,6 +44,7 @@ CREATE TABLE rosterusers ( jid text NOT NULL, nick text NOT NULL, subscription character(1) NOT NULL, + approved boolean NOT NULL, ask character(1) NOT NULL, askmessage text NOT NULL, server character(1) NOT NULL, diff --git a/sql/lite.sql b/sql/lite.sql index d9fa400b9..a08d6242b 100644 --- a/sql/lite.sql +++ b/sql/lite.sql @@ -40,6 +40,7 @@ CREATE TABLE rosterusers ( jid text NOT NULL, nick text NOT NULL, subscription character(1) NOT NULL, + approved boolean NOT NULL, ask character(1) NOT NULL, askmessage text NOT NULL, server character(1) NOT NULL, diff --git a/sql/mssql.new.sql b/sql/mssql.new.sql index 7a3a3168b..2739d19a8 100644 --- a/sql/mssql.new.sql +++ b/sql/mssql.new.sql @@ -324,6 +324,7 @@ CREATE TABLE [dbo].[rosterusers] ( [jid] [varchar] (250) NOT NULL, [nick] [text] NOT NULL, [subscription] [char] (1) NOT NULL, + [approved] [smallint] NOT NULL, [ask] [char] (1) NOT NULL, [askmessage] [text] NOT NULL, [server] [char] (1) NOT NULL, diff --git a/sql/mssql.sql b/sql/mssql.sql index 19522ace2..9af6a518a 100644 --- a/sql/mssql.sql +++ b/sql/mssql.sql @@ -305,6 +305,7 @@ CREATE TABLE [dbo].[rosterusers] ( [jid] [varchar] (250) NOT NULL, [nick] [text] NOT NULL, [subscription] [char] (1) NOT NULL, + [approved] [smallint] NOT NULL, [ask] [char] (1) NOT NULL, [askmessage] [text] NOT NULL, [server] [char] (1) NOT NULL, diff --git a/sql/mysql.new.sql b/sql/mysql.new.sql index d732fb24a..ecdc9567c 100644 --- a/sql/mysql.new.sql +++ b/sql/mysql.new.sql @@ -48,6 +48,7 @@ CREATE TABLE rosterusers ( jid varchar(191) NOT NULL, nick text NOT NULL, subscription character(1) NOT NULL, + approved boolean NOT NULL, ask character(1) NOT NULL, askmessage text NOT NULL, server character(1) NOT NULL, diff --git a/sql/mysql.sql b/sql/mysql.sql index 0aa9e799c..79779ef2c 100644 --- a/sql/mysql.sql +++ b/sql/mysql.sql @@ -44,6 +44,7 @@ CREATE TABLE rosterusers ( jid varchar(191) NOT NULL, nick text NOT NULL, subscription character(1) NOT NULL, + approved boolean NOT NULL, ask character(1) NOT NULL, askmessage text NOT NULL, server character(1) NOT NULL, diff --git a/sql/pg.new.sql b/sql/pg.new.sql index c63f9b97a..999092919 100644 --- a/sql/pg.new.sql +++ b/sql/pg.new.sql @@ -201,6 +201,7 @@ CREATE TABLE rosterusers ( jid text NOT NULL, nick text NOT NULL, subscription character(1) NOT NULL, + approved boolean NOT NULL, ask character(1) NOT NULL, askmessage text NOT NULL, server character(1) NOT NULL, diff --git a/sql/pg.sql b/sql/pg.sql index 39a8a2835..182e6636b 100644 --- a/sql/pg.sql +++ b/sql/pg.sql @@ -44,6 +44,7 @@ CREATE TABLE rosterusers ( jid text NOT NULL, nick text NOT NULL, subscription character(1) NOT NULL, + approved boolean NOT NULL, ask character(1) NOT NULL, askmessage text NOT NULL, server character(1) NOT NULL, diff --git a/src/mod_roster.erl b/src/mod_roster.erl index d840371cc..ae7a53390 100644 --- a/src/mod_roster.erl +++ b/src/mod_roster.erl @@ -44,9 +44,11 @@ import_info/0, process_local_iq/1, get_user_roster_items/2, import/5, get_roster/2, push_item/3, import_start/2, import_stop/2, is_subscribed/2, + user_send_packet/1, c2s_self_presence/1, in_subscription/2, out_subscription/1, set_items/3, remove_user/2, - get_jid_info/4, encode_item/1, get_versioning_feature/2, + get_jid_info/4, encode_item/1, + get_versioning_feature/2, pre_approval_stream_feature/2, roster_version/2, mod_doc/0, mod_opt_type/1, mod_options/1, set_roster/1, del_roster/3, process_rosteritems/5, @@ -101,6 +103,8 @@ start(Host, Opts) -> {hook, remove_user, remove_user, 50}, {hook, c2s_self_presence, c2s_self_presence, 50}, {hook, c2s_post_auth_features, get_versioning_feature, 50}, + {hook, c2s_post_auth_features, pre_approval_stream_feature, 50}, + {hook, user_send_packet, user_send_packet, 50}, {hook, webadmin_menu_hostuser, webadmin_menu_hostuser, 50}, {hook, webadmin_page_hostuser, webadmin_page_hostuser, 50}, {hook, webadmin_user, webadmin_user, 50}, @@ -205,6 +209,16 @@ get_versioning_feature(Acc, Host) -> Acc end. +%% Indicate support for pre-approval as of RFC6121 section 3.4 +-spec pre_approval_stream_feature([xmpp_element()], binary()) -> [xmpp_element()]. +pre_approval_stream_feature(Acc, Host) -> + case gen_mod:is_loaded(Host, ?MODULE) of + true -> + [#feature_pre_approval{} | Acc]; + false -> + Acc + end. + -spec roster_version(binary(), binary()) -> undefined | binary(). roster_version(LServer, LUser) -> case mod_roster_opt:store_current_id(LServer) of @@ -406,6 +420,7 @@ encode_item(Item) -> both -> subscribe; _ -> undefined end, + approved = Item#roster.approved, groups = Item#roster.groups}. -spec decode_item(roster_item(), #roster{}, boolean()) -> #roster{}. @@ -416,6 +431,7 @@ decode_item(#roster_item{subscription = remove} = Item, R, _) -> ask = none, groups = [], askmessage = <<"">>, + approved = false, xs = []}; decode_item(Item, R, Managed) -> R#roster{jid = jid:tolower(Item#roster_item.jid), @@ -424,6 +440,7 @@ decode_item(Item, R, Managed) -> Sub when Managed -> Sub; _ -> R#roster.subscription end, + approved = Item#roster_item.approved, groups = Item#roster_item.groups}. -spec process_iq_set(iq()) -> iq(). @@ -574,41 +591,58 @@ process_subscription(Direction, User, Server, JID1, Item = get_roster_item(LUser, LServer, LJID), NewState = case Direction of out -> - out_state_change(Item#roster.subscription, - Item#roster.ask, Type); + out_state_change(Item#roster.subscription, + Item#roster.ask, Type); in -> - in_state_change(Item#roster.subscription, - Item#roster.ask, Type) + case {Type, Item#roster.approved} of + {subscribe, true} -> + {TSub, TAsk} = in_state_change(Item#roster.subscription, + Item#roster.ask, Type), + out_state_change(TSub, TAsk, subscribed); + _ -> + in_state_change(Item#roster.subscription, + Item#roster.ask, Type) + end end, AutoReply = case Direction of out -> none; in -> - in_auto_reply(Item#roster.subscription, - Item#roster.ask, Type) + case {Type, Item#roster.approved} of + {subscribe, true} -> + subscribed; + _ -> + in_auto_reply(Item#roster.subscription, + Item#roster.ask, Type) + end end, AskMessage = case NewState of {_, both} -> Reason; {_, in} -> Reason; _ -> <<"">> end, + {Unapproved, Approved} = case {Direction, Type, Item#roster.approved} of + {out, unsubscribed, true} -> + {true, false}; + {_, _, Approved0} -> + {false, Approved0} + end, case NewState of none -> - {none, AutoReply}; + NewItem = update_item(Item, Item#roster.subscription, Approved, Item#roster.ask, SubEls, AskMessage), + {maybe_push_item(Unapproved, LUser, LServer, LJID, Item, NewItem), AutoReply}; {none, none} when Item#roster.subscription == none, Item#roster.ask == in -> - del_roster_t(LUser, LServer, LJID), {none, AutoReply}; + del_roster_t(LUser, LServer, LJID), + case Unapproved of + true -> + NewItem = update_item(Item, none, Approved, none, SubEls, AskMessage), + {{push, Item, NewItem}, AutoReply}; + false -> + {none, AutoReply} + end; {Subscription, Pending} -> - NewItem = Item#roster{subscription = Subscription, - ask = Pending, - name = get_nick_subels(SubEls, Item#roster.name), - xs = SubEls, - askmessage = AskMessage}, - roster_subscribe_t(LUser, LServer, LJID, NewItem), - case mod_roster_opt:store_current_id(LServer) of - true -> write_roster_version_t(LUser, LServer); - false -> ok - end, - {{push, Item, NewItem}, AutoReply} + NewItem = update_item(Item, Subscription, Approved, Pending, SubEls, AskMessage), + {prepare_push_item(LUser, LServer, LJID, Item, NewItem), AutoReply} end end, case transaction(LUser, LServer, [LJID], F) of @@ -631,7 +665,7 @@ process_subscription(Direction, User, Server, JID1, encode_item(OldItem), encode_item(NewItem)) end, - true; + not (Type == subscribe andalso Direction == in andalso NewItem#roster.approved); none -> false end; @@ -639,12 +673,33 @@ process_subscription(Direction, User, Server, JID1, false end. +update_item(Item, Subscription, Approved, Pending, SubEls, AskMessage) -> + Item#roster{subscription = Subscription, + approved = Approved, + ask = Pending, + name = get_nick_subels(SubEls, Item#roster.name), + xs = SubEls, + askmessage = AskMessage}. + get_nick_subels(SubEls, Default) -> case xmpp:get_subtag(#presence{sub_els = SubEls}, #nick{}) of {nick, N} -> N; _ -> Default end. +maybe_push_item(true, LUser, LServer, LJID, OldItem, NewItem) -> + prepare_push_item(LUser, LServer, LJID, OldItem, NewItem); +maybe_push_item(false, _LUser, _LServer, _LJID, _OldItem, _NewItem) -> + none. + +prepare_push_item(LUser, LServer, LJID, OldItem, NewItem) -> + roster_subscribe_t(LUser, LServer, LJID, NewItem), + case mod_roster_opt:store_current_id(LServer) of + true -> write_roster_version_t(LUser, LServer); + false -> ok + end, + {push, OldItem, NewItem}. + %% in_state_change(Subscription, Pending, Type) -> NewState %% NewState = none | {NewSubscription, NewPending} -ifdef(ROSTER_GATEWAY_WORKAROUND). @@ -946,6 +1001,43 @@ process_item_set_t(LUser, LServer, #roster_item{jid = JID1} = QueryItem) -> end; process_item_set_t(_LUser, _LServer, _) -> ok. +-spec user_send_packet({stanza(), ejabberd_c2s:state()}) -> {stanza(), ejabberd_c2s:state()}. +user_send_packet({#presence{type = subscribed, to = To} = Presence, + #{jid := #jid{luser = LUser, lserver = LServer} = Jid} = C2SState}) -> + LJID = jid:tolower(To), + {atomic, Item} = transaction( + LUser, LServer, [LJID], + fun() -> + get_roster_item(LUser, LServer, LJID) + end), + case Item of + #roster{subscription = Subscription, ask = Ask} + when Subscription == both; + Subscription == from, Ask == none; + Subscription == from, Ask == out -> + {drop, C2SState}; + #roster{subscription = Subscription, ask = Ask} + when Subscription == to, Ask == in; + Subscription == none, Ask == in; + Subscription == none, Ask == both -> + {Presence, C2SState}; + #roster{subscription = Subscription, ask = Ask} + when Subscription == to; + Subscription == none, Ask == none; + Subscription == none, Ask == out -> + transaction( + LUser, LServer, [LJID], + fun() -> + update_roster_t(LUser, LServer, LJID, Item#roster{approved = true}) + end), + OldItem = encode_item(Item), + NewItem = OldItem#roster_item{approved = true}, + push_item(Jid, OldItem, NewItem), + {drop, C2SState} + end; +user_send_packet(Acc) -> + Acc. + -spec c2s_self_presence({presence(), c2s_state()}) -> {presence(), c2s_state()}. c2s_self_presence({_, #{pres_last := _}} = Acc) -> Acc; @@ -1179,7 +1271,7 @@ export(LServer) -> import_info() -> [{<<"roster_version">>, 2}, {<<"rostergroups">>, 3}, - {<<"rosterusers">>, 10}]. + {<<"rosterusers">>, 11}]. import_start(LServer, DBType) -> Mod = gen_mod:db_mod(DBType, ?MODULE), diff --git a/src/mod_roster_mnesia.erl b/src/mod_roster_mnesia.erl index 0021125de..86ec10cbc 100644 --- a/src/mod_roster_mnesia.erl +++ b/src/mod_roster_mnesia.erl @@ -130,9 +130,24 @@ need_transform({roster_version, {U, S}, Ver}) when is_list(U) orelse is_list(S) orelse is_list(Ver) -> ?INFO_MSG("Mnesia table 'roster_version' will be converted to binary", []), true; +need_transform({roster, {U, S, _}, _, _, _, _, none, _, _, _, _}) -> + ?INFO_MSG("Mnesia table 'roster' will be converted to use new boolean() 'approved' attribute.", []), + true; need_transform(_) -> false. +transform({roster, USJ, US, Jid, Name, Sub, none, Ask, Groups, AskMsg, XS}) -> + #roster{ + us = US, + usj = USJ, + jid = Jid, + name = Name, + subscription = Sub, + approved = false, + ask = Ask, + groups = Groups, + askmessage = AskMsg, + xs = XS}; transform(#roster{usj = {U, S, {LU, LS, LR}}, us = {U1, S1}, jid = {U2, S2, R2}, diff --git a/src/mod_roster_sql.erl b/src/mod_roster_sql.erl index ca014ea45..227c56abd 100644 --- a/src/mod_roster_sql.erl +++ b/src/mod_roster_sql.erl @@ -53,6 +53,51 @@ init(Host, _Opts) -> sql_schemas() -> [#sql_schema{ + version = 2, + tables = + [#sql_table{ + name = <<"rosterusers">>, + columns = + [#sql_column{name = <<"username">>, type = text}, + #sql_column{name = <<"server_host">>, type = text}, + #sql_column{name = <<"jid">>, type = text}, + #sql_column{name = <<"nick">>, type = text}, + #sql_column{name = <<"subscription">>, type = {char, 1}}, + #sql_column{name = <<"approved">>, type = boolean}, + #sql_column{name = <<"ask">>, type = {char, 1}}, + #sql_column{name = <<"askmessage">>, type = text}, + #sql_column{name = <<"server">>, type = {char, 1}}, + #sql_column{name = <<"subscribe">>, type = text}, + #sql_column{name = <<"type">>, type = text}, + #sql_column{name = <<"created_at">>, type = timestamp, + default = true}], + indices = [#sql_index{ + columns = [<<"server_host">>, <<"username">>, + <<"jid">>], + unique = true}, + #sql_index{ + columns = [<<"server_host">>, <<"jid">>]}]}, + #sql_table{ + name = <<"rostergroups">>, + columns = + [#sql_column{name = <<"username">>, type = text}, + #sql_column{name = <<"server_host">>, type = text}, + #sql_column{name = <<"jid">>, type = text}, + #sql_column{name = <<"grp">>, type = text}], + indices = [#sql_index{ + columns = [<<"server_host">>, <<"username">>, + <<"jid">>]}]}, + #sql_table{ + name = <<"roster_version">>, + columns = + [#sql_column{name = <<"username">>, type = text}, + #sql_column{name = <<"server_host">>, type = text}, + #sql_column{name = <<"version">>, type = text}], + indices = [#sql_index{ + columns = [<<"server_host">>, <<"username">>], + unique = true}]}], + update = [{add_column, <<"rosterusers">>, <<"approved">>}]}, + #sql_schema{ version = 1, tables = [#sql_table{ @@ -118,7 +163,7 @@ write_roster_version(LUser, LServer, InTransaction, Ver) -> get_roster(LUser, LServer) -> case ejabberd_sql:sql_query( LServer, - ?SQL("select @(username)s, @(jid)s, @(nick)s, @(subscription)s, " + ?SQL("select @(username)s, @(jid)s, @(nick)s, @(subscription)s, @(approved)b, " "@(ask)s, @(askmessage)s, @(server)s, @(subscribe)s, " "@(type)s from rosterusers " "where username=%(LUser)s and %(LServer)H")) of @@ -281,7 +326,7 @@ get_roster_groups(LServer, LUser, SJID) -> ?SQL("select @(grp)s from rostergroups" " where username=%(LUser)s and %(LServer)H and jid=%(SJID)s")). -roster_subscribe({LUser, LServer, SJID, Name, SSubscription, SAsk, AskMessage}) -> +roster_subscribe({LUser, LServer, SJID, Name, SSubscription, BApproved, SAsk, AskMessage}) -> ?SQL_UPSERT_T( "rosterusers", ["!username=%(LUser)s", @@ -289,6 +334,7 @@ roster_subscribe({LUser, LServer, SJID, Name, SSubscription, SAsk, AskMessage}) "!jid=%(SJID)s", "nick=%(Name)s", "subscription=%(SSubscription)s", + "approved=%(BApproved)b", "ask=%(SAsk)s", "askmessage=%(AskMessage)s", "server='N'", @@ -297,7 +343,7 @@ roster_subscribe({LUser, LServer, SJID, Name, SSubscription, SAsk, AskMessage}) get_roster_by_jid(LServer, LUser, SJID) -> ejabberd_sql:sql_query_t( - ?SQL("select @(username)s, @(jid)s, @(nick)s, @(subscription)s," + ?SQL("select @(username)s, @(jid)s, @(nick)s, @(subscription)s, @(approved)b, " " @(ask)s, @(askmessage)s, @(server)s, @(subscribe)s," " @(type)s from rosterusers" " where username=%(LUser)s and %(LServer)H and jid=%(SJID)s")). @@ -314,7 +360,7 @@ get_subscription(LServer, LUser, SJID) -> ?SQL("select @(subscription)s, @(ask)s from rosterusers " "where username=%(LUser)s and %(LServer)H and jid=%(SJID)s")). -update_roster_sql({LUser, LServer, SJID, Name, SSubscription, SAsk, AskMessage}, +update_roster_sql({LUser, LServer, SJID, Name, SSubscription, BApproved, SAsk, AskMessage}, ItemGroups) -> [?SQL("delete from rosterusers where" " username=%(LUser)s and %(LServer)H and jid=%(SJID)s;"), @@ -325,6 +371,7 @@ update_roster_sql({LUser, LServer, SJID, Name, SSubscription, SAsk, AskMessage}, "jid=%(SJID)s", "nick=%(Name)s", "subscription=%(SSubscription)s", + "approved=%(BApproved)b", "ask=%(SAsk)s", "askmessage=%(AskMessage)s", "server='N'", @@ -342,19 +389,19 @@ update_roster_sql({LUser, LServer, SJID, Name, SSubscription, SAsk, AskMessage}, || ItemGroup <- ItemGroups]. raw_to_record(LServer, - [User, LServer, SJID, Nick, SSubscription, SAsk, SAskMessage, + [User, LServer, SJID, Nick, SSubscription, BApproved, SAsk, SAskMessage, SServer, SSubscribe, SType]) -> raw_to_record(LServer, - {User, LServer, SJID, Nick, SSubscription, SAsk, SAskMessage, + {User, LServer, SJID, Nick, SSubscription, BApproved, SAsk, SAskMessage, SServer, SSubscribe, SType}); raw_to_record(LServer, - {User, SJID, Nick, SSubscription, SAsk, SAskMessage, + {User, SJID, Nick, SSubscription, BApproved, SAsk, SAskMessage, SServer, SSubscribe, SType}) -> raw_to_record(LServer, - {User, LServer, SJID, Nick, SSubscription, SAsk, SAskMessage, + {User, LServer, SJID, Nick, SSubscription, BApproved, SAsk, SAskMessage, SServer, SSubscribe, SType}); raw_to_record(LServer, - {User, LServer, SJID, Nick, SSubscription, SAsk, SAskMessage, + {User, LServer, SJID, Nick, SSubscription, BApproved, SAsk, SAskMessage, _SServer, _SSubscribe, _SType}) -> try jid:decode(SJID) of JID -> @@ -363,7 +410,7 @@ raw_to_record(LServer, Ask = decode_ask(User, LServer, SAsk), #roster{usj = {User, LServer, LJID}, us = {User, LServer}, jid = LJID, name = Nick, - subscription = Subscription, ask = Ask, + subscription = Subscription, approved = BApproved, ask = Ask, askmessage = SAskMessage} catch _:{bad_jid, _} -> ?ERROR_MSG("~ts", [format_row_error(User, LServer, {jid, SJID})]), @@ -373,13 +420,14 @@ raw_to_record(LServer, record_to_row( #roster{us = {LUser, LServer}, jid = JID, name = Name, subscription = Subscription, - ask = Ask, askmessage = AskMessage}) -> + approved = Approved, ask = Ask, askmessage = AskMessage}) -> SJID = jid:encode(jid:tolower(JID)), {LUser, LServer, SJID, Name, encode_subscription(Subscription), + Approved, encode_ask(Ask), AskMessage}. diff --git a/test/ejabberd_SUITE.erl b/test/ejabberd_SUITE.erl index 678bdb421..0474ed725 100644 --- a/test/ejabberd_SUITE.erl +++ b/test/ejabberd_SUITE.erl @@ -345,21 +345,11 @@ init_per_testcase(TestCase, OrigConfig) -> bind(auth(connect(Config))); "replaced" ++ _ -> auth(connect(Config)); - "antispam" ++ _ -> - Password = ?config(password, Config), - ejabberd_auth:try_register(User, Server, Password), - open_session(bind(auth(connect(Config)))); - "invites_" ++ _ -> - Password = ?config(password, Config), - ejabberd_auth:try_register(User, Server, Password), - open_session(bind(auth(connect(Config)))); - _ when IsMaster or IsSlave -> - Password = ?config(password, Config), - ejabberd_auth:try_register(User, Server, Password), - open_session(bind(auth(connect(Config)))); _ when TestGroup == s2s_tests -> auth(connect(starttls(connect(Config)))); _ -> + Password = ?config(password, Config), + ejabberd_auth:try_register(User, Server, Password), open_session(bind(auth(connect(Config)))) end. diff --git a/test/roster_tests.erl b/test/roster_tests.erl index 5e60d23b1..5385de3e1 100644 --- a/test/roster_tests.erl +++ b/test/roster_tests.erl @@ -35,7 +35,8 @@ -record(state, {subscription = none :: none | from | to | both, peer_available = false, pending_in = false :: boolean(), - pending_out = false :: boolean()}). + pending_out = false :: boolean(), + approved = false :: boolean()}). %%%=================================================================== %%% API @@ -51,7 +52,7 @@ stop(_TestCase, Config) -> %%%=================================================================== single_cases() -> {roster_single, [sequence], - [single_test(feature_enabled), + [single_test(features_enabled), single_test(iq_set_many_items), single_test(iq_set_duplicated_groups), single_test(iq_get_item), @@ -60,9 +61,10 @@ single_cases() -> single_test(set_item), single_test(version)]}. -feature_enabled(Config) -> - ct:comment("Checking if roster versioning stream feature is set"), +features_enabled(Config) -> + ct:comment("Checking if roster stream feature (versioning and pre-approval) is set"), true = ?config(rosterver, Config), + true = ?config(pre_approval, Config), disconnect(Config). set_item(Config) -> @@ -160,6 +162,7 @@ subscribe_slave(Config) -> process_subscriptions_master(Config, Actions) -> EnumeratedActions = lists:zip(lists:seq(1, length(Actions)), Actions), + ct:pal("actions: ~p", [EnumeratedActions]), self_presence(Config, available), Peer = ?config(peer, Config), lists:foldl( @@ -265,10 +268,14 @@ recv_push(Config) -> {Ver, PushItem}. recv_push(Config, Subscription, Ask) -> + recv_push(Config, Subscription, Ask, false). + +recv_push(Config, Subscription, Ask, Approved) -> PeerJID = ?config(peer, Config), PeerBareJID = jid:remove_resource(PeerJID), Match = #roster_item{jid = PeerBareJID, subscription = Subscription, + approved = Approved, ask = Ask, groups = [], name = <<"">>}, @@ -316,11 +323,12 @@ check_roster([], _Config, _State) -> ok; check_roster([Roster], _Config, State) -> case {Roster#roster.subscription == State#state.subscription, + Roster#roster.approved == State#state.approved, Roster#roster.ask, State#state.pending_in, State#state.pending_out} of - {true, both, true, true} -> ok; - {true, in, true, false} -> ok; - {true, out, false, true} -> ok; - {true, none, false, false} -> ok; + {true, true, both, true, true} -> ok; + {true, true, in, true, false} -> ok; + {true, true, out, false, true} -> ok; + {true, true, none, false, false} -> ok; _ -> ct:fail({roster_mismatch, State, Roster}) end. @@ -340,43 +348,50 @@ check_roster_item(Config, State) -> %% RFC6121, A.2.1 transition(Id, Config, out, subscribe, - #state{subscription = Sub, pending_in = In, pending_out = Out} = State) -> + #state{subscription = Sub, pending_in = In, pending_out = Out, approved = Approved} = State) -> PeerJID = ?config(peer, Config), PeerBareJID = jid:remove_resource(PeerJID), + {is_approved, PeerApproved} = get_event(Config), send(Config, #presence{id = Id, to = PeerBareJID, type = subscribe}), - case {Sub, Out, In} of - {none, false, _} -> - recv_push(Config, none, subscribe), + case {Sub, Out, In, PeerApproved} of + {none, false, _, true} -> + recv_push(Config, none, subscribe, Approved), + recv_push(Config, to, undefined, Approved), + recv_subscription(Config, subscribed), + recv_presence(Config, available), + State#state{subscription = to}; + {none, false, _, false} -> + recv_push(Config, none, subscribe, Approved), State#state{pending_out = true}; - {none, true, false} -> + {none, true, false, false} -> %% BUG: we should not receive roster push here - recv_push(Config, none, subscribe), + recv_push(Config, none, subscribe, Approved), State; - {from, false, false} -> - recv_push(Config, from, subscribe), + {from, false, false, false} -> + recv_push(Config, from, subscribe, Approved), State#state{pending_out = true}; _ -> State end; %% RFC6121, A.2.2 transition(Id, Config, out, unsubscribe, - #state{subscription = Sub, pending_in = In, pending_out = Out} = State) -> + #state{subscription = Sub, pending_in = In, pending_out = Out, approved = Approved} = State) -> PeerJID = ?config(peer, Config), PeerBareJID = jid:remove_resource(PeerJID), send(Config, #presence{id = Id, to = PeerBareJID, type = unsubscribe}), case {Sub, Out, In} of {none, true, _} -> - recv_push(Config, none, undefined), + recv_push(Config, none, undefined, Approved), State#state{pending_out = false}; {to, false, _} -> - recv_push(Config, none, undefined), + recv_push(Config, none, undefined, Approved), recv_presence(Config, unavailable), State#state{subscription = none, peer_available = false}; {from, true, false} -> - recv_push(Config, from, undefined), + recv_push(Config, from, undefined, Approved), State#state{pending_out = false}; {both, false, false} -> - recv_push(Config, from, undefined), + recv_push(Config, from, undefined, Approved), recv_presence(Config, unavailable), State#state{subscription = from, peer_available = false}; _ -> @@ -390,59 +405,94 @@ transition(Id, Config, out, subscribed, send(Config, #presence{id = Id, to = PeerBareJID, type = subscribed}), case {Sub, Out, In} of {none, false, true} -> + put_event(Config, {is_approved, false}), recv_push(Config, from, undefined), State#state{subscription = from, pending_in = false}; {none, true, true} -> + put_event(Config, {is_approved, false}), recv_push(Config, from, subscribe), State#state{subscription = from, pending_in = false}; {to, false, true} -> - recv_push(Config, both, undefined), - State#state{subscription = both, pending_in = false}; - {to, false, _} -> + put_event(Config, {is_approved, false}), %% BUG: we should not transition to 'both' state recv_push(Config, both, undefined), - State#state{subscription = both}; + State#state{subscription = both, pending_in = false}; + {to, false, false} -> + put_event(Config, {is_approved, true}), + recv_push(Config, to, undefined, true), + State#state{approved = true}; + {none, true, false} -> + put_event(Config, {is_approved, true}), + recv_push(Config, none, subscribe, true), + State#state{approved = true}; + {none, false, false} -> + put_event(Config, {is_approved, true}), + recv_push(Config, none, undefined, true), + State#state{approved = true}; _ -> + put_event(Config, {is_approved, false}), State end; %% RFC6121, A.2.4 transition(Id, Config, out, unsubscribed, - #state{subscription = Sub, pending_in = In, pending_out = Out} = State) -> + #state{subscription = Sub, pending_in = In, pending_out = Out, approved = Approved} = State) -> PeerJID = ?config(peer, Config), PeerBareJID = jid:remove_resource(PeerJID), send(Config, #presence{id = Id, to = PeerBareJID, type = unsubscribed}), - case {Sub, Out, In} of - {none, false, true} -> - State#state{subscription = none, pending_in = false}; - {none, true, true} -> - recv_push(Config, none, subscribe), - State#state{subscription = none, pending_in = false}; - {to, _, true} -> - State#state{pending_in = false}; - {from, false, _} -> + case {Sub, Out, In, Approved} of + {none, false, true, true} -> recv_push(Config, none, undefined), - State#state{subscription = none}; - {from, true, _} -> + State#state{subscription = none, pending_in = false, approved = false}; + {none, false, true, false} -> + State#state{subscription = none, pending_in = false, approved = false}; + {none, true, true, _} -> recv_push(Config, none, subscribe), - State#state{subscription = none}; - {both, _, _} -> + State#state{subscription = none, pending_in = false, approved = false}; + {to, _, true, true} -> recv_push(Config, to, undefined), - State#state{subscription = to}; + State#state{pending_in = false, approved = false}; + {to, _, true, false} -> + State#state{pending_in = false, approved = false}; + {from, false, _, _} -> + recv_push(Config, none, undefined), + State#state{subscription = none, approved = false}; + {from, true, _, _} -> + recv_push(Config, none, subscribe), + State#state{subscription = none, approved = false}; + {both, _, _, _} -> + recv_push(Config, to, undefined), + State#state{subscription = to, approved = false}; + {_, true, _, true} -> + recv_push(Config, Sub, subscribe), + State#state{approved = false}; + {_, false, _, true} -> + recv_push(Config, Sub, undefined), + State#state{approved = false}; _ -> - State + State#state{approved = false} end; %% RFC6121, A.3.1 transition(_, Config, in, subscribe = Type, - #state{subscription = Sub, pending_in = In, pending_out = Out} = State) -> - case {Sub, Out, In} of - {none, false, false} -> + #state{subscription = Sub, pending_in = In, pending_out = Out, approved = Approved} = State) -> + put_event(Config, {is_approved, Approved}), + case {Sub, Out, In, Approved} of + {to, false, false, true} -> + recv_push(Config, both, undefined, true), + State#state{subscription = both}; + {none, true, false, true} -> + recv_push(Config, from, subscribe, true), + State#state{subscription = from}; + {none, false, false, true} -> + recv_push(Config, from, undefined, true), + State#state{subscription = from}; + {none, false, false, false} -> recv_subscription(Config, Type), State#state{pending_in = true}; - {none, true, false} -> + {none, true, false, false} -> recv_push(Config, none, subscribe), recv_subscription(Config, Type), State#state{pending_in = true}; - {to, false, false} -> + {to, false, false, false} -> %% BUG: we should not receive roster push in this state! recv_push(Config, to, undefined), recv_subscription(Config, Type), @@ -452,24 +502,24 @@ transition(_, Config, in, subscribe = Type, end; %% RFC6121, A.3.2 transition(_, Config, in, unsubscribe = Type, - #state{subscription = Sub, pending_in = In, pending_out = Out} = State) -> + #state{subscription = Sub, pending_in = In, pending_out = Out, approved = Approved} = State) -> case {Sub, Out, In} of {none, _, true} -> State#state{pending_in = false}; {to, _, true} -> - recv_push(Config, to, undefined), + recv_push(Config, to, undefined, Approved), recv_subscription(Config, Type), State#state{pending_in = false}; {from, false, _} -> - recv_push(Config, none, undefined), + recv_push(Config, none, undefined, Approved), recv_subscription(Config, Type), State#state{subscription = none}; {from, true, _} -> - recv_push(Config, none, subscribe), + recv_push(Config, none, subscribe, Approved), recv_subscription(Config, Type), State#state{subscription = none}; {both, _, _} -> - recv_push(Config, to, undefined), + recv_push(Config, to, undefined, Approved), recv_subscription(Config, Type), State#state{subscription = to}; _ -> @@ -477,52 +527,58 @@ transition(_, Config, in, unsubscribe = Type, end; %% RFC6121, A.3.3 transition(_, Config, in, subscribed = Type, - #state{subscription = Sub, pending_in = In, pending_out = Out} = State) -> + #state{subscription = Sub, pending_in = In, pending_out = Out, approved = Approve} = State) -> + {is_approved, PeerApproved} = get_event(Config), case {Sub, Out, In} of {none, true, _} -> - recv_push(Config, to, undefined), + recv_push(Config, to, undefined, Approve), recv_subscription(Config, Type), recv_presence(Config, available), State#state{subscription = to, pending_out = false, peer_available = true}; {from, true, _} -> - recv_push(Config, both, undefined), + recv_push(Config, both, undefined, Approve), recv_subscription(Config, Type), recv_presence(Config, available), State#state{subscription = both, pending_out = false, peer_available = true}; {from, false, _} -> - %% BUG: we should not transition to 'both' in this state - recv_push(Config, both, undefined), - recv_subscription(Config, Type), - recv_presence(Config, available), - State#state{subscription = both, pending_out = false, peer_available = true}; + case PeerApproved of + false -> + %% BUG: we should not transition to 'both' in this state + recv_push(Config, both, undefined, Approve), + recv_subscription(Config, Type), + recv_presence(Config, available), + State#state{subscription = both, pending_out = false, peer_available = true}; + true -> + State + end; _ -> State end; %% RFC6121, A.3.4 transition(_, Config, in, unsubscribed = Type, - #state{subscription = Sub, pending_in = In, pending_out = Out} = State) -> + #state{subscription = Sub, pending_in = In, pending_out = Out, approved = Approved} = State) -> case {Sub, Out, In} of {none, true, true} -> %% BUG: we should receive roster push in this state! recv_subscription(Config, Type), State#state{subscription = none, pending_out = false}; {none, true, false} -> - recv_push(Config, none, undefined), + recv_push(Config, none, undefined, Approved), recv_subscription(Config, Type), State#state{subscription = none, pending_out = false}; {none, false, false} -> State; {to, false, _} -> - recv_push(Config, none, undefined), + recv_push(Config, none, undefined, Approved), recv_presence(Config, unavailable), recv_subscription(Config, Type), State#state{subscription = none, peer_available = false}; {from, true, false} -> - recv_push(Config, from, undefined), + recv_push(Config, from, undefined, Approved), recv_subscription(Config, Type), State#state{subscription = from, pending_out = false}; {both, _, _} -> - recv_push(Config, from, undefined), + recv_push(Config, from, undefined, Approved), recv_presence(Config, unavailable), recv_subscription(Config, Type), State#state{subscription = from, peer_available = false}; @@ -550,15 +606,15 @@ transition(Id, Config, out, remove, #state{}; %% Incoming roster remove transition(_, Config, in, remove, - #state{subscription = Sub, pending_in = In, pending_out = Out} = State) -> + #state{subscription = Sub, pending_in = In, pending_out = Out, approved = Approved} = State) -> case {Sub, Out, In} of {none, true, _} -> ok; {from, false, _} -> - recv_push(Config, none, undefined), + recv_push(Config, none, undefined, Approved), recv_subscription(Config, unsubscribe); {from, true, _} -> - recv_push(Config, none, subscribe), + recv_push(Config, none, subscribe, Approved), recv_subscription(Config, unsubscribe); {to, false, _} -> %% BUG: we should receive push here @@ -567,9 +623,9 @@ transition(_, Config, in, remove, recv_subscription(Config, unsubscribed); {both, _, _} -> recv_presence(Config, unavailable), - recv_push(Config, to, undefined), + recv_push(Config, to, undefined, Approved), recv_subscription(Config, unsubscribe), - recv_push(Config, none, undefined), + recv_push(Config, none, undefined, Approved), recv_subscription(Config, unsubscribed); _ -> ok diff --git a/test/suite.erl b/test/suite.erl index 5a90b71b3..30066c9c3 100644 --- a/test/suite.erl +++ b/test/suite.erl @@ -130,6 +130,7 @@ init_config(Config) -> {db_xmlns, <<"">>}, {mechs, []}, {rosterver, false}, + {pre_approval, false}, {lang, <<"en">>}, {base_dir, BaseDir}, {receiver, undefined}, @@ -531,6 +532,8 @@ wait_auth_SASL_result(Config, ShouldFail) -> set_opt(csi, true, ConfigAcc); (#rosterver_feature{}, ConfigAcc) -> set_opt(rosterver, true, ConfigAcc); + (#feature_pre_approval{}, ConfigAcc) -> + set_opt(pre_approval, true, ConfigAcc); (#compression{methods = Ms}, ConfigAcc) -> set_opt(compression, Ms, ConfigAcc); (_, ConfigAcc) ->