processone / ejabberd-contrib

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

ejabberd 20, mod_filter problem #289

Closed slesru closed 4 years ago

slesru commented 4 years ago

Hello!

Examples do not work any longer with ejabberd 20:

  ## Admins can send anything.  Others are restricted in various ways.
  mod_filter:
    - allow: admin
    - restrict_local: local
    - restrict_foreign: all
  ## Local non-admin users can only send messages to other local users.
  restrict_local:
    - allow: local
    - deny: all
  ## Foreign users can only send messages to admins.
  restrict_foreign:
    - allow: admin
    - deny: all

Results in:

2020-04-27 15:58:58.716 [critical] <0.113.0>@ejabberd_app:start:71
 Failed to start ejabberd application: Invalid value of option
 access_rules->mod_filter:
 Unknown option: restrict_local. Did you mean allow?
 Available options are: allow, deny

Really, I have no idea how to rewrite this to make current ejabberd happy :-(

Thank you!

badlop commented 4 years ago

Right. Configuration verification is far more strict since past year, more concretely since https://github.com/processone/ejabberd/commit/a02cff0e780bb735531594c4ece81e8628f79782

The problem is that mod_filter uses support for an extended configuration that right now is not supported, because apparently only mod_filter used it.

The good news is that, apparently at least in my small test, adding support for that is as simple as adding a line in ejabberd source code:

diff --git a/src/acl.erl b/src/acl.erl
index d13c05601..c2a72fd9f 100644
--- a/src/acl.erl
+++ b/src/acl.erl
@@ -310,6 +310,7 @@ access_rules_validator() ->
       econf:non_empty(
    econf:options(
      #{allow => access_validator(),
+       '_' => access_validator(),
        deny => access_validator()},
      []))).

Can you apply this patch to your ejabberd, play with configuration as you wish, and report your results?

slesru commented 4 years ago

Just built ejabberd with this patch and it starts without complaining with my mod_fliter configuration. Quick test shows that everything works as expected, will do more tests tomorrow, but don't expect any problems.

btw, how can I ask to include this fix into next ejabberd release? Thank you!

slesru commented 4 years ago

Hello! Everything works as expected with patch , thank you!

badlop commented 4 years ago

I've documented this requirement in the mod_filter README file.

It seems that change is only useful for mod_filter, which is a module rarely used, considering that nobody else noticed this problem in the last year.

On the other hand, adding this patch to ejabberd would allow wrong configuration to get unnoticed. "Wrong" for anybody else than mod_fitler, that is.

So, unless other modules also benefit from this change, it doesn't seem a good idea to include it.

slesru commented 4 years ago

"considering that nobody else noticed this problem in the last year." - well, I use ejabberd 18 on ubuntu 18.04, now I tested migration to 20.04 and hit this. so, I think that if configuration is valid, it can't be wrong :-)

Thank you for help!

And, please, consider configuration parameter to allow , let's say, extended acl syntax.