Closed Bug 1190597 Opened 4 years ago Closed 4 years ago

regression: kidfox not detecting restricted profile?

Categories

(Firefox for Android :: Profile Handling, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 42
Tracking Status
firefox42 --- fixed

People

(Reporter: ally, Assigned: ally)

References

Details

Attachments

(1 file)

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
Any idea? Is anyone else seeing this?
Flags: needinfo?(s.kaspari)
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
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.
Blocks: FFB
Flags: needinfo?(s.kaspari)
OS: Unspecified → Android
Hardware: Unspecified → All
also on a nexus 9
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..
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.
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.
Attachment #8643188 - Flags: review?(ally)
sebastian, that patch should have its own bug
nexus 10 running 5.1.0 not displaying regression.
nexus 10 now running 5.1.1 not displaying regression either.
Assignee: nobody → 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.
(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.
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.
Sebastian's patch does fix this, in that the second bundle is nonempty and we get the right behavior on both tablets. Whee!
Comment on attachment 8643188 [details] [diff] [review]
1190597-isUserRestricted.patch

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

ship it! srsly
Attachment #8643188 - Flags: review?(ally) → review+
url:        https://hg.mozilla.org/integration/fx-team/rev/5f1f9037b080a4a1f0f68ef5417be6f50676e2da
changeset:  5f1f9037b080a4a1f0f68ef5417be6f50676e2da
user:       Sebastian Kaspari <s.kaspari@gmail.com>
date:       Tue Aug 04 20:27:49 2015 +0200
description:
Bug 1190597 - isRestrictedProfile() should check app and user restrictions. r=ally
https://hg.mozilla.org/mozilla-central/rev/5f1f9037b080
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
You need to log in before you can comment on or make changes to this bug.