adding ability to change credentials, tinode/tindroid/issues/38

server/db/adapter.go

@@ -52,10 +52,10 @@ type Adapter interface {
 
 	// Credential management
 
-	// CredAdd adds a credential record.
-	CredAdd(cred *t.Credential) error
-	// CredGet returns all credential records for the given method.
-	CredGet(uid t.Uid, method string) ([]*t.Credential, error)
+	// CredUpsert adds or updates a credential record.
+	CredUpsert(cred *t.Credential) error
+	// CredGetActive returns the currently active credential record for the given method.
+	CredGetActive(uid t.Uid, method string) (*t.Credential, error)
 	// CredIsConfirmed returns true if the given credential method has been verified, false otherwise.
 	CredIsConfirmed(uid t.Uid, metod string) (bool, error)
 	// CredDel deletes credentials for the given method. If method is empty, deletes all user's credentials.

server/db/mysql/adapter.go

@@ -2132,18 +2132,42 @@ func (a *adapter) DeviceDelete(uid t.Uid, deviceID string) error {
 
 // Credential management
 
-// CredAdd adds credential validation record: userd ID, method, credential value, possible response.
-func (a *adapter) CredAdd(cred *t.Credential) error {
+// CredUpsert adds or updates a validation record.
+func (a *adapter) CredUpsert(cred *t.Credential) error {
+	// If credential is validated it cannot be changed so assume it does not exist, just try to add it.
+	// If credential is not valiated, try to find it first: if it does not exist add it, otherwise
+	// update UpdatedAt and Resp.
+
+	userId := decodeUidString(cred.User)
+
+	// Deactivate all unverified records of this user and method.
+	_, err := a.db.Exec("UPDATE credentials SET done=-1 WHERE userid=? AND method=?", userId, cred.Method)
+
 	// Enforce uniqueness: if credential is confirmed, "method:value" must be unique.
 	// if credential is not yet confirmed, "userid:method:value" is unique.
 	synth := cred.Method + ":" + cred.Value
+	done := 1
 	if !cred.Done {
 		synth = cred.User + ":" + synth
+		done = 0
+
+		// Assume that the record exists and try to update it: mark as active, update timestamp and response value.
+		res, err := a.db.Exec("UPDATE credentials SET updatedat=?,resp=?,done=0 WHERE synthetic=?",
+			cred.UpdatedAt, cred.Resp, synth)
+		if err != nil {
+			return err
+		}
+
+		// If record was updated, then all is fine.
+		if numrows, _ := res.RowsAffected(); numrows > 0 {
+			return nil
+		}
 	}
-	_, err := a.db.Exec("INSERT INTO credentials(createdat,updatedat,method,value,synthetic,userid,resp,done) "+
+
+	// Add new record.
+	_, err = a.db.Exec("INSERT INTO credentials(createdat,updatedat,method,value,synthetic,userid,resp,done) "+
 		"VALUES(?,?,?,?,?,?,?,?)",
-		cred.CreatedAt, cred.UpdatedAt, cred.Method, cred.Value, synth,
-		decodeUidString(cred.User), cred.Resp, cred.Done)
+		cred.CreatedAt, cred.UpdatedAt, cred.Method, cred.Value, synth, userId, cred.Resp, done)
 	if isDupe(err) {
 		return t.ErrDuplicate
 	}
@@ -2164,9 +2188,8 @@ func (a *adapter) CredIsConfirmed(uid t.Uid, method string) (bool, error) {
 	return done > 0, err
 }
 
-// credDel deletes given validation method or ll methods of the given user.
+// credDel deletes given validation method or all methods of the given user.
 func credDel(tx *sqlx.Tx, uid t.Uid, method string) error {
-	// FIXME: There could be more than one credential of the same method.
 	query := "DELETE FROM credentials WHERE userid=?"
 	args := []interface{}{store.DecodeUid(uid)}
 	if method != "" {
@@ -2180,7 +2203,6 @@ func credDel(tx *sqlx.Tx, uid t.Uid, method string) error {
 // CredDel deletes either all credentials of the given user and method
 // or all credentials of the given user if the method is blank.
 func (a *adapter) CredDel(uid t.Uid, method string) error {
-	// FIXME: There could be more than one credential of the same method.
 	tx, err := a.db.Beginx()
 	if err != nil {
 		return err
@@ -2201,9 +2223,8 @@ func (a *adapter) CredDel(uid t.Uid, method string) error {
 
 // CredConfirm marks given credential method as confirmed.
 func (a *adapter) CredConfirm(uid t.Uid, method string) error {
-	// FIXME: There could be more than one credential of the same method.
 	res, err := a.db.Exec(
-		"UPDATE credentials SET updatedat=?,done=1,synthetic=CONCAT(method,':',value) WHERE userid=? AND method=?",
+		"UPDATE credentials SET updatedat=?,done=1,synthetic=CONCAT(method,':',value) WHERE userid=? AND method=? AND done=0",
 		t.TimeNow(), store.DecodeUid(uid), method)
 	if err != nil {
 		if isDupe(err) {
@@ -2219,38 +2240,25 @@ func (a *adapter) CredConfirm(uid t.Uid, method string) error {
 
 // CredFail increments failure count of the given validation method.
 func (a *adapter) CredFail(uid t.Uid, method string) error {
-	// FIXME: There could be more than one credential of the same method.
-	_, err := a.db.Exec("UPDATE credentials SET updatedat=?,retries=retries+1 WHERE userid=? AND method=?",
+	_, err := a.db.Exec("UPDATE credentials SET updatedat=?,retries=retries+1 WHERE userid=? AND method=? AND done=0",
 		t.TimeNow(), store.DecodeUid(uid), method)
 	return err
 }
 
-// CredGet returns all credentials of the given user and method, validated or not.
-func (a *adapter) CredGet(uid t.Uid, method string) ([]*t.Credential, error) {
-	query := "SELECT createdat,updatedat,method,value,resp,done,retries " +
-		"FROM credentials WHERE userid=? ORDER BY createdat DESC"
-	args := []interface{}{store.DecodeUid(uid)}
-	if method != "" {
-		query += " AND method=?"
-		args = append(args, method)
-	}
-	rows, err := a.db.Queryx(query, args...)
+// CredGetActive returns currently active unvalidated credential of the given user and method.
+func (a *adapter) CredGetActive(uid t.Uid, method string) (*t.Credential, error) {
+	var cred t.Credential
+	err := a.db.Get(&cred, "SELECT createdat,updatedat,method,value,resp,done,retries "+
+		"FROM credentials WHERE userid=? AND method=? AND done=0", store.DecodeUid(uid), method)
 	if err != nil {
-		return nil, err
-	}
-
-	var result []*t.Credential
-	for rows.Next() {
-		var cred t.Credential
-		if err = rows.StructScan(&cred); err != nil {
-			break
+		if err == sql.ErrNoRows {
+			err = nil
 		}
-		cred.User = uid.String()
-		result = append(result, &cred)
+		return nil, err
 	}
-	rows.Close()
+	cred.User = uid.String()
 
-	return result, err
+	return &cred, nil
 }
 
 // FileUploads

server/db/rethinkdb/adapter.go

@@ -304,7 +304,7 @@ func (a *adapter) CreateDb(reset bool) error {
 		return err
 	}
 
-	// User credentials - contact information such as "email:jdoe@example.com" or "tel:18003287448":
+	// User credentials - contact information such as "email:jdoe@example.com" or "tel:+18003287448":
 	// Id: "method:credential" like "email:jdoe@example.com". See types.Credential.
 	if _, err := rdb.DB(a.dbName).TableCreate("credentials", rdb.TableCreateOpts{PrimaryKey: "Id"}).RunWrite(a.conn); err != nil {
 		return err
@@ -1766,33 +1766,68 @@ func (a *adapter) DeviceDelete(uid t.Uid, deviceID string) error {
 }
 
 // Credential management
-func (a *adapter) CredAdd(cred *t.Credential) error {
+
+// CredUpsert adds or updates a validation record.
+func (a *adapter) CredUpsert(cred *t.Credential) error {
+	// If credential is validated it cannot be changed so assume it does not exist, just try to add it.
+	// If credential is not valiated, try to find it first: if it does not exist add it, otherwise
+	// update UpdatedAt and Resp, remove Closed.
+
+	// Deactivate all unverified records of this user and method.
+	_, err := rdb.DB(a.dbName).Table("credentials").
+		GetAllByIndex("User", cred.User).
+		Filter(map[string]interface{}{"Method": cred.Method, "Done": false}).Update(
+		map[string]interface{}{"Closed": true}).RunWrite(a.conn)
+	if err != nil {
+		return err
+	}
+
 	cred.Id = cred.Method + ":" + cred.Value
+
 	if !cred.Done {
 		// If credential is not confirmed, it should not block others
-		// from attempting to validate it.
+		// from attempting to validate it: make index user-unique instead of global-unique.
 		cred.Id = cred.User + ":" + cred.Id
+
+		// Assume that the record exists and try to update it: remove Closed, update timestamp and response.
+		result, err := rdb.DB(a.dbName).Table("credentials").Get(cred.Id).
+			Replace(rdb.Row.Without("Closed").
+				Merge(map[string]interface{}{
+					"UpdatedAt": cred.UpdatedAt,
+					"Resp":      cred.Resp})).RunWrite(a.conn)
+		if err != nil {
+			return err
+		}
+
+		// If record was updated, then all is fine.
+		if result.Updated > 0 {
+			return nil
+		}
 	}
 
-	_, err := rdb.DB(a.dbName).Table("credentials").Insert(cred).RunWrite(a.conn)
+	// Insert a new record.
+	_, err = rdb.DB(a.dbName).Table("credentials").Insert(cred).RunWrite(a.conn)
 	if rdb.IsConflictErr(err) {
 		return t.ErrDuplicate
 	}
+
 	return err
 }
 
+// CredIsConfirmed returns true if the given credential method has been verified, false otherwise.
 func (a *adapter) CredIsConfirmed(uid t.Uid, method string) (bool, error) {
-	creds, err := a.CredGet(uid, method)
+	cursor, err := rdb.DB(a.dbName).Table("credentials").
+		GetAllByIndex("User", uid.String()).
+		Filter(map[string]interface{}{"Method": method, "Done": true}).Run(a.conn)
 	if err != nil {
 		return false, err
 	}
+	defer cursor.Close()
 
-	if len(creds) > 0 {
-		return creds[0].Done, nil
-	}
-	return false, nil
+	return !cursor.IsNil(), nil
 }
 
+// CredDel deletes credentials for the given method. If method is empty, deletes all user's credentials.
 func (a *adapter) CredDel(uid t.Uid, method string) error {
 	q := rdb.DB(a.dbName).Table("credentials").
 		GetAllByIndex("User", uid.String())
@@ -1803,72 +1838,71 @@ func (a *adapter) CredDel(uid t.Uid, method string) error {
 	return err
 }
 
-func (a *adapter) CredConfirm(uid t.Uid, method string) error {
-	// RethinkDb does not allow primary key to be changed (userid:method:value -> method:value)
-	// We have to delete and re-insert with a different primary key.
-	creds, err := a.CredGet(uid, method)
+// credGetActive reads the currently active unvalidated credential
+func (a *adapter) credGetActive(uid t.Uid, method string) (*t.Credential, error) {
+	// Get the active unconfirmed credential:
+	cursor, err := rdb.DB(a.dbName).Table("credentials").GetAllByIndex("User", uid.String()).
+		Filter(rdb.Row.HasFields("Closed").Not()).
+		Filter(map[string]interface{}{"Method": method, "Done": false}).Run(a.conn)
 	if err != nil {
-		return err
+		return nil, err
 	}
-	if len(creds) == 0 {
-		return t.ErrNotFound
+	defer cursor.Close()
+
+	if cursor.IsNil() {
+		return nil, t.ErrNotFound
 	}
-	if creds[0].Done {
-		// Already confirmed
-		return nil
+
+	var cred t.Credential
+	if err = cursor.One(&cred); err != nil {
+		return nil, err
 	}
 
-	creds[0].Done = true
-	creds[0].UpdatedAt = t.TimeNow()
-	if err = a.CredAdd(creds[0]); err != nil {
-		if rdb.IsConflictErr(err) {
-			return t.ErrDuplicate
-		}
+	return &cred, nil
+}
+
+// CredConfirm marks given credential as validated.
+func (a *adapter) CredConfirm(uid t.Uid, method string) error {
+
+	cred, err := a.credGetActive(uid, method)
+	if err != nil {
+		return err
+	}
+
+	// RethinkDb does not allow primary key to be changed (userid:method:value -> method:value)
+	// We have to delete and re-insert with a different primary key.
+
+	cred.Done = true
+	cred.UpdatedAt = t.TimeNow()
+	if err = a.CredUpsert(cred); err != nil {
 		return err
 	}
 
 	rdb.DB(a.dbName).
 		Table("credentials").
-		Get(uid.String() + ":" + creds[0].Method + ":" + creds[0].Value).
+		Get(uid.String() + ":" + cred.Method + ":" + cred.Value).
 		Delete(rdb.DeleteOpts{Durability: "soft", ReturnChanges: false}).
 		RunWrite(a.conn)
 
 	return nil
 }
 
+// CredFail increments count of failed validation attepmts for the given credentials.
 func (a *adapter) CredFail(uid t.Uid, method string) error {
 	_, err := rdb.DB(a.dbName).Table("credentials").
 		GetAllByIndex("User", uid.String()).
-		Filter(map[string]interface{}{"Method": method}).
+		Filter(map[string]interface{}{"Method": method, "Done": false}).
+		Filter(rdb.Row.HasFields("Closed").Not()).
 		Update(map[string]interface{}{
 			"Retries":   rdb.Row.Field("Retries").Default(0).Add(1),
 			"UpdatedAt": t.TimeNow(),
 		}).RunWrite(a.conn)
 	return err
 }
 
-func (a *adapter) CredGet(uid t.Uid, method string) ([]*t.Credential, error) {
-	q := rdb.DB(a.dbName).Table("credentials").
-		GetAllByIndex("User", uid.String())
-	if method != "" {
-		q = q.Filter(map[string]interface{}{"Method": method})
-	}
-	cursor, err := q.Run(a.conn)
-	if err != nil {
-		return nil, err
-	}
-	defer cursor.Close()
-
-	if cursor.IsNil() {
-		return nil, nil
-	}
-
-	var result []*t.Credential
-	if err = cursor.All(&result); err != nil {
-		return nil, err
-	}
-
-	return result, nil
+// CredGetActive returns currently active credential record for the given method.
+func (a *adapter) CredGetActive(uid t.Uid, method string) (*t.Credential, error) {
+	return a.credGetActive(uid, method)
 }
 
 // FileUploads

server/db/rethinkdb/schema.md

@@ -264,7 +264,7 @@ Sample:
 ### Table `credentials`
 The tables stores user credentials used for validation.
 
-* `Id` unique credential, primary key
+* `Id` credential, primary key
 * `CreatedAt` timestamp when the record was created
 * `UpdatedAt` timestamp when the last validation attempt was performed (successful or not).
 * `Method` validation method
@@ -273,6 +273,7 @@ The tables stores user credentials used for validation.
 * `Retries` number of failed attempts at validation
 * `User` id of the user who owns this credential
 * `Value` value of the credential
+* `Closed` unvalidated credential is no longer being validated. Only one credential is not Closed for each user/method.
 
 Indexes:
 * `Id` Primary key composed either as `User`:`Method`:`Value` for unconfirmed credentials or as `Method`:`Value` for confirmed.
@@ -281,15 +282,15 @@ Indexes:
 Sample:
 ```js
 {
-  "Id": "tel:17025550001",
+  "Id": "tel:+17025550001",
   "CreatedAt": Sun Jun 10 2018 16:37:27 GMT+00:00 ,
+  "UpdatedAt": Sun Jun 10 2018 16:37:28 GMT+00:00 ,
   "Method":  "tel" ,
   "Done": true ,
   "Resp":  "123456" ,
   "Retries": 0 ,
-  "UpdatedAt": Sun Jun 10 2018 16:37:27 GMT+00:00 ,
   "User":  "k3srBRk9RYw" ,
-  "Value":  "17025550001"
+  "Value":  "+17025550001"
 }
 ```
 

server/hub.go

@@ -48,7 +48,7 @@ type sessionLeave struct {
 	// Leave and unsubscribe
 	unsub bool
 	// ID of originating request, if any
-	reqID string
+	id string
 }
 
 // Request to hub to remove the topic