respond with InfoNoAction to delete requests of credential is not found

sayi21cn · Apr 16, 2020 · 4da4a46 · 4da4a46
1 parent 00a5964
commit 4da4a46
Show file tree

Hide file tree

Showing 5 changed files with 70 additions and 19 deletions.
diff --git a/server/db/mongodb/adapter.go b/server/db/mongodb/adapter.go
@@ -601,7 +601,7 @@ func (a *adapter) UserDelete(uid t.Uid, hard bool) error {
 			}
 
 			// Delete credentials.
-			if err = a.credDel(sc, uid, "", ""); err != nil {
+			if err = a.credDel(sc, uid, "", ""); err != nil && err != t.ErrNotFound {
 				return err
 			}
 
@@ -921,7 +921,12 @@ func (a *adapter) credDel(ctx context.Context, uid t.Uid, method, value string)
 			filter["value"] = value
 		}
 	} else {
-		_, err := credCollection.DeleteMany(ctx, filter)
+		res, err := credCollection.DeleteMany(ctx, filter)
+		if err == nil {
+			if res.DeletedCount == 0 {
+				err = t.ErrNotFound
+			}
+		}
 		return err
 	}
 
@@ -930,12 +935,21 @@ func (a *adapter) credDel(ctx context.Context, uid t.Uid, method, value string)
 	hardDeleteFilter["$or"] = b.A{
 		b.M{"done": true},
 		b.M{"retries": 0}}
-	if _, err := credCollection.DeleteMany(ctx, hardDeleteFilter); err != nil {
+	res, err := credCollection.DeleteMany(ctx, hardDeleteFilter)
+	if err != nil {
 		return err
 	}
+	if res.DeletedCount > 0 {
+		return nil
+	}
 
 	// Soft-delete all other values.
-	_, err := credCollection.UpdateMany(ctx, filter, b.M{"$set": b.M{"deletedat": t.TimeNow()}})
+	res, err = credCollection.UpdateMany(ctx, filter, b.M{"$set": b.M{"deletedat": t.TimeNow()}})
+	if err == nil {
+		if res.DeletedCount == 0 {
+			err = t.ErrNotFound
+		}
+	}
 	return err
 }
 

diff --git a/server/db/mysql/adapter.go b/server/db/mysql/adapter.go
@@ -905,7 +905,7 @@ func (a *adapter) UserDelete(uid t.Uid, hard bool) error {
 		}
 
 		// Delete all credentials.
-		if err = credDel(tx, uid, "", ""); err != nil {
+		if err = credDel(tx, uid, "", ""); err != nil && err != t.ErrNotFound {
 			return err
 		}
 
@@ -2342,8 +2342,10 @@ func deviceDelete(tx *sqlx.Tx, uid t.Uid, deviceID string) error {
 		res, err = tx.Exec("DELETE FROM devices WHERE userid=? AND hash=?", store.DecodeUid(uid), deviceHasher(deviceID))
 	}
 
-	if count, _ := res.RowsAffected(); count == 0 && err == nil {
-		err = t.ErrNotFound
+	if err == nil {
+		if count, _ := res.RowsAffected(); count == 0 {
+			err = t.ErrNotFound
+		}
 	}
 
 	return err
@@ -2465,19 +2467,36 @@ func credDel(tx *sqlx.Tx, uid t.Uid, method, value string) error {
 		}
 	}
 
+	var err error
+	var res sql.Result
 	if method == "" {
-		_, err := tx.Exec("DELETE FROM credentials"+constraints, args...)
+		// Case 1
+		res, err = tx.Exec("DELETE FROM credentials"+constraints, args...)
+		if err == nil {
+			if count, _ := res.RowsAffected(); count == 0 {
+				err = t.ErrNotFound
+			}
+		}
 		return err
 	}
 
 	// Case 2.1
-	if _, err := tx.Exec("DELETE FROM credentials"+constraints+" AND (done=true OR retries=0)", args...); err != nil {
+	res, err = tx.Exec("DELETE FROM credentials"+constraints+" AND (done=true OR retries=0)", args...)
+	if err != nil {
 		return err
 	}
+	if count, _ := res.RowsAffected(); count > 0 {
+		return nil
+	}
 
 	// Case 2.2
 	args = append([]interface{}{t.TimeNow()}, args...)
-	_, err := tx.Exec("UPDATE credentials SET deletedat=?"+constraints, args...)
+	res, err = tx.Exec("UPDATE credentials SET deletedat=?"+constraints, args...)
+	if err == nil {
+		if count, _ := res.RowsAffected(); count >= 0 {
+			err = t.ErrNotFound
+		}
+	}
 
 	return err
 }

diff --git a/server/db/rethinkdb/adapter.go b/server/db/rethinkdb/adapter.go
@@ -772,7 +772,7 @@ func (a *adapter) UserDelete(uid t.Uid, hard bool) error {
 		}
 
 		// Delete credentials.
-		if err = a.CredDel(uid, "", ""); err != nil {
+		if err = a.CredDel(uid, "", ""); err != nil && err != t.ErrNotFound {
 			return err
 		}
 		// And finally delete the user.
@@ -2069,18 +2069,31 @@ func (a *adapter) CredDel(uid t.Uid, method, value string) error {
 	}
 
 	if method == "" {
-		_, err := q.Delete().RunWrite(a.conn)
+		res, err := q.Delete().RunWrite(a.conn)
+		if err == nil {
+			if res.Deleted == 0 {
+				err = t.ErrNotFound
+			}
+		}
 		return err
 	}
 
 	// Hard-delete all confirmed values or values with no attempts at confirmation.
-	_, err := q.Filter(rdb.Or(rdb.Row.Field("Done").Eq(true), rdb.Row.Field("Retries").Eq(0))).Delete().RunWrite(a.conn)
+	res, err := q.Filter(rdb.Or(rdb.Row.Field("Done").Eq(true), rdb.Row.Field("Retries").Eq(0))).Delete().RunWrite(a.conn)
 	if err != nil {
 		return err
 	}
+	if res.Deleted > 0 {
+		return nil
+	}
 
 	// Soft-delete all other values.
-	_, err = q.Update(map[string]interface{}{"DeletedAt": t.TimeNow()}).RunWrite(a.conn)
+	res, err = q.Update(map[string]interface{}{"DeletedAt": t.TimeNow()}).RunWrite(a.conn)
+	if err == nil {
+		if res.Deleted == 0 {
+			err = t.ErrNotFound
+		}
+	}
 	return err
 }
 

diff --git a/server/topic.go b/server/topic.go
@@ -2299,13 +2299,14 @@ func (t *Topic) replyDelCred(h *Hub, sess *Session, asUid types.Uid, authLvl aut
 		if len(removed) > 0 {
 			t.tags = tags
 			t.presSubsOnline("tags", "", nilPresParams, nilPresFilters, "")
-			sess.queueOut(decodeStoreError(err, del.Id, del.Topic, now, nil))
-		} else {
-			sess.queueOut(InfoNoAction(del.Id, del.Topic, now))
 		}
-	} else {
-		sess.queueOut(decodeStoreError(err, del.Id, del.Topic, now, nil))
+	} else if err == nil {
+		sess.queueOut(InfoNoAction(del.Id, del.Topic, now))
+		return nil
 	}
+
+	sess.queueOut(decodeStoreError(err, del.Id, del.Topic, now, nil))
+
 	return err
 }
 

diff --git a/server/user.go b/server/user.go
@@ -490,6 +490,10 @@ func deleteCred(uid types.Uid, authLvl auth.Level, cred *MsgCredClient) ([]strin
 	// The credential is either not required or more than one credential is validated for the given method.
 	err := vld.Remove(uid, cred.Value)
 	if err != nil {
+		if err == types.ErrNotFound {
+			// Credential is not deleted because it's not found
+			err = nil
+		}
 		return nil, err
 	}