processone / ejabberd-contrib

Growing and curated ejabberd contributions repository - PR or ask to join !
http://ejabberd.im
250 stars 140 forks source link

Issues with LDAP in mod_default_rooms #317

Closed jacobeva closed 1 year ago

jacobeva commented 1 year ago

When a new account is created in ejabberd via the LDAP authentication mechanism, mod_default_rooms does not trigger, as the register API hook is never called. Therefore, it may be more suitable to use another hook from ejabberd which can support this use case?

In an email to me, Holger described potentially triggering this module each time a user authenticates (but this is hacky and may cause the deletion of extra bookmarks?).

Any thoughts on what could be done?

Neustradamus commented 1 year ago

To follow

badlop commented 1 year ago

I wrote a simple patch following that idea, so you can try it and comment if it works or has some drawback.

This patch only sets default rooms when the client sets presence, client logins using an external auth method, and the private storage is empty. Hopefully that will have minimum performance impact and shouldn't have drawbacks.

```diff From c8998acfdb34670c8b95ae0314b910ee1091f63a Mon Sep 17 00:00:00 2001 From: Badlop Date: Mon, 15 May 2023 12:53:58 +0200 Subject: [PATCH 1/2] Set default rooms when login with external auth and no private data found (#317) This allows the feature to work when accounts are not registered in ejabberd because it's using an external authentication method like LDAP. --- mod_default_rooms/src/mod_default_rooms.erl | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/mod_default_rooms/src/mod_default_rooms.erl b/mod_default_rooms/src/mod_default_rooms.erl index 0ba4fa7..a58e71c 100644 --- a/mod_default_rooms/src/mod_default_rooms.erl +++ b/mod_default_rooms/src/mod_default_rooms.erl @@ -32,7 +32,7 @@ mod_doc/0]). %% ejabberd_hooks callbacks. --export([register_user/2]). +-export([register_user/2, set_presence/4]). -include("logger.hrl"). -include_lib("xmpp/include/xmpp.hrl"). @@ -42,10 +42,12 @@ %%-------------------------------------------------------------------- -spec start(binary(), gen_mod:opts()) -> ok. start(Host, _Opts) -> + ejabberd_hooks:add(set_presence_hook, Host, ?MODULE, set_presence, 50), ejabberd_hooks:add(register_user, Host, ?MODULE, register_user, 50). -spec stop(binary()) -> ok. stop(Host) -> + ejabberd_hooks:delete(set_presence_hook, Host, ?MODULE, set_presence_hook, 50), ejabberd_hooks:delete(register_user, Host, ?MODULE, register_user, 50). -spec reload(binary(), gen_mod:opts(), gen_mod:opts()) -> ok. @@ -73,6 +75,17 @@ mod_doc() -> %%-------------------------------------------------------------------- %% ejabberd_hooks callbacks. %%-------------------------------------------------------------------- +-spec set_presence(binary(), binary(), binary(), binary()) -> ok | {error, _}. +set_presence(LUser, LServer, _Resource, _Presence) -> + case ejabberd_auth:store_type(LServer) of + external -> + case mod_private:get_data(LUser, LServer) of + [] -> register_user(LUser, LServer); + _ -> ok + end; + _ -> ok + end. + -spec register_user(binary(), binary()) -> ok | {error, _}. register_user(LUser, LServer) -> JID = jid:make(LUser, LServer), -- 2.35.1 ```
Neustradamus commented 1 year ago

@jacobeva: Have you looked the @badlop patch?

jacobeva commented 1 year ago

My apologies, I've been busy as of late! I will attempt to apply this patch now.

jacobeva commented 1 year ago

LGTM!

badlop commented 1 year ago

Ok, patch applied to the module.