Merge pull request #343 from alfrunes/MEN-6087_1.18.x

Cherry-pick MEN-6087 to 1.18.x (mender-3.3.x)
mendersoftware · Nov 28, 2022 · 1339ffc · 1339ffc
2 parents db78f87 + fcd0e48
commit 1339ffc
Show file tree

Hide file tree

Showing 5 changed files with 50 additions and 21 deletions.
diff --git a/model/user.go b/model/user.go
@@ -83,7 +83,7 @@ type User struct {
 	// the User document.
 	// NOTE: The v1 API does not support ETags, so this is only used
 	// internally for checking pre-conditions before performing updates.
-	ETag ETag `json:"-" bson:"etag"`
+	ETag *ETag `json:"-" bson:"etag,omitempty"`
 
 	// user email address
 	Email string `json:"email" bson:"email"`
@@ -102,6 +102,9 @@ type User struct {
 }
 
 func (u User) NextETag() (ret ETag) {
+	if u.ETag == nil {
+		u.ETag = new(ETag)
+	}
 	if u.CreatedTs != nil {
 		// Weak part of the ETag
 		lsb := uint64(u.CreatedTs.Unix())

diff --git a/store/mongo/datastore_mongo.go b/store/mongo/datastore_mongo.go
@@ -209,10 +209,8 @@ func (db *DataStoreMongo) UpdateUser(
 			next.Increment()
 			u.ETagUpdate = &next
 		}
-	} else if u.ETagUpdate != nil {
-		return nil, errors.New(
-			"store: setting an etag with out providing a filter is not allowed",
-		)
+	} else {
+		f["etag"] = bson.D{{Key: "$exists", Value: false}}
 	}
 	up := bson.M{"$set": u}
 	fuOpts := mopts.FindOneAndUpdate().

diff --git a/store/mongo/datastore_mongo_test.go b/store/mongo/datastore_mongo_test.go
@@ -1,16 +1,16 @@
-// Copyright 2020 Northern.tech AS
+// Copyright 2022 Northern.tech AS
 //
-//    Licensed under the Apache License, Version 2.0 (the "License");
-//    you may not use this file except in compliance with the License.
-//    You may obtain a copy of the License at
+//	Licensed under the Apache License, Version 2.0 (the "License");
+//	you may not use this file except in compliance with the License.
+//	You may obtain a copy of the License at
 //
-//        http://www.apache.org/licenses/LICENSE-2.0
+//	    http://www.apache.org/licenses/LICENSE-2.0
 //
-//    Unless required by applicable law or agreed to in writing, software
-//    distributed under the License is distributed on an "AS IS" BASIS,
-//    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-//    See the License for the specific language governing permissions and
-//    limitations under the License.
+//	Unless required by applicable law or agreed to in writing, software
+//	distributed under the License is distributed on an "AS IS" BASIS,
+//	WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+//	See the License for the specific language governing permissions and
+//	limitations under the License.
 package mongo
 
 import (
@@ -189,6 +189,12 @@ func TestMongoUpdateUser(t *testing.T) {
 			Email:    "bar@bar.com",
 			Password: "pretenditsahash",
 		},
+		model.User{
+			ID:       "2bbde4d1-2a4c-47dc-9df4-f048285d2704",
+			Email:    "baz+mcetagface@bar.com",
+			Password: "pretenditsahash",
+			ETag:     new(model.ETag),
+		},
 	}
 
 	testCases := map[string]struct {
@@ -228,6 +234,28 @@ func TestMongoUpdateUser(t *testing.T) {
 			tenant:   "foo",
 			outErr:   "",
 		},
+		"ok with tenant and etag": {
+			inUserUpdate: model.UserUpdate{
+				ETag:       new(model.ETag),
+				Email:      "baz@bar.com",
+				Password:   "correcthorsebatterystaple",
+				ETagUpdate: &model.ETag{1},
+			},
+			inUserId: "2bbde4d1-2a4c-47dc-9df4-f048285d2704",
+			tenant:   "foo",
+			outErr:   "",
+		},
+		"error, etag mismatch": {
+			inUserUpdate: model.UserUpdate{
+				ETag:       &model.ETag{3},
+				Email:      "baz@bar.com",
+				Password:   "correcthorsebatterystaple",
+				ETagUpdate: &model.ETag{4},
+			},
+			inUserId: "2bbde4d1-2a4c-47dc-9df4-f048285d2704",
+			tenant:   "foo",
+			outErr:   store.ErrUserNotFound.Error(),
+		},
 		"duplicate email error": {
 			inUserUpdate: model.UserUpdate{
 				Email:    "foo@bar.com",
@@ -241,7 +269,7 @@ func TestMongoUpdateUser(t *testing.T) {
 				Email:    "foo@acme.com",
 				Password: "correcthorsebatterystaple",
 			},
-			inUserId: "3",
+			inUserId: "0",
 			outErr:   store.ErrUserNotFound.Error(),
 		},
 	}

diff --git a/user/useradm.go b/user/useradm.go
@@ -384,8 +384,8 @@ func (ua *UserAdm) UpdateUser(ctx context.Context, id string, userUpdate *model.
 		// Update without the support for etags.
 		next := user.NextETag()
 		userUpdate.ETagUpdate = &next
-		userUpdate.ETag = &user.ETag
-	} else if *userUpdate.ETag != user.ETag {
+		userUpdate.ETag = user.ETag
+	} else if user.ETag == nil || *userUpdate.ETag != *user.ETag {
 		return ErrETagMismatch
 	}
 

diff --git a/user/useradm_test.go b/user/useradm_test.go
@@ -973,7 +973,7 @@ func TestUserAdmUpdateUser(t *testing.T) {
 		DataStore: func(t *testing.T, self *testCase) *mstore.DataStore {
 			ds := new(mstore.DataStore)
 			ds.On("GetUserAndPasswordById", self.CTX, self.ID).
-				Return(&model.User{ID: self.ID, ETag: model.ETag{0}}, nil)
+				Return(&model.User{ID: self.ID, ETag: &model.ETag{0}}, nil)
 			return ds
 		},
 		Error: ErrCannotModifyPassword,
@@ -992,7 +992,7 @@ func TestUserAdmUpdateUser(t *testing.T) {
 		DataStore: func(t *testing.T, self *testCase) *mstore.DataStore {
 			ds := new(mstore.DataStore)
 			ds.On("GetUserAndPasswordById", self.CTX, self.ID).
-				Return(&model.User{ID: self.ID, ETag: model.ETag{1}}, nil)
+				Return(&model.User{ID: self.ID, ETag: &model.ETag{1}}, nil)
 			return ds
 		},
 		Error: ErrETagMismatch,
@@ -1011,7 +1011,7 @@ func TestUserAdmUpdateUser(t *testing.T) {
 		DataStore: func(t *testing.T, self *testCase) *mstore.DataStore {
 			ds := new(mstore.DataStore)
 			ds.On("GetUserAndPasswordById", self.CTX, self.ID).
-				Return(&model.User{ID: self.ID, ETag: model.ETag{0}}, nil).
+				Return(&model.User{ID: self.ID, ETag: &model.ETag{0}}, nil).
 				On("UpdateUser", self.CTX, self.ID, self.UserUpdate).
 				Return(nil, store.ErrUserNotFound)