Proposed behaviour for rlm_mruby (which might impact rlm_perl and rlm_python too)

[Alan DeKok]

On Nov 26, 2016, at 3:04 PM, Herwin Weststrate <herwin at quarantainenet.nl> wrote:
> 
> This weekend I've been trying to get some work done in this very old ticket[1] to remove rlm_ruby and replace it with rlm_mruby. mruby is a kind of minified ruby that is suitable for embedding in processes (as opposed to ruby, which does things like hooking its own signal handlers into freeradius). It was definitely an interesting experience, not in the least because documentation like this[2] is far from exceptional.

  Yeah...

> I don't like any of these interfaces, so I'm trying to propose a new one. We could use that in every rlm_language-module, so switching between languages would be easier and functionality will be preserved.
> 
> Use an object for the input. This object has methods like request, reply and session_state to get the correct lists. This means no more globale variables (perl) and no excessive lists of argument (python, ruby).
> The result of these method calls would be objects too. These object have methods like get_attribute and get_attributes. The first one returns a scalar value, if there are multiple attributes it returns the first one, If there is no attribute, it does something that is expected in the language (perl and ruby would return a NULL-like value here, in python a KeyError would be more suiteable). The second method always returns a list, that might contain 0 or 1 elements. In the general case, people are only interested in the first value.
> Updates should be performed via those objects too, so we could use code like `control.set_attribute('Cleartext-Password', 'hello')`. This means we can update all lists from rlm_python/rlm_ruby as well, and we no longer have to remember in what order "control" and "reply" were.

  I agree.  Maybe some minor changes;

 set_attribute(attribute, value)

  i.e. the attribute name always contains the list reference.  Otherwise it's hard to add new lists.

  We're also looking at adding nested groups at some point.  Having this API be forward compatible is nice.

  I think we can avoid the operator in the API, tho it may be useful.  We could otherwise do:

  replace_attribute(attribute, value)
  append_attribute(attribute, value)

> One of the drawbacks of the current implementation is that a lot of attributes have to be copied from the request and converted into language-specific strings. It is not unlikely that we copy and convert a load of EAP-data, while the called script is only interested in the User-Name attribute. By converting everything to objects we can make these conversions on-demand, instead of doing everything up front.

  Yes.

> We might even want to skip the parameter to the methods, and just define an abstract base class that has to be subclassed in the script. The downside of this being that it would become hard to test the script without freeradius, because the base class has been implemented there. Then again, the same problem arises with the rest of this proposal.

  Possibly.  I'll have to think about it.

> The biggest drawbacks here are that everything has to be coded, and that the language modules become backwards incompatible with older version. The changes for rlm_python are relatively small (I just hope nobody is using rlm_ruby at the moment, so nothing would be backwards incompatible here), but the behaviour of rlm_perl really changes here.

  That's fine.  Doing it better is worthwhile.

> I would like to hear your comments on this proposal, I can't be the only one who didn't like the interface of the scripting language modules.

  It grew over time.  It's annoying. :(

  Alan DeKok.