From bab84d7d0ffc9b805eed52e2a9accb4d094fe424 Mon Sep 17 00:00:00 2001 From: Ricki Hirner Date: Fri, 5 Aug 2016 14:55:44 +0200 Subject: [PATCH] Improve HTTP authentication * use preemptive Basic auth automatically for HTTPS connections * cache auth parameters (Basic/Digest) --- .../ui/setup/DavResourceFinderTest.java | 11 +++-- .../at/bitfire/davdroid/AccountSettings.java | 7 +-- .../java/at/bitfire/davdroid/HttpClient.java | 49 ++++++------------- .../davdroid/ui/AccountSettingsActivity.java | 11 ----- .../davdroid/ui/DebugInfoActivity.java | 1 - .../ui/setup/AccountDetailsFragment.java | 2 +- .../davdroid/ui/setup/DavResourceFinder.java | 5 +- .../davdroid/ui/setup/LoginCredentials.java | 5 +- .../ui/setup/LoginCredentialsFragment.java | 6 +-- .../res/layout/login_credentials_fragment.xml | 7 --- app/src/main/res/xml/settings_account.xml | 7 --- dav4android | 2 +- 12 files changed, 30 insertions(+), 83 deletions(-) diff --git a/app/src/androidTest/java/at/bitfire/davdroid/ui/setup/DavResourceFinderTest.java b/app/src/androidTest/java/at/bitfire/davdroid/ui/setup/DavResourceFinderTest.java index 806cd803..2db8fd8a 100644 --- a/app/src/androidTest/java/at/bitfire/davdroid/ui/setup/DavResourceFinderTest.java +++ b/app/src/androidTest/java/at/bitfire/davdroid/ui/setup/DavResourceFinderTest.java @@ -53,11 +53,11 @@ public class DavResourceFinderTest extends InstrumentationTestCase { server.setDispatcher(new TestDispatcher()); server.start(); - credentials = new LoginCredentials(URI.create("/"), "mock", "12345", true); + credentials = new LoginCredentials(URI.create("/"), "mock", "12345"); finder = new DavResourceFinder(getInstrumentation().getContext(), credentials); client = HttpClient.create(); - client = HttpClient.addAuthentication(client, credentials.userName, credentials.password, credentials.authPreemptive); + client = HttpClient.addAuthentication(client, credentials.userName, credentials.password); } @Override @@ -129,8 +129,11 @@ public class DavResourceFinderTest extends InstrumentationTestCase { @Override public MockResponse dispatch(RecordedRequest rq) throws InterruptedException { - if (!checkAuth(rq)) - return new MockResponse().setResponseCode(401); + if (!checkAuth(rq)) { + MockResponse authenticate = new MockResponse().setResponseCode(401); + authenticate.setHeader("WWW-Authenticate", "Basic realm=\"test\""); + return authenticate; + } String path = rq.getPath(); diff --git a/app/src/main/java/at/bitfire/davdroid/AccountSettings.java b/app/src/main/java/at/bitfire/davdroid/AccountSettings.java index 6cd99e59..2a006403 100644 --- a/app/src/main/java/at/bitfire/davdroid/AccountSettings.java +++ b/app/src/main/java/at/bitfire/davdroid/AccountSettings.java @@ -60,7 +60,6 @@ public class AccountSettings { KEY_SETTINGS_VERSION = "version", KEY_USERNAME = "user_name", - KEY_AUTH_PREEMPTIVE = "auth_preemptive", KEY_WIFI_ONLY = "wifi_only", // sync on WiFi only (default: false) KEY_WIFI_ONLY_SSID = "wifi_only_ssid"; // restrict sync to specific WiFi SSID @@ -147,11 +146,10 @@ public class AccountSettings { } } - public static Bundle initialUserData(String userName, boolean preemptive) { + public static Bundle initialUserData(String userName) { Bundle bundle = new Bundle(); bundle.putString(KEY_SETTINGS_VERSION, String.valueOf(CURRENT_VERSION)); bundle.putString(KEY_USERNAME, userName); - bundle.putString(KEY_AUTH_PREEMPTIVE, Boolean.toString(preemptive)); return bundle; } @@ -164,9 +162,6 @@ public class AccountSettings { public String password() { return accountManager.getPassword(account); } public void password(@NonNull String password) { accountManager.setPassword(account, password); } - public boolean preemptiveAuth() { return Boolean.parseBoolean(accountManager.getUserData(account, KEY_AUTH_PREEMPTIVE)); } - public void preemptiveAuth(boolean preemptive) { accountManager.setUserData(account, KEY_AUTH_PREEMPTIVE, Boolean.toString(preemptive)); } - // sync. settings diff --git a/app/src/main/java/at/bitfire/davdroid/HttpClient.java b/app/src/main/java/at/bitfire/davdroid/HttpClient.java index 4c4dc9f7..3758bc46 100644 --- a/app/src/main/java/at/bitfire/davdroid/HttpClient.java +++ b/app/src/main/java/at/bitfire/davdroid/HttpClient.java @@ -12,6 +12,7 @@ import android.accounts.Account; import android.content.Context; import android.os.Build; import android.support.annotation.NonNull; +import android.support.annotation.Nullable; import java.io.IOException; import java.text.SimpleDateFormat; @@ -21,9 +22,7 @@ import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.Logger; -import at.bitfire.dav4android.BasicDigestAuthenticator; -import lombok.RequiredArgsConstructor; -import okhttp3.Credentials; +import at.bitfire.dav4android.BasicDigestAuthHandler; import okhttp3.Interceptor; import okhttp3.OkHttpClient; import okhttp3.Request; @@ -46,13 +45,9 @@ public class HttpClient { public static OkHttpClient create(@NonNull Context context, @NonNull Account account, @NonNull final Logger logger) throws InvalidAccountException { OkHttpClient.Builder builder = defaultBuilder(logger); - // use account settings for authentication and logging + // use account settings for authentication AccountSettings settings = new AccountSettings(context, account); - - if (settings.preemptiveAuth()) - builder.addNetworkInterceptor(new PreemptiveAuthenticationInterceptor(settings.username(), settings.password())); - else - builder.authenticator(new BasicDigestAuthenticator(null, settings.username(), settings.password())); + builder = addAuthentication(builder, null, settings.username(), settings.password()); return builder.build(); } @@ -69,6 +64,7 @@ public class HttpClient { return create(App.log); } + private static OkHttpClient.Builder defaultBuilder(@NonNull final Logger logger) { OkHttpClient.Builder builder = client.newBuilder(); @@ -107,24 +103,23 @@ public class HttpClient { return builder; } - private static OkHttpClient.Builder addAuthentication(@NonNull OkHttpClient.Builder builder, @NonNull String username, @NonNull String password, boolean preemptive) { - if (preemptive) - builder.addNetworkInterceptor(new PreemptiveAuthenticationInterceptor(username, password)); - else - builder.authenticator(new BasicDigestAuthenticator(null, username, password)); - return builder; + private static OkHttpClient.Builder addAuthentication(@NonNull OkHttpClient.Builder builder, @Nullable String host, @NonNull String username, @NonNull String password) { + BasicDigestAuthHandler authHandler = new BasicDigestAuthHandler(host, username, password); + return builder + .addNetworkInterceptor(authHandler) + .authenticator(authHandler); } - public static OkHttpClient addAuthentication(@NonNull OkHttpClient client, @NonNull String username, @NonNull String password, boolean preemptive) { + public static OkHttpClient addAuthentication(@NonNull OkHttpClient client, @NonNull String username, @NonNull String password) { OkHttpClient.Builder builder = client.newBuilder(); - addAuthentication(builder, username, password, preemptive); + addAuthentication(builder, null, username, password); return builder.build(); } public static OkHttpClient addAuthentication(@NonNull OkHttpClient client, @NonNull String host, @NonNull String username, @NonNull String password) { - return client.newBuilder() - .authenticator(new BasicDigestAuthenticator(host, username, password)) - .build(); + OkHttpClient.Builder builder = client.newBuilder(); + addAuthentication(builder, host, username, password); + return builder.build(); } @@ -140,18 +135,4 @@ public class HttpClient { } } - @RequiredArgsConstructor - static class PreemptiveAuthenticationInterceptor implements Interceptor { - final String username, password; - - @Override - public Response intercept(Chain chain) throws IOException { - App.log.fine("Adding basic authorization header for user " + username); - Request request = chain.request().newBuilder() - .header("Authorization", Credentials.basic(username, password)) - .build(); - return chain.proceed(request); - } - } - } diff --git a/app/src/main/java/at/bitfire/davdroid/ui/AccountSettingsActivity.java b/app/src/main/java/at/bitfire/davdroid/ui/AccountSettingsActivity.java index 735532bc..4a37994c 100644 --- a/app/src/main/java/at/bitfire/davdroid/ui/AccountSettingsActivity.java +++ b/app/src/main/java/at/bitfire/davdroid/ui/AccountSettingsActivity.java @@ -114,17 +114,6 @@ public class AccountSettingsActivity extends AppCompatActivity { } }); - final SwitchPreferenceCompat prefPreemptive = (SwitchPreferenceCompat)findPreference("preemptive"); - prefPreemptive.setChecked(settings.preemptiveAuth()); - prefPreemptive.setOnPreferenceChangeListener(new Preference.OnPreferenceChangeListener() { - @Override - public boolean onPreferenceChange(Preference preference, Object newValue) { - settings.preemptiveAuth((Boolean)newValue); - refresh(); - return false; - } - }); - // category: synchronization final ListPreference prefSyncContacts = (ListPreference)findPreference("sync_interval_contacts"); final Long syncIntervalContacts = settings.getSyncInterval(ContactsContract.AUTHORITY); diff --git a/app/src/main/java/at/bitfire/davdroid/ui/DebugInfoActivity.java b/app/src/main/java/at/bitfire/davdroid/ui/DebugInfoActivity.java index 21926dae..93ee19b7 100644 --- a/app/src/main/java/at/bitfire/davdroid/ui/DebugInfoActivity.java +++ b/app/src/main/java/at/bitfire/davdroid/ui/DebugInfoActivity.java @@ -213,7 +213,6 @@ public class DebugInfoActivity extends AppCompatActivity implements LoaderManage " Address book sync. interval: ").append(syncStatus(settings, ContactsContract.AUTHORITY)).append("\n" + " Calendar sync. interval: ").append(syncStatus(settings, CalendarContract.AUTHORITY)).append("\n" + " OpenTasks sync. interval: ").append(syncStatus(settings, "org.dmfs.tasks")).append("\n" + - " Preemptive auth: ").append(settings.preemptiveAuth()).append("\n" + " WiFi only: ").append(settings.getSyncWifiOnly()); if (settings.getSyncWifiOnlySSID() != null) report.append(", SSID: ").append(settings.getSyncWifiOnlySSID()); diff --git a/app/src/main/java/at/bitfire/davdroid/ui/setup/AccountDetailsFragment.java b/app/src/main/java/at/bitfire/davdroid/ui/setup/AccountDetailsFragment.java index a044a84f..01601990 100644 --- a/app/src/main/java/at/bitfire/davdroid/ui/setup/AccountDetailsFragment.java +++ b/app/src/main/java/at/bitfire/davdroid/ui/setup/AccountDetailsFragment.java @@ -107,7 +107,7 @@ public class AccountDetailsFragment extends Fragment { Account account = new Account(accountName, Constants.ACCOUNT_TYPE); // create Android account - Bundle userData = AccountSettings.initialUserData(config.userName, config.preemptive); + Bundle userData = AccountSettings.initialUserData(config.userName); App.log.log(Level.INFO, "Creating Android account with initial config", new Object[] { account, userData }); AccountManager accountManager = AccountManager.get(getContext()); diff --git a/app/src/main/java/at/bitfire/davdroid/ui/setup/DavResourceFinder.java b/app/src/main/java/at/bitfire/davdroid/ui/setup/DavResourceFinder.java index 7c9f45eb..21a297ff 100644 --- a/app/src/main/java/at/bitfire/davdroid/ui/setup/DavResourceFinder.java +++ b/app/src/main/java/at/bitfire/davdroid/ui/setup/DavResourceFinder.java @@ -78,7 +78,7 @@ public class DavResourceFinder { log.addHandler(logBuffer); httpClient = HttpClient.create(log); - httpClient = HttpClient.addAuthentication(httpClient, credentials.userName, credentials.password, credentials.authPreemptive); + httpClient = HttpClient.addAuthentication(httpClient, credentials.userName, credentials.password); } @@ -88,7 +88,7 @@ public class DavResourceFinder { calDavConfig = findInitialConfiguration(Service.CALDAV); return new Configuration( - credentials.userName, credentials.password, credentials.authPreemptive, + credentials.userName, credentials.password, cardDavConfig, calDavConfig, logBuffer.toString() ); @@ -371,7 +371,6 @@ public class DavResourceFinder { } public final String userName, password; - public final boolean preemptive; public final ServiceInfo cardDAV; public final ServiceInfo calDAV; diff --git a/app/src/main/java/at/bitfire/davdroid/ui/setup/LoginCredentials.java b/app/src/main/java/at/bitfire/davdroid/ui/setup/LoginCredentials.java index 648d51a0..9fb3960a 100644 --- a/app/src/main/java/at/bitfire/davdroid/ui/setup/LoginCredentials.java +++ b/app/src/main/java/at/bitfire/davdroid/ui/setup/LoginCredentials.java @@ -19,7 +19,6 @@ import lombok.RequiredArgsConstructor; public class LoginCredentials implements Parcelable { public final URI uri; public final String userName, password; - public final boolean authPreemptive; @Override public int describeContents() { @@ -31,7 +30,6 @@ public class LoginCredentials implements Parcelable { dest.writeSerializable(uri); dest.writeString(userName); dest.writeString(password); - dest.writeValue(authPreemptive); } public static final Creator CREATOR = new Creator() { @@ -39,8 +37,7 @@ public class LoginCredentials implements Parcelable { public LoginCredentials createFromParcel(Parcel source) { return new LoginCredentials( (URI)source.readSerializable(), - source.readString(), source.readString(), - (boolean)source.readValue(null) + source.readString(), source.readString() ); } diff --git a/app/src/main/java/at/bitfire/davdroid/ui/setup/LoginCredentialsFragment.java b/app/src/main/java/at/bitfire/davdroid/ui/setup/LoginCredentialsFragment.java index 954df682..0a3b057b 100644 --- a/app/src/main/java/at/bitfire/davdroid/ui/setup/LoginCredentialsFragment.java +++ b/app/src/main/java/at/bitfire/davdroid/ui/setup/LoginCredentialsFragment.java @@ -43,7 +43,6 @@ public class LoginCredentialsFragment extends Fragment implements CompoundButton LinearLayout urlDetails; EditText editBaseURL, editUserName; EditPassword editUrlPassword; - CheckBox checkPreemptiveAuth; @Override @@ -60,7 +59,6 @@ public class LoginCredentialsFragment extends Fragment implements CompoundButton editBaseURL = (EditText)v.findViewById(R.id.base_url); editUserName = (EditText)v.findViewById(R.id.user_name); editUrlPassword = (EditPassword)v.findViewById(R.id.url_password); - checkPreemptiveAuth = (CheckBox)v.findViewById(R.id.preemptive_auth); radioUseEmail.setOnCheckedChangeListener(this); radioUseURL.setOnCheckedChangeListener(this); @@ -114,7 +112,7 @@ public class LoginCredentialsFragment extends Fragment implements CompoundButton valid = false; } - return valid ? new LoginCredentials(uri, email, password, true) : null; + return valid ? new LoginCredentials(uri, email, password) : null; } else if (radioUseURL.isChecked()) { URI uri = null; @@ -159,7 +157,7 @@ public class LoginCredentialsFragment extends Fragment implements CompoundButton valid = false; } - return valid ? new LoginCredentials(uri, userName, password, checkPreemptiveAuth.isChecked()) : null; + return valid ? new LoginCredentials(uri, userName, password) : null; } return null; diff --git a/app/src/main/res/layout/login_credentials_fragment.xml b/app/src/main/res/layout/login_credentials_fragment.xml index bbc04c56..3ea46896 100644 --- a/app/src/main/res/layout/login_credentials_fragment.xml +++ b/app/src/main/res/layout/login_credentials_fragment.xml @@ -94,13 +94,6 @@ android:layout_height="wrap_content" android:hint="@string/login_password"/> - - diff --git a/app/src/main/res/xml/settings_account.xml b/app/src/main/res/xml/settings_account.xml index 3b3eff8c..9cd603c2 100644 --- a/app/src/main/res/xml/settings_account.xml +++ b/app/src/main/res/xml/settings_account.xml @@ -25,13 +25,6 @@ android:summary="@string/settings_password_summary" android:dialogTitle="@string/settings_enter_password" /> - - diff --git a/dav4android b/dav4android index bf2004e9..ff40bfd6 160000 --- a/dav4android +++ b/dav4android @@ -1 +1 @@ -Subproject commit bf2004e9052241345c435c2d46090ab9baf43eb8 +Subproject commit ff40bfd61f40a445b0e9414ae15b4a02fb95f64b