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. diff --git a/lib/go-tc/users.go b/lib/go-tc/users.go index 9fe8296716..0a650978cb 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 == 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) } @@ -192,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")) } } 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) {