On Friday, March 3, 2017, Adam Brightwell <adam.brightw...@crunchydata.com>
> I've given an initial review of this patch. It applies cleanly and
> compiles without issue as of 6da9759. I'm going to continue with
> testing it against a set of RADIUS servers in the next few days. But
> in the meantime, I have a few questions (below).
> > It supports multiple RADIUS servers. For all other parameters (secret,
> > identifier) one can specify either the exact same number of entries, in
> > which case each server gets it's own, or exactly one entry in which case
> > that entry will apply to all servers. (Or zero entries for everything
> > secret, which will make it the default).
> I wonder if removing the complexity of maintaining two separate lists
> for the server and port would be a better/less complex approach. For
> instance, why not go with a list of typical 'host:port' strings for
> 'radiusservers'? If no port is specified, then simply use the default
> for that specific host. Therefore, we would not have to worry about
> keeping the two lists in sync. Thoughts?
If we do that we should do it for all the parameters, no? So not just
host:port, but something like host:port:secret:identifier? Mixing the two
ways of doing it would be quite confusing I think.
And I wonder if that format wouldn't get even more confusing if you for
example want to use default ports, but non-default secrets.
I can see how it would probably be easier in some of the simple cases, but
I wonder if it wouldn't make it worse in a lot of other cases.
> > Each server is tried in order. If it responds positive, auth is OK. If it
> > responds negative, auth is rejected. If it does not respond at all, we
> > on to the next one.
> > I'm wondering if in doing this we should also make the RADIUS timeout a
> > configurable as HBA option, since it might become more important now?
> Yes, I think this would make sense and would be a good idea.