1138354 - Allow browser app to get correct information for proxy authentication

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Shian-Yow Wu [:swu]]

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.

Comment 1

[Shian-Yow Wu [:swu]]

Attachment: Patch: Add proxy authencation info for 'mozbrowserusernameandpasswordrequired' event.

Here is the solution 2 patch.

Comment 2

[Shian-Yow Wu [:swu]]

Attachment: 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.

Comment 3

[Shian-Yow Wu [:swu]]

Modify the summary to reflect what's actually done in the patch.

Comment 4

[Shian-Yow Wu [:swu]]

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

Hi Honza, could you help to review this patch?

Comment 5

[Shian-Yow Wu [:swu]]

Attachment: 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

Comment 6

[Honza Bambas (:mayhemer)]

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;

Comment 7

[Shian-Yow Wu [:swu]]

(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!

Comment 8

[Shian-Yow Wu [:swu]]

Attachment: 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

Comment 9

[Shian-Yow Wu [:swu]]

Try result looks ok.

https://hg.mozilla.org/integration/mozilla-inbound/rev/106f8198c67e

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/106f8198c67e