[TIMOB-24748] Android: HTTPClient - can't use streaming mode with authentication

[ypbnv]

Fix:
Remove Authenticator and add Basic authentication headers manually to be able to support it with streamed requests.

JIRA: https://jira.appcelerator.org/browse/TIMOB-24748

Test case In the ticket.

Optional Description:
According to the documentation:
https://developer.android.com/reference/java/net/HttpURLConnection.html
HttpUrlConnection supports only Basic Authentication by default using Authenticator, so that should be reasonable.

[jquick-axway]

Question:
Should we still use the "Authenticator" class along with your new changes in case we get other security challenges (perhaps TLS) from the server? I don't know if we need to (I would hope the Authorization header would be preserved), just asking the question.

I imagine streaming was broken because the 1st HTTP request sent was rejected due to a 401 "authorization challenge" response from the server. The system had to send a 2nd HTTP request responding to the challenge with the authorization header set (thanks to the Authenticator). But I imagine the same issue can happen due to a TLS security challenge as well, right? (If there is a TLS issue too, then we can make that a separate bug/fix.)

[garymathews]

Set line 106 to:

private HTTPClientProxy proxy;

and line 402 to:

this.proxy = (HTTPClientProxy) proxy;

so you can use proxy.getUsername() and proxy.getPassword() instead?

[garymathews]

Maybe just inline this?

String encodedCredentials = Base64.encodeToString((username + ":" + password).getBytes(), Base64.DEFAULT);

[ypbnv]

@jquick-axway I can't find reliable info whether Authenticator class handles TLS challenges.
In general I think exposing it will be a good decision - after all it is a class designed to deal with authentication. Although this introduces a new problem - we use fixed length streaming mode in HttpUrlConnection to properly report the progress of sending data and that does not work with the Authenticator.

Maybe a decent compromise will be:

• expose the Authenticator class;
• if we have username/password pair in the proxy - create a default one (the behaviour we have now);
• disable the internal buffer only when we have a "ondatastream" listener attached;
• document that they do not work well together and propose manually adding the headers as a workaround;
  I think these will cover basic usage and functionality. And for cases where both of these are needed (ondatastream listening and authentication) we have a solution. For more complex scenarios manual handling of challenges, redirects, and responses should be left for the developer.

@garymathews What are your thoughts about that?

[garymathews]

I don't think Authenticator affects TLS, we only use it for basic auth. Although, we should create some test cases to prove that. I have found reports of issues using Authenticator and it looks like the only workaround is to manually add the request header. I don't see any problems with this, but we should create a thorough test case.

[jquick-axway]

I may be over-thinking the TLS handling. My understanding is that our TiSocketFactory (which derives from SSLSocketFactory) handles TLS. With this class, the system is supposed to automatically handle the handshaking and respond with a client certificate for us (if requested). I was just curious if we would run into the same streaming bug with websites using TLS that do client certificate requests. But we may not, because the handshake may happen before the HTTP request is sent to the server (so there is no re-send in this case). We're probably fine. I was just posing the question.

One more thing. The "Authenticator" class is supposed to handle authorization challenges from proxy servers too. In this case, HTTP responses having a "Proxy-Authenticate" header instead of the typical "Authenticate" header and require the client to re-send the HTTP request with a "Proxy-Authorization" header set to username:password. I'm pretty sure our old code would handle both cases with the given username/password pair (not sure if that's a good thing). Our new code breaks proxy authorization support. @ypbnv, you may be right... proxy handling may need to be handled manually by the developer.

[garymathews]

You're right, the handshake happens before the request, this is why it is recommended to use HTTPS when using basic authentication, for added security since the credentials are pretty much pain-text.

Also, when a proxy is set (Wi-Fi or APN) the proxy authentication is handled by Android and not us, the user can't connect to a proxy server manually either as we don't support CONNECT in Ti.Network.HTTPClient.open - so we don't need to worry about that.

Here's some test cases:

var win = Ti.UI.createWindow({backgroundColor: 'gray', layout: 'vertical'}),
    btn = function(title, click) {
        var b = Ti.UI.createButton({
            title: title,
            width: '100%',
            height: 40
        });
        b.addEventListener('click', click);
        return b;
    },
    config = {
        onload: function(e) {
            Ti.API.info(JSON.stringify(e, null, ' '));
            alert('success: ' + e.source.status);
        },
        onerror: function(e) {
            Ti.API.error(JSON.stringify(e, null, ' '));
            alert('error: ' + e.error);
        },
        timeout: 5000
    };

win.add(btn('BASIC AUTH - GET HTTP', function() {
    var http = Ti.Network.createHTTPClient(Object.assign(config, {
        username: 'user',
        password: 'passwd'
    }));
    http.open('GET', 'http://httpbin.org/basic-auth/user/passwd');
    http.send();
}));
win.add(btn('BASIC AUTH - GET HTTPS', function() {
    var http = Ti.Network.createHTTPClient(Object.assign(config, {
        username: 'user',
        password: 'passwd'
    }));
    http.open('GET', 'https://httpbin.org/basic-auth/user/passwd');
    http.send();
}));
win.add(btn('BASIC DIGEST - GET HTTPS', function() { // TiAuthenticator does not support this
    var http = Ti.Network.createHTTPClient(Object.assign(config, {
        username: 'user',
        password: 'passwd'
    }));
    http.open('GET', 'https://httpbin.org/digest-auth/auth/user/passwd/MD5');
    http.send();
}));
win.add(btn('BASIC AUTH - POST HTTPS', function() {
    var http = Ti.Network.createHTTPClient(Object.assign(config, {
        username: 'user',
        password: 'p$wd'
    }));
    http.open('POST', 'https://srvasic.upv.es/recursos/error401.asp');
    http.send();
}));

win.open();

[jquick-axway]

@garymathews, I was thinking more of something like a "transparent proxy". That is, a proxy server that you'll hit along the way to the endpoint and isn't needed to gain access to the Internet. These type of proxies tend to intercept HTTP requests for fast cached responses, logging, manipulating the requests (not necessarily for nefarious reasons), etc. Although, if such a proxy asked for a username/password, then it wouldn't be very transparent. :)

But if it did respond with a "Proxy-Authenticate", then I don't think that should be handled by the username/password passed to the HttpClient API because that login was intended for the endpoint. So, breaking it makes sense to me.

[jquick-axway]

CR: Pass

[mukherjee2]

Tested with this environment:
Node Version: 6.10.3
NPM Version: 3.10.10
Mac OS: 10.12.4
Appc CLI: 6.2.2
Appc CLI NPM: 4.2.9
Titanium SDK version: PR/9104
Appcelerator Studio, build: 4.9.0.201705302345
Xcode 8.3.2
FR Passed. After verifying the regresssion (no errors in SDK 6.0.4, but errors in SDK 6.1.0) I tested with the above testcase, and no errors were seen in the latest fix.