1181565 - Lockscreen passcode does not survive reboot

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::System, defect)

Description

[Frederik Braun [:freddy]]

** This was introduced by bug 1123325 **

My patch to the settings migrator did not take into account that the default settings are a passcode of "0000", even if it is not enabled.

That means that when you set a different passcode (using the new settings attribute) the migrator will not notice that *both* exist and overwrite the new-style with the old-style since it thinks a migration is necessary.

STR:
1) Create a passcode (e.g., 4444). Locking and Unlocking works fine
2) Reboot
3) Migrator will overwrite new-style value with "0000", because old-style value with "0000" exists.
4) Unlock only possible with "4444".


I am fixing this by modifying the existing check to not only see if an old-style passcode is existant but also to bail out if a new one has been already set.

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] mozfreddyb:1181565-settings-migrator-bug > mozilla-b2g:master

Comment 2

[Frederik Braun [:freddy]]

Attachment: github pull request

Fred, can you review?

Comment 3

[[e-mail]]

Comment on attachment 8631018 [details] [review]
github pull request

please add unit test since the migrator is a high risk area

Comment 4

[Frederik Braun [:freddy]]

Comment on attachment 8631017 [details] [review]
[gaia] mozfreddyb:1181565-settings-migrator-bug > mozilla-b2g:master

It's hard to test something that shoots something off asynchronously but does not return the promise. So I had to rewrite a few bits, so that it's returns the promises for things that it kicks off. :-)

We could also rewrite the other unit tests to not use a fake clock but look into the promises that are now returned. I'd prefer if this happens in a follow up.
For now, I left them untouched as they are passing fine.

Fred, can you take another look please?

Comment 5

[[e-mail]]

Comment on attachment 8631017 [details] [review]
[gaia] mozfreddyb:1181565-settings-migrator-bug > mozilla-b2g:master

please try mockLazyLoader which turns lazy loader to synchronized and make inner code testable

Comment 6

[Frederik Braun [:freddy]]

I don't think this would make a big difference, since neither start() nor keyMigration() return anything. We still need to rewrite those functions. Can we please do this in a follow-up?

I do not want passcodes broken on master for so long.

Comment 7

[[e-mail]]

I understand that its hard to test inside of lazyloader, so you don't have to.

The expect tests are:

1. when certain settings value already exists, make sure it not trigger the migration
2. when certain settings value does not exist, the migration (lazyloader part) is called

Comment 8

[Frederik Braun [:freddy]]

Comment on attachment 8631017 [details] [review]
[gaia] mozfreddyb:1181565-settings-migrator-bug > mozilla-b2g:master

Can you review again, please?

Comment 9

[[e-mail]]

Comment on attachment 8631017 [details] [review]
[gaia] mozfreddyb:1181565-settings-migrator-bug > mozilla-b2g:master

Thanks for hard work!

r+ if comment addressed in github. 

Please add suite('when old passcode is not present') test case to make sure its not run to LazyLoader.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Master: https://github.com/mozilla-b2g/gaia/commit/506a55fcb5b5992c64f3a776f29294b8b15af6b3

Comment 12

[Shally Li（leave from Mozilla）]

This bug has been verified as "pass" on the latest build of Flame KK 2.5 and Aires KK 2.5 by the STR in comment 0.

Actual results: Lockscreen passcode is correct and not reset to "0000" after reboot.
See attachment: verified_FlameKK_v2.5.3gp
Reproduce rate: 0/10


Device: Flame KK 2.5 (Pass)
Build ID               20150818150207
Gaia Revision          507ba38fb64b27f87d11f4104dfcc58448e12b1a
Gaia Date              2015-08-18 10:50:12
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/2c272af993c23e803f6ea7798a812b0c8abfad4d
Gecko Version          43.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150818.184156
Firmware Date          Tue Aug 18 18:42:07 EDT 2015
Firmware Version       v18D v4
Bootloader             L1TC000118D0

Device: Aries KK 2.5(Pass)
Build ID               20150818233027
Gaia Revision          1e1197e0e8e64307aa382ffba4711d1c661de7ca
Gaia Date              2015-08-18 16:54:35
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/88e1d293ffa9faf065b2a944c94a2705aabf1fb9
Gecko Version          43.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20150818.225517
Firmware Date          Tue Aug 18 22:55:24 UTC 2015
Bootloader             s1

Comment 13

[Shally Li（leave from Mozilla）]

Attachment: verified_FlameKK_v2.5.3gp