Skip to content
This repository has been archived by the owner on Jun 23, 2021. It is now read-only.

Wrong user used in second step #14

Closed
cornelinux opened this issue Dec 19, 2018 · 8 comments
Closed

Wrong user used in second step #14

cornelinux opened this issue Dec 19, 2018 · 8 comments
Assignees
Labels

Comments

@cornelinux
Copy link
Contributor

We realized that a wrong username can be used in the authentication request of the second step.

We configured a service account to do triggerchallenge to send SMS.

Observation

We observe the following:

  1. UserA enters his windows credentials. We see a validate/triggerchallenge call in privacyIDEA for UserA. The user receives the SMS on his phone.

  2. On another browser a UserB enters his windows credentials. We see a validate/triggerchallenge call in privacyIDEA for UserB. The user receives the SMS on his phone.

  3. Now UserA enters his OTP value. But now an API call is sent to privacyIDEA validate/check with the username UserB. Of course authentication fails.

Analysis

Obvious the IdentityClaim you are using are not bound to the right session or is only a current snapshot.

The username is currently defined as a member variable of the Authenticator Class. Obviously ADFS does not create a new instance of the class on each request or the variable is shared over all instances.


Thus the second user trying to authenticate will overwrite the username causing the first user fail to login.

One possible solution

I think a possible solution could be to add the username and realm as hidden fields to the login form and also read the username from the HTTP request just like the otp value.

Please drop me a note, if you need any more information or assistance.

@sbidy sbidy self-assigned this Dec 19, 2018
@sbidy sbidy added the bug label Dec 19, 2018
@sbidy
Copy link
Owner

sbidy commented Dec 19, 2018

Hey @cornelinux ! Thanks for the report!
So I assumed that the ADFS creates a object for each request from the class ... But that seems not correct 😞 .

I'll look into that and try to tied the realm/username to the corresponding "session". A hidden HTML form field can be a feasible solution. Thank you for that suggestion! 😉

@tedbear
Copy link

tedbear commented Dec 19, 2018

Hi sbidy,

I was working/debugging with cornelinux on this issue. Let me know if you need me for any testing etc, as I have a team of 25 colleagues more or less just sitting and waiting to test (and use the plugin when it works) this plugin for ADFS/privacyidea.
Also: Do you have any idea of how long it would possibly take for you to fix the issue?

Thank you in advance!

Best regards,
Teddy Strand

@sbidy
Copy link
Owner

sbidy commented Dec 19, 2018

I'm still working at the issue at the moment 😉 ... maybe a fix is coming today.

sbidy added a commit that referenced this issue Dec 19, 2018
…alm to the ID3A.

Version riesd from 1.3.2 to 1.3.3 - install script also updated with new version!!
@sbidy
Copy link
Owner

sbidy commented Dec 19, 2018

Please check the 1.3.3a version - I added the username and realm to hidden input fields and use this for the OTP request to the ID3A. This should fix the problem.

!! -> the install script should be updated too (changed version number) !!

@cornelinux
Copy link
Contributor Author

cornelinux commented Dec 19, 2018

Hi @sbidy
thanks for the realy quick respsone.
When I look at this line,

return otp_prov.getAuthOTP(username, otpvalue, session_realm);

I think it must be session_user in the parameters and not username.
Thx
Cornelius

PS: I added a quick pull request :-)

@sbidy
Copy link
Owner

sbidy commented Dec 19, 2018

Bug fixed with last commit but still not in the last release (1.3.3).
New release is coming with bug fix for #15.

@tedbear
Copy link

tedbear commented Dec 20, 2018

Hi @sbidy
Thanks for the update. Do you reckon the new release will be available later today?

@sbidy
Copy link
Owner

sbidy commented Dec 21, 2018

Sorry for the delay 😞 ... pre-Christmas-stress 🎄 ...
I'll publish an new 1.3.3 pre-realase "now".

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants