As always, patches are welcome for suggested behavior.
For the delayed-return-when-you-don't-expect-it case: authenticate { ... previous module sets reject without returning (reject=1) if (cond) { update request { ... ok so far } !! return happens here !! } } It seems to me that if the reject suppressed the return previously, then the 'if' block should suppress it too. In fact, an 'if' block could behave like an 'update' and simply perform no action at its end. I've made a patch for if/elsif/else/case/switch: https://github.com/candlerb/freeradius-server/commit/5d4b9923010de8f24dff266... This makes the authenticate { } section behave how I expected it to in the first place. You can still turn on the old behaviour if you want: if (cond) { ... reject = return } I arrived at that patch after examining why the 'update' block has a zero actions array. This turns out to be the right thing: if you *did* have the standard actions for 'update', and a 'reject' was set by a previous module, then a subsequent update{...} statement will trigger an immediately return. That's not very useful. Furthermore, the syntax for 'update' doesn't allow overriding return codes like this: update reply { reject = 1 Reply-Message += "foo" } Hence it's important that update shouldn't take any action based on the return code from the previous module. What about update setting the result to 'updated', instead of propagating the result of the previous module? I tried that, and it was OK in simple cases, but is probably undesirable in 'redundant' groups. After all, you don't want a simple update { } to mark the request as if it had done a successful database lookup. However, I still haven't worked out why the default AUTZ actions are being used for AUTH modules in some circumstances. The following patch would undo it, but it's probably a bad idea (I just don't understand why): --- a/src/main/modcall.c +++ b/src/main/modcall.c @@ -1950,13 +1938,8 @@ static modcallable *do_compile_modsingle(modcallable *parent, csingle = mod_singletocallable(single); csingle->parent = parent; csingle->next = NULL; - if (!parent || (component != RLM_COMPONENT_AUTH)) { - memcpy(csingle->actions, defaultactions[component][grouptype], - sizeof csingle->actions); - } else { /* inside Auth-Type has different rules */ - memcpy(csingle->actions, defaultactions[RLM_COMPONENT_AUTZ][grouptype], - sizeof csingle->actions); - } + memcpy(csingle->actions, defaultactions[component][grouptype], + sizeof csingle->actions); rad_assert(modrefname != NULL); csingle->name = modrefname; csingle->type = MOD_SINGLE; @@ -2132,11 +2115,7 @@ static modcallable *do_compile_modgroup(modcallable *parent, */ for (i = 0; i < RLM_MODULE_NUMCODES; i++) { if (!c->actions[i]) { - if (!parent || (component != RLM_COMPONENT_AUTH)) { - c->actions[i] = defaultactions[component][parentgrouptype][i]; - } else { /* inside Auth-Type has different rules */ - c->actions[i] = defaultactions[RLM_COMPONENT_AUTZ][parentgrouptype][i]; - } + c->actions[i] = defaultactions[component][parentgrouptype][i]; } } Redundant and append groups are identical between AUTH and AUTZ anyway, so the only difference is in the behaviour of a simple group, depending on whether it's at the top level or not (!parent) Regards, Brian.