Closed Bug 580349 Opened 14 years ago Closed 6 years ago

Need a way to lock/block user access to about:config options

Categories

(Toolkit :: Preferences, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 1421707

People

(Reporter: ilanio, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [testday-20110603][parity-ie][has patch][needs test per comment 36])

Attachments

(4 files, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; pt-BR; rv:1.9.2) Gecko/20100115 Firefox/3.6 ( .NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; pt-BR; rv:1.9.2) Gecko/20100115 Firefox/3.6 ( .NET CLR 3.5.30729)

Need to create a way to avoid user change about:config options by typing "about:config" at address line of browser.

Reproducible: Always

Steps to Reproduce:
1. Type "about:config" at address line of browser
2. User can change all browser configurations

Actual Results:  
User have access to all browser configurations

Expected Results:  
Avoid user to access about:config options, by LockPrefs or some other way.
We already show a warning, so I think this is enough to avoid accidental changes. But it's a nice idea for an extension.
Whiteboard: wontfix?
(In reply to comment #2)
> It already exists :
> 
> https://developer.mozilla.org/en/Automatic_Mozilla_Configurator/Locked_config_settings
> http://ilias.ca/blog/2005/03/locking-mozilla-firefox-settings/
> http://www.pcc-services.com/kixtart/firefox-lockdown.html

No, it not exists. Firefox lockdown settings does not have an option to disable user access to about:config settings.

You can lockdown all settings but if user types "about:config" at address line he/she can change all settings, even the locked ones.

Michael, please, note in a corporate LAN, I need a way to avoid user to change the settings of browser and not just warning he/she.
This is important for corporate users.
YOu have to lock all values that only exists in about:config and not via GUI. It would be much easier to block the access to about:config itself.
At my workplace we reinstall mozilla.cfg everytime the application starts. Power-users can still access about:config if they like (I'm often messing with proxies for instance), but it will be overwritten the next time you restart the application. Unless you know how to bypass the startup-script of course.

You can't really expect that you can really lock down ALL settings, as there are numerous ways to bypass them. That includes using different profiles, installing different browsers, using Portable Firefox, etc...

about:config can indeed be made better hidden (protected with a password, etc...), but don't forget that users will always find an easy way  to work around that "problem".
At my workplace you can't install applications or start/copy .exe files from somewhere. But you can access about:config in Firefox :-)
IE also allows locking/hiding some of its options probably by some global policy. Yes, there may be options to bypass even about:config (e.g. editing user.js directly). But that can also be done on IE. But it may still stop some percentage of less determined users.

Confirming as feature request.
Severity: major → enhancement
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Need a way to lock user to access about:config options → Need a way to lock/block user access to about:config options
Whiteboard: wontfix? → [testday-20110603][wontfix?][parity-ie]
My suggestion would be that if the pref:

general.warnOnAboutConfig

is locked, then about:config doesn't show

That would be an easy way to add this functionality without major change.
Nice idea. I suppose there should be some explanation that the config is locked and there would be no button to continue.
I'll try this.
Assignee: nobody → acelists
Version: unspecified → Trunk
Yeah. I figured there could be a third page in the deck that had a simple message.

And in the locked case, it would display that page.

Or if we don't want to impact l10n, just have an empty box.
Are you the right person to review the patch?
Whiteboard: [testday-20110603][wontfix?][parity-ie] → [testday-20110603][parity-ie]
Attached patch patch per comment 8 (obsolete) — Splinter Review
Attachment #589993 - Flags: review?(mozilla)
Status: NEW → ASSIGNED
Comment on attachment 589993 [details] [diff] [review]
patch per comment 8

The code is actually in toolkit (and affects TB too, where I tested it).
Attachment #589993 - Flags: review?(dtownsend)
Assignee: acelists → nobody
Product: Firefox → Toolkit
QA Contact: preferences → preferences
Attachment #589993 - Flags: ui-review?(ux-review)
aceman: Per the guidelines [1], can you please attach a screenshot or screencast/video of your changes for ui-review?

[1] https://developer.mozilla.org/En/Developer_Guide/Requesting_feedback_and_ui-review_for_desktop_Firefox_front-end_changes#Looking_for_feedback_or_ui-review_on_your_patch.3F
Assignee: nobody → acelists
Comment on attachment 589993 [details] [diff] [review]
patch per comment 8

Clearing ui-review request for now. Please add a screenshot, screencast/video, or link to try server build and reflag for ui-review.

Thanks for using the new ux-review alias :)
Attachment #589993 - Flags: ui-review?(ux-review)
Yeah, I accidentally noticed the new alias on planet.mozilla.org.
You could put it on https://wiki.mozilla.org/Modules/Firefox . That module looks poorly described compared to the others (I do not know from whom to request reviews for various submodules).
(In reply to :aceman from comment #16)
> Yeah, I accidentally noticed the new alias on planet.mozilla.org.

Don't feel bad, I'm happy you noticed it :)

> You could put it on https://wiki.mozilla.org/Modules/Firefox . That module
> looks poorly described compared to the others (I do not know from whom to
> request reviews for various submodules).

Good idea. I'll talk to some people about it.
Comment on attachment 589993 [details] [diff] [review]
patch per comment 8

dtownsend is definitely the right person for final review on this.

I think that since this code uses a deck already, it would be cleaner to just put the new label in a third deck and simply show that page and return.

That would avoid the need for all the collapsing.

Also, for the text, I think we should use "your administrator"

This is consistent with the text for xpinstall

xpinstallDisabledMessageLocked=Software installation has been disabled by your system administrator.
Attachment #589993 - Flags: review?(mozilla)
(In reply to Jared Wein [:jaws] from comment #17)
> > You could put it on https://wiki.mozilla.org/Modules/Firefox . That module
> > looks poorly described compared to the others (I do not know from whom to
> > request reviews for various submodules).
> 
> Good idea. I'll talk to some people about it.

Since this is toolkit code the module you actually want is Toolkit: https://wiki.mozilla.org/Modules/Toolkit
Comment on attachment 589993 [details] [diff] [review]
patch per comment 8

Why aren't we just locking the actual locked prefs themselves, making them impossible to change from about:config rather than implementing a complete block to about:config?
We do lock prefs, but we can't lock every Firefox preerences.

Enterprises want to prevent users from accessing about:config.

Or are you saying we should make about:config read only versus not displaying it at all?
Comment on attachment 589993 [details] [diff] [review]
patch per comment 8

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

Oh sorry I misread the intent of the bug. I'd rather see a new pref for this rather than overloading the warning pref. Probably general.blockAboutConfig, just check if it exists and is true. Please get ux-review on the displayed message before requesting the next review.
Attachment #589993 - Flags: review?(dtownsend) → review-
(In reply to Michael Kaply (mkaply) from comment #18)
> I think that since this code uses a deck already, it would be cleaner to
> just put the new label in a third deck and simply show that page and return.

I do not know how to use decks. I only see one <deck> there. How should I interpret the XUL code there?
> I do not know how to use decks. I only see one <deck> there. How should I interpret the XUL code there?

So basically a deck allows switching between it's children (no matter what they are).

So if you have a deck that looks like this:

<deck>
  <button>
  <box>
  <textbox>
</deck>

selectedIndex of 1 would be the box, 2 would be the testbox, 0 would be the button.

More info here: https://developer.mozilla.org/en/XUL/deck

So all you need to do is give the deck one more child at the same level as the other two.
Thanks. That would work now. But it seems I would have to define all the styling on it so that it looks so nice as the current warning. I don't think it is worth messing with the theme for this.
> But it seems I would have to define all the styling on it so that it looks so nice as the current warning. I don't think it is worth messing with the theme for this.

Looking at the code, I agree with you.

It's unfortunate that IDs were used for all this instead of classes.
Attached patch patch, fix wording (obsolete) — Splinter Review
Attachment #589993 - Attachment is obsolete: true
Attachment #590821 - Flags: ui-review?(ux-review)
Screenshot from Thunderbird, therefore there is no FF UI around the content. I hope it does not matter.
Comment on attachment 590821 [details] [diff] [review]
patch, fix wording

This works for me, if this is indeed a setting we want to support.

Would be nice if the text could be centered relative to the icon, though. :)
Attachment #590821 - Flags: ui-review?(ux-review) → ui-review+
I'd gladly center it vertically if I knew how. It needs to be in the .xul file, not in some css in the theme. I tried align="center" but it didn't work. Any ideas?
I think I got it how to center it. However, if I do that, then it also centers vertically the exclamation icon when there is the large warning text (common situation). That icon is currently on the top. Would it be a problem?
OK, I think I got it. The original warning unaffected, the new info text centered.
Attachment #590821 - Attachment is obsolete: true
Attachment #591600 - Flags: review?(dtownsend)
Comment on attachment 591600 [details] [diff] [review]
patch, vertically center the label

This would be so much nicer with a deck but I guess isn't worth the effort. Can we get an automated test though?
Attachment #591600 - Flags: review?(dtownsend) → review+
Is there documentation on how to write automated tests with something like this?
Sorry, I have no idea how to make tests in Firefox.
Browser chrome tests are probably the best choice, they are documented here: http://mxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/test/browser/browser_aboutCrashes.js

A very basic example that loads about:crashes and checks some of its content is here: http://mxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/test/browser/browser_aboutCrashes.js it has some bits that wouldn't be necessary for about:config though.

We'd probably want a test that set the blocking pref, opened about:config, checked only the warning was visible, then closed the tab, reset the pref and opened about:config again to check it worked.

Bonus points for also checking the warning message shows as expected but not necessary.

I'd expect the test to be added in toolkit/components/viewconfig/tests/
Dave, not trying to be a jerk.

Those links you pointed to are not documentation.

For a new contributor, is there any good documentation on how to to write a test?

What we have here is someone who has contributed code (and done a nice job) but is now blocked because we have a test requirement for checkin, but have provided no real documentation on how to make that happen.

This seems like a major contributor issue to me.
Thanks for stepping in.
However, it is good for me to copy and tweak existing code. I have now looked into the linked file and it seems quite relevant here. Maybe I can use it.

On the other hand, I am not sure I could test/run the test as I am developing on comm-central so it will not build whole Firefox.
(In reply to Michael Kaply (mkaply) from comment #37)
> Dave, not trying to be a jerk.
> 
> Those links you pointed to are not documentation.
> 
> For a new contributor, is there any good documentation on how to to write a
> test?

My apologies, I didn't notice I accidentally copied the same link twice. The first link was meant to be to the actual documentation here: https://developer.mozilla.org/en/Browser_chrome_tests
I do not have (and do not aim to have) the infrastructure/knowledge to build and run Firefox tests dependably as I mostly work on Thunderbird.

If anybody can add the missing test then please pick this up. Thanks.
Assignee: acelists → nobody
Status: ASSIGNED → NEW
Flags: in-testsuite?
Whiteboard: [testday-20110603][parity-ie] → [testday-20110603][parity-ie][has patch][needs test per comment 36]
(In reply to Mike Kaply [:mkaply] from comment #8)
> My suggestion would be that if the pref:
> 
> general.warnOnAboutConfig
> 
> is locked, then about:config doesn't show
> 
> That would be an easy way to add this functionality without major change.

when you set  
lockPref("general.warnOnAboutConfig", false)

then no warning message appears and the about:config still works :(
That solution wasn't implemented. If you want to block about:config, you can use the CCK2:

http://mike.kaply.com/cck2/
After XUL is ended, (current) CCK2 won't work and we still need another solution to deactivate about:config. I think now it is the time to move toward.
> After XUL is ended, (current) CCK2 won't work and we still need another solution to deactivate about:config. I think now it is the time to move toward.

You will have to run the CCK2 on Firefox 52, but the functionality will still work on Firefox 57 and beyond.

I have no plans to deprecate the CCK@ until I have to.
Hmm, to be honest, I'm afraid of death of very base technologies including chrome.manifest, Components.utils.import() ("Components" itself seems a part of the dying XPCOM system), and so on, at more future versions. (Am I too nervous?)
(In reply to YUKI "Piro" Hiroshi from comment #48)
> Hmm, to be honest, I'm afraid of death of very base technologies including
> chrome.manifest, Components.utils.import() ("Components" itself seems a part
> of the dying XPCOM system), and so on, at more future versions. (Am I too
> nervous?)

At some point, yes, those things will change and hopefully by then we have a better solution for enterprise type changes.

Until then, I'm hopeful that most of the things we need today will stick around. I don't see XUL and XPCOM being ripped out as soon as 57 ships.
I've created a patch file which is based on aceman's work - attached as https://bugzilla.mozilla.org/attachment.cgi?id=591600

There are two points which is updated from original patch

* follow recent mozilla-central source code changes
* add browser chrome test case

Any thought?
Attachment #8932811 - Flags: review?(enndeakin)
Comment on attachment 8932811 [details] [diff] [review]
Patch to block user access to about:config

I've updated requestee because this patch modifies config.js.
Attachment #8932811 - Flags: review?(enndeakin) → review?(jaws)
Comment on attachment 8932814 [details]
test case which is about:config is disabled by https://bugzilla.mozilla.org/attachment.cgi?id=8932811

I'm really not sure if this is something that we should do. I'm requesting ui-review from Stephen to see if this (a) something we should consider and (b) how the UI/text should look.
Attachment #8932814 - Flags: ui-review?(shorlander)
Comment on attachment 8932811 [details] [diff] [review]
Patch to block user access to about:config

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

I'm going to hold off on doing a full review until I hear Stephen's response.

::: toolkit/components/viewconfig/content/config.js
@@ +316,5 @@
>    gTypeStrs[nsIPrefBranch.PREF_INT] = gConfigBundle.getString("int");
>    gTypeStrs[nsIPrefBranch.PREF_BOOL] = gConfigBundle.getString("bool");
>  
> +  // Lock/Block user access to about:config
> +  if (gPrefBranch.getBoolPref("general.blockAboutConfig")) {

Comment #8 had a good recommendation to just check if `general.warnOnAboutConfig` was locked. I would prefer using that versus introducing a new pref.
Attachment #8932811 - Flags: review?(jaws)
It looks like this is intended to be covered by the new policy engine work.
Flags: needinfo?(felipc)
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #54)
> ::: toolkit/components/viewconfig/content/config.js
> @@ +316,5 @@
> >    gTypeStrs[nsIPrefBranch.PREF_INT] = gConfigBundle.getString("int");
> >    gTypeStrs[nsIPrefBranch.PREF_BOOL] = gConfigBundle.getString("bool");
> >  
> > +  // Lock/Block user access to about:config
> > +  if (gPrefBranch.getBoolPref("general.blockAboutConfig")) {
> 
> Comment #8 had a good recommendation to just check if
> `general.warnOnAboutConfig` was locked. I would prefer using that versus
> introducing a new pref.

At first, I thought that it is reasonable to add new
pref(general.blockAboutConfig) as
same as original patch from aceman's because role of pref is different.

Thus, I assumed the following situations. (A)

* pref("general.warnOnAboutConfig", true)
  Allow to access about:config but it requires confirmation
* pref("general.warnOnAboutConfig", false)
  Allow to access about:config directly
* pref("general.blockAboutConfig", true)
  Block user access to about:config completely
* pref("general.blockAboutConfig", false)
  Don't block user access to about:config, so it depends on
  general.warnOnAboutConfig preference whether user can access to
  about:config with/without warning.

With feedback Comment #8 and Comment #54, I've reconsidered that it
should be the followings as same as above comments - I mean (B):

* lockPref("general.warnOnAboutConfig", true)
  Block user access to about:config completely
* lockPref("general.warnOnAboutConfig", false)
  Allow to access about:config without confirmation
* pref("general.warnOnAboutConfig", true)
  Allow to access about:config but it requires confirmation
* pref("general.warnOnAboutConfig", false)
  Allow to access about:config without confirmation

With above suggestion, one user experience will be lost.

* Administrator allow users to access about:config but always
  need confirmation before showing about:config. (C)

The behavior of lockPref("general.warnOnAboutConfig", true) will be
changed in contrast to the previous versions, but there is no problem
in corporate use.

I had some experiences with customized version of Firefox in corporate
use, but there is no such a situation (C) because most of
administrators want to disable access to about:config for their
users. So there is no problem even though the behavior is changed to
(B).

In corporate use, health report is always disabled, so use case (C)
may be not monitored through collection of health report by mozilla,
so it may be hard to judge whether above suggestion is reasonable from it,
but in real world for corporate use case, this feature is really
needed.

Any thoughts?
(In reply to Dave Townsend [:mossop] from comment #55)
> It looks like this is intended to be covered by the new policy engine work.

Indeed, there's ongoing work for this, tracked by bug 1421707. When about:config is blocked, we'll show a neterror page saying "This page has been blocked by your system administrator" or something like that.
Flags: needinfo?(felipc)
Thanks, I'll close this bug and dupe it forward to bug 1421707. Thanks for bringing this discussion back Kentaro.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → DUPLICATE
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #58)
> Thanks, I'll close this bug and dupe it forward to bug 1421707. Thanks for
> bringing this discussion back Kentaro.

bug 1421707 is private issue, so I can't follow working status of that bug.
Could you mind to add me (:kenhys) to bug 1421707 Cc: list, please?
Because This bug affects our customer which use customized version of Firefox in corporate use.
Comment on attachment 8932814 [details]
test case which is about:config is disabled by https://bugzilla.mozilla.org/attachment.cgi?id=8932811

Closing old requests...
Attachment #8932814 - Flags: ui-review?(shorlander)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: