My MacPorts-based PHP 5.4.11 install also has the mhash module available.

Adding patch information for a commit with the updated library version.

Thanks for reporting and patching that, Justin. Feel free to request a peer review.

Petr: perhaps you could peer review this.

+1, there is not much we can review, the code is from upstream, I think testing is enough

Testing is the tough one. It's what has prevented me from sending this for review so far. We have a client who is currently suffering from this problem. We can check that the problem goes away and their RADIUS auth starts working correctly for them. I checked the QA tests from 2.4 and there were no tests for this auth plug-in. Is it worth while to come up with some instructions for how to setup a test RADIUS server to validate that this plug-in works?

I am planning to set up some read only test VMs with all these external services necessary for testing of auth plugins. Hopefully it will be available for HQ CI servers before 2.5 release.

Sounds good to me. We'll get my branch tested on our clent's site that is broken right now to verify if it works and then I'll push this through the rest of the process.

I'm requesting a peer review on this one as we can now confirm that this fix has allowed the RADIUS auth plugin to work with our affected client installation.

Hi Justin,

It looks good to me, can't really argue with the upstream changes.

We do need some testing instructions for this though, even if its difficult to setup the test, could you give us the requirements to setup a test of it?

Also, I assume you have tested it, so it at least gives us some confidence in applying this fix

[-] Syntax
[-] Output
[-] Whitespace
[-] Language
[-] Databases
[N] Testing
[-] Security
[-] Documentation
[Y] Git
[Y] Sanity check

Thanks, Dan.

I put in rough instructions. In our case, the client we host who had the problem was connecting to their own RADIUS server that was external from our network. We do not maintain any setups internally so I can't really provide any further detailed instructions on how to do the RADIUS setup. =(

We did test this, though, and it resolved the issue and allowed users to successfully authenticated into the client's Moodle site again.

Thanks Justin, this has been integrated now on 23, 24, and master.

It passes, tested in 22, 23, 24 and master with a RADIUS VM using Microsoft CHAP 1 authentication and freeradius.

I've received a strict standards error in 23 and 24, but IIRC we are not compliant with strict standards until 24. I wait for integrators feedback to pass it or fail it.

Strict Standards: Non-static method PEAR::loadExtension() should not be called statically, assuming $this from incompatible context in /var/www/integration/MOODLE_24_STABLE/lib/pear/Auth/RADIUS.php on line 49 Strict Standards: Non-static method PEAR::isError() should not be called statically, assuming $this from incompatible context in /var/www/integration/MOODLE_24_STABLE/auth/radius/auth.php on line 108

Edit: I've just noticed that it hasn't been integrated in 22; anyway I also have mhash, that's why it hasn't failed.

it seems that the class has not been changed as trunk continue showing that static behavior:

http://svn.php.net/viewvc/pear/packages/Auth_RADIUS/trunk/RADIUS.php?revision=257341&view=markup

So, or we fix that in the PEAR wrapper or we consider switching to another (pure or wrapper) lib. If this had to be voted, I'd say "pure" is better (less dependencies). Like, for example:

http://www.phpclasses.org/package/4326-PHP-Authenticate-users-with-a-RADIUS-server.html

Although I don't know much really. Adding Jonathan Harker here coz he did the initial (and current) implementation.

Ciao

I've created MDL-38373 and provided a trivial solution there for the stricts standard thingy. So I think this can be passed safely.

Ciao

This is valid for unlimited entries to the, soon to be unveiled, Moodle Codebase Gardens. It includes free access to all facilities.

Personal and non-transferable to all assignees, reviewers and testers in this issue. Valid until switching to Blackboard (100000€ penalization will be applied).

Thanks, closing as fixed!

For the record, there is a VM in the HQ servers to use in further RADIUS-related tests