Update idp error detection

[C0ntroller]

Should not only fix error detection on login, but prevent people from killing their 2FA account.

Can't test changes, as I don't have an ZIH account anymore.

[OliEfr]

Thanks for the very quick workaround. Will release now.

@A-K-O-R-A @t0mbrn can you confirm that this works with your ZIH account?

[A-K-O-R-A]

can you confirm that this works with your ZIH account?

Sure, could you give me any specific scenarios / configuration combinations that should be tested? I don't use the feature so I'm not familiar with the problem this PR solves 😅

[t0mbrn]

can you confirm that this works with your ZIH account?

Sure, could you give me any specific scenarios / configuration combinations that should be tested? I don't use the feature so I'm not familiar with the problem this PR solves 😅

A friend of mine just today faced the issue that when he changed his 2FA Token TUFast still used the old one and spammend the system with retries causing it to block the account login. I think it's trying to address this but it's just a guess.

I'm currently only using safari so I'm not using the tufast login either 😅

[OliEfr]

Thanks for responding so quickly.

Yes, that is the issue that was reported by ZIH (that's why it is quite urgent.) Apparently student accounts got locked.

I think it's hard to test this then. @C0ntroller can it be an issue that the token is not properly set it TUfast?

[t0mbrn]

Thanks for responding so quickly.

Yes, that is the issue that was reported by ZIH (that's why it is quite urgent.) Apparently student accounts got locked.

I think it's hard to test this then. @C0ntroller can it be an issue that the token is not properly set it TUfast?

My friend encountered it when he setup a new 2FA token in iCloud Keychain invalidating the one present in TUFast.

[C0ntroller]

I think it's hard to test this then. @C0ntroller can it be an issue that the token is not properly set it TUfast?

Could be. Can't access self service anymore to see, if the scraping fails.
But there are a few checks in place before TUfast uses it, I can't really imagine that it would break that hard.

My friend encountered it when he setup a new 2FA token in iCloud Keychain invalidating the one present in TUFast.

But that is exactly how to break it. Manually provide wrong key in the TUfast settings or reset the 2FA token (so revisit self service) on a different device, so the TUfast one is revoked.

TUFast still used the old one and spammend the system with retries causing it to block the account login.

Thats what the PR shoudl have fixed: if an error is shown (so you inserted a wrong key), TUfast will not try it again. The prior check was probably broken by redesign.

Sure, could you give me any specific scenarios / configuration combinations that should be tested?

@A-K-O-R-A Scenario would be trying to login with an invalid but set 2FA token.

[A-K-O-R-A]

I have not yet been able to test the change yet, but I seems like this could happen again if the IDP website changes. Could a persistent retry counter (using the chrome storage for example) in the common.Login class potentially be a safer option to mitigate the impact of problems like this in the future?

[C0ntroller]

Could a persistent retry counter (using the chrome storage for example) in the common.Login class potentially be a safer option to mitigate the impact of problems like this in the future?

Not trivially.
There is not really a way to detect a successful login to reset the counter.

I guess there is a browser api function for timers to reset it after eg. 10 minutes. But that would not be a minor change anymore, and I don't really have the time.

[A-K-O-R-A]

My suggestion would be something similar to the following:
Save the current timestamp on each start() to a list, then before each Login attempt check the list, count how many of the entries have been in the last 30 seconds (remove all older ones) and if there are more than 3 (or some other threshold value) attempts (no matter successful or not) show an alert / wait for ~30 seconds.

This way an infinite retry loop would be prevented, I feel like there is no real world use case where a user would attempt 3 consequent successful logins in under 30s, so this should not hinder the normal user interactions

If you think this could be a viable solution then I could implement this myself and open a PR

[A-K-O-R-A]

Scenario would be trying to login with an invalid but set 2FA token.

Just tested it and it works for me, the extension stops after the "Verification failed" banner is shown 👍

[OliEfr]

I got a mail from ZIH saying thanks that we deployed the fix so quickly and they were pretty happy.