From 81f5454b30b64859b28079545d9391aa663563c6 Mon Sep 17 00:00:00 2001 From: or-else Date: Fri, 10 Apr 2020 18:46:05 +0300 Subject: [PATCH] workaroud for rethinkdb/rethinkdb-go/issues/486; a few other bug fixes --- server/db/rethinkdb/adapter.go | 14 +++++---- server/topic.go | 16 +++++++--- server/user.go | 53 ++++++++++++++++++++++++---------- 3 files changed, 59 insertions(+), 24 deletions(-) diff --git a/server/db/rethinkdb/adapter.go b/server/db/rethinkdb/adapter.go index 8cb0cf953..1a992c353 100644 --- a/server/db/rethinkdb/adapter.go +++ b/server/db/rethinkdb/adapter.go @@ -889,16 +889,20 @@ func (a *adapter) UserUpdateTags(uid t.Uid, add, remove, reset []string) ([]stri return nil, err } - // Get the new tags - cursor, err := q.Field("Tags").Run(a.conn) + // Get the new tags. + // Using Pluck instead of Field because of https://github.com/rethinkdb/rethinkdb-go/issues/486 + cursor, err := q.Pluck("Tags").Run(a.conn) if err != nil { return nil, err } defer cursor.Close() - var tags []string - err = cursor.One(&tags) - return tags, err + var tagsField struct{ Tags []string } + err = cursor.One(&tagsField) + if err != nil { + return nil, err + } + return tagsField.Tags, nil } // UserGetByCred returns user ID for the given validated credential. diff --git a/server/topic.go b/server/topic.go index ac9334e14..eebe201d1 100644 --- a/server/topic.go +++ b/server/topic.go @@ -2104,7 +2104,7 @@ func (t *Topic) replySetCred(sess *Session, asUid types.Uid, authLevel auth.Leve _, tags, err = addCreds(asUid, creds, nil, sess.lang, tmpToken) } - if err == nil && len(tags) > 0 { + if tags != nil { t.tags = tags t.presSubsOnline("tags", "", nilPresParams, nilPresFilters, "") } @@ -2287,11 +2287,19 @@ func (t *Topic) replyDelCred(h *Hub, sess *Session, asUid types.Uid, authLvl aut sess.queueOut(ErrPermissionDenied(del.Id, t.original(asUid), now)) return errors.New("del.cred: invalid topic category") } + if del.Cred == nil || del.Cred.Method == "" { + sess.queueOut(ErrMalformed(del.Id, t.original(asUid), now)) + return errors.New("del.cred: missing method") + } tags, err := deleteCred(asUid, authLvl, del.Cred) - if err == nil { - t.tags = tags - t.presSubsOnline("tags", "", nilPresParams, nilPresFilters, "") + if tags != nil { + // Check if anything has been actuallt removed. + _, removed := stringSliceDelta(t.tags, tags) + if len(removed) > 0 { + t.tags = tags + t.presSubsOnline("tags", "", nilPresParams, nilPresFilters, "") + } } sess.queueOut(decodeStoreError(err, del.Id, del.Topic, now, nil)) return err diff --git a/server/user.go b/server/user.go index 19c5e4f3c..651eb2fcf 100644 --- a/server/user.go +++ b/server/user.go @@ -311,8 +311,10 @@ func updateUserAuth(msg *ClientComMessage, user *types.User, rec *auth.Rec) erro } // Tags may have been changed by authhdl.UpdateRecord, reset them. - // Can't do much with the error here, so ignoring it. - store.Users.UpdateTags(user.Uid(), nil, nil, rec.Tags) + // Can't do much with the error here, logging it but not returning. + if _, err = store.Users.UpdateTags(user.Uid(), nil, nil, rec.Tags); err != nil { + log.Println("updateUserAuth tags update failed:", err) + } return nil } @@ -322,9 +324,8 @@ func updateUserAuth(msg *ClientComMessage, user *types.User, rec *auth.Rec) erro // addCreds adds new credentials and re-send validation request for existing ones. It also adds credential-defined // tags if necessary. -// Returns methods validated in this call only, full set of tags. -func addCreds(uid types.Uid, creds []MsgCredClient, tags []string, lang string, - tmpToken []byte) ([]string, []string, error) { +// Returns methods validated in this call only. Returns either a full set of tags or nil for tags when tags are unchanged. +func addCreds(uid types.Uid, creds []MsgCredClient, extraTags []string, lang string, tmpToken []byte) ([]string, []string, error) { var validated []string for i := range creds { cr := &creds[i] @@ -346,20 +347,27 @@ func addCreds(uid types.Uid, creds []MsgCredClient, tags []string, lang string, // Generate tags for these confirmed credentials. if globals.validators[cr.Method].addToTags { - tags = append(tags, cr.Method+":"+cr.Value) + extraTags = append(extraTags, cr.Method+":"+cr.Value) } } } // Save tags potentially changed by the validator. - if len(tags) > 0 { - tags, _ = store.Users.UpdateTags(uid, tags, nil, nil) + if len(extraTags) > 0 { + if utags, err := store.Users.UpdateTags(uid, extraTags, nil, nil); err != nil { + extraTags = utags + } else { + log.Println("add cred tags update failed:", err) + } + } else { + extraTags = nil } - return validated, tags, nil + return validated, extraTags, nil } // validatedCreds returns the list of validated credentials including those validated in this call. -// Returns all validated methods including those validated earlier and now, full set of tags. +// Returns all validated methods including those validated earlier and now. +// Returns either a full set of tags or nil for tags if tags are unchanged. func validatedCreds(uid types.Uid, authLvl auth.Level, creds []MsgCredClient, errorOnFail bool) ([]string, []string, error) { // Check if credential validation is required. @@ -417,7 +425,14 @@ func validatedCreds(uid types.Uid, authLvl auth.Level, creds []MsgCredClient, er var tags []string if len(tagsToAdd) > 0 { // Save update to tags - tags, _ = store.Users.UpdateTags(uid, tagsToAdd, nil, nil) + if utags, err := store.Users.UpdateTags(uid, tagsToAdd, nil, nil); err == nil { + tags = utags + } else { + log.Println("validated creds tags update failed:", err) + tags = nil + } + } else { + tags = nil } var validated []string @@ -429,6 +444,7 @@ func validatedCreds(uid types.Uid, authLvl auth.Level, creds []MsgCredClient, er } // deleteCred deletes user's credential. +// Returns full set of remaining tags or nil if tags are unchanged. func deleteCred(uid types.Uid, authLvl auth.Level, cred *MsgCredClient) ([]string, error) { vld := store.GetValidator(cred.Method) if vld == nil || cred.Value == "" { @@ -447,7 +463,6 @@ func deleteCred(uid types.Uid, authLvl auth.Level, cred *MsgCredClient) ([]strin // If credential is required, make sure the method remains validated even after this credential is deleted. if isRequired { - // There could be multiple validated credentials for the same method thus we are getting a map with count // for each method. @@ -457,10 +472,10 @@ func deleteCred(uid types.Uid, authLvl auth.Level, cred *MsgCredClient) ([]strin return nil, err } - // Check if it's OK to delete: there is another validated value. + // Check if it's OK to delete: there is another validated value or this value is not validated in the first place. var okTodelete bool for _, cr := range allCreds { - if cr.Done && cr.Value != cred.Value { + if (cr.Done && cr.Value != cred.Value) || (!cr.Done && cr.Value == cred.Value) { okTodelete = true break } @@ -482,8 +497,16 @@ func deleteCred(uid types.Uid, authLvl auth.Level, cred *MsgCredClient) ([]strin var tags []string if globals.validators[cred.Method].addToTags { // This error should not be returned to user. - tags, _ = store.Users.UpdateTags(uid, nil, []string{cred.Method + ":" + cred.Value}, nil) + if utags, err := store.Users.UpdateTags(uid, nil, []string{cred.Method + ":" + cred.Value}, nil); err == nil { + tags = utags + } else { + log.Println("delete cred: failed to update tags:", err) + tags = nil + } + } else { + tags = nil } + return tags, nil }