move HTTP Auth to HttpDownloader to make it testable

This also encapsulates the HTTP Auth stuff better so that it will be easier
to wrap it all into a event-based service.
This commit is contained in:
Hans-Christoph Steiner 2016-03-28 22:17:36 +02:00
parent 591b23b5ab
commit ae0976d24a
9 changed files with 75 additions and 142 deletions

View File

@ -84,7 +84,6 @@ import org.fdroid.fdroid.data.Apk;
import org.fdroid.fdroid.data.ApkProvider;
import org.fdroid.fdroid.data.App;
import org.fdroid.fdroid.data.AppProvider;
import org.fdroid.fdroid.data.Credentials;
import org.fdroid.fdroid.data.InstalledAppProvider;
import org.fdroid.fdroid.data.Repo;
import org.fdroid.fdroid.data.RepoProvider;
@ -857,19 +856,8 @@ public class AppDetails extends AppCompatActivity implements ProgressListener, A
return repo.address;
}
@Nullable
private Credentials getRepoCredentials(Apk apk) {
final String[] projection = {RepoProvider.DataColumns.USERNAME, RepoProvider.DataColumns.PASSWORD};
Repo repo = RepoProvider.Helper.findById(this, apk.repo, projection);
if (repo == null || repo.username == null || repo.password == null) {
return null;
}
return repo.getCredentials();
}
private void startDownload(Apk apk, String repoAddress) {
downloadHandler = new ApkDownloader(getBaseContext(), app, apk, repoAddress);
downloadHandler.setCredentials(getRepoCredentials(apk));
localBroadcastManager.registerReceiver(downloaderProgressReceiver,
new IntentFilter(Downloader.LOCAL_ACTION_PROGRESS));

View File

@ -98,9 +98,7 @@ public class RepoUpdater {
Downloader downloader = null;
try {
downloader = DownloaderFactory.create(context,
getIndexAddress(), File.createTempFile("index-", "-downloaded", context.getCacheDir()),
repo.getCredentials()
);
getIndexAddress(), File.createTempFile("index-", "-downloaded", context.getCacheDir()));
downloader.setCacheTag(repo.lastetag);
downloader.download();

View File

@ -1,36 +0,0 @@
/*
* Copyright (C) 2015 Christian Morgner (christian.morgner@structr.com)
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
* as published by the Free Software Foundation; either version 3
* of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
* MA 02110-1301, USA.
*/
package org.fdroid.fdroid.data;
import java.net.HttpURLConnection;
/**
* Credentials to authenticate HTTP requests. Implementations if this interface
* should encapsulate the authentication of an HTTP request in the authenticate
* method.
*/
public interface Credentials {
/**
* Implement this method to provide authentication for the given connection.
* @param connection the HTTP connection to authenticate
*/
void authenticate(final HttpURLConnection connection);
}

View File

@ -5,7 +5,6 @@ import android.database.Cursor;
import android.text.TextUtils;
import org.fdroid.fdroid.Utils;
import org.fdroid.fdroid.net.auth.HttpBasicCredentials;
import java.net.MalformedURLException;
import java.net.URL;
@ -149,21 +148,6 @@ public class Repo extends ValueObject {
return value;
}
/**
* Returns the credentials for this repo, or null of no authentication
* method is configured.
* @return the credentials or null
*/
public Credentials getCredentials() {
// return the only credentials implementation we have right now
if (!TextUtils.isEmpty(username) && !TextUtils.isEmpty(password)) {
return new HttpBasicCredentials(username, password);
}
return null;
}
public void setValues(ContentValues values) {
if (values.containsKey(RepoProvider.DataColumns._ID)) {

View File

@ -36,7 +36,6 @@ import org.fdroid.fdroid.Utils;
import org.fdroid.fdroid.compat.FileCompat;
import org.fdroid.fdroid.data.Apk;
import org.fdroid.fdroid.data.App;
import org.fdroid.fdroid.data.Credentials;
import org.fdroid.fdroid.data.SanitizedFile;
import java.io.File;
@ -69,7 +68,6 @@ public class ApkDownloader implements AsyncDownloader.Listener {
private ProgressListener listener;
private AsyncDownloader dlWrapper;
private Credentials credentials;
private boolean isComplete;
private final long id = ++downloadIdCounter;
@ -188,7 +186,7 @@ public class ApkDownloader implements AsyncDownloader.Listener {
Utils.debugLog(TAG, "Downloading apk from " + remoteAddress + " to " + localFile);
try {
dlWrapper = DownloaderFactory.createAsync(context, remoteAddress, localFile, credentials, this);
dlWrapper = DownloaderFactory.createAsync(context, remoteAddress, localFile, this);
dlWrapper.download();
return true;
} catch (IOException e) {
@ -271,8 +269,4 @@ public class ApkDownloader implements AsyncDownloader.Listener {
public int getTotalBytes() {
return dlWrapper != null ? dlWrapper.getTotalBytes() : 0;
}
public void setCredentials(final Credentials credentials) {
this.credentials = credentials;
}
}

View File

@ -4,7 +4,9 @@ import android.content.Context;
import android.content.Intent;
import android.support.v4.content.LocalBroadcastManager;
import org.fdroid.fdroid.data.Credentials;
import org.apache.commons.io.FilenameUtils;
import org.fdroid.fdroid.data.Repo;
import org.fdroid.fdroid.data.RepoProvider;
import java.io.File;
import java.io.IOException;
@ -23,10 +25,10 @@ public class DownloaderFactory {
throws IOException {
File destFile = File.createTempFile("dl-", "", context.getCacheDir());
destFile.deleteOnExit(); // this probably does nothing, but maybe...
return create(context, new URL(urlString), destFile, null);
return create(context, new URL(urlString), destFile);
}
public static Downloader create(Context context, URL url, File destFile, Credentials credentials)
public static Downloader create(Context context, URL url, File destFile)
throws IOException {
Downloader downloader = null;
if (localBroadcastManager == null) {
@ -39,7 +41,14 @@ public class DownloaderFactory {
} else if (isLocalFile(url)) {
downloader = new LocalFileDownloader(url, destFile);
} else {
downloader = new HttpDownloader(url, destFile, credentials);
final String[] projection = {RepoProvider.DataColumns.USERNAME, RepoProvider.DataColumns.PASSWORD};
String repoUrlString = FilenameUtils.getBaseName(url.toString());
Repo repo = RepoProvider.Helper.findByAddress(context, repoUrlString, projection);
if (repo == null) {
downloader = new HttpDownloader(url, destFile);
} else {
downloader = new HttpDownloader(url, destFile, repo.username, repo.password);
}
}
downloader.setListener(new Downloader.DownloaderProgressListener() {
@ -63,9 +72,9 @@ public class DownloaderFactory {
return "file".equalsIgnoreCase(url.getProtocol());
}
public static AsyncDownloader createAsync(Context context, String urlString, File destFile, Credentials credentials, AsyncDownloader.Listener listener)
public static AsyncDownloader createAsync(Context context, String urlString, File destFile, AsyncDownloader.Listener listener)
throws IOException {
URL url = new URL(urlString);
return new AsyncDownloadWrapper(create(context, url, destFile, credentials), listener);
return new AsyncDownloadWrapper(create(context, url, destFile), listener);
}
}

View File

@ -6,7 +6,7 @@ import com.nostra13.universalimageloader.core.download.BaseImageDownloader;
import org.fdroid.fdroid.FDroidApp;
import org.fdroid.fdroid.Utils;
import org.fdroid.fdroid.data.Credentials;
import org.spongycastle.util.encoders.Base64;
import java.io.BufferedInputStream;
import java.io.File;
@ -28,20 +28,33 @@ public class HttpDownloader extends Downloader {
protected static final String HEADER_IF_NONE_MATCH = "If-None-Match";
protected static final String HEADER_FIELD_ETAG = "ETag";
public final String username;
public final String password;
protected HttpURLConnection connection;
private Credentials credentials;
private int statusCode = -1;
HttpDownloader(URL url, File destFile)
throws FileNotFoundException, MalformedURLException {
this(url, destFile, null);
this(url, destFile, null, null);
}
HttpDownloader(URL url, File destFile, final Credentials credentials)
/**
* Create a downloader that can authenticate via HTTP Basic Auth using the supplied
* {@code username} and {@code password}.
*
* @param url The file to download
* @param destFile Where the download is saved
* @param username Username for HTTP Basic Auth, use {@code null} to ignore
* @param password Password for HTTP Basic Auth, use {@code null} to ignore
* @throws FileNotFoundException
* @throws MalformedURLException
*/
HttpDownloader(URL url, File destFile, String username, String password)
throws FileNotFoundException, MalformedURLException {
super(url, destFile);
this.credentials = credentials;
this.username = username;
this.password = password;
}
/**
@ -106,8 +119,10 @@ public class HttpDownloader extends Downloader {
((HttpsURLConnection) connection).setSSLSocketFactory(HttpsURLConnection.getDefaultSSLSocketFactory());
}
if (credentials != null) {
credentials.authenticate(connection);
if (username != null && password != null) {
// add authorization header from username / password if set
String authString = username + ":" + password;
connection.setRequestProperty("Authorization", "Basic " + Base64.toBase64String(authString.getBytes()));
}
}

View File

@ -1,51 +0,0 @@
/*
* Copyright (C) 2015 Christian Morgner (christian.morgner@structr.com)
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
* as published by the Free Software Foundation; either version 3
* of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
* MA 02110-1301, USA.
*/
package org.fdroid.fdroid.net.auth;
import android.text.TextUtils;
import org.apache.commons.net.util.Base64;
import org.fdroid.fdroid.data.Credentials;
import java.net.HttpURLConnection;
/**
* Credentials implementation for HTTP Basic Authentication.
*/
public class HttpBasicCredentials implements Credentials {
private final String username;
private final String password;
public HttpBasicCredentials(final String username, final String password) {
this.username = username;
this.password = password;
}
@Override
public void authenticate(final HttpURLConnection connection) {
if (!TextUtils.isEmpty(username) && !TextUtils.isEmpty(password)) {
// add authorization header from username / password if set
connection.setRequestProperty("Authorization", "Basic " + Base64.encodeBase64String((username + ":" + password).getBytes()));
}
}
}

View File

@ -9,6 +9,7 @@ import java.net.URL;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
@ -32,7 +33,7 @@ public class HttpDownloaderTest {
URL url = new URL(urlString);
File destFile = File.createTempFile("dl-", "");
destFile.deleteOnExit(); // this probably does nothing, but maybe...
HttpDownloader httpDownloader = new HttpDownloader(url, destFile, null);
HttpDownloader httpDownloader = new HttpDownloader(url, destFile);
httpDownloader.download();
assertTrue(destFile.exists());
assertTrue(destFile.canRead());
@ -46,7 +47,7 @@ public class HttpDownloaderTest {
URL url = new URL(urlString);
File destFile = File.createTempFile("dl-", "");
destFile.deleteOnExit(); // this probably does nothing, but maybe...
HttpDownloader httpDownloader = new HttpDownloader(url, destFile, null);
HttpDownloader httpDownloader = new HttpDownloader(url, destFile);
httpDownloader.setListener(new Downloader.DownloaderProgressListener() {
@Override
public void sendProgress(URL sourceUrl, int bytesRead, int totalBytes) {
@ -60,13 +61,44 @@ public class HttpDownloaderTest {
}
}
@Test
public void downloadHttpBasicAuth() throws IOException, InterruptedException {
URL url = new URL("https://httpbin.org/basic-auth/myusername/supersecretpassword");
File destFile = File.createTempFile("dl-", "");
destFile.deleteOnExit(); // this probably does nothing, but maybe...
HttpDownloader httpDownloader = new HttpDownloader(url, destFile, "myusername", "supersecretpassword");
httpDownloader.download();
assertTrue(destFile.exists());
assertTrue(destFile.canRead());
}
@Test(expected = IOException.class)
public void downloadHttpBasicAuthWrongPassword() throws IOException, InterruptedException {
URL url = new URL("https://httpbin.org/basic-auth/myusername/supersecretpassword");
File destFile = File.createTempFile("dl-", "");
destFile.deleteOnExit(); // this probably does nothing, but maybe...
HttpDownloader httpDownloader = new HttpDownloader(url, destFile, "myusername", "wrongpassword");
httpDownloader.download();
assertFalse(destFile.exists());
}
@Test(expected = IOException.class)
public void downloadHttpBasicAuthWrongUsername() throws IOException, InterruptedException {
URL url = new URL("https://httpbin.org/basic-auth/myusername/supersecretpassword");
File destFile = File.createTempFile("dl-", "");
destFile.deleteOnExit(); // this probably does nothing, but maybe...
HttpDownloader httpDownloader = new HttpDownloader(url, destFile, "wrongusername", "supersecretpassword");
httpDownloader.download();
assertFalse(destFile.exists());
}
@Test
public void downloadThenCancel() throws IOException, InterruptedException {
final CountDownLatch latch = new CountDownLatch(5);
URL url = new URL("https://f-droid.org/repo/index.jar");
File destFile = File.createTempFile("dl-", "");
destFile.deleteOnExit(); // this probably does nothing, but maybe...
final HttpDownloader httpDownloader = new HttpDownloader(url, destFile, null);
final HttpDownloader httpDownloader = new HttpDownloader(url, destFile);
httpDownloader.setListener(new Downloader.DownloaderProgressListener() {
@Override
public void sendProgress(URL sourceUrl, int bytesRead, int totalBytes) {