Openvpn authentication with otp as suffix #8058
Conversation
| @@ -54,7 +54,7 @@ trait TOTP | |||
| /** | |||
| * @var bool token after password | |||
| */ | |||
| private $passwordFirst = false; | |||
| public $passwordFirst = false; | |||
There was a problem hiding this comment.
I'm not sure I understand the problem. This looks like a workaround for a different issue.
There was a problem hiding this comment.
This is needed to know if the authenticator has the pin as suffix.
There's a readonly option but then it needs to be initialized in the creator of the class.
| $tmp = explode(':', $password); | ||
| if (count($tmp) == 3) { | ||
| $pass = base64_decode($tmp[1]); | ||
| $pinpassword = base64_decode($tmp[1]); |
There was a problem hiding this comment.
all of this just obscures the intention...
There was a problem hiding this comment.
We might move this also inside the authenticator loop for clarity, just it didn't seem programmatically correct to me.
| $checkpassword = $pin . $pinpassword; | ||
| } | ||
| } | ||
| if ($authenticator->authenticate($username, $checkpassword)) { |
There was a problem hiding this comment.
if we had a third argument for the pin this might be possible to handle gracefully inside the authenticator but I am not sure of the benefit of all of this CC @AdSchellevis
There was a problem hiding this comment.
we shouldn't do this, you can configure openvpn to ask for the pin separately, see also #3290
There was a problem hiding this comment.
I thought also about that, but it would impact more code. If needed I might just change the Auth backends to accept the pin.
There was a problem hiding this comment.
About #3290 that's exactly the point. Enabling the pin request does not handle the reverse option enabled.
There was a problem hiding this comment.
Should we maybe move the "SCRV1:" handling code inside the authenticator itself?
There was a problem hiding this comment.
easiest option is just to add 2 authenticators if you need both orders, use one for openvpn, the other for the ui. I don't think we need more glue to be honest, a note in the documentation about limitations should be more than enough.
|
I can understand you not accepting this, as the solution for you would simply be to remove the "reverse" option. |
AdSchellevis
force-pushed
the
master
branch
5 times, most recently
from
6586a65
to
607e32a
Compare
Authentication server as Local and LDAP with TOTP have an option which permits to enter the TOTP code after the password (default is as prefix)
In this case if you enable the TOTP question in Openvpn Client (static-challenge) the authentication fails as the PIN is presented as prefix and not as a suffix.
I currently have many configurations where static-challenge is not enabled and thus the users write the password and the PIN in the password input on the client.
Migrating to support the static-challenge would require them to be changed all at the same time.
I propose the move of the check (and how the password is composed) based on the Authentication method settings.