Merge pull request #2008 from dhis2/androsdk-1739

fix: [ANDROSDK-1739] Replace authentication encoding to support special characters

core/src/main/java/org/hisp/dhis/android/core/arch/api/authentication/internal/OpenIDConnectAuthenticator.kt

@@ -32,7 +32,6 @@ import javax.inject.Inject
 import okhttp3.Interceptor
 import okhttp3.Request
 import okhttp3.Response
-import org.hisp.dhis.android.core.arch.api.authentication.internal.UserIdAuthenticatorHelper.Companion.AUTHORIZATION_KEY
 import org.hisp.dhis.android.core.arch.storage.internal.Credentials
 import org.hisp.dhis.android.core.arch.storage.internal.CredentialsSecureStore
 import org.hisp.dhis.android.core.user.openid.OpenIDConnectLogoutHandler
@@ -70,6 +69,9 @@ internal class OpenIDConnectAuthenticator @Inject constructor(
     }
 
     private fun addTokenHeader(builder: Request.Builder, token: String): Request.Builder {
-        return builder.addHeader(AUTHORIZATION_KEY, "Bearer $token")
+        return builder.addHeader(
+            UserIdAuthenticatorHelper.AUTHORIZATION_KEY,
+            "Bearer $token"
+        )
     }
 }

core/src/main/java/org/hisp/dhis/android/core/arch/api/authentication/internal/PasswordAndCookieAuthenticator.kt

@@ -32,8 +32,6 @@ import javax.inject.Inject
 import okhttp3.Interceptor
 import okhttp3.Request
 import okhttp3.Response
-import org.hisp.dhis.android.core.arch.api.authentication.internal.UserIdAuthenticatorHelper.Companion.AUTHORIZATION_KEY
-import org.hisp.dhis.android.core.arch.helpers.UserHelper
 import org.hisp.dhis.android.core.arch.storage.internal.Credentials
 
 @Reusable
@@ -73,11 +71,9 @@ internal class PasswordAndCookieAuthenticator @Inject constructor(
     }
 
     private fun addPasswordHeader(builder: Request.Builder, credentials: Credentials): Request.Builder {
-        return builder.addHeader(AUTHORIZATION_KEY, getAuthorizationForPassword(credentials))
-    }
-
-    private fun getAuthorizationForPassword(credentials: Credentials): String {
-        val base64Credentials = UserHelper.base64(credentials.username, credentials.password)
-        return "Basic $base64Credentials"
+        return builder.addHeader(
+            UserIdAuthenticatorHelper.AUTHORIZATION_KEY,
+            UserIdAuthenticatorHelper.basic(credentials)
+        )
     }
 }

core/src/main/java/org/hisp/dhis/android/core/arch/api/authentication/internal/UserIdAuthenticatorHelper.kt

@@ -31,6 +31,8 @@ import dagger.Reusable
 import javax.inject.Inject
 import okhttp3.Interceptor
 import okhttp3.Request
+import org.hisp.dhis.android.core.arch.helpers.UserHelper
+import org.hisp.dhis.android.core.arch.storage.internal.Credentials
 import org.hisp.dhis.android.core.arch.storage.internal.UserIdInMemoryStore
 
 @Reusable
@@ -41,6 +43,15 @@ internal class UserIdAuthenticatorHelper @Inject constructor(
     companion object {
         const val AUTHORIZATION_KEY = "Authorization"
         private const val USER_ID_KEY = "x-dhis2-user-id"
+
+        fun basic(credentials: Credentials): String {
+            return basic(credentials.username, credentials.password!!)
+        }
+
+        fun basic(username: String, password: String): String {
+            val base64Credentials = UserHelper.base64(username, password)
+            return "Basic $base64Credentials"
+        }
     }
 
     fun builderWithUserId(chain: Interceptor.Chain): Request.Builder {

core/src/main/java/org/hisp/dhis/android/core/arch/helpers/UserHelper.java → core/src/main/java/org/hisp/dhis/android/core/arch/helpers/UserHelper.kt

@@ -25,77 +25,71 @@
  *  (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
  *  SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
+package org.hisp.dhis.android.core.arch.helpers
 
-package org.hisp.dhis.android.core.arch.helpers;
-
-
-import java.nio.charset.StandardCharsets;
-import java.security.MessageDigest;
-import java.security.NoSuchAlgorithmException;
-
-import okio.ByteString;
-
-public final class UserHelper {
-
-    private UserHelper() {
-        // no instances
-    }
+import java.nio.charset.StandardCharsets
+import java.security.MessageDigest
+import java.security.NoSuchAlgorithmException
+import okio.ByteString
 
+object UserHelper {
     /**
-     * Encode the given username and password to a base 64 {@link String}.
+     * Encode the given username and password to a base 64 [String].
      *
      * @param username The username of the user account.
      * @param password The password of the user account.
-     * @return An encoded base 64 {@link String}.
+     * @return An encoded base 64 [String].
      */
-    public static String base64(String username, String password) {
-        return base64(username + ":" + password);
+    fun base64(username: String, password: String): String {
+        return base64(usernameAndPassword(username, password))
     }
 
     /**
-     * Encode the given string to a base 64 {@link String}.
+     * Encode the given string to a base 64 [String].
      *
      * @param value The value to encode
-     * @return An encoded base 64 {@link String}.
+     * @return An encoded base 64 [String].
      */
-    public static String base64(String value) {
-        byte[] bytes = value.getBytes(StandardCharsets.ISO_8859_1);
-        return ByteString.of(bytes).base64();
+    @Suppress("SpreadOperator")
+    fun base64(value: String): String {
+        val bytes = value.toByteArray(StandardCharsets.UTF_8)
+        return ByteString.of(*bytes).base64()
     }
 
     /**
-     * Encode the given username and password to a MD5 {@link String}.
+     * Encode the given username and password to a MD5 [String].
      *
      * @param username The username of the user account.
      * @param password The password of the user account.
-     * @return An encoded MD5 {@link String}.
+     * @return An encoded MD5 [String].
      */
-    @SuppressWarnings({"PMD.UseLocaleWithCaseConversions"})
-    public static String md5(String username, String password) {
-        try {
-            String credentials = usernameAndPassword(username, password);
-            MessageDigest md = MessageDigest.getInstance("MD5");
-            md.reset();
-            md.update(credentials.getBytes(StandardCharsets.ISO_8859_1));
-            return bytesToHex(md.digest()).toLowerCase();
-        } catch (NoSuchAlgorithmException noSuchAlgorithmException) {
+    fun md5(username: String, password: String): String {
+        return try {
+            val credentials = usernameAndPassword(username, password)
+            val md = MessageDigest.getInstance("MD5")
+            md.reset()
+            md.update(credentials.toByteArray(StandardCharsets.UTF_8))
+            bytesToHex(md.digest()).lowercase()
+        } catch (noSuchAlgorithmException: NoSuchAlgorithmException) {
             // noop. Every implementation of Java is required to support MD5
-            throw new AssertionError(noSuchAlgorithmException);
+            throw AssertionError(noSuchAlgorithmException)
         }
     }
 
-    private static String bytesToHex(byte[] bytes) {
-        char[] hexArray = "0123456789ABCDEF".toCharArray();
-        char[] hexChars = new char[bytes.length * 2];
-        for (int j = 0; j < bytes.length; j++) {
-            int v = bytes[j] & 0xFF;
-            hexChars[j * 2] = hexArray[v >>> 4];
-            hexChars[j * 2 + 1] = hexArray[v & 0x0F];
+    @Suppress("MagicNumber")
+    private fun bytesToHex(bytes: ByteArray): String {
+        val hexArray = "0123456789ABCDEF".toCharArray()
+        val hexChars = CharArray(bytes.size * 2)
+
+        for (j in bytes.indices) {
+            val v = bytes[j].toInt() and 0xFF
+            hexChars[j * 2] = hexArray[v ushr 4]
+            hexChars[j * 2 + 1] = hexArray[v and 0x0F]
         }
-        return new String(hexChars);
+        return String(hexChars)
     }
 
-    private static String usernameAndPassword(String username, String password) {
-        return username + ":" + password;
+    private fun usernameAndPassword(username: String, password: String): String {
+        return "$username:$password"
     }
 }

core/src/main/java/org/hisp/dhis/android/core/arch/storage/internal/Credentials.kt

@@ -37,7 +37,7 @@ data class Credentials(
     val openIDConnectState: AuthState?
 ) {
     fun getHash(): String? {
-        return password.let { UserHelper.md5(username, it) }
+        return password?.let { UserHelper.md5(username, it) }
     }
 
     override fun equals(other: Any?) =

core/src/main/java/org/hisp/dhis/android/core/user/internal/LogInCall.kt

@@ -31,6 +31,7 @@ import dagger.Reusable
 import io.reactivex.Single
 import javax.inject.Inject
 import net.openid.appauth.AuthState
+import org.hisp.dhis.android.core.arch.api.authentication.internal.UserIdAuthenticatorHelper
 import org.hisp.dhis.android.core.arch.api.executors.internal.APICallExecutor
 import org.hisp.dhis.android.core.arch.api.internal.ServerURLWrapper
 import org.hisp.dhis.android.core.arch.db.access.DatabaseAdapter
@@ -87,7 +88,7 @@ internal class LogInCall @Inject internal constructor(
         ServerURLWrapper.setServerUrl(parsedServerUrl.toString())
 
         val authenticateCall = userService.authenticate(
-            okhttp3.Credentials.basic(username!!, password!!),
+            UserIdAuthenticatorHelper.basic(username!!, password!!),
             UserFields.allFieldsWithoutOrgUnit(null)
         )
 

core/src/main/java/org/hisp/dhis/android/core/user/internal/UserService.java

@@ -28,6 +28,7 @@
 
 package org.hisp.dhis.android.core.user.internal;
 
+import org.hisp.dhis.android.core.arch.api.authentication.internal.UserIdAuthenticatorHelper;
 import org.hisp.dhis.android.core.arch.api.fields.internal.Fields;
 import org.hisp.dhis.android.core.arch.api.filters.internal.Which;
 import org.hisp.dhis.android.core.user.User;
@@ -40,7 +41,7 @@
 interface UserService {
 
     @GET("me")
-    Call<User> authenticate(@Header("Authorization") String credentials,
+    Call<User> authenticate(@Header(UserIdAuthenticatorHelper.AUTHORIZATION_KEY) String credentials,
                             @Query("fields") @Which Fields<User> fields);
 
     @GET("me")

core/src/test/java/org/hisp/dhis/android/core/arch/helpers/UserHelperShould.java → core/src/test/java/org/hisp/dhis/android/core/arch/helpers/UserHelperShould.kt

@@ -25,38 +25,54 @@
  *  (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
  *  SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
+package org.hisp.dhis.android.core.arch.helpers
 
-package org.hisp.dhis.android.core.arch.helpers;
+import com.google.common.truth.Truth.assertThat
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.JUnit4
 
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
-
-import static com.google.common.truth.Truth.assertThat;
+@RunWith(JUnit4::class)
+class UserHelperShould {
+    @Test
+    fun md5_evaluate_same_string() {
+        val md5s1 = UserHelper.md5("user1", "password1")
+        val md5s2 = UserHelper.md5("user1", "password1")
 
-@RunWith(JUnit4.class)
-public class UserHelperShould {
+        assertThat(md5s1.length).isEqualTo(32)
+        assertThat(md5s2.length).isEqualTo(32)
+        assertThat(md5s1 == md5s2).isTrue()
+    }
 
     @Test
-    public void md5_evaluate_same_string() {
-        String md5s1 = UserHelper.md5("user1","password1");
-        String md5s2 = UserHelper.md5("user1","password1");
-
-        assertThat(md5s1.length()).isEqualTo(32);
-        assertThat(md5s2.length()).isEqualTo(32);
+    fun md5_evaluate_different_string() {
+        val md5s1 = UserHelper.md5("user2", "password2")
+        val md5s2 = UserHelper.md5("user3", "password3")
 
-        assertThat(md5s1.equals(md5s2)).isTrue();
+        assertThat(md5s1.length).isEqualTo(32)
+        assertThat(md5s2.length).isEqualTo(32)
+        assertThat(md5s1 == md5s2).isFalse()
     }
 
     @Test
-    public void md5_evaluate_different_string() {
-        String md5s1 = UserHelper.md5("user2", "password2");
-        String md5s2 = UserHelper.md5("user3", "password3");
+    fun md5_evaluate_special_chars() {
+        val md5s1 = UserHelper.md5("user1", "pässword")
+        val md5s2 = UserHelper.md5("user1", "password")
 
-        assertThat(md5s1.length()).isEqualTo(32);
-        assertThat(md5s2.length()).isEqualTo(32);
+        assertThat(md5s1.length).isEqualTo(32)
+        assertThat(md5s2.length).isEqualTo(32)
+        assertThat(md5s1 == md5s2).isFalse()
+    }
 
-        assertThat(md5s1.equals(md5s2)).isFalse();
+    @Test
+    fun base64_encode_credentials() {
+        val base64 = UserHelper.base64("user", "password")
+        assertThat(base64).isEqualTo("dXNlcjpwYXNzd29yZA==")
     }
 
+    @Test
+    fun base64_encode_special_chars() {
+        val base64 = UserHelper.base64("user", "pässword")
+        assertThat(base64).isEqualTo("dXNlcjpww6Rzc3dvcmQ=")
+    }
 }