search

OpenConext / Stepup-tiqr

tiqr IdP for step-up authentication
Apache License 2.0
3 stars 2 forks source link

2.1.1 doesn't work well without push notification #59

Closed tvdijen closed 5 years ago

tvdijen commented 5 years ago

Leaving push notification to the default values will cause an error after scanning QR: afbeelding After clicking the retry button, authentication is finished normally and I'm redirected to the SP.

I've also tested with non-default but non-working settings, but the result is the same..

MKodde commented 5 years ago

Thanks for your report @tvdijen, we have looked into this problem and suspect the Tiqr server timeout (set to 5min) and our own state might be in misalignment here. The fact that the registration passes after a retry fortifies this suspicion.

We have not yet been able to reproduce this behavior though. We have also thought up a solution that might fix this problem, should not be too much effort, but adding a Behat test to verify this behavior might prove to be a bit more difficult.

The unverified hotfix we had in mind would target: https://github.com/OpenConext/Stepup-tiqr/blob/develop/src/AppBundle/Controller/RegistrationController.php#L98

Add below line 98:

if ($this->tiqrService->enrollmentFinalized()) {
    return new Response('5'); // This translates to the Tiqr_Service::ENROLLMENT_STATUS_FINALIZED status
}

Would you be so kind as to test if this would work out? This would help us to create a more robust fix.

tvdijen commented 5 years ago

Unfortunately, that made no difference at all...

MKodde commented 5 years ago

Too bad that didn't fix it. Maybe my understanding of the actual problem is not fully developed yet.

My assumption is that the tiqr server did return a finished state at some point, maybe you can verify this in your browsers developer tools? This should occur right after scanning the QR code.

tvdijen commented 5 years ago

SAML-tracer-export-2018-10-16T11_54_01.049Z.zip I can see 8 subsequent calls to /authentication with all different X-Stepup-Request-Id's.. That doesn't seem right, but I'm not sure what it represents

MKodde commented 5 years ago

The status calls are the result of polling actions that are performed on the Tiqr server. The client polls for the status on https://tiqr.stepup.moo-archive.nl/authentication/status Once the finished status is returned, the client knows authentication finished successfully in the app. The drawback of your tracer dump is that the actual response value was not included. Maybe your browsers developer tools allow the export of the status polling requests in the networks section of the dev tools? Firefox allows the export in 'har' format.

tvdijen commented 5 years ago

Archive 18-10-18 09-43-10.zip

tvdijen commented 5 years ago

Polling status is "pending", until I scan the QR-code+PIN.. Then a final poll is answered with the "challenge-expired" status

MKodde commented 5 years ago

@tvdijen, sorry that it takes so long for us to look into this problem. We are currently trying to plan the fixing of this issue for one of the tiqr contributors. Linking this thread in the Pivotal story.

MKodde commented 5 years ago

@tvdijen We started this new year with something that might make you happy #66

tvdijen commented 5 years ago

Thanks @mkodde! I'll see if I can test it later this week

MKodde commented 5 years ago

@tvdijen we baked a new 2.1.2 release yesterday including the fix for this problem. I'm closing this issue. Thanks again for your contribution :+1: