639: Invalidate card frontend

[f1sh1918]

Testing:

1. Change expirationDay: 19446, in dev_settings_view
2. Set invalid card in dev settings

resolve #639
resolves #677

[sarahsporck]

Shouldn't we check with the backend if the card is still valid as it could not only have been exbired but there is also the possibility it's been revoked?

[f1sh1918]

#639

good point. I just read the criteria in the issue which said. Take the expirationDate from the Anmeldecode.
The point is right now we have no functionality that revokes cards, so i think its not needed yet and should be implemented when this functionality is provided
We have also this issue #677

[michael-markl]

Shouldn't we check with the backend if the card is still valid as it could not only have been exbired but there is also the possibility it's been revoked?

I think we should do both:

• Check locally if the card has expired (which would be quite annoying to work around (for the user) by changing the system time)
• Check via API request if it is still valid (can be worked around rather easily by using flight mode)

Of course, we could also make the app API-validate the card on every start, and if it this failed (because of connection errors) for two weeks (which again can be tricked by changing system time) we don't show the QR code. In that case, we can probably omit the local expiration date check.

We should probably API-validate the card on app-start anyways, as the card might even get invalidated by scanning the anmeldecode on another device. Then, it would be great if the user could see this right away, after relaunching the app on the old device.

[f1sh1918]

Shouldn't we check with the backend if the card is still valid as it could not only have been exbired but there is also the possibility it's been revoked?

I think we should do both:

• Check locally if the card has expired (which would be quite annoying to work around (for the user) by changing the system time)
• Check via API request if it is still valid (can be worked around rather easily by using flight mode)

Of course, we could also make the app API-validate the card on every start, and if it this failed (because of connection errors) for two weeks (which again can be tricked by changing system time) we don't show the QR code. In that case, we can probably omit the local expiration date check.

We should probably API-validate the card on app-start anyways, as the card might even get invalidated by scanning the anmeldecode on another device. Then, it would be great if the user could see this right away, after relaunching the app on the old device.

Not sure if you read my comment above. I'm aware these checks are needed at some point and therefore is another ticket but for now we don't have any functionality for "scanning anmeldecode on other device", that it gets invalid yet (currently you can use it on x devices at least as i tested). I also mentioned this in the other issue #677.
The revoke functionality has a lot of issues to consider and is not implemented yet. Until there is no possibility to revoke the card there is no need to check this in my opinion because the value will stay false and the expirationDate is also immutable in the backend atm.

[michael-markl]

In this whole conversation we use "Anmeldecode" synonomously to "Aktivierungscode"?

[...] for now we don't have any functionality for "scanning anmeldecode on other device", that it gets invalid yet (currently you can use it on x devices at least as i tested)

Afaik, this is (or at least should be) wrong. Once you scan an "Aktivierungscode", the app makes an API-request ("activate card"), the backend generates a new TOTP secret and overwrites the old TOTP secret (thus invalidating the old activation).

[f1sh1918]

In this whole conversation we use "Anmeldecode" synonomously to "Aktivierungscode"?

[...] for now we don't have any functionality for "scanning anmeldecode on other device", that it gets invalid yet (currently you can use it on x devices at least as i tested)

Afaik, this is (or at least should be) wrong. Once you scan an "Aktivierungscode", the app makes an API-request ("activate card"), the backend generates a new TOTP secret and overwrites the old TOTP secret (thus invalidating the old activation).

Yes we do. Well i tested the same activation code on two devices locally. Even the secret changes it seem not to be revalidated on the first activated device. At least i couldn't reproduce that

Update: Okay you are right. If i scan the first card its invalid. But its not deactivated in the frontend (looks still valid that was my issue). So we need a visible feedback on the first device to also look invalid :)
Then this was a missunderstanding i expected the activation code not to work on a second device and that the user has to revoke first on the old device (which is not yet implemented).

[michael-markl]

[...] So we need a visible feedback on the first device to also look invalid :)

Right, that's why we need to API-validate on a regular basis, to actually detect this.

Then this was a missunderstanding i expected the activation code not to work on a second device and that the user has to revoke first on the old device (which is not yet implemented).

Just to clarify: We never planned to allow the user to revoke his card; only administrators should be able to revoke a card. (Also once revoked, it cannot be activated).

[f1sh1918]

Alright just as a summary:

• Check regulary against the CurrentTimestamp of the backend if the card is valid
• check the topt secret
• we could save the last check date in the device settings
• if no check was possible since 1 week or sth f.e due to no Internet, we could deactivate the qr code and tell the User to enable Internet and sync with the backend

What do you think? Did I forget sth?

[michael-markl]

What do you think

sounds good to me.

[f1sh1918]

Ok i updated the pr. Now additionally to the frontend check i validate:

• on each app start when the "Ausweisen" Tab was opened its getting checked for validity and a timestamp of this check will be written in UserCode secureStorage.
• if this check wasn't possible for more than 7 days (no internet) the qrCode will be hidden with a message to enable internet and to resync the card.
• if the verification process returns invalid due to card is revoked or old totp was sent, the card will be set invalid
  Note: There is no alert yet if you use the same Anmeldecode for another device Anmeldecode Reactivation Confirmation #949

Testing: You can set an invalid lastCheckDate in the dev settings

Updated my pr @michael-markl & @sarahsporck
One thing is just missing, to deprecate the verifyCardInProject endpoint, since i'm not sure if we really need this timestamp here or use the client one. Since we use the Date.now() from the client to calculate if the lastCheck was within the lastWeek, the user can even cheat with the timestamp from the backend.
So let me know your opinion.

[michael-markl]

Does it make sense to keep the card in the app if invalid like revoked or old totp?

As I explained in one of my comments above: YES

Maybe then we should send back different VerificationStates for each case

We cannot detect whether the otp code sent along with the request belongs to an old totp secret or is simply invalid (as we don't keep old totp secrets). I think it's enough to just show some text like "this card is invalid and these are the possible reasons: [possible reasons]".

[sarahsporck]

Tested and works fine.

However I find the revalidation doesn't look very pleasing. Especially the icon followed by "Weitere Aktionen". Maybe putting the icon above the text or also adding a text to the icon can help. Maybe @michael-markl has an idea.

[michael-markl]

Except one comment, I'm fine with these changes.

Good stuff 🚀

[f1sh1918]

Tested and works fine. However I find the revalidation doesn't look very pleasing. Especially the icon followed by "Weitere Aktionen". Maybe putting the icon above the text or also adding a text to the icon can help. Maybe @michael-markl has an idea.

i think this case is happening very rarely. I added a label to the button. But to change order means to change at lot in the widgets which is not really necessary imo

Added deprecation message, v2 and the adjustment of the data type to uint64

[michael-markl]

After addressing the remaining unresolved comments, you can merge imo 🚀