1190597 - regression: kidfox not detecting restricted profile?

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Profile Handling, defect)

Description

[Allison Naaktgeboren :ally]

RestrictedProfiles.isUserRestricted() is returning false in the restricted profile.

It also appears that RestrictedProfiles.isAllowed() is returning true for everything...

In other words, kidfox is trying to behave exactly like regular fennec. 

I think this is a regression

Comment 1

[Allison Naaktgeboren :ally]

Any idea? Is anyone else seeing this?

Comment 2

[:Margaret Leibovic]

Which call to `isUserRestricted` appears to be doing the wrong thing? How is fennec behaving differently than you would expect it to?

Looking at this code briefly, could it be that `isUserRestricted` is getting called with the wrong context? Maybe `GeckoAppShell.getContext()` isn't the context that the rest of the logic is expecting?

http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/RestrictedProfiles.java#92

Comment 3

[Sebastian Kaspari (:sebastian; :pocmo)]

Interesting. I have never seen this but I'm only testing on my N9. Will test on my N7 a bit today. What device are you using?

It would be interesting to know what UserManager.getUserRestrictions() and UserManager.getApplicationRestrictions() for this user returns. At least the first bundle should never be empty for a restricted profile.

Comment 4

[Allison Naaktgeboren :ally]

also on a nexus 9

Comment 5

[Sebastian Kaspari (:sebastian; :pocmo)]

As already mentioned in the vidyo call: I couldn't reproduce on my N7 either.

To see whether our code is broken or Android dropped all restrictions we should look into the Bundle returned by UserManager.getUserRestrictions(). This bundle should contain restrictions of the system. I used the following debug code to print them to the log previously (BrowserApp.onResume()):

>         UserManager userManager = (UserManager) getSystemService(Context.USER_SERVICE);
>         Bundle restrictions = userManager.getUserRestrictions();
> 
>         Log.d("Restricted/Debugging", restrictions.size() + " restrictions");
>         for (String key : restrictions.keySet()) {
>             Log.d("Restricted/Debugging", " Restriction " + key + " = " + restrictions.get(key));
>         }

On my N9 this returns for a restricted profile:

> D/Restricted/Debugging( 4024): 4 restrictions
> D/Restricted/Debugging( 4024):  Restriction no_sms = true
> D/Restricted/Debugging( 4024):  Restriction no_modify_accounts = true
> D/Restricted/Debugging( 4024):  Restriction no_share_location = true
> D/Restricted/Debugging( 4024):  Restriction no_outgoing_calls = true

I wonder what you get on your tablet..

Comment 6

[Sebastian Kaspari (:sebastian; :pocmo)]

I found bug 1190982 while trying to understand how this can happen. After fixing this other bug it's kind-of reproducible by disabling all app restriction in the settings. Because we are only checking app restrictions isUserRestricted() will return false, even though we are on a restricted profile. We obviously should check user restrictions (from the system) too. This is probably not triggering the bug you are experiencing but it's related so I'll attach a patch here.

Comment 7

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: 1190597-isUserRestricted.patch

This is a patch for the possible bug mentioned in comment#6. It will not necessarily fix your issue but it has the same effect.

Comment 8

[Allison Naaktgeboren :ally]

sebastian, that patch should have its own bug

Comment 9

[Allison Naaktgeboren :ally]

nexus 10 running 5.1.0 not displaying regression.

Comment 10

[Allison Naaktgeboren :ally]

nexus 10 now running 5.1.1 not displaying regression either.

Comment 11

[Allison Naaktgeboren :ally]

(In reply to Sebastian Kaspari (:sebastian) from comment #5)
> As already mentioned in the vidyo call: I couldn't reproduce on my N7 either.
> 
> To see whether our code is broken or Android dropped all restrictions we
> should look into the Bundle returned by UserManager.getUserRestrictions().
> This bundle should contain restrictions of the system. I used the following
> debug code to print them to the log previously (BrowserApp.onResume()):
> 
> >         UserManager userManager = (UserManager) getSystemService(Context.USER_SERVICE);
> >         Bundle restrictions = userManager.getUserRestrictions();
> > 
> >         Log.d("Restricted/Debugging", restrictions.size() + " restrictions");
> >         for (String key : restrictions.keySet()) {
> >             Log.d("Restricted/Debugging", " Restriction " + key + " = " + restrictions.get(key));
> >         }
> 
> On my N9 this returns for a restricted profile:
> 
> > D/Restricted/Debugging( 4024): 4 restrictions
> > D/Restricted/Debugging( 4024):  Restriction no_sms = true
> > D/Restricted/Debugging( 4024):  Restriction no_modify_accounts = true
> > D/Restricted/Debugging( 4024):  Restriction no_share_location = true
> > D/Restricted/Debugging( 4024):  Restriction no_outgoing_calls = true
> 
> I wonder what you get on your tablet..

N9 5.1.1 restricted profile, misbehaving fennec
D/Restricted/Debugging﹕ 4 restrictions
D/Restricted/Debugging﹕ Restriction no_sms = true
D/Restricted/Debugging﹕ Restriction no_modify_accounts = true
D/Restricted/Debugging﹕ Restriction no_share_location = true
D/Restricted/Debugging﹕ Restriction no_outgoing_calls = true

N9 5.1.1 regular profile
D/Restricted/Debugging﹕ 0 restrictions

N10 5.1.1 restricted profile, fennec behaving correctly
D/Restricted/Debugging﹕ 4 restrictions
D/Restricted/Debugging﹕ Restriction no_sms = true
D/Restricted/Debugging﹕ Restriction no_modify_accounts = true
D/Restricted/Debugging﹕ Restriction no_share_location = true
D/Restricted/Debugging﹕ Restriction no_outgoing_calls = true

Breakpointing, I see

isuserrestricted()
	returning false at nativeRun() in GeckoLoader context=BrowserApp
		//These methods are implemented in mozglue/android/APKOpen.cpp
	repeated call several times
		return false context=GeckoApplication
		return false, nativeRun() invocation, context=BrowserApp
		return false nativeRun() invocation, context=BrowserApp

isAllowed
	DISALLOW_SHARE -> true
	DISALLOW_INSTALL_EXTENSION -> true
	DISALLOW_DOWNLOADs -> true
	DISALLOW_TOOLS_MENU -> true
	DISALLOW_PRIVATE_BROWSING -> true
		loops through this set a couple of times

...only one of the 5 usages of isUserRestricted() in the frontend ever seems to trigger the breakpoint in isUserRestricted. Makes me wonder if the recent refactor could be whats biting us here.

Comment 12

[Allison Naaktgeboren :ally]

(In reply to :Margaret Leibovic from comment #2)
> Which call to `isUserRestricted` appears to be doing the wrong thing? How is
> fennec behaving differently than you would expect it to?
> 
> Looking at this code briefly, could it be that `isUserRestricted` is getting
> called with the wrong context? Maybe `GeckoAppShell.getContext()` isn't the
> context that the rest of the logic is expecting?
> 
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/
> RestrictedProfiles.java#92

All of them are. 

It returns false for calls with either BrowserApp or GeckoApp. I did try switching all the calls over to using BrowserApp, and still no dice.

Comment 13

[Allison Naaktgeboren :ally]

aha! isRestrictedProfile() in the createConfiguration() stack claims to have an empty restriction bundle, (which causes us to create a defaultconfiguration which translates to a normal fennec) even though the debug code of sebastian's prints out 4 in onResume() in browserapp.

Comment 14

[Allison Naaktgeboren :ally]

Sebastian's patch does fix this, in that the second bundle is nonempty and we get the right behavior on both tablets. Whee!

Comment 15

[Allison Naaktgeboren :ally]

Comment on attachment 8643188 [details] [diff] [review]
1190597-isUserRestricted.patch

Review of attachment 8643188 [details] [diff] [review]:
-----------------------------------------------------------------

ship it! srsly

Comment 16

[Sebastian Kaspari (:sebastian; :pocmo)]

url:        https://hg.mozilla.org/integration/fx-team/rev/5f1f9037b080a4a1f0f68ef5417be6f50676e2da
changeset:  5f1f9037b080a4a1f0f68ef5417be6f50676e2da
user:       Sebastian Kaspari <[e-mail]>
date:       Tue Aug 04 20:27:49 2015 +0200
description:
Bug 1190597 - isRestrictedProfile() should check app and user restrictions. r=ally

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/5f1f9037b080