Support piefxis with external authentication

[dseomn]

Is your feature request related to a problem? Please describe.
When I run ejabberdctl export_piefxis on a server (version 21.01) configured to use external authentication, I get output that looks like this:

20211101-210548.xml:

<?xml version='1.0' encoding='UTF-8'?><server-data xmlns='urn:xmpp:pie:0' xmlns:xi='http://www.w3.org/2001/XInclude'><xi:include href='20211101-210548_mandelberg_org.xml'/></server-data>

20211101-210548_mandelberg_org.xml:

<?xml version='1.0' encoding='UTF-8'?><host xmlns='urn:xmpp:pie:0' xmlns:xi='http://www.w3.org/2001/XInclude' jid='mandelberg.org'></host>

When I run the same command without external authentication, the second file has a bunch more data.

Describe the solution you'd like
I'd like export_piefxis to export all of the data except for passwords (since it doesn't have access to those with external authentication). If that's not possible, it would be nice if the command at least returned an error instead of giving the impression that backups are working, when they're actually mostly empty.

Describe alternatives you've considered
ejabberdctl dump looks like it works with external authentication, but I haven't tried restoring from it yet.

Additional context
Is the underlying issue that https://docs.ejabberd.im/developer/guide/#external doesn't define a way for the script to list known users? If that were added to the protocol, it wouldn't be hard for me to return a list of users from my script.

[badlop]

As mentioned in the extauth section you can enable auth_use_cache. When doing this with extauth, the accounts existence is cached, and this allows piefxis (and webadmin, and other parts of ejabberd) to behave as if the list of accounts is known.

[dseomn]

I don't think I can use the cache though:

Note that caching interferes with the ability to maintain multiple passwords per account. So if your authentication mechanism supports application-specific passwords, caching must be disabled.

[badlop]

For your experiments:

From 2487ba5e602d96229780dd8515bffa6bd9fe1ed8 Mon Sep 17 00:00:00 2001
From: Badlop <badlop@process-one.net>
Date: Thu, 4 Nov 2021 13:38:19 +0100
Subject: [PATCH] Experimental support for getting users list in extauth
 (#3705)

New action get_users:
- Response should be AAB
- AA indicates the size of B
- B must start with the integer 6 (this is just a funny number)
- The remaining of B are usernames separated with :

Check the changes in the example extauth.py:
- It indicates that the size of B is 11 characters (0b in hexadecimal)
- Then it has the funny number 6
- And then returns the usernames u3, u6 and u901, separated by :
---
 src/ejabberd_auth_external.erl      |  8 +++++++-
 src/extauth.erl                     | 11 ++++++++++-
 test/ejabberd_SUITE_data/extauth.py |  6 ++++++
 3 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/src/ejabberd_auth_external.erl b/src/ejabberd_auth_external.erl
index a513c24ba..defd4250f 100644
--- a/src/ejabberd_auth_external.erl
+++ b/src/ejabberd_auth_external.erl
@@ -30,7 +30,7 @@
 -behaviour(ejabberd_auth).
 
 -export([start/1, stop/1, reload/1, set_password/3, check_password/4,
-	 try_register/3, user_exists/2, remove_user/2,
+	 try_register/3, user_exists/2, remove_user/2, get_users/2,
 	 store_type/1, plain_password_required/1]).
 
 -include("logger.hrl").
@@ -51,6 +51,12 @@ plain_password_required(_) -> true.
 
 store_type(_) -> external.
 
+get_users(Server, []) ->
+    case extauth:get_users(Server) of
+	Res when is_list(Res) -> [{list_to_binary(U), Server} || U <- Res];
+	{error, Reason} -> failure(<<"">>, Server, get_users, Reason)
+    end.
+
 check_password(User, AuthzId, Server, Password) ->
     if AuthzId /= <<>> andalso AuthzId /= User ->
 	    {nocache, false};
diff --git a/src/extauth.erl b/src/extauth.erl
index 6462df1cb..ce0534ea3 100644
--- a/src/extauth.erl
+++ b/src/extauth.erl
@@ -31,7 +31,7 @@
 %% API
 -export([start/1, stop/1, reload/1, start_link/2]).
 -export([check_password/3, set_password/3, try_register/3, remove_user/2,
-	 remove_user/3, user_exists/2, check_certificate/3]).
+	 remove_user/3, user_exists/2, check_certificate/3, get_users/1]).
 -export([prog_name/1, pool_name/1, worker_name/2, pool_size/1]).
 -export([init/1, handle_call/3, handle_cast/2, handle_info/2,
 	 terminate/2, code_change/3]).
@@ -79,6 +79,9 @@ remove_user(User, Server) ->
 remove_user(User, Server, Password) ->
     call_port(Server, [<<"removeuser3">>, User, Server, Password]).
 
+get_users(Server) ->
+    call_port(Server, [<<"getusers">>]).
+
 -spec prog_name(binary()) -> string() | undefined.
 prog_name(Host) ->
     ejabberd_option:extauth_program(Host).
@@ -127,6 +130,10 @@ handle_call({cmd, Cmd, EndTime}, _From, State) ->
 		    ?DEBUG("Received response from external authentication "
 			   "program: ~p", [Data]),
 		    {reply, decode_bool(N), State};
+		{Port, {data, [$6 | Str] = Data}} when Cmd == <<"getusers">> ->
+		    ?DEBUG("Received users list from external authentication "
+			   "program: ~p", [Data]),
+		    {reply, decode_string_list(Str), State};
 		{Port, Data} ->
 		    ?ERROR_MSG("Received unexpected response from external "
 			       "authentication program '~ts': ~p "
@@ -217,3 +224,5 @@ do_call(Cmd, I, Max, Pool, PoolSize, EndTime, CurrTime) ->
 
 decode_bool(0) -> false;
 decode_bool(1) -> true.
+
+decode_string_list(S) -> string:tokens(S, ":").
diff --git a/test/ejabberd_SUITE_data/extauth.py b/test/ejabberd_SUITE_data/extauth.py
index 394c2126a..57778d372 100755
--- a/test/ejabberd_SUITE_data/extauth.py
+++ b/test/ejabberd_SUITE_data/extauth.py
@@ -35,10 +35,16 @@ def read():
     elif cmd == 'removeuser3':
         u, s, p = pkt.split(':', 3)[1:]
         write(True)
+    elif cmd == 'getusers':
+        write_string('\x00\x0b6u3:u6:u901')
     else:
         write(False)
     read()
 
+def write_string(result):
+    sys.stdout.write(result)
+    sys.stdout.flush()
+
 def write(result):
     if result:
         sys.stdout.write('\x00\x02\x00\x01')
-- 
2.20.1

[dseomn]

Thanks, I'll try to test this out sometime in the next couple days! A few questions/comments in the meantime:

Response should be AAB

This will work fine for me since I only have one user. I imagine larger servers might have trouble with this though since it means getting an atomic snapshot of all the users and calculating the length before sending back any data. Would it make sense to stream results back? Maybe something like length + result + username, where length is the usual uint16 and result is a uint16 with values 0 = failure, 1 = end of list, 2 = not end of list? For usernames a, b, and c, that would look like '\x00\x03\x00\x02a\x00\x03\x00\x02b\x00\x03\x00\x01c'.

The remaining of B are usernames separated with :

So just the usernames, not the server names? Or user@server?

[badlop]

I imagine larger servers might have trouble with this

I imagine that's one of the reasons your request was never implemented, in the first place, in all the past 19 years of ejabberd.

So just the usernames, not the server names? Or user@server?

Just the usernames.

[dseomn]

I imagine that's one of the reasons your request was never implemented, in the first place, in all the past 19 years of ejabberd.

Fair enough.

Thanks, I'll try to test this out sometime in the next couple days!

It looks like it worked, thank you!

Does <user name='david' password=''> mean that there's no valid password, or that there's a valid empty password? If the latter, would it make sense to document that a piefxis export with extauth shouldn't be imported on a server that isn't also configured to use extauth?

[badlop]

Does mean that there's no valid password, or that there's a valid empty password?

I imagine an empty password is not valid in XMPP (https://xmpp.org/extensions/xep-0077.xml#nt-id0x2c600), so here it means the password is unknown. In that case, I guess it would be preferable to not include the password attribute at all (https://xmpp.org/extensions/xep-0227.html#users). For example:

diff --git a/src/ejabberd_piefxis.erl b/src/ejabberd_piefxis.erl
index d62efb300..fa0ba3ce9 100644
--- a/src/ejabberd_piefxis.erl
+++ b/src/ejabberd_piefxis.erl
@@ -166,6 +166,7 @@ export_user(User, Server, Fd) ->
     LServer = jid:nameprep(Server),
     {PassPlain, PassScram} = case ejabberd_auth:password_format(LServer) of
 	       scram -> {[], [format_scram_password(Password)]};
+	       _ when Password == <<"">> -> {[], []};
 	       _ -> {[{<<"password">>, Password}], []}
 	   end,
     Els =

[dseomn]

Is this actually fixed? 91c9b04 looks like it's just about the password attribute, and I don't see anything at https://github.com/processone/ejabberd/commits/master/src/extauth.erl or https://github.com/processone/ejabberd/commits/master/src/ejabberd_auth_external.erl about the actual feature.

[badlop]

The patch is available in that comment for anybody interested. But it was not merged into upstream; for that reason this issue is tagged "To reconsider" and in the "Parking Lot".