Implement basic auth prompt

[MortimerGoro]

Fixes #943

Note: The text message from https://www.httpwatch.com/httpgallery/authentication/authenticatedimage/default.aspx is a bit cropped. It looks like a GeckoView issue, the string is already cropped when the GV event is received.

[bluemarvin]

If GV is truncating the string, please file.

[bluemarvin]

Looks good. Would be nice to find a more generic solution to the setShowSoftInputOnFocus() issue.

[philip-lamb]

I've got an open question with Mozilla Security Engineering team on whether we have a policy about warning the user if the password would be transmitted in cleartext. Will report back once I hear. https://mozilla.slack.com/archives/C8XGKM694/p1551904945003300

[bluemarvin]

It looks like Firefox doesn't warn:
Fennec:

Firefox Desktop:

GeckoView Example:

Chrome for Android doesn't:

But Chrome for desktop does:

Safari on both iOS and macOS do
iOS:

macOS:

I think it would be okay to buck the Mozilla trend and warn the user.

[cvan]

I've got an open question with Mozilla Security Engineering team on whether we have a policy about warning the user if the password would be transmitted in cleartext. Will report back once I hear. https://mozilla.slack.com/archives/C8XGKM694/p1551904945003300

@philip-lamb: this is a good observation. Gecko bug 966754 tracks the issue of plaintext transmission of credentials via Basic Authentication over HTTP.

In basic HTTP authentication, a request contains a header field of the form Authorization: Basic <credentials>, where credentials is the base64 encoding of id and password joined by a colon.

heh, yep.

on a related note, there was a Chrome bug filed to indicate in the UI the security of the connection (to denote insecure origins for Basic Auth on http:// URLs: Chromium platform bug 153917) was filed, reported fix, but it doesn't appear in Chrome's Basic-Auth UI for me anymore (in Chrome M67).

[bluemarvin]

I might look into why GV is returning a truncated string, but I noticed most other browsers don't even display the message. I wonder if this is for security reasons.

[cvan]

I've been having trouble loading pages with FxR, so I cannot properly review this.

I'll review this again when my issues stabilise. And, likely I'll need to file some GV platform bugs for this.

Thanks for working on this. 👍

[cvan]

update: I've been able to successfully test this. Basic Authentication now appears to work correctly. I discovered a couple bugs, a few minor requests to simplify and secure the UI + make the UX consistent, and some interoperability observations between Oculus Browser and Firefox on other platforms. I am compiling screenshots and will post a full review today shortly. thanks for your patience. and thank you for fixing this, @MortimerGoro 👍

[cvan]

solid work here. 👍

see my comments for a few minor changes requested.

let me know if you have any questions.

assuming these items are addressed, feel free to update this branch and merge w/o an additional re-review from me.

thank you for tackling this, @MortimerGoro. 👍

[cvan]

nit: add a newline at the EOF?

[cvan]

nit: a authentication -> an authentication
nit: period as well

[cvan]

re: the issue with the truncated message (Gecko platform bug 1533468): I tried to investigate, but no debug-log messages appeared when I called adb logcat | grep -i onAuthPrompt when going through a Basic-Auth flow:

diff --git a/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/WindowWidget.java b/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/WindowWidget.java
index 8fb90ee..b494fb4 100644
--- a/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/WindowWidget.java
+++ b/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/WindowWidget.java
@@ -548,7 +548,7 @@ public class WindowWidget extends UIWidget implements SessionStore.SessionChange
         GeckoSession session = SessionStore.get().getSession(mSessionId);
         return (session != null) && session.getTextInput().onKeyMultiple(aKeyCode, repeatCount, aEvent);
     }
-    
+
     @Override
     protected void onFocusChanged(boolean aGainFocus, int aDirection, Rect aPreviouslyFocusedRect) {
         super.onFocusChanged(aGainFocus, aDirection, aPreviouslyFocusedRect);
@@ -607,6 +607,7 @@ public class WindowWidget extends UIWidget implements SessionStore.SessionChange
 
     @Override
     public void onAuthPrompt(GeckoSession session, String title, String msg, AuthOptions options, AuthCallback callback) {
+        Log.d(LOGTAG, "onAuthPrompt: title:[" + title.toString() + "] msg:[" + msg.toString() + "]");
         mAuthPrompt = new AuthPromptWidget(getContext());
         mAuthPrompt.mWidgetPlacement.parentHandle = getHandle();
         mAuthPrompt.setTitle(title);

diff --git a/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/WindowWidget.java b/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/WindowWidget.java
index 8fb90ee..efeaf3e 100644
--- a/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/WindowWidget.java
+++ b/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/WindowWidget.java
@@ -548,7 +548,7 @@ public class WindowWidget extends UIWidget implements SessionStore.SessionChange
         GeckoSession session = SessionStore.get().getSession(mSessionId);
         return (session != null) && session.getTextInput().onKeyMultiple(aKeyCode, repeatCount, aEvent);
     }
-    
+
     @Override
     protected void onFocusChanged(boolean aGainFocus, int aDirection, Rect aPreviouslyFocusedRect) {
         super.onFocusChanged(aGainFocus, aDirection, aPreviouslyFocusedRect);
@@ -607,6 +607,7 @@ public class WindowWidget extends UIWidget implements SessionStore.SessionChange
 
     @Override
     public void onAuthPrompt(GeckoSession session, String title, String msg, AuthOptions options, AuthCallback callback) {
+        Log.d(LOGTAG, "called onAuthPrompt method");
         mAuthPrompt = new AuthPromptWidget(getContext());
         mAuthPrompt.mWidgetPlacement.parentHandle = getHandle();
         mAuthPrompt.setTitle(title);

any ideas why the debug-log message is not appearing when I call adb logcat | grep -i onAuthPrompt?

[MortimerGoro]

Thanks for the review @cvan . I addressed most of the issues and asked to file a different issue for others.