Allow browser app to get correct information for proxy authentication

RESOLVED FIXED in Firefox 39

Status

Firefox OS
General
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: swu, Assigned: swu)

Tracking

unspecified
2.2 S9 (3apr)
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox39 fixed)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

3 years ago
Background:
     In some conditions, the browser app wants to automatically login with pre-defined username/password when HTTP authentication from proxy server is required, instead of prompting a dialog to user.

Below are two possible solutions.

Solution #1:
     Store username/password/proxy/realm in "Saved Password" by nsILoginManager, and set the preference "singon.autologin.proxy" to true.  Currently the "Saved Password" feature is not supported in browser element, to use this approach, we need to add it first.

Solution #2:
     When HTTP authentication is required inside browser iframe, the 'mozbrowserusernameandpasswordrequired' event will be fired and browser app will prompt a dialog for username/password.  Currently the event doesn't specify whether it's for www or proxy authentication.  Add a flag in the event to specify whether it's proxy authentication, and browser app can then reply with pre-defined username/password(from mozSettings) when it's proxy authentication.
(Assignee)

Updated

3 years ago
Depends on: 1115495
(Assignee)

Comment 1

3 years ago
Created attachment 8571276 [details] [diff] [review]
Patch: Add proxy authencation info for 'mozbrowserusernameandpasswordrequired' event.

Here is the solution 2 patch.
(Assignee)

Comment 2

3 years ago
Created attachment 8579298 [details] [diff] [review]
Patch: Add proxy authencation info for 'mozbrowserusernameandpasswordrequired' event.

The patch allows browser element to:
1. Distinguish whether the authentication is coming from www or proxy server.
2. Display correct hostname if the authentication request is coming from proxy server.
Assignee: nobody → swu
Attachment #8571276 - Attachment is obsolete: true
(Assignee)

Comment 3

3 years ago
Modify the summary to reflect what's actually done in the patch.
Summary: Allow browser app to automatically login for proxy authentication → Allow browser app to get correct information for proxy authentication
(Assignee)

Comment 4

3 years ago
Comment on attachment 8579298 [details] [diff] [review]
Patch: Add proxy authencation info for 'mozbrowserusernameandpasswordrequired' event.

Hi Honza, could you help to review this patch?
Attachment #8579298 - Flags: review?(honzab.moz)
(Assignee)

Comment 5

3 years ago
Created attachment 8581567 [details] [diff] [review]
Patch: Add proxy authencation info for 'mozbrowserusernameandpasswordrequired' event.

This patch fixes a try server error on Android in previous patch.

Try result for this patch looks ok.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2c57db4e3a6c
Attachment #8579298 - Attachment is obsolete: true
Attachment #8579298 - Flags: review?(honzab.moz)
Attachment #8581567 - Flags: review?(honzab.moz)
Comment on attachment 8581567 [details] [diff] [review]
Patch: Add proxy authencation info for 'mozbrowserusernameandpasswordrequired' event.

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

Not locally tested.

::: dom/browser-element/BrowserElementPromptService.jsm
@@ +391,5 @@
>    _getAuthTarget : function (channel, authInfo) {
> +    let hostname, realm;
> +
> +    // If our proxy is demanding authentication, don't use the
> +    // channel's actual destination.

maybe comment this is taken from nsLoginManagerPrompter.js.

any chance to share/call on that code?

::: dom/browser-element/mochitest/browserElement_Auth.js
@@ +113,5 @@
> +      const interfaces = [Ci.nsIProtocolProxyCallback, Ci.nsISupports];
> +
> +      if (!interfaces.some( function(v) { return iid.equals(v) } ))
> +        throw SpecialPowers.Cr.NS_ERROR_NO_INTERFACE;
> +        return this;

if {
  ...
}

return this;
Attachment #8581567 - Flags: review?(honzab.moz) → review+
(Assignee)

Comment 7

3 years ago
(In reply to Honza Bambas (:mayhemer) from comment #6)
> Comment on attachment 8581567 [details] [diff] [review]
> Patch: Add proxy authencation info for
> 'mozbrowserusernameandpasswordrequired' event.
> 
> Review of attachment 8581567 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Not locally tested.

I've tested it on B2G desktop.  It shows correct host information of proxy authentication, just some wordings might be improved by the browser app for proxy authentication.  Will file a new bug for this.

> 
> ::: dom/browser-element/BrowserElementPromptService.jsm
> @@ +391,5 @@
> >    _getAuthTarget : function (channel, authInfo) {
> > +    let hostname, realm;
> > +
> > +    // If our proxy is demanding authentication, don't use the
> > +    // channel's actual destination.
> 
> maybe comment this is taken from nsLoginManagerPrompter.js.

OK, thanks!

> 
> any chance to share/call on that code?

It's a good idea, but I didn't see a simple/clean way to do it.

> 
> ::: dom/browser-element/mochitest/browserElement_Auth.js
> @@ +113,5 @@
> > +      const interfaces = [Ci.nsIProtocolProxyCallback, Ci.nsISupports];
> > +
> > +      if (!interfaces.some( function(v) { return iid.equals(v) } ))
> > +        throw SpecialPowers.Cr.NS_ERROR_NO_INTERFACE;
> > +        return this;
> 
> if {
>   ...
> }
> 
> return this;

OK, thanks!
(Assignee)

Updated

3 years ago
Blocks: 1147772
(Assignee)

Comment 8

3 years ago
Created attachment 8583673 [details] [diff] [review]
Patch: Add proxy authencation info for 'mozbrowserusernameandpasswordrequired' event. (r=mayhermer)

This patched addressed review comments.

Try server:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=aa4bd469980a
Attachment #8581567 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/106f8198c67e
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox39: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S9 (3apr)
You need to log in before you can comment on or make changes to this bug.