Merge pull request #242 from mozilla/loginsapi-add-update

- Implement add/update support into android logins-api
- Bump android library version number to 0.4.1

logins-api/android/build.gradle

@@ -4,7 +4,7 @@ buildscript {
     ext.kotlin_version = '1.2.50'
 
     ext.library = [
-        version: '0.4.0'
+        version: '0.4.1'
     ]
 
     ext.build = [

logins-api/android/library/src/main/java/org/mozilla/sync15/logins/DatabaseLoginsStorage.kt

@@ -123,6 +123,22 @@ class DatabaseLoginsStorage(private val dbPath: String) : Closeable, LoginsStora
         }
     }
 
+    override fun add(login: ServerPassword): SyncResult<String> {
+        return safeAsyncString {
+            val s = login.toJSON().toString()
+            PasswordSyncAdapter.INSTANCE.sync15_passwords_add(this.raw!!, s, it)
+        }.then {
+            SyncResult.fromValue(it!!)
+        }
+    }
+
+    override fun update(login: ServerPassword): SyncResult<Unit> {
+        return safeAsync {
+            val s = login.toJSON().toString()
+            PasswordSyncAdapter.INSTANCE.sync15_passwords_update(this.raw!!, s, it)
+        }
+    }
+
     override fun close() {
         synchronized(PasswordSyncAdapter.INSTANCE) {
             var raw = this.raw;

logins-api/android/library/src/main/java/org/mozilla/sync15/logins/LoginsStorage.kt

@@ -41,7 +41,7 @@ interface LoginsStorage : Closeable {
     fun wipe(): SyncResult<Unit>
 
     /**
-     * Delete a password with the given ID. TODO: should be SyncResult<bool>!
+     * Delete a password with the given ID.
      */
     fun delete(id: String): SyncResult<Boolean>
 
@@ -59,4 +59,39 @@ interface LoginsStorage : Closeable {
      * Fetch the full list of passwords from the underlying storage layer.
      */
     fun list(): SyncResult<List<ServerPassword>>
+
+    /**
+     * Insert the provided login into the database.
+     *
+     * This function ignores values in metadata fields (`timesUsed`,
+     * `timeCreated`, `timeLastUsed`, and `timePasswordChanged`).
+     *
+     * If login has an empty id field, then a GUID will be
+     * generated automatically. The format of generated guids
+     * are left up to the implementation of LoginsStorage (in
+     * practice the [DatabaseLoginsStorage] generates 12-character
+     * base64url (RFC 4648) encoded strings, and [MemoryLoginsStorage]
+     * generates strings using [java.util.UUID.toString])
+     *
+     * This will return an error result if a GUID is provided but
+     * collides with an existing record, or if the provided record
+     * is invalid (missing password, hostname, or doesn't have exactly
+     * one of formSubmitURL and httpRealm).
+     */
+    fun add(login: ServerPassword): SyncResult<String>
+
+    /**
+     * Update the fields in the provided record.
+     *
+     * This will return an error if `login.id` does not refer to
+     * a record that exists in the database, or if the provided record
+     * is invalid (missing password, hostname, or doesn't have exactly
+     * one of formSubmitURL and httpRealm).
+     *
+     * Like `add`, this function will ignore values in metadata
+     * fields (`timesUsed`, `timeCreated`, `timeLastUsed`, and
+     * `timePasswordChanged`).
+     */
+    fun update(login: ServerPassword): SyncResult<Unit>
+
 }

logins-api/android/library/src/main/java/org/mozilla/sync15/logins/LoginsStorageException.kt

@@ -8,13 +8,37 @@
  * specific language governing permissions and limitations under the License. */
 package org.mozilla.sync15.logins
 
-// TODO: Get more descriptive errors here.
+// TODO: More descriptive errors would be nice here...
 open class LoginsStorageException(msg: String): Exception(msg)
 
-/** Indicates that the sync authentication is invalid, likely due to having
+/** This indicates that the sync authentication is invalid, likely due to having
  * expired.
  */
 class SyncAuthInvalidException(msg: String): LoginsStorageException(msg)
 
-// This doesn't really belong in this file...
+/**
+ * This is thrown if `lock()`/`unlock()` pairs don't match up.
+ */
 class MismatchedLockException(msg: String): LoginsStorageException(msg)
+
+/**
+ * This is thrown if `update()` is performed with a record whose ID
+ * does not exist.
+ */
+class NoSuchRecordException(msg: String): LoginsStorageException(msg)
+
+/**
+ * This is thrown if `add()` is given a record that has an ID, and
+ * that ID does not exist.
+ */
+class IdCollisionException(msg: String): LoginsStorageException(msg)
+
+/**
+ * This is thrown on attempts to insert or update a record so that it
+ * is no longer valid. Valid records have:
+ *
+ * - non-empty hostnames
+ * - non-empty passwords
+ * - and exactly one of `httpRealm` or `formSubmitUrl` is non-null.
+ */
+class InvalidRecordException(msg: String): LoginsStorageException(msg)

logins-api/android/library/src/main/java/org/mozilla/sync15/logins/MemoryLoginsStorage.kt

@@ -11,6 +11,7 @@ package org.mozilla.sync15.logins
 import android.util.Log
 import kotlinx.coroutines.experimental.launch
 import java.io.Closeable
+import java.util.UUID
 
 private enum class LoginsStorageState {
     Unlocked,
@@ -109,25 +110,69 @@ class MemoryLoginsStorage(private var list: List<ServerPassword>) : Closeable, L
                 // add a new one with updated properties
                 list = list.filter { it.id != id };
                 // Is there a better way to do this?
-                val newsp = ServerPassword(
-                        id = sp.id,
-                        hostname = sp.hostname,
-                        username = sp.username,
-                        password = sp.password,
-                        httpRealm = sp.httpRealm,
-                        formSubmitURL = sp.formSubmitURL,
-                        timeCreated = sp.timeCreated,
-                        timePasswordChanged = sp.timePasswordChanged,
-                        usernameField = sp.usernameField,
-                        passwordField = sp.passwordField,
-                        timeLastUsed = System.currentTimeMillis(),
-                        timesUsed = sp.timesUsed + 1
+                val newsp = sp.copy(
+                    timeLastUsed = System.currentTimeMillis(),
+                    timesUsed = sp.timesUsed + 1
                 )
                 list += newsp
             }
         }
     }
 
+    override fun add(login: ServerPassword): SyncResult<String> {
+        return asyncResult {
+            checkUnlocked()
+
+            val toInsert = if (login.id.isEmpty()) {
+                // This isn't anything like what the IDs we generate in rust look like
+                // but whatever.
+                login.copy(id = UUID.randomUUID().toString())
+            } else {
+                login
+            }.copy(
+                timesUsed = 1,
+                timeLastUsed = System.currentTimeMillis(),
+                timeCreated = System.currentTimeMillis(),
+                timePasswordChanged = System.currentTimeMillis()
+            )
+
+            checkValid(toInsert)
+
+            val sp = list.find { it.id == toInsert.id }
+            if (sp != null) {
+                // Note: Not the way this is formatted in rust -- don't rely on the formatting!
+                throw IdCollisionException("Id already exists " + toInsert.id)
+            }
+
+            list += toInsert
+            toInsert.id
+        }
+    }
+
+    override fun update(login: ServerPassword): SyncResult<Unit> {
+        return asyncResult {
+            checkUnlocked()
+            val current = list.find { it.id == login.id }
+                    ?: throw NoSuchRecordException("No such record: " + login.id)
+
+            val newRecord = login.copy(
+                    timeLastUsed = System.currentTimeMillis(),
+                    timesUsed = current.timesUsed + 1,
+                    timeCreated = current.timeCreated,
+                    timePasswordChanged = if (current.password == login.password) {
+                        current.timePasswordChanged
+                    } else {
+                        System.currentTimeMillis()
+                    })
+
+            checkValid(newRecord)
+
+            list = list.filter { it.id != login.id }
+
+            list += newRecord
+        }
+    }
+
     override fun list(): SyncResult<List<ServerPassword>> {
         return asyncResult {
             checkUnlocked()
@@ -149,6 +194,24 @@ class MemoryLoginsStorage(private var list: List<ServerPassword>) : Closeable, L
         }
     }
 
+    private fun checkValid(login: ServerPassword) {
+        if (login.hostname == "") {
+            throw InvalidRecordException("Invalid login: Hostname is empty")
+        }
+        if (login.password == "") {
+            throw InvalidRecordException("Invalid login: Password is empty")
+        }
+        if (login.formSubmitURL != null && login.httpRealm != null) {
+            throw InvalidRecordException(
+                    "Invalid login: Both `formSubmitUrl` and `httpRealm` are present")
+        }
+        if (login.formSubmitURL == null && login.httpRealm == null) {
+            throw InvalidRecordException(
+                    "Invalid login: Neither `formSubmitUrl` and `httpRealm` are present")
+        }
+    }
+
+
     private fun <T> asyncResult(callback: () -> T): SyncResult<T> {
         // Roughly mimic the semantics of MentatLoginsStorage -- serialize all calls to this API (
         // unlike MentatLoginsStorage we don't serialize calls to separate instances, but that

logins-api/android/library/src/main/java/org/mozilla/sync15/logins/ServerPassword.kt

@@ -14,7 +14,7 @@ import org.json.JSONObject;
 /**
  * Raw password data that is stored by the LoginsStorage implementation.
  */
-class ServerPassword (
+data class ServerPassword (
 
      /**
       * The unique ID associated with this login.
@@ -25,9 +25,18 @@ class ServerPassword (
       */
     val id: String,
 
+    /**
+     * The hostname this record corresponds to. It is an error to
+     * attempt to insert or update a record to have a blank hostname.
+     */
     val hostname: String,
+
     val username: String?,
 
+    /**
+     * The password field of this record. It is an error to attempt to insert or update
+     * a record to have a blank password.
+     */
     val password: String,
 
     /**
@@ -42,16 +51,56 @@ class ServerPassword (
      */
     val formSubmitURL: String? = null,
 
+    /**
+     * Number of times this password has been used.
+     */
     val timesUsed: Int,
 
+    /**
+     * Time of creation in milliseconds from the unix epoch.
+     */
     val timeCreated: Long,
+
+    /**
+     * Time of last use in milliseconds from the unix epoch.
+     */
     val timeLastUsed: Long,
+
+    /**
+     * Time of last password change in milliseconds from the unix epoch.
+     */
     val timePasswordChanged: Long,
 
     val usernameField: String? = null,
     val passwordField: String? = null
 ) {
 
+    fun toJSON(): JSONObject {
+        val o = JSONObject()
+        o.put("id", this.id)
+        o.put("hostname", this.hostname)
+        o.put("password", password)
+        o.put("timesUsed", timesUsed)
+        o.put("timeCreated", timeCreated)
+        o.put("timeLastUsed", timeLastUsed)
+        o.put("timePasswordChanged", timePasswordChanged)
+        if (username != null) {
+            o.put("username", username)
+        }
+        if (httpRealm != null) {
+            o.put("httpRealm", httpRealm)
+        }
+        if (formSubmitURL != null) {
+            o.put("formSubmitURL", formSubmitURL)
+        }
+        if (usernameField != null) {
+            o.put("usernameField", usernameField)
+        }
+        if (passwordField != null) {
+            o.put("passwordField", passwordField)
+        }
+        return o
+    }
 
     companion object {
         fun fromJSON(jsonObject: JSONObject): ServerPassword {
@@ -77,6 +126,7 @@ class ServerPassword (
             )
         }
 
+
         fun fromJSON(jsonText: String): ServerPassword {
             return fromJSON(JSONObject(jsonText))
         }

logins-api/android/library/src/main/java/org/mozilla/sync15/logins/rust/RustError.kt

@@ -10,7 +10,9 @@ package org.mozilla.sync15.logins.rust
 
 import com.sun.jna.Pointer
 import com.sun.jna.Structure
+import org.mozilla.sync15.logins.IdCollisionException
 import org.mozilla.sync15.logins.LoginsStorageException
+import org.mozilla.sync15.logins.NoSuchRecordException
 import org.mozilla.sync15.logins.SyncAuthInvalidException
 import java.util.Arrays
 
@@ -48,6 +50,12 @@ open class RustError : Structure() {
             // ever hit! (But we shouldn't ever hit it?)
             throw RuntimeException("[Bug] intoException called on non-failure!");
         }
+        if (code == 3) {
+            return IdCollisionException(this.consumeErrorMessage())
+        }
+        if (code == 2) {
+            return NoSuchRecordException(this.consumeErrorMessage())
+        }
         if (code == 1) {
             return SyncAuthInvalidException(this.consumeErrorMessage());
         }

logins-sql/ffi/src/error.rs

@@ -126,6 +126,16 @@ pub enum ExternErrorCode {
     /// Indicates the FxA credentials are invalid, and should be refreshed.
     AuthInvalidError = 1,
 
+    /// Returned from an `update()` call where the record ID did not exist.
+    NoSuchRecord = 2,
+
+    /// Returned from an `add()` call that was provided an ID, where the ID
+    /// already existed.
+    DuplicateGuid = 3,
+
+    /// Attempted to insert or update a record so that it is invalid
+    InvalidLogin = 4,
+
     // TODO: lockbox indicated that they would want to know when we fail to open
     // the DB due to invalid key.
     // https://github.com/mozilla/application-services/issues/231
@@ -183,6 +193,18 @@ fn get_code(err: &Error) -> ExternErrorCode {
                 _ => ExternErrorCode::OtherError,
             }
         }
+        ErrorKind::DuplicateGuid(id) => {
+            error!("Guid already exists: {}", id);
+            ExternErrorCode::DuplicateGuid
+        }
+        ErrorKind::NoSuchRecord(id) => {
+            error!("No record exists with id {}", id);
+            ExternErrorCode::NoSuchRecord
+        }
+        ErrorKind::InvalidLogin(desc) => {
+            error!("Invalid login: {}", desc);
+            ExternErrorCode::InvalidLogin
+        }
         err => {
             error!("Unexpected error: {:?}", err);
             ExternErrorCode::OtherError