vzakharchenko / keycloak-radius-plugin

Make the radius server as part of keycloak SSO
Apache License 2.0
188 stars 46 forks source link

OTP-only mode also supported for PAP #619

Open elkman opened 2 years ago

elkman commented 2 years ago

Is your feature request related to a problem? Please describe.

I am trying to use keycloak-radius-plugin for OTP token verification configured as a secondary RADIUS service with the Cisco AnyConnect VPN stuff. The primary RADIUS service is AD/Windows and performs all the usual authentication and authorization for VPN login.

Cisco just sends the username and OTP token to the secondary RADIUS service and prefers to use PAP. The switch to MSCHAPv2 comes with some other drawbacks and it does not seem to be possible to force the VPN to send password and OTP token to the secondary RADIUS service. Since the password is already checked, it also makes sense not to send it to other services without a need to do so .

I have not found any reference to why OTP-only mode is limited to CHAP/MSCHAPv2 and does not work with PAP. Only the code shows me that someone did a very good job by explicitly excluding PAP from the OTP configuration flag. This is so well done that there must be a very good reason for this, which I am very curious to know.

Describe the solution you'd like

A) Let PAP behave like CHAP/MSCHAPv2. Would be most consistent.

Describe alternatives you've considered

B) Add an additional configuration flag for PAP or change the otp flag to a list of PAP, CHAP, MSCHAPv2. (true means CHAP, MSCHAPv2 for compatibillity).

C) Discard the otp flag in the configuration, move it to the client configuration with the possibility to change the behavior for all or PAP, CHAP, MSCHAPv2 separately. (I'm not sure if this will work, since the client seems to provide at best a realm, but no client ID. Therefore a configuration in the realm would also be thinkable).

Additional context

I think I could create a PR for it (if it doesn't get too deep into Keycloak internals), but I'd like to know beforehand if this is a completely stupid idea respectively which variant would be preferred.

And many thanks to @vzakharchenko for his great work on this plugin.

dickpepperfield commented 2 years ago

I've ran into almost the exact same issue with Cisco ASAv's, and would like to say I don't think it's a completely stupid idea to apply your alternative recommendations, I'm completely inexperienced with Java development so unfortunately only a comment/bump is all I can offer

A massive thanks to vzakharchenko for their work on this plugin thus far, it has ticked all my boxes and I appreciate the work done.

elkman commented 8 months ago

Hi @vzakharchenko, hi all

I hope I have solved this issue with PR https://github.com/elkman/keycloak-radius-plugin/pull/84 in my fork of this repo. Since a lot of changes/updates have already been made, it makes no sense to submit it as a PR here without the history since the fork.

I'll keep this open as a reminder for a later code reunion and as a hint for people who have the same problem.