From 301ca226fe4d3e82c3d25ca1462b9f84c41fe210 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Tue, 28 Jan 2020 12:30:39 -0700 Subject: [PATCH 1/4] Email address cannot be empty any longer on user/current PUT --- lib/go-tc/users.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/go-tc/users.go b/lib/go-tc/users.go index 9fe8296716..6691938d3b 100644 --- a/lib/go-tc/users.go +++ b/lib/go-tc/users.go @@ -182,6 +182,8 @@ func (u *CurrentUserUpdateRequestUser) UnmarshalAndValidate(user *User) error { if u.Email != nil { if err := json.Unmarshal(u.Email, &user.Email); err != nil { errs = append(errs, fmt.Errorf("email: %v", err)) + } else if *user.Email == "" { + errs = append(errs, errors.New("email: cannot be an empty string")) } else if err = validation.Validate(*user.Email, is.Email); err != nil { errs = append(errs, err) } From 38c72150cf6eeef91ad41977d2b3fbd29bd7a4e8 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Tue, 28 Jan 2020 12:34:26 -0700 Subject: [PATCH 2/4] Updated documentation --- docs/source/api/user_current.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/source/api/user_current.rst b/docs/source/api/user_current.rst index 0134798d75..c7f09d6deb 100644 --- a/docs/source/api/user_current.rst +++ b/docs/source/api/user_current.rst @@ -124,7 +124,7 @@ Request Structure :company: The name of the company for which the user works :confirmLocalPasswd: An optional 'confirm' field in a new user's password specification. This has no known effect and in fact *doesn't even need to match* ``localPasswd`` :country: The name of the country wherein the user resides - :email: The user's email address\ [#notnull]_ + :email: The user's email address - cannot be an empty string\ [#notnull]_ .. versionchanged:: ATCv4 Prior to version ATCv4, the email was validated using the `Email::Valid Perl package `_ but is now validated (circuitously) by `GitHub user asaskevich's regular expression `_ . Note that neither method can actually distinguish a valid, deliverable, email address but merely ensure the email is in a commonly-found format. From 3c658c2e2d3b1bf277964a101b66d87cf23e42f4 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Tue, 28 Jan 2020 12:43:31 -0700 Subject: [PATCH 3/4] Added constraint to client/api integration tests --- traffic_ops/testing/api/v1/user_test.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/traffic_ops/testing/api/v1/user_test.go b/traffic_ops/testing/api/v1/user_test.go index 334fc33729..18171e44dc 100644 --- a/traffic_ops/testing/api/v1/user_test.go +++ b/traffic_ops/testing/api/v1/user_test.go @@ -218,6 +218,30 @@ func UserSelfUpdateTest(t *testing.T) { } else if *resp2[0].Email != "operator@example.com" { t.Errorf("Expected Email to be restored to 'operator@example.com', but it was '%s'", *resp2[0].Email) } + + // now test using an invalid email address + currentEmail := *user.Email + user.Email = new(string); + updateResp, _, err = TOSession.UpdateCurrentUser(user) + if err == nil { + t.Fatal("error was expected updating user with email: '' - got none") + } + + // Ensure it wasn't actually updated + resp2, _, err = TOSession.GetUserByID(*user.ID) + if err != nil { + t.Fatalf("error getting user #%d: %v", *user.ID, err) + } + + if len(resp2) < 1 { + t.Fatalf("no user returned when requesting user #%d", *user.ID) + } + + if resp2[0].Email == nil { + t.Errorf("Email missing or null after update") + } else if *resp2[0].Email != currentEmail { + t.Errorf("Expected Email to still be '%s', but it was '%s'", currentEmail, *resp2[0].Email) + } } func UserUpdateOwnRoleTest(t *testing.T) { From 1d5d0c9b8a5670292b1de6bd8c21e53c64ede9c8 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Wed, 29 Jan 2020 08:19:17 -0700 Subject: [PATCH 4/4] Fixed segfault with null email addresses --- lib/go-tc/users.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/go-tc/users.go b/lib/go-tc/users.go index 6691938d3b..0a650978cb 100644 --- a/lib/go-tc/users.go +++ b/lib/go-tc/users.go @@ -182,8 +182,8 @@ func (u *CurrentUserUpdateRequestUser) UnmarshalAndValidate(user *User) error { if u.Email != nil { if err := json.Unmarshal(u.Email, &user.Email); err != nil { errs = append(errs, fmt.Errorf("email: %v", err)) - } else if *user.Email == "" { - errs = append(errs, errors.New("email: cannot be an empty string")) + } else if user.Email == nil || *user.Email == "" { + errs = append(errs, errors.New("email: cannot be null or an empty string")) } else if err = validation.Validate(*user.Email, is.Email); err != nil { errs = append(errs, err) } @@ -194,7 +194,7 @@ func (u *CurrentUserUpdateRequestUser) UnmarshalAndValidate(user *User) error { errs = append(errs, fmt.Errorf("fullName: %v", err)) } else if user.FullName == nil || *user.FullName == "" { // Perl enforced this - errs = append(errs, fmt.Errorf("fullName: cannot be set to 'null' or empty string")) + errs = append(errs, errors.New("fullName: cannot be set to 'null' or empty string")) } }