search

mooltipass / minible

Github repository containing the firmwares running on the Mooltipass Mini BLE
GNU General Public License v3.0
97 stars 21 forks source link

Hiding digits by default #399

Closed Da-xy closed 1 year ago

Da-xy commented 1 year ago

The idea of this feature is to have an even more secure mode when entering PIN code. Feature request linked to this issue : #398 The code is related to the 4th implementation proposal in the issue.

limpkin commented 1 year ago

thanks a lot :)

limpkin commented 2 months ago

@Da-xy : hey there :). I recently noticed that this PR doesn't enable continuous push and scroll... do you have the same issue?

limpkin commented 2 months ago

seems that the root cause issue is https://github.com/mooltipass/minible/blame/master/source_code/main_mcu/src/GUI/gui_prompts.c#L814

Da-xy commented 2 months ago

Hi @limpkin ! Indeed, I have exactly the same issue ! Unfortunately, I did not get the time to investigate deeper, after the release... Yes, it seems that the root cause come from this function because removing this line fixes the issue. However, I am not able to tell if it will bring some sort of side effects if we modify or remove it. Do you have an idea on a possible fix or do you want me to give it a look ? Thank you very much for pointing it out !

Da-xy commented 2 months ago

You're right I should have done this before.. sorry about that. If you want, I can create the related issue with the fix proposal and send a PR with the line removed ?

limpkin commented 2 months ago

sorry... had left a comment using another account. it's alright, can you make sure removing this line doesn't break anything?

Da-xy commented 2 months ago

OK, I will perform some functional tests and I come back to you to inform you if there are any regressions.

Da-xy commented 2 months ago

Hi again @limpkin, I checked a lot of different use cases mixing the different options :

I also checked with the changing pin display. I did not notice any issues with the fix. So I think, that this instruction can be deleted.

However, I felt that mixing these options are not very good and could lead to a very weird user experience. I think that I will create an issue on Moolticute repository in order to submit a feature request. It will prevent new users to select all options which might in my opinion will make no sense especially : paranoid entry pin + display full pin on entry.