From cc06c1a4c5db79e1ed0a6d3a51e3ca0ccb1d23ee Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Thu, 12 May 2022 20:43:45 -0600 Subject: [PATCH 01/14] Convert APIv4 user/current response to use APIv4 /users representation --- .../cdn-in-a-box/enroller/enroller.go | 5 +- lib/go-tc/users.go | 171 ++++++------ lib/go-tc/users_test.go | 251 +++++++++++------- traffic_ops/testing/api/v4/user_test.go | 4 +- .../traffic_ops_golang/user/current.go | 27 +- 5 files changed, 252 insertions(+), 206 deletions(-) diff --git a/infrastructure/cdn-in-a-box/enroller/enroller.go b/infrastructure/cdn-in-a-box/enroller/enroller.go index a773532882..2b31ef5061 100644 --- a/infrastructure/cdn-in-a-box/enroller/enroller.go +++ b/infrastructure/cdn-in-a-box/enroller/enroller.go @@ -845,7 +845,7 @@ func enrollFederation(toSession *session, r io.Reader) error { } cdnFederation = resp.Response if cdnFederation.ID == nil { - err = fmt.Errorf("Federation returned from creation through Traffic Ops with null or undefined ID") + err = fmt.Errorf("federation returned from creation through Traffic Ops with null or undefined ID") log.Infoln(err) return err } @@ -869,8 +869,7 @@ func enrollFederation(toSession *session, r io.Reader) error { } resp, _, err := toSession.CreateFederationUsers(*cdnFederation.ID, []int{*user.Response.ID}, true, client.RequestOptions{}) if err != nil { - var username string - username = user.Response.UserName + username := user.Response.Username err = fmt.Errorf("assigning User '%s' to Federation with ID %d: %v - alerts: %+v", username, *cdnFederation.ID, err, resp.Alerts) log.Infoln(err) return err diff --git a/lib/go-tc/users.go b/lib/go-tc/users.go index ba1b0d17ed..e616d8f6ea 100644 --- a/lib/go-tc/users.go +++ b/lib/go-tc/users.go @@ -75,7 +75,7 @@ func (u User) Upgrade() UserV4 { ret.Tenant = copyStringIfNotNil(u.Tenant) ret.Token = copyStringIfNotNil(u.Token) ret.UID = copyIntIfNotNil(u.UID) - ret.FullName = u.FullName + ret.FullName = copyStringIfNotNil(u.FullName) if u.LastUpdated != nil { ret.LastUpdated = u.LastUpdated.Time } @@ -184,7 +184,6 @@ type User struct { // NOTE: RoleName db:"-" tag is required due to clashing with the DB query here: // https://github.com/apache/trafficcontrol/blob/3b5dd406bf1a0bb456c062b0f6a465ec0617d8ef/traffic_ops/traffic_ops_golang/user/user.go#L197 // It's done that way in order to maintain "rolename" vs "roleName" JSON field capitalization for the different users APIs. - // TODO: make the breaking API change to make all user APIs use "roleName" consistently. RoleName *string `json:"roleName,omitempty" db:"role_name"` commonUserFields } @@ -197,52 +196,27 @@ type UserCurrent struct { commonUserFields } -// UserCurrentV4 is an alias for the UserCurrent struct used for the latest minor version associated with api major version 4. -type UserCurrentV4 UserCurrentV40 - -// UserCurrentV40 represents the structure for the "current" user, and has the "Role" field as a *string, as opposed to a *int found in the older versions. -type UserCurrentV40 struct { - UserName string `json:"username"` - LocalUser *bool `json:"localUser"` - AddressLine1 *string `json:"addressLine1"` - AddressLine2 *string `json:"addressLine2"` - City *string `json:"city"` - Company *string `json:"company"` - Country *string `json:"country"` - Email *string `json:"email"` - FullName *string `json:"fullName"` - GID *int `json:"gid"` - ID *int `json:"id"` - NewUser *bool `json:"newUser"` - PhoneNumber *string `json:"phoneNumber"` - PostalCode *string `json:"postalCode"` - PublicSSHKey *string `json:"publicSshKey"` - Role string `json:"role"` - StateOrProvince *string `json:"stateOrProvince"` - Tenant *string `json:"tenant"` - TenantID *int `json:"tenantId"` - Token *string `json:"-"` - UID *int `json:"uid"` - LastUpdated time.Time `json:"lastUpdated"` - LastAuthenticated time.Time `json:"lastAuthenticated"` -} - -// Downgrade will convert a UserCurrentV4 struct into an instance of the UserCurrent struct. -func (u UserCurrentV4) Downgrade() UserCurrent { +// ToLegacyCurrentUser will convert an APIv4 user to an APIv3 "current user" +// representation. A Role ID and "local user" value must be supplied, since the +// APIv4 User doesn't have them. +func (u UserV4) ToLegacyCurrentUser(roleID int, localUser bool) UserCurrent { var ret UserCurrent ret.FullName = new(string) - ret.FullName = u.FullName + *ret.FullName = *u.FullName ret.LastUpdated = TimeNoModFromTime(u.LastUpdated) ret.NewUser = new(bool) - ret.NewUser = u.NewUser + *ret.NewUser = u.NewUser ret.RoleName = new(string) *ret.RoleName = u.Role - ret.Role = nil + ret.Role = new(int) + *ret.Role = roleID ret.TenantID = new(int) - ret.TenantID = u.TenantID + *ret.TenantID = u.TenantID ret.Tenant = u.Tenant - ret.UserName = &u.UserName - ret.LocalUser = u.LocalUser + ret.UserName = new(string) + *ret.UserName = u.Username + ret.LocalUser = new(bool) + *ret.LocalUser = localUser ret.Token = copyStringIfNotNil(u.Token) ret.AddressLine1 = copyStringIfNotNil(u.AddressLine1) ret.AddressLine2 = copyStringIfNotNil(u.AddressLine2) @@ -270,33 +244,37 @@ type UserV4 UserV40 // A UserV40 is a representation of a Traffic Ops user as it appears in version // 4.0 of Traffic Ops's API. type UserV40 struct { - AddressLine1 *string `json:"addressLine1" db:"address_line1"` - AddressLine2 *string `json:"addressLine2" db:"address_line2"` - ChangeLogCount *int `json:"changeLogCount" db:"change_log_count"` - City *string `json:"city" db:"city"` - Company *string `json:"company" db:"company"` - ConfirmLocalPassword *string `json:"confirmLocalPasswd,omitempty" db:"confirm_local_passwd"` - Country *string `json:"country" db:"country"` - Email *string `json:"email" db:"email"` - FullName *string `json:"fullName" db:"full_name"` - GID *int `json:"gid"` - ID *int `json:"id" db:"id"` - LastAuthenticated *time.Time `json:"lastAuthenticated" db:"last_authenticated"` - LastUpdated time.Time `json:"lastUpdated" db:"last_updated"` - LocalPassword *string `json:"localPasswd,omitempty" db:"local_passwd"` - NewUser bool `json:"newUser" db:"new_user"` - PhoneNumber *string `json:"phoneNumber" db:"phone_number"` - PostalCode *string `json:"postalCode" db:"postal_code"` - PublicSSHKey *string `json:"publicSshKey" db:"public_ssh_key"` - RegistrationSent *time.Time `json:"registrationSent" db:"registration_sent"` - Role string `json:"role" db:"role"` - StateOrProvince *string `json:"stateOrProvince" db:"state_or_province"` - Tenant *string `json:"tenant"` - TenantID int `json:"tenantId" db:"tenant_id"` - Token *string `json:"-" db:"token"` - UCDN string `json:"ucdn"` - UID *int `json:"uid"` - Username string `json:"username" db:"username"` + AddressLine1 *string `json:"addressLine1" db:"address_line1"` + AddressLine2 *string `json:"addressLine2" db:"address_line2"` + ChangeLogCount *int `json:"changeLogCount" db:"change_log_count"` + City *string `json:"city" db:"city"` + Company *string `json:"company" db:"company"` + ConfirmLocalPassword *string `json:"confirmLocalPasswd,omitempty" db:"confirm_local_passwd"` + Country *string `json:"country" db:"country"` + Email *string `json:"email" db:"email"` + FullName *string `json:"fullName" db:"full_name"` + // Deprecated: This has no known use, and will likely be removed in future + // API versions. + GID *int `json:"gid"` + ID *int `json:"id" db:"id"` + LastAuthenticated *time.Time `json:"lastAuthenticated" db:"last_authenticated"` + LastUpdated time.Time `json:"lastUpdated" db:"last_updated"` + LocalPassword *string `json:"localPasswd,omitempty" db:"local_passwd"` + NewUser bool `json:"newUser" db:"new_user"` + PhoneNumber *string `json:"phoneNumber" db:"phone_number"` + PostalCode *string `json:"postalCode" db:"postal_code"` + PublicSSHKey *string `json:"publicSshKey" db:"public_ssh_key"` + RegistrationSent *time.Time `json:"registrationSent" db:"registration_sent"` + Role string `json:"role" db:"role"` + StateOrProvince *string `json:"stateOrProvince" db:"state_or_province"` + Tenant *string `json:"tenant"` + TenantID int `json:"tenantId" db:"tenant_id"` + Token *string `json:"-" db:"token"` + UCDN string `json:"ucdn"` + // Deprecated: This has no known use, and will likely be removed in future + // API versions. + UID *int `json:"uid"` + Username string `json:"username" db:"username"` } // UsersResponseV4 is the type of a response from Traffic Ops to requests made @@ -344,9 +322,9 @@ type CurrentUserUpdateRequestUser struct { Username json.RawMessage `json:"username"` } -// Upgrade converts a UserCurrent struct into an instance of UserCurrentV4. -func (u UserCurrent) Upgrade() UserCurrentV4 { - var ret UserCurrentV4 +// Upgrade converts an APIv3 and earlier "current user" to an APIv4 User. +func (u UserCurrent) Upgrade() UserV4 { + var ret UserV4 ret.AddressLine1 = copyStringIfNotNil(u.AddressLine1) ret.AddressLine2 = copyStringIfNotNil(u.AddressLine2) ret.City = copyStringIfNotNil(u.City) @@ -367,17 +345,17 @@ func (u UserCurrent) Upgrade() UserCurrentV4 { ret.LastUpdated = u.LastUpdated.Time } if u.NewUser != nil { - ret.NewUser = u.NewUser + ret.NewUser = *u.NewUser } if u.RoleName != nil { ret.Role = *u.RoleName } if u.TenantID != nil { - ret.TenantID = u.TenantID + ret.TenantID = *u.TenantID } if u.UserName != nil { - ret.UserName = *u.UserName + ret.Username = *u.UserName } return ret } @@ -388,25 +366,25 @@ func (u *CurrentUserUpdateRequestUser) UnmarshalAndValidate(user *User, useV4Use errs := []error{} if u.AddressLine1 != nil { if err := json.Unmarshal(u.AddressLine1, &user.AddressLine1); err != nil { - errs = append(errs, fmt.Errorf("addressLine1: %v", err)) + errs = append(errs, fmt.Errorf("addressLine1: %w", err)) } } if u.AddressLine2 != nil { if err := json.Unmarshal(u.AddressLine2, &user.AddressLine2); err != nil { - errs = append(errs, fmt.Errorf("addressLine2: %v", err)) + errs = append(errs, fmt.Errorf("addressLine2: %w", err)) } } if u.City != nil { if err := json.Unmarshal(u.City, &user.City); err != nil { - errs = append(errs, fmt.Errorf("city: %v", err)) + errs = append(errs, fmt.Errorf("city: %w", err)) } } if u.Company != nil { if err := json.Unmarshal(u.Company, &user.Company); err != nil { - errs = append(errs, fmt.Errorf("company: %v", err)) + errs = append(errs, fmt.Errorf("company: %w", err)) } } @@ -415,13 +393,13 @@ func (u *CurrentUserUpdateRequestUser) UnmarshalAndValidate(user *User, useV4Use if u.Country != nil { if err := json.Unmarshal(u.Country, &user.Country); err != nil { - errs = append(errs, fmt.Errorf("country: %v", err)) + errs = append(errs, fmt.Errorf("country: %w", err)) } } if u.Email != nil { if err := json.Unmarshal(u.Email, &user.Email); err != nil { - errs = append(errs, fmt.Errorf("email: %v", err)) + errs = append(errs, fmt.Errorf("email: %w", 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 { @@ -431,7 +409,7 @@ func (u *CurrentUserUpdateRequestUser) UnmarshalAndValidate(user *User, useV4Use if u.FullName != nil { if err := json.Unmarshal(u.FullName, &user.FullName); err != nil { - errs = append(errs, fmt.Errorf("fullName: %v", err)) + errs = append(errs, fmt.Errorf("fullName: %w", err)) } else if user.FullName == nil || *user.FullName == "" { // Perl enforced this errs = append(errs, errors.New("fullName: cannot be set to 'null' or empty string")) @@ -440,14 +418,14 @@ func (u *CurrentUserUpdateRequestUser) UnmarshalAndValidate(user *User, useV4Use if u.GID != nil { if err := json.Unmarshal(u.GID, &user.GID); err != nil { - errs = append(errs, fmt.Errorf("gid: %v", err)) + errs = append(errs, fmt.Errorf("gid: %w", err)) } } if u.ID != nil { var uid int if err := json.Unmarshal(u.ID, &uid); err != nil { - errs = append(errs, fmt.Errorf("id: %v", err)) + errs = append(errs, fmt.Errorf("id: %w", err)) } else if user.ID != nil && *user.ID != uid { errs = append(errs, errors.New("id: cannot change user id")) } else { @@ -457,48 +435,47 @@ func (u *CurrentUserUpdateRequestUser) UnmarshalAndValidate(user *User, useV4Use if u.PhoneNumber != nil { if err := json.Unmarshal(u.PhoneNumber, &user.PhoneNumber); err != nil { - errs = append(errs, fmt.Errorf("phoneNumber: %v", err)) + errs = append(errs, fmt.Errorf("phoneNumber: %w", err)) } } if u.PostalCode != nil { if err := json.Unmarshal(u.PostalCode, &user.PostalCode); err != nil { - errs = append(errs, fmt.Errorf("postalCode: %v", err)) + errs = append(errs, fmt.Errorf("postalCode: %w", err)) } } if u.PublicSSHKey != nil { if err := json.Unmarshal(u.PublicSSHKey, &user.PublicSSHKey); err != nil { - errs = append(errs, fmt.Errorf("publicSshKey: %v", err)) + errs = append(errs, fmt.Errorf("publicSshKey: %w", err)) } } if u.Role != nil { if useV4User { if err := json.Unmarshal(u.Role, &user.RoleName); err != nil { - errs = append(errs, fmt.Errorf("role: %v", err)) + errs = append(errs, fmt.Errorf("role: %w", err)) } else if user.RoleName == nil { errs = append(errs, errors.New("role: cannot be null")) } } else { if err := json.Unmarshal(u.Role, &user.Role); err != nil { - errs = append(errs, fmt.Errorf("role: %v", err)) + errs = append(errs, fmt.Errorf("role: %w", err)) } else if user.Role == nil { errs = append(errs, errors.New("role: cannot be null")) } } - } if u.StateOrProvince != nil { if err := json.Unmarshal(u.StateOrProvince, &user.StateOrProvince); err != nil { - errs = append(errs, fmt.Errorf("stateOrProvince: %v", err)) + errs = append(errs, fmt.Errorf("stateOrProvince: %w", err)) } } if u.TenantID != nil { if err := json.Unmarshal(u.TenantID, &user.TenantID); err != nil { - errs = append(errs, fmt.Errorf("tenantID: %v", err)) + errs = append(errs, fmt.Errorf("tenantID: %w", err)) } else if user.TenantID == nil { errs = append(errs, errors.New("tenantID: cannot be null")) } @@ -506,13 +483,13 @@ func (u *CurrentUserUpdateRequestUser) UnmarshalAndValidate(user *User, useV4Use if u.UID != nil { if err := json.Unmarshal(u.UID, &user.UID); err != nil { - errs = append(errs, fmt.Errorf("uid: %v", err)) + errs = append(errs, fmt.Errorf("uid: %w", err)) } } if u.Username != nil { if err := json.Unmarshal(u.Username, &user.Username); err != nil { - errs = append(errs, fmt.Errorf("username: %v", err)) + errs = append(errs, fmt.Errorf("username: %w", err)) } else if user.Username == nil || *user.Username == "" { errs = append(errs, errors.New("username: cannot be null or empty string")) } @@ -577,7 +554,7 @@ type UserCurrentResponse struct { // UserCurrentResponseV4 is the latest 4.x Traffic Ops API version variant of UserResponse. type UserCurrentResponseV4 struct { - Response UserCurrentV4 `json:"response"` + Response UserV4 `json:"response"` Alerts } @@ -618,11 +595,11 @@ type UserRegistrationRequestV40 struct { func (urr *UserRegistrationRequestV4) Validate(tx *sql.Tx) error { var errs = []error{} if urr.Role == "" { - errs = append(errs, errors.New("role: required and cannot be empty.")) + errs = append(errs, errors.New("role: required and cannot be empty")) } if urr.TenantID == 0 { - errs = append(errs, errors.New("tenantId: required and cannot be zero.")) + errs = append(errs, errors.New("tenantId: required and cannot be zero")) } // This can only happen if an email isn't present in the request; the JSON parse handles actually @@ -640,11 +617,11 @@ func (urr *UserRegistrationRequestV4) Validate(tx *sql.Tx) error { func (urr *UserRegistrationRequest) Validate(tx *sql.Tx) error { var errs = []error{} if urr.Role == 0 { - errs = append(errs, errors.New("role: required and cannot be zero.")) + errs = append(errs, errors.New("role: required and cannot be zero")) } if urr.TenantID == 0 { - errs = append(errs, errors.New("tenantId: required and cannot be zero.")) + errs = append(errs, errors.New("tenantId: required and cannot be zero")) } // This can only happen if an email isn't present in the request; the JSON parse handles actually diff --git a/lib/go-tc/users_test.go b/lib/go-tc/users_test.go index b756999b5a..5cacaa4241 100644 --- a/lib/go-tc/users_test.go +++ b/lib/go-tc/users_test.go @@ -24,6 +24,149 @@ import ( "time" ) +func compareIntPtrs(t *testing.T, name string, want, got *int, operation string) { + if want == nil { + t.Error("incorrect calling of compareStrPtrs - want must not be nil") + return + } + + if got == nil { + t.Errorf("wrong %s after %s; want: %d, got: nil pointer", name, operation, *want) + } else if want == got { + t.Errorf("expected %s to be deeply copied, but it was a pointer to the original struct's field", name) + } else if *want != *got { + t.Errorf("wrong %s after %s; want: %d, got: %d", name, operation, *want, *got) + } +} + +func compareStrPtrs(t *testing.T, name string, want, got *string, operation string) { + if want == nil { + t.Error("incorrect calling of compareStrPtrs - want must not be nil") + return + } + + if got == nil { + t.Errorf("wrong %s after %s; want: '%s', got: nil pointer", name, operation, *want) + } else if want == got { + t.Errorf("expected %s to be deeply copied, but it was a pointer to the original struct's field", name) + } else if *want != *got { + t.Errorf("wrong %s after %s; want: '%s', got: '%s'", name, operation, *want, *got) + } +} + +func TestUserV4_ToLegacyCurrentUser(t *testing.T) { + addressLine1 := "Address Line 1" + addressLine2 := "Address Line 2" + city := "City" + company := "Company" + confirmLocalPassword := "Confirm LocalPasswd" + country := "Country" + email := "em@i.l" + fullName := "Full Name" + gid := 1 + id := 2 + lastAuthenticated := time.Time{} + lastUpdated := time.Now() + localPassword := "LocalPasswd" + localUser := true + newUser := true + phoneNumber := "555-555-5555" + postalCode := "55555" + publicSSHKey := "Public SSH Key" + registrationSent, _ := time.Parse(time.RFC3339, "2000-01-02T03:04:05Z") + role := "Role Name" + roleID := 3 + stateOrProvince := "State or Province" + tenant := "Tenant" + tenantID := 4 + token := "Token" + uid := 5 + username := "Username" + + user := UserV4{ + AddressLine1: &addressLine1, + AddressLine2: &addressLine2, + City: &city, + Company: &company, + ConfirmLocalPassword: &confirmLocalPassword, + Country: &country, + Email: &email, + FullName: &fullName, + GID: &gid, + ID: &id, + LastAuthenticated: &lastAuthenticated, + LastUpdated: lastUpdated, + LocalPassword: &localPassword, + NewUser: newUser, + PhoneNumber: &phoneNumber, + PostalCode: &postalCode, + PublicSSHKey: &publicSSHKey, + RegistrationSent: ®istrationSent, + Role: role, + StateOrProvince: &stateOrProvince, + Tenant: &tenant, + TenantID: tenantID, + Token: &token, + UCDN: "UCDN", + UID: &uid, + Username: username, + } + + currentUser := user.ToLegacyCurrentUser(roleID, localUser) + compareStrPtrs(t, "AddressLine1", user.AddressLine1, currentUser.AddressLine1, "downgrade") + compareStrPtrs(t, "AddressLine2", user.AddressLine2, currentUser.AddressLine2, "downgrade") + compareStrPtrs(t, "City", user.City, currentUser.City, "downgrade") + compareStrPtrs(t, "Company", user.Company, currentUser.Company, "downgrade") + compareStrPtrs(t, "Country", user.Country, currentUser.Country, "downgrade") + compareStrPtrs(t, "Email", user.Email, currentUser.Email, "downgrade") + compareStrPtrs(t, "FullName", user.FullName, currentUser.FullName, "downgrade") + compareIntPtrs(t, "GID", user.GID, currentUser.GID, "downgrade") + compareIntPtrs(t, "ID", user.ID, currentUser.ID, "downgrade") + if currentUser.LastUpdated == nil { + t.Errorf("wrong LastUpdated after downgrade; want: '%s', got: nil pointer", lastUpdated) + } else if !currentUser.LastUpdated.Time.Equal(lastUpdated) { + t.Errorf("wrong LastUpdated after downgrade; want: '%s', got: '%s'", lastUpdated, currentUser.LastUpdated.Time) + } + if currentUser.LocalUser == nil { + t.Errorf("wrong LocalUser after downgrade; want: %t, got: nil pointer", localUser) + } else if *currentUser.LocalUser != localUser { + t.Errorf("wrong LocalUser after downgrade; want: %t, got: %t", localUser, *currentUser.LocalUser) + } + if currentUser.NewUser == nil { + t.Errorf("wrong NewUser after downgrade; want: %t, got: nil pointer", newUser) + } else if *currentUser.NewUser != newUser { + t.Errorf("wrong NewUser after downgrade; want: %t, got: %t", newUser, *currentUser.NewUser) + } + compareStrPtrs(t, "PhoneNumber", user.PhoneNumber, currentUser.PhoneNumber, "downgrade") + compareStrPtrs(t, "PostalCode", user.PostalCode, currentUser.PostalCode, "downgrade") + compareStrPtrs(t, "PublicSSHKey", user.PublicSSHKey, currentUser.PublicSSHKey, "downgrade") + if currentUser.Role == nil { + t.Errorf("wrong Role after downgrade; want: %d, got: nil pointer", roleID) + } else if *currentUser.Role != roleID { + t.Errorf("wrong Role after downgrade; want: %d, got: %d", roleID, *currentUser.Role) + + } + if currentUser.RoleName == nil { + t.Errorf("wrong RoleName after downgrade; want: '%s', got: nil pointer", role) + } else if *currentUser.RoleName != role { + t.Errorf("wrong RoleName after downgrade; want: '%s', got: '%s'", role, *currentUser.RoleName) + } + compareStrPtrs(t, "StateOrProvince", user.StateOrProvince, currentUser.StateOrProvince, "downgrade") + compareStrPtrs(t, "Tenant", user.Tenant, currentUser.Tenant, "downgrade") + if currentUser.TenantID == nil { + t.Errorf("wrong TenantID after downgrade; want: %d, got: nil pointer", tenantID) + } else if *currentUser.TenantID != tenantID { + t.Errorf("wrong TenantID after downgrade; want: %d, got: %d", tenantID, *currentUser.TenantID) + } + compareStrPtrs(t, "Token", user.Token, currentUser.Token, "downgrade") + compareIntPtrs(t, "UID", user.UID, currentUser.UID, "downgrade") + if currentUser.UserName == nil { + t.Errorf("wrong UserName after downgrade; want: '%s', got: nil pointer", username) + } else if *currentUser.UserName != username { + t.Errorf("wrong UserName after downgrade; want: '%s', got: '%s'", username, *currentUser.UserName) + } +} + func TestUser_UpgradeFromLegacyUser(t *testing.T) { addressLine1 := "Address Line 1" addressLine2 := "Address Line 2" @@ -78,82 +221,26 @@ func TestUser_UpgradeFromLegacyUser(t *testing.T) { user.Username = &username upgraded := user.Upgrade() - if upgraded.AddressLine1 == nil { - t.Error("AddressLine1 became nil after upgrade") - } else if *upgraded.AddressLine1 != addressLine1 { - t.Errorf("Incorrect AddressLine1 after upgrade; want: '%s', got: '%s'", addressLine1, *upgraded.AddressLine1) - } - if upgraded.AddressLine2 == nil { - t.Error("AddressLine2 became nil after upgrade") - } else if *upgraded.AddressLine2 != addressLine2 { - t.Errorf("Incorrect AddressLine2 after upgrade; want: '%s', got: '%s'", addressLine2, *upgraded.AddressLine2) - } - if upgraded.City == nil { - t.Error("City became nil after upgrade") - } else if *upgraded.City != city { - t.Errorf("Incorrect City after upgrade; want: '%s', got: '%s'", city, *upgraded.City) - } - if upgraded.Company == nil { - t.Error("Company became nil after upgrade") - } else if *upgraded.Company != company { - t.Errorf("Incorrect Company after upgrade; want: '%s', got: '%s'", company, *upgraded.Company) - } - if upgraded.ConfirmLocalPassword == nil { - t.Error("ConfirmLocalPassword became nil after upgrade") - } else if *upgraded.ConfirmLocalPassword != confirmLocalPassword { - t.Errorf("Incorrect ConfirmLocalPassword after upgrade; want: '%s', got: '%s'", confirmLocalPassword, *upgraded.ConfirmLocalPassword) - } - if upgraded.Country == nil { - t.Error("Country became nil after upgrade") - } else if *upgraded.Country != country { - t.Errorf("Incorrect Country after upgrade; want: '%s', got: '%s'", country, *upgraded.Country) - } - if upgraded.Email == nil { - t.Error("Email became nil after upgrade") - } else if *upgraded.Email != email { - t.Errorf("Incorrect Email after upgrade; want: '%s', got: '%s'", email, *upgraded.Email) - } - if upgraded.FullName == nil { - t.Error("Fullname became nil after upgrade") - } else if *upgraded.FullName != fullName { - t.Errorf("Incorrect FullName after upgrade; want: '%s', got: '%s'", fullName, *upgraded.FullName) - } - if upgraded.GID == nil { - t.Error("GID became nil after upgrade") - } else if *upgraded.GID != gid { - t.Errorf("Incorrect GID after upgrade; want: %d, got: %d", gid, *upgraded.GID) - } - if upgraded.ID == nil { - t.Error("ID became nil after upgrade") - } else if *upgraded.ID != id { - t.Errorf("Incorrect ID after upgrade; want: %d, got: %d", id, *upgraded.ID) - } + compareStrPtrs(t, "AddressLine1", user.AddressLine1, upgraded.AddressLine1, "upgrade") + compareStrPtrs(t, "AddressLine2", user.AddressLine2, upgraded.AddressLine2, "upgrade") + compareStrPtrs(t, "City", user.City, upgraded.City, "upgrade") + compareStrPtrs(t, "Company", user.Company, upgraded.Company, "upgrade") + compareStrPtrs(t, "ConfirmLocalPassword", user.ConfirmLocalPassword, upgraded.ConfirmLocalPassword, "upgrade") + compareStrPtrs(t, "Country", user.Country, upgraded.Country, "upgrade") + compareStrPtrs(t, "Email", user.Email, upgraded.Email, "upgrade") + compareStrPtrs(t, "FullName", user.FullName, upgraded.FullName, "upgrade") + compareIntPtrs(t, "GID", user.GID, upgraded.GID, "upgrade") + compareIntPtrs(t, "ID", user.ID, upgraded.ID, "upgrade") if !upgraded.LastUpdated.Equal(lastUpdated.Time) { t.Errorf("Incorrect LastUpdated after upgrade; want: %v, got: %v", lastUpdated.Time, upgraded.LastUpdated) } - if upgraded.LocalPassword == nil { - t.Error("LocalPassword became nil after upgrade") - } else if *upgraded.LocalPassword != localPassword { - t.Errorf("Incorrect LocalPassword after upgrade; want: '%s', got: '%s'", localPassword, *upgraded.LocalPassword) - } + compareStrPtrs(t, "LocalPassword", user.LocalPassword, upgraded.LocalPassword, "upgrade") if upgraded.NewUser != newUser { t.Errorf("Incorrect NewUser after upgrade; want: %t, got: %t", newUser, upgraded.NewUser) } - if upgraded.PhoneNumber == nil { - t.Error("PhoneNumber became nil after upgrade") - } else if *upgraded.PhoneNumber != phoneNumber { - t.Errorf("Incorrect PhoneNumber after upgrade; want: '%s', got: '%s'", phoneNumber, *upgraded.PhoneNumber) - } - if upgraded.PostalCode == nil { - t.Error("PostalCode became nil after upgrade") - } else if *upgraded.PostalCode != postalCode { - t.Errorf("Incorrect PostalCode after upgrade; want: '%s', got: '%s'", postalCode, *upgraded.PostalCode) - } - if upgraded.PublicSSHKey == nil { - t.Error("PublicSSHKey became nil after upgrade") - } else if *upgraded.PublicSSHKey != publicSSHKey { - t.Errorf("Incorrect PublicSSHKey after upgrade; want: '%s', got: '%s'", publicSSHKey, *upgraded.PublicSSHKey) - } + compareStrPtrs(t, "PhoneNumber", user.PhoneNumber, upgraded.PhoneNumber, "upgrade") + compareStrPtrs(t, "PostalCode", user.PostalCode, upgraded.PostalCode, "upgrade") + compareStrPtrs(t, "PublicSSHKey", user.PublicSSHKey, upgraded.PublicSSHKey, "upgrade") if upgraded.RegistrationSent == nil { t.Error("RegistrationSent became nil after upgrade") } else if !upgraded.RegistrationSent.Equal(registrationSent.Time) { @@ -162,29 +249,13 @@ func TestUser_UpgradeFromLegacyUser(t *testing.T) { if upgraded.Role != role { t.Errorf("Incorrect Role after upgrade; want: '%s', got: '%s'", role, upgraded.Role) } - if upgraded.StateOrProvince == nil { - t.Error("StateOrProvince became nil after upgrade") - } else if *upgraded.StateOrProvince != stateOrProvince { - t.Errorf("Incorrect StateOrProvince after upgrade; want: '%s', got: '%s'", stateOrProvince, *upgraded.StateOrProvince) - } - if upgraded.Tenant == nil { - t.Error("Tenant became nil after upgrade") - } else if *upgraded.Tenant != tenant { - t.Errorf("Incorrect Tenant after upgrade; want: '%s', got: '%s'", tenant, *upgraded.Tenant) - } + compareStrPtrs(t, "StateOrProvince", user.StateOrProvince, upgraded.StateOrProvince, "upgrade") + compareStrPtrs(t, "Tenant", user.Tenant, upgraded.Tenant, "upgrade") if upgraded.TenantID != tenantID { t.Errorf("Incorrect TenantID after upgrade; want: %d, got: %d", tenantID, upgraded.TenantID) } - if upgraded.Token == nil { - t.Error("Token became nil after upgrade") - } else if *upgraded.Token != token { - t.Errorf("Incorrect Token after upgrade; want: '%s', got: '%s'", token, *upgraded.Token) - } - if upgraded.UID == nil { - t.Error("UID became nil after upgrade") - } else if *upgraded.UID != uid { - t.Errorf("Incorrect UID after upgrade; want: %d, got: %d", uid, *upgraded.UID) - } + compareStrPtrs(t, "Token", user.Token, upgraded.Token, "upgrade") + compareIntPtrs(t, "UID", user.UID, upgraded.UID, "upgrade") if upgraded.Username != username { t.Errorf("Incorrect Username after upgrade; want: '%s', got: '%s'", username, upgraded.Username) } diff --git a/traffic_ops/testing/api/v4/user_test.go b/traffic_ops/testing/api/v4/user_test.go index 8acd77eaf8..35864892b4 100644 --- a/traffic_ops/testing/api/v4/user_test.go +++ b/traffic_ops/testing/api/v4/user_test.go @@ -390,8 +390,8 @@ func GetTestUserCurrent(t *testing.T) { if err != nil { t.Errorf("cannot get current user: %v - alerts: %+v", err, user.Alerts) } - if user.Response.UserName != SessionUserName { - t.Errorf("current user expected: '%s' actual: '%s'", SessionUserName, user.Response.UserName) + if user.Response.Username != SessionUserName { + t.Errorf("current user expected: '%s' actual: '%s'", SessionUserName, user.Response.Username) } } diff --git a/traffic_ops/traffic_ops_golang/user/current.go b/traffic_ops/traffic_ops_golang/user/current.go index d2f2365bc8..7cf4b8f243 100644 --- a/traffic_ops/traffic_ops_golang/user/current.go +++ b/traffic_ops/traffic_ops_golang/user/current.go @@ -106,27 +106,26 @@ func Current(w http.ResponseWriter, r *http.Request) { } defer inf.Close() - currentUser, role, err := getUser(inf.Tx.Tx, inf.User.ID) + currentUser, role, localUser, err := getUser(inf.Tx.Tx, inf.User.ID) if err != nil { - api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("getting current user: "+err.Error())) + api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("getting current user: %w", err)) return } version := inf.Version if version == nil { - api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, fmt.Errorf("TOUsers.Read called with invalid API version"), nil) + api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, errors.New("TOUsers.Read called with invalid API version"), nil) return } if version.Major >= 4 { api.WriteResp(w, r, currentUser) } else { - legacyUser := currentUser.Downgrade() - legacyUser.Role = &role + legacyUser := currentUser.ToLegacyCurrentUser(role, localUser) api.WriteResp(w, r, legacyUser) } } -func getUser(tx *sql.Tx, id int) (tc.UserCurrentV4, int, error) { +func getUser(tx *sql.Tx, id int) (tc.UserV4, int, bool, error) { q := ` SELECT u.address_line1, @@ -144,8 +143,8 @@ u.new_user, u.phone_number, u.postal_code, u.public_ssh_key, -u.role, -r.name as role_name, +r.name as "role", +u.role as role_id, u.state_or_province, t.name as tenant, u.tenant_id, @@ -155,14 +154,14 @@ LEFT JOIN role as r ON r.id = u.role INNER JOIN tenant as t ON t.id = u.tenant_id WHERE u.id=$1 ` - u := tc.UserCurrentV4{} - localPassword := sql.NullString{} + var u tc.UserV4 + var localPassword sql.NullString var role int - if err := tx.QueryRow(q, id).Scan(&u.AddressLine1, &u.AddressLine2, &u.City, &u.Company, &u.Country, &u.Email, &u.FullName, &u.ID, &u.LastUpdated, &u.LastAuthenticated, &localPassword, &u.NewUser, &u.PhoneNumber, &u.PostalCode, &u.PublicSSHKey, &role, &u.Role, &u.StateOrProvince, &u.Tenant, &u.TenantID, &u.UserName); err != nil { - return tc.UserCurrentV4{}, role, errors.New("querying current user: " + err.Error()) + err := tx.QueryRow(q, id).Scan(&u.AddressLine1, &u.AddressLine2, &u.City, &u.Company, &u.Country, &u.Email, &u.FullName, &u.ID, &u.LastUpdated, &u.LastAuthenticated, &localPassword, &u.NewUser, &u.PhoneNumber, &u.PostalCode, &u.PublicSSHKey, &role, &u.Role, &u.StateOrProvince, &u.Tenant, &u.TenantID, &u.Username) + if err != nil { + err = fmt.Errorf("querying current user: %w", err) } - u.LocalUser = util.BoolPtr(localPassword.Valid) - return u, role, nil + return u, role, localPassword.Valid, err } func ReplaceCurrent(w http.ResponseWriter, r *http.Request) { From fc16cb2f7a361f9914a717c65cca1cad0ab6e841 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Fri, 13 May 2022 09:47:41 -0600 Subject: [PATCH 02/14] Add function to convert ozzo errors to errors and join them in one step --- lib/go-tc/tovalidate/validate.go | 20 ++++++++++++++++++++ lib/go-tc/tovalidate/validate_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) create mode 100644 lib/go-tc/tovalidate/validate_test.go diff --git a/lib/go-tc/tovalidate/validate.go b/lib/go-tc/tovalidate/validate.go index 7422cc97d0..50daba18e9 100644 --- a/lib/go-tc/tovalidate/validate.go +++ b/lib/go-tc/tovalidate/validate.go @@ -13,7 +13,9 @@ package tovalidate // limitations under the License. import ( + "errors" "fmt" + "strings" ) // ToErrors converts a map of strings to errors into an array of errors. @@ -43,3 +45,21 @@ func ToErrors(err map[string]error) []error { } return vErrors } + +// ToError converts a map of strings to errors into a single error. +// +// Because multiple errors are collapsed, errors cannot be wrapped and therefore +// error identity cannot be preserved. +func ToError(err map[string]error) error { + var b strings.Builder + for key, value := range err { + if value != nil { + b.WriteRune('\'') + b.WriteString(key) + b.WriteString("' ") + b.WriteString(value.Error()) + b.WriteString(", ") + } + } + return errors.New(strings.TrimSuffix(b.String(), ", ")) +} diff --git a/lib/go-tc/tovalidate/validate_test.go b/lib/go-tc/tovalidate/validate_test.go new file mode 100644 index 0000000000..fcced2174e --- /dev/null +++ b/lib/go-tc/tovalidate/validate_test.go @@ -0,0 +1,27 @@ +package tovalidate + +// 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 + +// 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. + +import ( + "errors" + "fmt" +) + +func ExampleToError() { + errs := map[string]error{ + "propA": errors.New("bad value"), + "propB": errors.New("cannot be blank"), + } + fmt.Println(ToError(errs)) + // Output: 'propA' bad value, 'propB' cannot be blank +} From 040b3bb63201bb357ac43211b5c7639604bdc172 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Fri, 13 May 2022 09:53:14 -0600 Subject: [PATCH 03/14] Make user/current GET requests return UserV40s, remove ConfirmLocalPassword Also fixed undocumented error where "current user" representations didn't include GID or UID, and fixed unable to edit UCDN with the user/current endpoint, and remove the top-level "user" key from v4 that holds the actual request body, and fix upgrading CurrentUser not setting v4 fields and fix PUT request responses not showing changeLogCount, GID, UID, and UCDN. --- lib/go-tc/users.go | 45 +- lib/go-tc/users_test.go | 59 +- .../traffic_ops_golang/routing/routes.go | 2 +- .../traffic_ops_golang/user/current.go | 525 ++++++++++++++---- 4 files changed, 479 insertions(+), 152 deletions(-) diff --git a/lib/go-tc/users.go b/lib/go-tc/users.go index e616d8f6ea..e7fc6ff889 100644 --- a/lib/go-tc/users.go +++ b/lib/go-tc/users.go @@ -62,7 +62,6 @@ func (u User) Upgrade() UserV4 { ret.AddressLine2 = copyStringIfNotNil(u.AddressLine2) ret.City = copyStringIfNotNil(u.City) ret.Company = copyStringIfNotNil(u.Company) - ret.ConfirmLocalPassword = copyStringIfNotNil(u.ConfirmLocalPassword) ret.Country = copyStringIfNotNil(u.Country) ret.Email = copyStringIfNotNil(u.Email) ret.GID = copyIntIfNotNil(u.GID) @@ -118,7 +117,7 @@ func (u UserV4) Downgrade() User { ret.AddressLine2 = copyStringIfNotNil(u.AddressLine2) ret.City = copyStringIfNotNil(u.City) ret.Company = copyStringIfNotNil(u.Company) - ret.ConfirmLocalPassword = copyStringIfNotNil(u.ConfirmLocalPassword) + ret.ConfirmLocalPassword = copyStringIfNotNil(u.LocalPassword) ret.Country = copyStringIfNotNil(u.Country) ret.Email = copyStringIfNotNil(u.Email) ret.GID = copyIntIfNotNil(u.GID) @@ -244,15 +243,14 @@ type UserV4 UserV40 // A UserV40 is a representation of a Traffic Ops user as it appears in version // 4.0 of Traffic Ops's API. type UserV40 struct { - AddressLine1 *string `json:"addressLine1" db:"address_line1"` - AddressLine2 *string `json:"addressLine2" db:"address_line2"` - ChangeLogCount *int `json:"changeLogCount" db:"change_log_count"` - City *string `json:"city" db:"city"` - Company *string `json:"company" db:"company"` - ConfirmLocalPassword *string `json:"confirmLocalPasswd,omitempty" db:"confirm_local_passwd"` - Country *string `json:"country" db:"country"` - Email *string `json:"email" db:"email"` - FullName *string `json:"fullName" db:"full_name"` + AddressLine1 *string `json:"addressLine1" db:"address_line1"` + AddressLine2 *string `json:"addressLine2" db:"address_line2"` + ChangeLogCount *int `json:"changeLogCount" db:"change_log_count"` + City *string `json:"city" db:"city"` + Company *string `json:"company" db:"company"` + Country *string `json:"country" db:"country"` + Email *string `json:"email" db:"email"` + FullName *string `json:"fullName" db:"full_name"` // Deprecated: This has no known use, and will likely be removed in future // API versions. GID *int `json:"gid"` @@ -323,22 +321,27 @@ type CurrentUserUpdateRequestUser struct { } // Upgrade converts an APIv3 and earlier "current user" to an APIv4 User. -func (u UserCurrent) Upgrade() UserV4 { +// Fields not present in earlier API versions need to be passed explicitly +func (u UserCurrent) Upgrade(registrationSent, lastAuthenticated *time.Time, ucdn string, changeLogCount *int) UserV4 { var ret UserV4 ret.AddressLine1 = copyStringIfNotNil(u.AddressLine1) ret.AddressLine2 = copyStringIfNotNil(u.AddressLine2) + ret.ChangeLogCount = changeLogCount ret.City = copyStringIfNotNil(u.City) ret.Company = copyStringIfNotNil(u.Company) ret.Country = copyStringIfNotNil(u.Country) ret.Email = copyStringIfNotNil(u.Email) ret.GID = copyIntIfNotNil(u.GID) ret.ID = copyIntIfNotNil(u.ID) + ret.LastAuthenticated = lastAuthenticated ret.PhoneNumber = copyStringIfNotNil(u.PhoneNumber) ret.PostalCode = copyStringIfNotNil(u.PostalCode) ret.PublicSSHKey = copyStringIfNotNil(u.PublicSSHKey) + ret.RegistrationSent = registrationSent ret.StateOrProvince = copyStringIfNotNil(u.StateOrProvince) ret.Tenant = copyStringIfNotNil(u.Tenant) ret.Token = copyStringIfNotNil(u.Token) + ret.UCDN = ucdn ret.UID = copyIntIfNotNil(u.UID) ret.FullName = u.FullName if u.LastUpdated != nil { @@ -362,7 +365,7 @@ func (u UserCurrent) Upgrade() UserV4 { // UnmarshalAndValidate validates the request and returns a User into which the request's information // has been unmarshalled. -func (u *CurrentUserUpdateRequestUser) UnmarshalAndValidate(user *User, useV4User bool) error { +func (u *CurrentUserUpdateRequestUser) UnmarshalAndValidate(user *User) error { errs := []error{} if u.AddressLine1 != nil { if err := json.Unmarshal(u.AddressLine1, &user.AddressLine1); err != nil { @@ -452,18 +455,10 @@ func (u *CurrentUserUpdateRequestUser) UnmarshalAndValidate(user *User, useV4Use } if u.Role != nil { - if useV4User { - if err := json.Unmarshal(u.Role, &user.RoleName); err != nil { - errs = append(errs, fmt.Errorf("role: %w", err)) - } else if user.RoleName == nil { - errs = append(errs, errors.New("role: cannot be null")) - } - } else { - if err := json.Unmarshal(u.Role, &user.Role); err != nil { - errs = append(errs, fmt.Errorf("role: %w", err)) - } else if user.Role == nil { - errs = append(errs, errors.New("role: cannot be null")) - } + if err := json.Unmarshal(u.Role, &user.Role); err != nil { + errs = append(errs, fmt.Errorf("role: %w", err)) + } else if user.Role == nil { + errs = append(errs, errors.New("role: cannot be null")) } } diff --git a/lib/go-tc/users_test.go b/lib/go-tc/users_test.go index 5cacaa4241..9d6440ad5b 100644 --- a/lib/go-tc/users_test.go +++ b/lib/go-tc/users_test.go @@ -59,7 +59,6 @@ func TestUserV4_ToLegacyCurrentUser(t *testing.T) { addressLine2 := "Address Line 2" city := "City" company := "Company" - confirmLocalPassword := "Confirm LocalPasswd" country := "Country" email := "em@i.l" fullName := "Full Name" @@ -84,32 +83,31 @@ func TestUserV4_ToLegacyCurrentUser(t *testing.T) { username := "Username" user := UserV4{ - AddressLine1: &addressLine1, - AddressLine2: &addressLine2, - City: &city, - Company: &company, - ConfirmLocalPassword: &confirmLocalPassword, - Country: &country, - Email: &email, - FullName: &fullName, - GID: &gid, - ID: &id, - LastAuthenticated: &lastAuthenticated, - LastUpdated: lastUpdated, - LocalPassword: &localPassword, - NewUser: newUser, - PhoneNumber: &phoneNumber, - PostalCode: &postalCode, - PublicSSHKey: &publicSSHKey, - RegistrationSent: ®istrationSent, - Role: role, - StateOrProvince: &stateOrProvince, - Tenant: &tenant, - TenantID: tenantID, - Token: &token, - UCDN: "UCDN", - UID: &uid, - Username: username, + AddressLine1: &addressLine1, + AddressLine2: &addressLine2, + City: &city, + Company: &company, + Country: &country, + Email: &email, + FullName: &fullName, + GID: &gid, + ID: &id, + LastAuthenticated: &lastAuthenticated, + LastUpdated: lastUpdated, + LocalPassword: &localPassword, + NewUser: newUser, + PhoneNumber: &phoneNumber, + PostalCode: &postalCode, + PublicSSHKey: &publicSSHKey, + RegistrationSent: ®istrationSent, + Role: role, + StateOrProvince: &stateOrProvince, + Tenant: &tenant, + TenantID: tenantID, + Token: &token, + UCDN: "UCDN", + UID: &uid, + Username: username, } currentUser := user.ToLegacyCurrentUser(roleID, localUser) @@ -225,7 +223,6 @@ func TestUser_UpgradeFromLegacyUser(t *testing.T) { compareStrPtrs(t, "AddressLine2", user.AddressLine2, upgraded.AddressLine2, "upgrade") compareStrPtrs(t, "City", user.City, upgraded.City, "upgrade") compareStrPtrs(t, "Company", user.Company, upgraded.Company, "upgrade") - compareStrPtrs(t, "ConfirmLocalPassword", user.ConfirmLocalPassword, upgraded.ConfirmLocalPassword, "upgrade") compareStrPtrs(t, "Country", user.Country, upgraded.Country, "upgrade") compareStrPtrs(t, "Email", user.Email, upgraded.Email, "upgrade") compareStrPtrs(t, "FullName", user.FullName, upgraded.FullName, "upgrade") @@ -271,7 +268,6 @@ func TestUserV4_Downgrade(t *testing.T) { addressLine2 := "Address Line 2" city := "City" company := "Company" - confirmLocalPassword := "Confirm LocalPasswd" country := "Country" email := "em@i.l" fullName := "Full Name" @@ -304,7 +300,6 @@ func TestUserV4_Downgrade(t *testing.T) { user.AddressLine2 = &addressLine2 user.City = &city user.Company = &company - user.ConfirmLocalPassword = &confirmLocalPassword user.Country = &country user.Email = &email user.GID = &gid @@ -342,8 +337,8 @@ func TestUserV4_Downgrade(t *testing.T) { } if downgraded.ConfirmLocalPassword == nil { t.Error("ConfirmLocalPassword became nil after downgrade") - } else if *downgraded.ConfirmLocalPassword != confirmLocalPassword { - t.Errorf("Incorrect ConfirmLocalPassword after downgrade; want: '%s', got: '%s'", confirmLocalPassword, *downgraded.ConfirmLocalPassword) + } else if *downgraded.ConfirmLocalPassword != localPassword { + t.Errorf("Incorrect ConfirmLocalPassword after downgrade; want: '%s', got: '%s'", localPassword, *downgraded.ConfirmLocalPassword) } if downgraded.Country == nil { t.Error("Country became nil after downgrade") diff --git a/traffic_ops/traffic_ops_golang/routing/routes.go b/traffic_ops/traffic_ops_golang/routing/routes.go index 452ab76157..bdcfa2d277 100644 --- a/traffic_ops/traffic_ops_golang/routing/routes.go +++ b/traffic_ops/traffic_ops_golang/routing/routes.go @@ -248,7 +248,7 @@ func Routes(d ServerData) ([]Route, http.Handler, error) { {Version: api.Version{Major: 4, Minor: 0}, Method: http.MethodPost, Path: `users/?$`, Handler: user.Create, RequiredPrivLevel: auth.PrivLevelOperations, RequiredPermissions: []string{"USER:CREATE", "USER:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 4762448163}, {Version: api.Version{Major: 4, Minor: 0}, Method: http.MethodGet, Path: `user/current/?$`, Handler: user.Current, RequiredPrivLevel: auth.PrivLevelReadOnly, RequiredPermissions: nil, Authenticated: Authenticated, Middlewares: nil, ID: 46107016143}, - {Version: api.Version{Major: 4, Minor: 0}, Method: http.MethodPut, Path: `user/current/?$`, Handler: user.ReplaceCurrent, RequiredPrivLevel: auth.PrivLevelReadOnly, RequiredPermissions: nil, Authenticated: Authenticated, Middlewares: nil, ID: 4203}, + {Version: api.Version{Major: 4, Minor: 0}, Method: http.MethodPut, Path: `user/current/?$`, Handler: user.ReplaceCurrentV4, RequiredPrivLevel: auth.PrivLevelReadOnly, RequiredPermissions: nil, Authenticated: Authenticated, Middlewares: nil, ID: 4203}, //Parameter: CRUD {Version: api.Version{Major: 4, Minor: 0}, Method: http.MethodGet, Path: `parameters/?$`, Handler: api.ReadHandler(¶meter.TOParameter{}), RequiredPrivLevel: auth.PrivLevelReadOnly, RequiredPermissions: []string{"PARAMETER:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 42125542923}, diff --git a/traffic_ops/traffic_ops_golang/user/current.go b/traffic_ops/traffic_ops_golang/user/current.go index 7cf4b8f243..b6c2ef9e70 100644 --- a/traffic_ops/traffic_ops_golang/user/current.go +++ b/traffic_ops/traffic_ops_golang/user/current.go @@ -25,9 +25,13 @@ import ( "errors" "fmt" "net/http" + "strings" "github.com/apache/trafficcontrol/lib/go-tc" + "github.com/apache/trafficcontrol/lib/go-tc/tovalidate" "github.com/apache/trafficcontrol/lib/go-util" + validation "github.com/go-ozzo/ozzo-validation" + "github.com/go-ozzo/ozzo-validation/is" "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api" "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/auth" @@ -47,107 +51,183 @@ SET confirm_local_passwd=$1 WHERE id=$2 ` +const replacePasswordV4Query = ` +UPDATE tm_user +SET + confirm_local_passwd=$1, + local_passwd=$1 +WHERE id=$2 +` + const replaceCurrentQuery = ` UPDATE tm_user -SET address_line1=$1, - address_line2=$2, - city=$3, - company=$4, - country=$5, - email=$6, - full_name=$7, - gid=$8, - new_user=FALSE, - phone_number=$9, - postal_code=$10, - public_ssh_key=$11, - state_or_province=$12, - tenant_id=$13, - token=NULL, - uid=$14, - username=$15 +SET + address_line1=$1, + address_line2=$2, + city=$3, + company=$4, + country=$5, + email=$6, + full_name=$7, + gid=$8, + new_user=FALSE, + phone_number=$9, + postal_code=$10, + public_ssh_key=$11, + state_or_province=$12, + tenant_id=$13, + token=NULL, + uid=$14, + username=$15 WHERE id=$16 -RETURNING address_line1, - address_line2, - city, - company, - country, - email, - full_name, - gid, - id, - last_updated, - new_user, - phone_number, - postal_code, - public_ssh_key, - role, - ( - SELECT role.name - FROM role - WHERE role.id=tm_user.role - ) AS role_name, - state_or_province, - ( - SELECT tenant.name - FROM tenant - WHERE tenant.id=tm_user.tenant_id - ) AS tenant, - tenant_id, - uid, - username +RETURNING + address_line1, + address_line2, + city, + company, + country, + email, + full_name, + gid, + id, + last_updated, + new_user, + phone_number, + postal_code, + public_ssh_key, + role, + ( + SELECT role.name + FROM role + WHERE role.id=tm_user.role + ) AS role_name, + state_or_province, + ( + SELECT tenant.name + FROM tenant + WHERE tenant.id=tm_user.tenant_id + ) AS tenant, + tenant_id, + uid, + username +` + +const replaceCurrentV4Query = ` +UPDATE tm_user +SET + address_line1=$1, + address_line2=$2, + city=$3, + company=$4, + country=$5, + email=$6, + full_name=$7, + gid=$8, + new_user=FALSE, + phone_number=$9, + postal_code=$10, + public_ssh_key=$11, + state_or_province=$12, + tenant_id=$13, + token=NULL, + ucdn=$14, + uid=$15, + username=$16 +WHERE id=$17 +RETURNING + address_line1, + address_line2, + ( + SELECT count(l.tm_user) + FROM log AS l + WHERE l.tm_user = u.id + ), + city, + company, + country, + email, + full_name, + gid, + id, + last_authenticated, + last_updated, + new_user, + phone_number, + postal_code, + public_ssh_key, + registration_sent, + ( + SELECT role.name + FROM role + WHERE role.id=tm_user.role + ), + state_or_province, + ( + SELECT tenant.name + FROM tenant + WHERE tenant.id=tm_user.tenant_id + ), + tenant_id, + ucdn, + uid, + username ` func Current(w http.ResponseWriter, r *http.Request) { inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil) + tx := inf.Tx.Tx if userErr != nil || sysErr != nil { - api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr) + api.HandleErr(w, r, tx, errCode, userErr, sysErr) return } defer inf.Close() - currentUser, role, localUser, err := getUser(inf.Tx.Tx, inf.User.ID) - if err != nil { - api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("getting current user: %w", err)) + if inf.Version.Major < 4 { + cu, err := getLegacyUser(tx, inf.User.ID) + if err != nil { + api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("getting legacy current user: %w", err)) + return + } + api.WriteResp(w, r, cu) return } - - version := inf.Version - if version == nil { - api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, errors.New("TOUsers.Read called with invalid API version"), nil) + currentUser, err := getUser(inf.Tx.Tx, inf.User.ID) + if err != nil { + api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("getting current user: %w", err)) return } - if version.Major >= 4 { - api.WriteResp(w, r, currentUser) - } else { - legacyUser := currentUser.ToLegacyCurrentUser(role, localUser) - api.WriteResp(w, r, legacyUser) - } + api.WriteResp(w, r, currentUser) } -func getUser(tx *sql.Tx, id int) (tc.UserV4, int, bool, error) { +func getUser(tx *sql.Tx, id int) (tc.UserV4, error) { q := ` SELECT u.address_line1, u.address_line2, +( + SELECT count(l.tm_user) + FROM log AS l + WHERE l.tm_user = u.id +), u.city, u.company, u.country, u.email, u.full_name, +u.gid, u.id, -u.last_updated, u.last_authenticated, -u.local_passwd, +u.last_updated, u.new_user, u.phone_number, u.postal_code, u.public_ssh_key, r.name as "role", -u.role as role_id, u.state_or_province, t.name as tenant, u.tenant_id, +u.ucdn, +u.uid, u.username FROM tm_user as u LEFT JOIN role as r ON r.id = u.role @@ -155,17 +235,100 @@ INNER JOIN tenant as t ON t.id = u.tenant_id WHERE u.id=$1 ` var u tc.UserV4 - var localPassword sql.NullString - var role int - err := tx.QueryRow(q, id).Scan(&u.AddressLine1, &u.AddressLine2, &u.City, &u.Company, &u.Country, &u.Email, &u.FullName, &u.ID, &u.LastUpdated, &u.LastAuthenticated, &localPassword, &u.NewUser, &u.PhoneNumber, &u.PostalCode, &u.PublicSSHKey, &role, &u.Role, &u.StateOrProvince, &u.Tenant, &u.TenantID, &u.Username) + err := tx.QueryRow(q, id).Scan( + &u.AddressLine1, + &u.AddressLine2, + &u.ChangeLogCount, + &u.City, + &u.Company, + &u.Country, + &u.Email, + &u.FullName, + &u.GID, + &u.ID, + &u.LastAuthenticated, + &u.LastUpdated, + &u.NewUser, + &u.PhoneNumber, + &u.PostalCode, + &u.PublicSSHKey, + &u.RegistrationSent, + &u.Role, + &u.StateOrProvince, + &u.Tenant, + &u.TenantID, + &u.UCDN, + &u.UID, + &u.Username, + ) if err != nil { err = fmt.Errorf("querying current user: %w", err) } - return u, role, localPassword.Valid, err + return u, err +} + +func getLegacyUser(tx *sql.Tx, id int) (tc.UserCurrent, error) { + q := ` +SELECT +u.address_line1, +u.address_line2, +u.city, +u.company, +u.country, +u.email, +u.full_name, +u.gid, +u.id, +u.last_updated, +u.local_passwd IS NOT NULL, +u.new_user, +u.phone_number, +u.postal_code, +u.public_ssh_key, +u.role as "role", +r.name as role_name, +u.state_or_province, +t.name as tenant, +u.tenant_id, +u.uid, +u.username +FROM tm_user as u +LEFT JOIN role as r ON r.id = u.role +INNER JOIN tenant as t ON t.id = u.tenant_id +WHERE u.id=$1 +` + var u tc.UserCurrent + err := tx.QueryRow(q, id).Scan( + &u.AddressLine1, + &u.AddressLine2, + &u.City, + &u.Company, + &u.Country, + &u.Email, + &u.FullName, + &u.GID, + &u.ID, + &u.LastUpdated, + &u.LocalUser, + &u.NewUser, + &u.PhoneNumber, + &u.PostalCode, + &u.PublicSSHKey, + &u.Role, + &u.RoleName, + &u.StateOrProvince, + &u.Tenant, + &u.TenantID, + &u.UID, + &u.UserName, + ) + if err != nil { + err = fmt.Errorf("querying legacy current user: %w", err) + } + return u, err } func ReplaceCurrent(w http.ResponseWriter, r *http.Request) { - var useV4User bool inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil) tx := inf.Tx.Tx if userErr != nil || sysErr != nil { @@ -177,7 +340,7 @@ func ReplaceCurrent(w http.ResponseWriter, r *http.Request) { var userRequest tc.CurrentUserUpdateRequest if err := json.NewDecoder(r.Body).Decode(&userRequest); err != nil { errCode = http.StatusBadRequest - userErr = fmt.Errorf("couldn't parse request: %v", err) + userErr = fmt.Errorf("couldn't parse request: %w", err) api.HandleErr(w, r, tx, errCode, userErr, nil) return } @@ -189,12 +352,9 @@ func ReplaceCurrent(w http.ResponseWriter, r *http.Request) { return } - if inf.Version.Major >= 4 { - useV4User = true - } user, exists, err := dbhelpers.GetUserByID(inf.User.ID, tx) if err != nil { - sysErr = fmt.Errorf("getting user by ID %d: %v", inf.User.ID, err) + sysErr = fmt.Errorf("getting user by ID %d: %w", inf.User.ID, err) errCode = http.StatusInternalServerError api.HandleErr(w, r, tx, errCode, nil, sysErr) return @@ -206,9 +366,9 @@ func ReplaceCurrent(w http.ResponseWriter, r *http.Request) { return } - if err := userRequest.User.UnmarshalAndValidate(&user, useV4User); err != nil { + if err := userRequest.User.UnmarshalAndValidate(&user); err != nil { errCode = http.StatusBadRequest - userErr = fmt.Errorf("couldn't parse request: %v", err) + userErr = fmt.Errorf("couldn't parse request: %w", err) api.HandleErr(w, r, tx, errCode, userErr, nil) return } @@ -224,7 +384,7 @@ func ReplaceCurrent(w http.ResponseWriter, r *http.Request) { if err != nil { userErr = err } else { - userErr = fmt.Errorf("Unacceptable password") + userErr = errors.New("unacceptable password") } api.HandleErr(w, r, tx, errCode, userErr, nil) return @@ -232,7 +392,7 @@ func ReplaceCurrent(w http.ResponseWriter, r *http.Request) { hashPass, err := auth.DerivePassword(*user.LocalPassword) if err != nil { - sysErr = fmt.Errorf("Hashing new password: %v", err) + sysErr = fmt.Errorf("Hashing new password: %w", err) errCode = http.StatusInternalServerError api.HandleErr(w, r, tx, errCode, nil, sysErr) return @@ -245,7 +405,7 @@ func ReplaceCurrent(w http.ResponseWriter, r *http.Request) { if user.ConfirmLocalPassword != nil && *user.ConfirmLocalPassword != "" { hashPass, err := auth.DerivePassword(*user.ConfirmLocalPassword) if err != nil { - sysErr = fmt.Errorf("Hashing new 'confirm' password: %v", err) + sysErr = fmt.Errorf("hashing new 'confirm' password: %w", err) errCode = http.StatusInternalServerError api.HandleErr(w, r, tx, errCode, nil, sysErr) return @@ -254,10 +414,10 @@ func ReplaceCurrent(w http.ResponseWriter, r *http.Request) { changeConfirmPasswd = true } - if *user.Role != inf.User.Role && !useV4User { + if *user.Role != inf.User.Role { privLevel, exists, err := dbhelpers.GetPrivLevelFromRoleID(tx, *user.Role) if err != nil { - sysErr = fmt.Errorf("Getting privLevel for Role #%d: %v", *user.Role, err) + sysErr = fmt.Errorf("getting privLevel for Role #%d: %w", *user.Role, err) errCode = http.StatusInternalServerError api.HandleErr(w, r, tx, errCode, nil, sysErr) return @@ -289,44 +449,157 @@ func ReplaceCurrent(w http.ResponseWriter, r *http.Request) { } else if !ok { // unlike Perl, this endpoint will not disclose the existence of tenants over which the current // user has no permission - in keeping with the behavior of the '/tenants' endpoint. - userErr = errors.New("No such tenant!") + userErr = errors.New("no such tenant") errCode = http.StatusNotFound api.HandleErr(w, r, tx, errCode, userErr, sysErr) return } if *user.Username != inf.User.UserName { - if ok, err := dbhelpers.UsernameExists(*user.Username, tx); err != nil { - sysErr = fmt.Errorf("Checking existence of user %s: %v", *user.Username, err) + sysErr = fmt.Errorf("Checking existence of user %s: %w", *user.Username, err) errCode = http.StatusInternalServerError api.HandleErr(w, r, tx, errCode, nil, sysErr) return } else if ok { // TODO users are tenanted, so theoretically I should be hiding the existence of the // conflicting user - but then how do I tell the client how to fix their request? - userErr = fmt.Errorf("Username %s already exists!", *user.Username) + userErr = fmt.Errorf("username %s already exists", *user.Username) errCode = http.StatusConflict api.HandleErr(w, r, tx, errCode, userErr, nil) return } } - if err = updateUser(&user, tx, changePasswd, changeConfirmPasswd); err != nil { + if err = updateLegacyUser(&user, tx, changePasswd, changeConfirmPasswd); err != nil { errCode = http.StatusInternalServerError - sysErr = fmt.Errorf("updating user: %v", err) + sysErr = fmt.Errorf("updating user: %w", err) api.HandleErr(w, r, tx, errCode, nil, sysErr) return } - if useV4User { - api.WriteRespAlertObj(w, r, tc.SuccessLevel, "User profile was successfully updated", user.Upgrade()) - } else { - api.WriteRespAlertObj(w, r, tc.SuccessLevel, "User profile was successfully updated", user) + api.WriteRespAlertObj(w, r, tc.SuccessLevel, "User profile was successfully updated", user) +} + +func validateV4(user tc.UserV4, inf *api.APIInfo) (error, error) { + validateErrs := validation.Errors{ + "email": validation.Validate(user.Email, validation.Required, is.Email), + "fullName": validation.Validate(user.FullName, validation.Required), + "role": validation.Validate(user.Role, validation.Required), + "username": validation.Validate(user.Username, validation.Required), + "tenantID": validation.Validate(user.TenantID, validation.Required), + } + + // Password is not required for update + if user.LocalPassword != nil { + ok, err := auth.IsGoodLoginPair(user.Username, *user.LocalPassword) + if err != nil { + return err, nil + } + if !ok { + return errors.New("unacceptable password"), nil + } + } + + if err := tovalidate.ToError(validateErrs); err != nil { + return err, nil + } + + caps, err := dbhelpers.GetCapabilitiesFromRoleName(inf.Tx.Tx, user.Role) + if err != nil { + return nil, fmt.Errorf("getting capabilities for user's requested Role (%s): %w", user.Role, err) + } + + missing := inf.User.MissingPermissions(caps...) + if len(missing) > 0 { + return nil, fmt.Errorf("cannot request more than assigned permissions, current user needs %s permissions", strings.Join(missing, ",")) + } + + if user.Username != inf.User.UserName { + if ok, err := dbhelpers.UsernameExists(user.Username, inf.Tx.Tx); err != nil { + return nil, fmt.Errorf("Checking existence of user %s: %w", user.Username, err) + } else if ok { + return fmt.Errorf("username %s already exists", user.Username), nil + } + } + + return nil, nil +} + +// ReplaceCurrentV4 replaces the current user with the definition in the user's +// request (assuming it meets validation constraints). +func ReplaceCurrentV4(w http.ResponseWriter, r *http.Request) { + inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil) + tx := inf.Tx.Tx + if userErr != nil || sysErr != nil { + api.HandleErr(w, r, tx, errCode, userErr, sysErr) + return + } + defer inf.Close() + + var user tc.UserV4 + if err := json.NewDecoder(r.Body).Decode(&user); err != nil { + errCode = http.StatusBadRequest + userErr = fmt.Errorf("couldn't parse request: %w", err) + api.HandleErr(w, r, tx, errCode, userErr, nil) + return + } + // Token must never be updated this way + user.Token = nil + + userErr, sysErr = validateV4(user, inf) + if userErr != nil || sysErr != nil { + errCode = http.StatusBadRequest + if sysErr != nil { + errCode = http.StatusInternalServerError + } + api.HandleErr(w, r, tx, errCode, userErr, sysErr) + return + } + + changePasswd := false + + // obfuscate password + if user.LocalPassword != nil { + hashPass, err := auth.DerivePassword(*user.LocalPassword) + if err != nil { + sysErr = fmt.Errorf("hashing new password for user %s (#%d): %w", inf.User.UserName, inf.User.ID, err) + errCode = http.StatusInternalServerError + api.HandleErr(w, r, tx, errCode, nil, sysErr) + return + } + changePasswd = true + *user.LocalPassword = hashPass + } + + if ok, err := tenant.IsResourceAuthorizedToUserTx(user.TenantID, inf.User, tx); err != nil { + if errors.Is(err, sql.ErrNoRows) { + userErr = fmt.Errorf("no such tenant: #%d", user.TenantID) + errCode = http.StatusNotFound + } else { + sysErr = fmt.Errorf("checking user %s permissions on tenant #%d: %w", inf.User.UserName, user.TenantID, err) + errCode = http.StatusInternalServerError + } + api.HandleErr(w, r, tx, errCode, userErr, sysErr) + return + } else if !ok { + userErr = fmt.Errorf("no such tenant: #%d", user.TenantID) + errCode = http.StatusNotFound + api.HandleErr(w, r, tx, errCode, userErr, sysErr) + return + } + + if err := updateUser(&user, tx, changePasswd); err != nil { + errCode = http.StatusInternalServerError + sysErr = fmt.Errorf("updating user: %v", err) + api.HandleErr(w, r, tx, errCode, nil, sysErr) + return } + + api.WriteRespAlertObj(w, r, tc.SuccessLevel, "User profile was successfully updated", user) } -func updateUser(u *tc.User, tx *sql.Tx, changePassword bool, changeConfirmPasswd bool) error { +func updateLegacyUser(u *tc.User, tx *sql.Tx, changePassword bool, changeConfirmPasswd bool) error { row := tx.QueryRow(replaceCurrentQuery, u.AddressLine1, u.AddressLine2, @@ -339,14 +612,15 @@ func updateUser(u *tc.User, tx *sql.Tx, changePassword bool, changeConfirmPasswd u.PhoneNumber, u.PostalCode, u.PublicSSHKey, + u.Role, u.StateOrProvince, u.TenantID, u.UID, u.Username, u.ID, ) - - err := row.Scan(&u.AddressLine1, + err := row.Scan( + &u.AddressLine1, &u.AddressLine2, &u.City, &u.Company, @@ -390,3 +664,66 @@ func updateUser(u *tc.User, tx *sql.Tx, changePassword bool, changeConfirmPasswd u.ConfirmLocalPassword = nil return nil } + +func updateUser(u *tc.UserV4, tx *sql.Tx, changePassword bool) error { + row := tx.QueryRow(replaceCurrentV4Query, + u.AddressLine1, + u.AddressLine2, + u.City, + u.Company, + u.Country, + u.Email, + u.FullName, + u.GID, + u.PhoneNumber, + u.PostalCode, + u.PublicSSHKey, + u.Role, + u.StateOrProvince, + u.TenantID, + u.UCDN, + u.UID, + u.Username, + u.ID, + ) + + err := row.Scan( + &u.AddressLine1, + &u.AddressLine2, + &u.ChangeLogCount, + &u.City, + &u.Company, + &u.Country, + &u.Email, + &u.FullName, + &u.GID, + &u.ID, + &u.LastAuthenticated, + &u.LastUpdated, + &u.NewUser, + &u.PhoneNumber, + &u.PostalCode, + &u.PublicSSHKey, + &u.RegistrationSent, + &u.Role, + &u.StateOrProvince, + &u.Tenant, + &u.TenantID, + &u.UCDN, + &u.UID, + &u.Username, + ) + if err != nil { + return err + } + + if changePassword { + _, err = tx.Exec(replacePasswordQuery, u.LocalPassword, u.ID) + if err != nil { + return fmt.Errorf("resetting password: %v", err) + } + } + + u.LocalPassword = nil + return nil +} From 52113dfe72ac05ab6e4aca7f8c728004407a805f Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Fri, 13 May 2022 12:46:15 -0600 Subject: [PATCH 04/14] Fix ToError never returning a nil error --- lib/go-tc/tovalidate/validate.go | 7 +++++++ lib/go-tc/tovalidate/validate_test.go | 28 +++++++++++++++++++++++++-- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/lib/go-tc/tovalidate/validate.go b/lib/go-tc/tovalidate/validate.go index 50daba18e9..eb721c6920 100644 --- a/lib/go-tc/tovalidate/validate.go +++ b/lib/go-tc/tovalidate/validate.go @@ -51,6 +51,9 @@ func ToErrors(err map[string]error) []error { // Because multiple errors are collapsed, errors cannot be wrapped and therefore // error identity cannot be preserved. func ToError(err map[string]error) error { + if len(err) == 0 { + return nil + } var b strings.Builder for key, value := range err { if value != nil { @@ -61,5 +64,9 @@ func ToError(err map[string]error) error { b.WriteString(", ") } } + msg := strings.TrimSuffix(b.String(), ", ") + if msg == "" { + return nil + } return errors.New(strings.TrimSuffix(b.String(), ", ")) } diff --git a/lib/go-tc/tovalidate/validate_test.go b/lib/go-tc/tovalidate/validate_test.go index fcced2174e..a646383253 100644 --- a/lib/go-tc/tovalidate/validate_test.go +++ b/lib/go-tc/tovalidate/validate_test.go @@ -15,6 +15,7 @@ package tovalidate import ( "errors" "fmt" + "testing" ) func ExampleToError() { @@ -22,6 +23,29 @@ func ExampleToError() { "propA": errors.New("bad value"), "propB": errors.New("cannot be blank"), } - fmt.Println(ToError(errs)) - // Output: 'propA' bad value, 'propB' cannot be blank + err := ToError(errs).Error() + // Iteration order of Go maps is random, so this is the best we can do. + fmt.Println( + err == "'propA' bad value, 'propB' cannot be blank" || + err == "'propB' cannot be blank, 'propA' bad value", + ) + // Output: true +} + +func TestToError(t *testing.T) { + var errs map[string]error + err := ToError(errs) + if err != nil { + t.Error("a nil error map should yield a nil error, got:", err) + } + errs = map[string]error{} + err = ToError(errs) + if err != nil { + t.Error("an empty error map should yield a nil error, got:", err) + } + errs["something"] = nil + err = ToError(errs) + if err != nil { + t.Error("an error map with no non-nil errors should yield a nil error, got:", err) + } } From d69b43936edec2e2521055bed5b7918f3e6d18e5 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Fri, 13 May 2022 14:05:20 -0600 Subject: [PATCH 05/14] Fix unable to update user through APIv4 --- .../traffic_ops_golang/user/current.go | 38 ++++++++++++------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/traffic_ops/traffic_ops_golang/user/current.go b/traffic_ops/traffic_ops_golang/user/current.go index b6c2ef9e70..1b499200f2 100644 --- a/traffic_ops/traffic_ops_golang/user/current.go +++ b/traffic_ops/traffic_ops_golang/user/current.go @@ -74,12 +74,13 @@ SET phone_number=$9, postal_code=$10, public_ssh_key=$11, - state_or_province=$12, - tenant_id=$13, + role=$12, + state_or_province=$13, + tenant_id=$14, token=NULL, - uid=$14, - username=$15 -WHERE id=$16 + uid=$15, + username=$16 +WHERE id=$17 RETURNING address_line1, address_line2, @@ -100,13 +101,13 @@ RETURNING SELECT role.name FROM role WHERE role.id=tm_user.role - ) AS role_name, + ), state_or_province, ( SELECT tenant.name FROM tenant WHERE tenant.id=tm_user.tenant_id - ) AS tenant, + ), tenant_id, uid, username @@ -127,20 +128,25 @@ SET phone_number=$9, postal_code=$10, public_ssh_key=$11, - state_or_province=$12, - tenant_id=$13, + role=( + SELECT role.id + FROM role + WHERE name=$12 + ), + state_or_province=$13, + tenant_id=$14, token=NULL, - ucdn=$14, - uid=$15, - username=$16 -WHERE id=$17 + ucdn=$15, + uid=$16, + username=$17 +WHERE id=$18 RETURNING address_line1, address_line2, ( SELECT count(l.tm_user) FROM log AS l - WHERE l.tm_user = u.id + WHERE l.tm_user = tm_user.id ), city, company, @@ -222,6 +228,7 @@ u.new_user, u.phone_number, u.postal_code, u.public_ssh_key, +u.registration_sent, r.name as "role", u.state_or_province, t.name as tenant, @@ -547,6 +554,9 @@ func ReplaceCurrentV4(w http.ResponseWriter, r *http.Request) { // Token must never be updated this way user.Token = nil + user.ID = new(int) + *user.ID = inf.User.ID + userErr, sysErr = validateV4(user, inf) if userErr != nil || sysErr != nil { errCode = http.StatusBadRequest From b42e28869081e6626cc546d383021e6e75e1e619 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Fri, 13 May 2022 14:06:23 -0600 Subject: [PATCH 06/14] Fix APIv4/Go client integration test compilation errors --- traffic_ops/testing/api/v4/cdns_test.go | 9 ++++----- traffic_ops/testing/api/v4/crconfig_test.go | 9 ++++----- traffic_ops/testing/api/v4/parameters_test.go | 9 ++++----- traffic_ops/testing/api/v4/profile_parameters_test.go | 9 ++++----- traffic_ops/testing/api/v4/profiles_test.go | 9 ++++----- traffic_ops/testing/api/v4/servers_test.go | 9 ++++----- traffic_ops/testing/api/v4/staticdnsentries_test.go | 9 ++++----- traffic_ops/testing/api/v4/topologies_test.go | 9 ++++----- 8 files changed, 32 insertions(+), 40 deletions(-) diff --git a/traffic_ops/testing/api/v4/cdns_test.go b/traffic_ops/testing/api/v4/cdns_test.go index c7a154366a..c346b5d516 100644 --- a/traffic_ops/testing/api/v4/cdns_test.go +++ b/traffic_ops/testing/api/v4/cdns_test.go @@ -64,11 +64,10 @@ func TestCDNs(t *testing.T) { func UpdateDeleteCDNWithLocks(t *testing.T) { // Create a new user with operations level privileges user1 := tc.UserV4{ - Username: "lock_user1", - RegistrationSent: new(time.Time), - LocalPassword: util.StrPtr("test_pa$$word"), - ConfirmLocalPassword: util.StrPtr("test_pa$$word"), - Role: "operations", + Username: "lock_user1", + RegistrationSent: new(time.Time), + LocalPassword: util.StrPtr("test_pa$$word"), + Role: "operations", } user1.Email = util.StrPtr("lockuseremail@domain.com") user1.TenantID = 1 diff --git a/traffic_ops/testing/api/v4/crconfig_test.go b/traffic_ops/testing/api/v4/crconfig_test.go index aac5ab4ba0..df99d7535a 100644 --- a/traffic_ops/testing/api/v4/crconfig_test.go +++ b/traffic_ops/testing/api/v4/crconfig_test.go @@ -57,11 +57,10 @@ func SnapshotWithReadOnlyUser(t *testing.T) { toReqTimeout := time.Second * time.Duration(Config.Default.Session.TimeoutInSecs) user := tc.UserV4{ - Username: "test_user_tm", - RegistrationSent: new(time.Time), - LocalPassword: util.StrPtr("test_pa$$word"), - ConfirmLocalPassword: util.StrPtr("test_pa$$word"), - Role: "read-only", + Username: "test_user_tm", + RegistrationSent: new(time.Time), + LocalPassword: util.StrPtr("test_pa$$word"), + Role: "read-only", } user.Email = util.StrPtr("email_tm@domain.com") user.TenantID = resp.Response[0].ID diff --git a/traffic_ops/testing/api/v4/parameters_test.go b/traffic_ops/testing/api/v4/parameters_test.go index 92d8d76202..3ebec7ef35 100644 --- a/traffic_ops/testing/api/v4/parameters_test.go +++ b/traffic_ops/testing/api/v4/parameters_test.go @@ -104,11 +104,10 @@ func GetTestSecureParameter(t *testing.T) { // Create a new user with operations level privileges user1 := tc.UserV4{ - Username: "lock_user1", - RegistrationSent: new(time.Time), - LocalPassword: util.StrPtr("test_pa$$word"), - ConfirmLocalPassword: util.StrPtr("test_pa$$word"), - Role: "operations", + Username: "lock_user1", + RegistrationSent: new(time.Time), + LocalPassword: util.StrPtr("test_pa$$word"), + Role: "operations", } user1.Email = util.StrPtr("lockuseremail@domain.com") user1.TenantID = tenantResp.Response[0].ID diff --git a/traffic_ops/testing/api/v4/profile_parameters_test.go b/traffic_ops/testing/api/v4/profile_parameters_test.go index 17e5a885a4..f9999e9d67 100644 --- a/traffic_ops/testing/api/v4/profile_parameters_test.go +++ b/traffic_ops/testing/api/v4/profile_parameters_test.go @@ -46,11 +46,10 @@ func TestProfileParameters(t *testing.T) { func CreateDeleteProfileParameterWithLocks(t *testing.T) { // Create a new user with operations level privileges user1 := tc.UserV4{ - Username: "lock_user1", - RegistrationSent: new(time.Time), - LocalPassword: util.StrPtr("test_pa$$word"), - ConfirmLocalPassword: util.StrPtr("test_pa$$word"), - Role: "operations", + Username: "lock_user1", + RegistrationSent: new(time.Time), + LocalPassword: util.StrPtr("test_pa$$word"), + Role: "operations", } user1.Email = util.StrPtr("lockuseremail@domain.com") user1.TenantID = 1 diff --git a/traffic_ops/testing/api/v4/profiles_test.go b/traffic_ops/testing/api/v4/profiles_test.go index 091c1bc240..c0383e650c 100644 --- a/traffic_ops/testing/api/v4/profiles_test.go +++ b/traffic_ops/testing/api/v4/profiles_test.go @@ -65,11 +65,10 @@ func CUDProfileWithLocks(t *testing.T) { // Create a new user with operations level privileges user1 := tc.UserV4{ - Username: "lock_user1", - RegistrationSent: new(time.Time), - LocalPassword: util.StrPtr("test_pa$$word"), - ConfirmLocalPassword: util.StrPtr("test_pa$$word"), - Role: "operations", + Username: "lock_user1", + RegistrationSent: new(time.Time), + LocalPassword: util.StrPtr("test_pa$$word"), + Role: "operations", } user1.Email = util.StrPtr("lockuseremail@domain.com") user1.TenantID = resp.Response[0].ID diff --git a/traffic_ops/testing/api/v4/servers_test.go b/traffic_ops/testing/api/v4/servers_test.go index af9adf9dd5..34b7694add 100644 --- a/traffic_ops/testing/api/v4/servers_test.go +++ b/traffic_ops/testing/api/v4/servers_test.go @@ -71,11 +71,10 @@ func CUDServerWithLocks(t *testing.T) { // Create a new user with operations level privileges user1 := tc.UserV4{ - Username: "lock_user1", - RegistrationSent: new(time.Time), - LocalPassword: util.StrPtr("test_pa$$word"), - ConfirmLocalPassword: util.StrPtr("test_pa$$word"), - Role: "operations", + Username: "lock_user1", + RegistrationSent: new(time.Time), + LocalPassword: util.StrPtr("test_pa$$word"), + Role: "operations", } user1.Email = util.StrPtr("lockuseremail@domain.com") user1.TenantID = resp.Response[0].ID diff --git a/traffic_ops/testing/api/v4/staticdnsentries_test.go b/traffic_ops/testing/api/v4/staticdnsentries_test.go index 266bf76030..71a4d56528 100644 --- a/traffic_ops/testing/api/v4/staticdnsentries_test.go +++ b/traffic_ops/testing/api/v4/staticdnsentries_test.go @@ -55,11 +55,10 @@ func TestStaticDNSEntries(t *testing.T) { func CreateUpdateDeleteStaticDNSEntriesWithLocks(t *testing.T) { // Create a new user with operations level privileges user1 := tc.UserV4{ - Username: "lock_user1", - RegistrationSent: new(time.Time), - LocalPassword: util.StrPtr("test_pa$$word"), - ConfirmLocalPassword: util.StrPtr("test_pa$$word"), - Role: "operations", + Username: "lock_user1", + RegistrationSent: new(time.Time), + LocalPassword: util.StrPtr("test_pa$$word"), + Role: "operations", } user1.Email = util.StrPtr("lockuseremail@domain.com") user1.TenantID = 1 diff --git a/traffic_ops/testing/api/v4/topologies_test.go b/traffic_ops/testing/api/v4/topologies_test.go index b8a5b94b26..bef21bfdce 100644 --- a/traffic_ops/testing/api/v4/topologies_test.go +++ b/traffic_ops/testing/api/v4/topologies_test.go @@ -992,11 +992,10 @@ func CRUDTopologyReadOnlyUser(t *testing.T) { toReqTimeout := time.Second * time.Duration(Config.Default.Session.TimeoutInSecs) user := tc.UserV4{ - Username: "test_user", - RegistrationSent: new(time.Time), - LocalPassword: util.StrPtr("test_pa$$word"), - ConfirmLocalPassword: util.StrPtr("test_pa$$word"), - Role: "read-only", + Username: "test_user", + RegistrationSent: new(time.Time), + LocalPassword: util.StrPtr("test_pa$$word"), + Role: "read-only", } user.Email = util.StrPtr("email@domain.com") user.TenantID = resp.Response[0].ID From 2aea0abbdd58f3e8b8e805526fc6a58b38439971 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Fri, 13 May 2022 14:06:39 -0600 Subject: [PATCH 07/14] Better error handling --- .../traffic_ops_golang/user/current.go | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/traffic_ops/traffic_ops_golang/user/current.go b/traffic_ops/traffic_ops_golang/user/current.go index 1b499200f2..87860fef9d 100644 --- a/traffic_ops/traffic_ops_golang/user/current.go +++ b/traffic_ops/traffic_ops_golang/user/current.go @@ -399,7 +399,7 @@ func ReplaceCurrent(w http.ResponseWriter, r *http.Request) { hashPass, err := auth.DerivePassword(*user.LocalPassword) if err != nil { - sysErr = fmt.Errorf("Hashing new password: %w", err) + sysErr = fmt.Errorf("hashing new password: %w", err) errCode = http.StatusInternalServerError api.HandleErr(w, r, tx, errCode, nil, sysErr) return @@ -444,11 +444,11 @@ func ReplaceCurrent(w http.ResponseWriter, r *http.Request) { } if ok, err := tenant.IsResourceAuthorizedToUserTx(*user.TenantID, inf.User, tx); err != nil { - if err == sql.ErrNoRows { - userErr = errors.New("No such tenant!") + if errors.Is(err, sql.ErrNoRows) { + userErr = errors.New("no such tenant") errCode = http.StatusNotFound } else { - sysErr = fmt.Errorf("Checking user %s permissions on tenant #%d: %v", inf.User.UserName, *user.TenantID, err) + sysErr = fmt.Errorf("checking user %s permissions on tenant #%d: %w", inf.User.UserName, *user.TenantID, err) errCode = http.StatusInternalServerError } api.HandleErr(w, r, tx, errCode, userErr, sysErr) @@ -464,7 +464,7 @@ func ReplaceCurrent(w http.ResponseWriter, r *http.Request) { if *user.Username != inf.User.UserName { if ok, err := dbhelpers.UsernameExists(*user.Username, tx); err != nil { - sysErr = fmt.Errorf("Checking existence of user %s: %w", *user.Username, err) + sysErr = fmt.Errorf("checking existence of user %s: %w", *user.Username, err) errCode = http.StatusInternalServerError api.HandleErr(w, r, tx, errCode, nil, sysErr) return @@ -480,7 +480,7 @@ func ReplaceCurrent(w http.ResponseWriter, r *http.Request) { if err = updateLegacyUser(&user, tx, changePasswd, changeConfirmPasswd); err != nil { errCode = http.StatusInternalServerError - sysErr = fmt.Errorf("updating user: %w", err) + sysErr = fmt.Errorf("updating legacy user: %w", err) api.HandleErr(w, r, tx, errCode, nil, sysErr) return } @@ -524,7 +524,7 @@ func validateV4(user tc.UserV4, inf *api.APIInfo) (error, error) { if user.Username != inf.User.UserName { if ok, err := dbhelpers.UsernameExists(user.Username, inf.Tx.Tx); err != nil { - return nil, fmt.Errorf("Checking existence of user %s: %w", user.Username, err) + return nil, fmt.Errorf("checking existence of user %s: %w", user.Username, err) } else if ok { return fmt.Errorf("username %s already exists", user.Username), nil } @@ -601,7 +601,7 @@ func ReplaceCurrentV4(w http.ResponseWriter, r *http.Request) { if err := updateUser(&user, tx, changePasswd); err != nil { errCode = http.StatusInternalServerError - sysErr = fmt.Errorf("updating user: %v", err) + sysErr = fmt.Errorf("updating user: %w", err) api.HandleErr(w, r, tx, errCode, nil, sysErr) return } @@ -659,14 +659,14 @@ func updateLegacyUser(u *tc.User, tx *sql.Tx, changePassword bool, changeConfirm if changePassword { _, err = tx.Exec(replacePasswordQuery, u.LocalPassword, u.ID) if err != nil { - return fmt.Errorf("resetting password: %v", err) + return fmt.Errorf("resetting password: %w", err) } } if changeConfirmPasswd { _, err = tx.Exec(replaceConfirmPasswordQuery, u.ConfirmLocalPassword, u.ID) if err != nil { - return fmt.Errorf("resetting confirm password: %v", err) + return fmt.Errorf("resetting confirm password: %w", err) } } @@ -730,7 +730,7 @@ func updateUser(u *tc.UserV4, tx *sql.Tx, changePassword bool) error { if changePassword { _, err = tx.Exec(replacePasswordQuery, u.LocalPassword, u.ID) if err != nil { - return fmt.Errorf("resetting password: %v", err) + return fmt.Errorf("resetting password: %w", err) } } From 4591093baaefbcb41fa548d54c2e1a5ea24c4648 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Fri, 13 May 2022 14:07:44 -0600 Subject: [PATCH 08/14] Fix Go APIv4 client using incorrect body structure for PUT /user/current --- traffic_ops/v4-client/user.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/traffic_ops/v4-client/user.go b/traffic_ops/v4-client/user.go index eb6edaf61e..38b46be17c 100644 --- a/traffic_ops/v4-client/user.go +++ b/traffic_ops/v4-client/user.go @@ -45,11 +45,8 @@ func (to *Session) GetUserCurrent(opts RequestOptions) (UserCurrentResponseV4, t // UpdateCurrentUser replaces the current user data with the provided tc.UserV4 structure. func (to *Session) UpdateCurrentUser(u tc.UserV4, opts RequestOptions) (tc.UpdateUserResponseV4, toclientlib.ReqInf, error) { - user := struct { - User tc.UserV4 `json:"user"` - }{u} var clientResp tc.UpdateUserResponseV4 - reqInf, err := to.put("/user/current", opts, user, &clientResp) + reqInf, err := to.put("/user/current", opts, u, &clientResp) return clientResp, reqInf, err } From 9dc4c6ed90b0100c960d8d1c8de174a94ba4e196 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Fri, 13 May 2022 16:33:24 -0600 Subject: [PATCH 09/14] Fix `changeLogCount` null in POST responses --- lib/go-tc/users.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/go-tc/users.go b/lib/go-tc/users.go index e7fc6ff889..dd4ff5c4e1 100644 --- a/lib/go-tc/users.go +++ b/lib/go-tc/users.go @@ -245,7 +245,7 @@ type UserV4 UserV40 type UserV40 struct { AddressLine1 *string `json:"addressLine1" db:"address_line1"` AddressLine2 *string `json:"addressLine2" db:"address_line2"` - ChangeLogCount *int `json:"changeLogCount" db:"change_log_count"` + ChangeLogCount int `json:"changeLogCount" db:"change_log_count"` City *string `json:"city" db:"city"` Company *string `json:"company" db:"company"` Country *string `json:"country" db:"country"` @@ -322,7 +322,7 @@ type CurrentUserUpdateRequestUser struct { // Upgrade converts an APIv3 and earlier "current user" to an APIv4 User. // Fields not present in earlier API versions need to be passed explicitly -func (u UserCurrent) Upgrade(registrationSent, lastAuthenticated *time.Time, ucdn string, changeLogCount *int) UserV4 { +func (u UserCurrent) Upgrade(registrationSent, lastAuthenticated *time.Time, ucdn string, changeLogCount int) UserV4 { var ret UserV4 ret.AddressLine1 = copyStringIfNotNil(u.AddressLine1) ret.AddressLine2 = copyStringIfNotNil(u.AddressLine2) From d071d84994076083c8e09818500cb0c2307dd795 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Fri, 13 May 2022 16:52:19 -0600 Subject: [PATCH 10/14] Update documentation --- docs/source/api/v4/user_current.rst | 230 +++++++++++++++++----------- docs/source/api/v4/users.rst | 204 +++++++++++++----------- docs/source/api/v4/users_id.rst | 189 +++++++++++++---------- 3 files changed, 366 insertions(+), 257 deletions(-) diff --git a/docs/source/api/v4/user_current.rst b/docs/source/api/v4/user_current.rst index 3efe4a82f9..b1e8e16c85 100644 --- a/docs/source/api/v4/user_current.rst +++ b/docs/source/api/v4/user_current.rst @@ -34,19 +34,35 @@ Request Structure ----------------- No parameters available. +.. code-block:: http + :caption: Request Example + + GET /api/4.0/user/current HTTP/1.1 + User-Agent: python-requests/2.25.1 + Accept-Encoding: gzip, deflate + Accept: */* + Connection: keep-alive + Cookie: mojolicious=... + + Response Structure ------------------ -:addressLine1: The user's address - including street name and number -:addressLine2: An additional address field for e.g. apartment number -:city: The name of the city wherein the user resides -:company: The name of the company for which the user works -:country: The name of the country wherein the user resides -:email: The user's email address -:fullName: The user's full name, e.g. "John Quincy Adams" -:gid: A deprecated field only kept for legacy compatibility reasons that used to contain the UNIX group ID of the user +:addressLine1: The user's address - including street name and number +:addressLine2: An additional address field for e.g. apartment number +:changeLogCount: The number of change log entries created by the user +:city: The name of the city wherein the user resides +:company: The name of the company for which the user works +:country: The name of the country wherein the user resides +:email: The user's email address +:fullName: The user's full name, e.g. "John Quincy Adams" +:gid: A deprecated field only kept for legacy compatibility reasons that used to contain the UNIX group ID of the user + + .. deprecated:: 4.0 + This field is serves no known purpose, and shouldn't be used for anything so it can be removed in the future. + :id: An integral, unique identifier for this user -:lastAuthenticated: The date and time at which the user was last authenticated, in :rfc:`3339` -:lastUpdated: The date and time at which the user was last modified, in :ref:`non-rfc-datetime` +:lastAuthenticated: The date and time at which the user was last authenticated, in :rfc:`3339` format +:lastUpdated: The date and time at which the user was last modified, in :rfc:`3339` format :newUser: A meta field with no apparent purpose that is usually ``null`` unless explicitly set during creation or modification of a user via some API endpoint :phoneNumber: The user's phone number :postalCode: The postal code of the area in which the user resides @@ -56,47 +72,56 @@ Response Structure :stateOrProvince: The name of the state or province where this user resides :tenant: The name of the :term:`Tenant` to which this user belongs :tenantId: The integral, unique identifier of the :term:`Tenant` to which this user belongs -:uid: A deprecated field only kept for legacy compatibility reasons that used to contain the UNIX user ID of the user -:username: The user's username +:ucdn: The name of the :abbr:`uCDN (Upstream Content Delivery Network)` to which the user belongs + + .. versionadded:: 4.0 + +:uid: A deprecated field only kept for legacy compatibility reasons that used to contain the UNIX user ID of the user + + .. deprecated:: 4.0 + This field is serves no known purpose, and shouldn't be used for anything so it can be removed in the future. + +:username: The user's username .. code-block:: http :caption: Response Example + HTTP/1.1 200 OK - Access-Control-Allow-Credentials: true - Access-Control-Allow-Headers: Origin, X-Requested-With, Content-Type, Accept, Set-Cookie, Cookie - Access-Control-Allow-Methods: POST,GET,OPTIONS,PUT,DELETE - Access-Control-Allow-Origin: * + Content-Encoding: gzip Content-Type: application/json - Set-Cookie: mojolicious=...; Path=/; Expires=Mon, 18 Nov 2019 17:40:54 GMT; Max-Age=3600; HttpOnly - Whole-Content-Sha512: HQwu9FxFyinXSVFK5+wpEhSxU60KbqXuokFbMZ3OoerOoM5ZpWpglsHz7mRch8VAw0dzwsJzpPJivj07RiKaJg== + Permissions-Policy: interest-cohort=() + Set-Cookie: mojolicious=...; Path=/; Expires=Fri, 13 May 2022 23:42:05 GMT; Max-Age=3600; HttpOnly + Vary: Accept-Encoding X-Server-Name: traffic_ops_golang/ - Date: Thu, 13 Dec 2018 15:14:45 GMT - Content-Length: 631 + Date: Fri, 13 May 2022 22:42:05 GMT + Content-Length: 311 { "response": { - "username": "admin", - "localUser": true, - "addressLine1": "not a real address", - "addressLine2": "not a real address either", - "city": "not a real city", - "company": "not a real company", - "country": "not a real country", - "email": "not@real.email", - "fullName": "Not a real fullName", + "addressLine1": null, + "addressLine2": null, + "changeLogCount": 1, + "city": null, + "company": null, + "country": null, + "email": "admin@no-reply.atc.test", + "fullName": "Development Admin User", "gid": null, "id": 2, + "lastAuthenticated": "2022-05-13T22:42:05.495439Z", + "lastUpdated": "2022-05-13T22:42:05.495439Z", "newUser": false, - "phoneNumber": "not a real phone number", - "postalCode": "not a real postal code", - "publicSshKey": "not a real ssh key", + "phoneNumber": null, + "postalCode": null, + "publicSshKey": null, + "registrationSent": null, "role": "admin", - "stateOrProvince": "not a real state or province", + "stateOrProvince": null, "tenant": "root", "tenantId": 1, + "ucdn": "", "uid": null, - "lastUpdated": "2021-09-16T09:55:09.309863-06:00", - "lastAuthenticated": "2021-09-16T09:55:09.309863-06:00" + "username": "admin" }} ``PUT`` @@ -114,70 +139,95 @@ Updates the date for the authenticated user. Request Structure ----------------- -:user: The entire request must be inside a top-level "user" key for legacy reasons - - :addressLine1: The user's address - including street name and number - :addressLine2: An additional address field for e.g. apartment number - :city: The name of the city wherein the user resides - :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 - cannot be an empty string\ [#notnull]_. The given email is validated (circuitously) by `GitHub user asaskevich's regular expression `_ . Note that it can't actually distinguish a valid, deliverable, email address but merely ensure the email is in a commonly-found format. - :fullName: The user's full name, e.g. "John Quincy Adams" - :gid: A legacy field only kept for legacy compatibility reasons that used to contain the UNIX group ID of the user - please don't use this - :id: The user's integral, unique, identifier - this cannot be changed\ [#notnull]_ - :localPasswd: Optionally, the user's password. This should never be given if it will not be changed. An empty string or ``null`` can be used to explicitly specify no change. - :phoneNumber: The user's phone number - :postalCode: The user's postal code - :publicSshKey: The user's public encryption key used for the SSH protocol - :role: The integral, unique identifier of the highest permission :term:`Role` which will be permitted to the user - this cannot be altered from the user's current :term:`Role`\ [#notnull]_ - :stateOrProvince: The state or province in which the user resides - :tenantId: The integral, unique identifier of the :term:`Tenant` to which the new user shall belong\ [#tenancy]_\ [#notnull]_ - :uid: A legacy field only kept for legacy compatibility reasons that used to contain the UNIX user ID of the user - please don't use this - :username: The user's new username\ [#notnull]_ +:addressLine1: The user's address - including street name and number +:addressLine2: An additional address field for e.g. apartment number +:city: The name of the city wherein the user resides +:company: The name of the company for which the user works +:country: The name of the country wherein the user resides +:email: The user's email address - cannot be an empty string\ [#notnull]_. The given email is validated (circuitously) by `GitHub user asaskevich's regular expression `_ . Note that it can't actually distinguish a valid, deliverable, email address but merely ensure the email is in a commonly-found format. +:fullName: The user's full name, e.g. "John Quincy Adams" +:gid: A legacy field only kept for legacy compatibility reasons that used to contain the UNIX group ID of the user - please don't use this + + .. deprecated:: 4.0 + This field is serves no known purpose, and shouldn't be used for anything so it can be removed in the future. + +:id: The user's integral, unique, identifier - this cannot be changed\ [#notnull]_ +:localPasswd: Optionally, the user's password. This should never be given if it will not be changed. An empty string or ``null`` can be used to explicitly specify no change. +:phoneNumber: The user's phone number +:postalCode: The user's postal code +:publicSshKey: The user's public encryption key used for the SSH protocol +:role: The integral, unique identifier of the highest permission :term:`Role` which will be permitted to the user - this cannot be altered from the user's current :term:`Role`\ [#notnull]_ +:stateOrProvince: The state or province in which the user resides +:tenantId: The integral, unique identifier of the :term:`Tenant` to which the new user shall belong\ [#tenancy]_\ [#notnull]_ +:ucdn: The name of the :abbr:`uCDN (Upstream Content Delivery Network)` to which the user belongs + + .. versionadded:: 4.0 + +:uid: A legacy field only kept for legacy compatibility reasons that used to contain the UNIX user ID of the user - please don't use this + + .. deprecated:: 4.0 + This field is serves no known purpose, and shouldn't be used for anything so it can be removed in the future. + +:username: The user's new username\ [#notnull]_ + +.. versionchanged:: 4.0 + In all earlier versions of the API, all of these objects needed to be properties of the "user" property of the JSON object request body. .. code-block:: http :caption: Request Example PUT /api/4.0/user/current HTTP/1.1 - Host: trafficops.infra.ciab.test - User-Agent: curl/7.47.0 + User-Agent: python-requests/2.25.1 + Accept-Encoding: gzip, deflate Accept: */* + Connection: keep-alive Cookie: mojolicious=... - Content-Length: 469 - Content-Type: application/json + Content-Length: 562 - { "user": { + { "addressLine1": null, "addressLine2": null, + "changeLogCount": 1, "city": null, "company": null, "country": null, - "email": "admin@infra.trafficops.ciab.test", - "fullName": "admin", + "email": "admin@no-reply.atc.test", + "fullName": "Development Admin User", "gid": null, "id": 2, + "lastAuthenticated": "2022-05-13T22:42:05.495439Z", + "lastUpdated": "2022-05-13T22:42:05.495439Z", + "newUser": false, "phoneNumber": null, "postalCode": null, "publicSshKey": null, + "registrationSent": null, "role": "admin", "stateOrProvince": null, + "tenant": "root", "tenantId": 1, + "ucdn": "", "uid": null, "username": "admin" - }} + } Response Structure ------------------ -:addressLine1: The user's address - including street name and number -:addressLine2: An additional address field for e.g. apartment number -:city: The name of the city wherein the user resides -:company: The name of the company for which the user works -:country: The name of the country wherein the user resides -:email: The user's email address validated (circuitously) by `GitHub user asaskevich's regular expression `_ . Note that it can't actually distinguish a valid, deliverable, email address but merely ensure the email is in a commonly-found format. -:fullName: The user's full name, e.g. "John Quincy Adams" -:gid: A legacy field only kept for legacy compatibility reasons that used to contain the UNIX group ID of the user +:addressLine1: The user's address - including street name and number +:addressLine2: An additional address field for e.g. apartment number +:changeLogCount: The number of change log entries created by the user +:city: The name of the city wherein the user resides +:company: The name of the company for which the user works +:country: The name of the country wherein the user resides +:email: The user's email address validated (circuitously) by `GitHub user asaskevich's regular expression `_ . Note that it can't actually distinguish a valid, deliverable, email address but merely ensure the email is in a commonly-found format. +:fullName: The user's full name, e.g. "John Quincy Adams" +:gid: A legacy field only kept for legacy compatibility reasons that used to contain the UNIX group ID of the user + + .. deprecated:: 4.0 + This field is serves no known purpose, and shouldn't be used for anything so it can be removed in the future. + :id: An integral, unique identifier for this user +:lastAuthenticated: The date and time at which the user was last authenticated, in :rfc:`3339` :lastUpdated: The date and time at which the user was last modified, in :ref:`non-rfc-datetime` :newUser: A meta field with no apparent purpose :phoneNumber: The user's phone number @@ -188,24 +238,29 @@ Response Structure :stateOrProvince: The name of the state or province where this user resides :tenant: The name of the :term:`Tenant` to which this user belongs :tenantId: The integral, unique identifier of the :term:`Tenant` to which this user belongs -:uid: A legacy field only kept for legacy compatibility reasons that used to contain the UNIX user ID of the user -:username: The user's username +:ucdn: The name of the :abbr:`uCDN (Upstream Content Delivery Network)` to which the user belongs + + .. versionadded:: 4.0 + +:uid: A legacy field only kept for legacy compatibility reasons that used to contain the UNIX user ID of the user + + .. deprecated:: 4.0 + This field is serves no known purpose, and shouldn't be used for anything so it can be removed in the future. + +:username: The user's username .. code-block:: http :caption: Response Example HTTP/1.1 200 OK - Access-Control-Allow-Credentials: true - Access-Control-Allow-Headers: Origin, X-Requested-With, Content-Type, Accept, Set-Cookie, Cookie - Access-Control-Allow-Methods: POST,GET,OPTIONS,PUT,DELETE - Access-Control-Allow-Origin: * + Content-Encoding: gzip Content-Type: application/json - Date: Thu, 13 Dec 2018 21:05:49 GMT - X-Server-Name: traffic_ops_golang/ - Set-Cookie: mojolicious=...; Path=/; Expires=Mon, 18 Nov 2019 17:40:54 GMT; Max-Age=3600; HttpOnly + Permissions-Policy: interest-cohort=() + Set-Cookie: mojolicious=...; Path=/; Expires=Fri, 13 May 2022 23:45:22 GMT; Max-Age=3600; HttpOnly Vary: Accept-Encoding - Whole-Content-Sha512: sHFqZQ4Cv7IIWaIejoAvM2Fr/HSupcX3D16KU/etjw+4jcK9EME3Bq5ohLC+eQ52BDCKW2Ra+AC3TfFtworJww== - Content-Length: 462 + X-Server-Name: traffic_ops_golang/ + Date: Fri, 13 May 2022 22:45:22 GMT + Content-Length: 370 { "alerts": [ { @@ -216,14 +271,16 @@ Response Structure "response": { "addressLine1": null, "addressLine2": null, + "changeLogCount": 1, "city": null, "company": null, "country": null, - "email": "admin@infra.trafficops.ciab.test", - "fullName": null, + "email": "admin@no-reply.atc.test", + "fullName": "Development Admin User", "gid": null, "id": 2, - "lastUpdated": "2019-10-08 20:14:25+00", + "lastAuthenticated": "2022-05-13T22:44:55.973452Z", + "lastUpdated": "2022-05-13T22:45:22.505401Z", "newUser": false, "phoneNumber": null, "postalCode": null, @@ -233,6 +290,7 @@ Response Structure "stateOrProvince": null, "tenant": "root", "tenantId": 1, + "ucdn": "", "uid": null, "username": "admin" }} diff --git a/docs/source/api/v4/users.rst b/docs/source/api/v4/users.rst index a10335f41b..2410b21d79 100644 --- a/docs/source/api/v4/users.rst +++ b/docs/source/api/v4/users.rst @@ -24,7 +24,7 @@ Retrieves all requested users. :Auth. Required: Yes -:Roles Required: None\ [1]_ +:Roles Required: None\ [#tenancy]_ :Permissions Required: USER:READ :Response Type: Array @@ -61,23 +61,29 @@ Request Structure .. code-block:: http :caption: Request Example - GET /api/4.0/users?username=admin HTTP/1.1 - Host: trafficops.infra.ciab.test - User-Agent: curl/7.47.0 + GET /api/4.0/users?username=mike HTTP/1.1 + User-Agent: python-requests/2.25.1 + Accept-Encoding: gzip, deflate Accept: */* + Connection: keep-alive Cookie: mojolicious=... + Response Structure ------------------ -:addressLine1: The user's address - including street name and number -:addressLine2: An additional address field for e.g. apartment number -:changeLogCount: The number of change log entries created by the user -:city: The name of the city wherein the user resides -:company: The name of the company for which the user works -:country: The name of the country wherein the user resides -:email: The user's email address -:fullName: The user's full name, e.g. "John Quincy Adams" -:gid: A deprecated field only kept for legacy compatibility reasons that used to contain the UNIX group ID of the user - now it is always ``null`` +:addressLine1: The user's address - including street name and number +:addressLine2: An additional address field for e.g. apartment number +:changeLogCount: The number of change log entries created by the user +:city: The name of the city wherein the user resides +:company: The name of the company for which the user works +:country: The name of the country wherein the user resides +:email: The user's email address +:fullName: The user's full name, e.g. "John Quincy Adams" +:gid: A deprecated field only kept for legacy compatibility reasons that used to contain the UNIX group ID of the user - now it is always ``null`` + + .. deprecated:: 4.0 + This field is serves no known purpose, and shouldn't be used for anything so it can be removed in the future. + :id: An integral, unique identifier for this user :lastAuthenticated: The date and time at which the user was last authenticated, in :rfc:`3339` :lastUpdated: The date and time at which the user was last modified, in :ref:`non-rfc-datetime` @@ -91,54 +97,58 @@ Response Structure :tenant: The name of the tenant to which this user belongs :tenantId: The integral, unique identifier of the tenant to which this user belongs :ucdn: The name of the :abbr:`uCDN (Upstream Content Delivery Network)` to which the user belongs -:uid: A deprecated field only kept for legacy compatibility reasons that used to contain the UNIX user ID of the user - now it is always ``null`` -:username: The user's username + + .. versionadded:: 4.0 + +:uid: A deprecated field only kept for legacy compatibility reasons that used to contain the UNIX user ID of the user - now it is always ``null`` + + .. deprecated:: 4.0 + This field is serves no known purpose, and shouldn't be used for anything so it can be removed in the future. + +:username: The user's username .. code-block:: http :caption: Response Example HTTP/1.1 200 OK - Access-Control-Allow-Credentials: true - Access-Control-Allow-Headers: Origin, X-Requested-With, Content-Type, Accept, Set-Cookie, Cookie - Access-Control-Allow-Methods: POST,GET,OPTIONS,PUT,DELETE - Access-Control-Allow-Origin: * + Content-Encoding: gzip Content-Type: application/json - Set-Cookie: mojolicious=...; Path=/; Expires=Mon, 18 Nov 2019 17:40:54 GMT; Max-Age=3600; HttpOnly - Whole-Content-Sha512: YBJLN8NbOxOvECe1RGtcwCzIPDhyhLpW56nTJHQM5WI2WUDe2mAKREpaEE72nzrfBliq1GABwJlsxq2OdhcFkw== + Permissions-Policy: interest-cohort=() + Set-Cookie: mojolicious=...; Path=/; Expires=Fri, 13 May 2022 23:16:14 GMT; Max-Age=3600; HttpOnly + Vary: Accept-Encoding X-Server-Name: traffic_ops_golang/ - Date: Thu, 13 Dec 2018 01:03:53 GMT - Content-Length: 454 + Date: Fri, 13 May 2022 22:16:14 GMT + Content-Length: 350 { "response": [ { - "username": "admin", - "registrationSent": null, - "addressLine1": null, + "addressLine1": "22 Mike Wazowski You've Got Your Life Back Lane", "addressLine2": null, - "city": null, + "changeLogCount": 0, + "city": "Monstropolis", "company": null, "country": null, - "email": null, - "fullName": null, + "email": "mwazowski@minc.biz", + "fullName": "Mike Wazowski", "gid": null, - "id": 2, - "newUser": false, + "id": 3, + "lastAuthenticated": null, + "lastUpdated": "2022-05-13T22:13:54.605052Z", + "newUser": true, "phoneNumber": null, "postalCode": null, "publicSshKey": null, + "registrationSent": null, "role": "admin", "stateOrProvince": null, "tenant": "root", "tenantId": 1, + "ucdn": "", "uid": null, - "lastUpdated": "2021-08-25T14:08:13.974447-06:00", - "changeLogCount": 20, - "lastAuthenticated": "2021-07-09T14:44:10.371708-06:00" + "username": "mike" } ]} -.. [1] While no roles are required, this endpoint does respect tenancy. A user will only be able to see, create, delete or modify other users belonging to the same tenant, or its descendants. - ``POST`` ======== Creates a new user. @@ -154,23 +164,32 @@ Request Structure :addressLine2: An optional field which should contain an additional address field for e.g. apartment number :city: An optional field which should contain the name of the city wherein the user resides :company: An optional field which should contain the name of the company for which the user works -:confirmLocalPasswd: The 'confirm' field in a new user's password specification - must match ``localPasswd`` :country: An optional field which should contain the name of the country wherein the user resides :email: The user's email address The given email is validated (circuitously) by `GitHub user asaskevich's regular expression `_ . Note that it can't actually distinguish a valid, deliverable, email address but merely ensure the email is in a commonly-found format. :fullName: The user's full name, e.g. "John Quincy Adams" -:localPasswd: The user's password -:newUser: An optional meta field with no apparent purpose - don't use this -:phoneNumber: An optional field which should contain the user's phone number -:postalCode: An optional field which should contain the user's postal code -:publicSshKey: An optional field which should contain the user's public encryption key used for the SSH protocol -:role: The name that corresponds to the highest permission role which will be permitted to the user -:stateOrProvince: An optional field which should contain the name of the state or province in which the user resides -:tenantId: The integral, unique identifier of the tenant to which the new user shall belong +:gid: A deprecated field only kept for legacy compatibility reasons that used to contain the UNIX group ID of the user + + .. deprecated:: 4.0 + This field is serves no known purpose, and shouldn't be used for anything so it can be removed in the future. + +:localPasswd: The user's password +:newUser: An optional meta field with no apparent purpose - don't use this +:phoneNumber: An optional field which should contain the user's phone number +:postalCode: An optional field which should contain the user's postal code +:publicSshKey: An optional field which should contain the user's public encryption key used for the SSH protocol +:role: The name that corresponds to the highest permission role which will be permitted to the user +:stateOrProvince: An optional field which should contain the name of the state or province in which the user resides +:tenantId: The integral, unique identifier of the tenant to which the new user shall belong +:ucdn: The name of the :abbr:`uCDN (Upstream Content Delivery Network)` to which the user belongs + + .. versionadded:: 4.0 - .. note:: This field is optional if and only if tenancy is not enabled in Traffic Control +:uid: A deprecated field only kept for legacy compatibility reasons that used to contain the UNIX user ID of the user -:ucdn: An optional field (only used if :abbr:`CDNi (Content Delivery Network Interconnect)` is in use) containing the name of the :abbr:`uCDN (Upstream Content Delivery Network)` to which the user belongs -:username: The new user's username + .. deprecated:: 4.0 + This field is serves no known purpose, and shouldn't be used for anything so it can be removed in the future. + +:username: The new user's username .. code-block:: http :caption: Request Example @@ -199,50 +218,61 @@ Request Structure Response Structure ------------------ -:addressLine1: The user's address - including street name and number -:addressLine2: An additional address field for e.g. apartment number -:city: The name of the city wherein the user resides -:company: The name of the company for which the user works -:country: The name of the country wherein the user resides -:email: The user's email address -:fullName: The user's full name, e.g. "John Quincy Adams" -:gid: A deprecated field only kept for legacy compatibility reasons that used to contain the UNIX group ID of the user - now it is always ``null`` -:id: An integral, unique identifier for this user -:lastUpdated: The date and time at which the user was last modified, in :ref:`non-rfc-datetime` -:newUser: A meta field with no apparent purpose that is usually ``null`` unless explicitly set during creation or modification of a user via some API endpoint -:phoneNumber: The user's phone number -:postalCode: The postal code of the area in which the user resides -:publicSshKey: The user's public key used for the SSH protocol -:registrationSent: If the user was created using the :ref:`to-api-users-register` endpoint, this will be the date and time at which the registration email was sent - otherwise it will be ``null`` -:role: The name of the role assigned to this user -:stateOrProvince: The name of the state or province where this user resides -:tenant: The name of the tenant to which this user belongs -:tenantId: The integral, unique identifier of the tenant to which this user belongs -:uid: A deprecated field only kept for legacy compatibility reasons that used to contain the UNIX user ID of the user - now it is always ``null`` -:username: The user's username +:addressLine1: The user's address - including street name and number +:addressLine2: An additional address field for e.g. apartment number +:changeLogCount: The number of change log entries created by the user +:city: The name of the city wherein the user resides +:company: The name of the company for which the user works +:country: The name of the country wherein the user resides +:email: The user's email address +:fullName: The user's full name, e.g. "John Quincy Adams" +:gid: A deprecated field only kept for legacy compatibility reasons that used to contain the UNIX group ID of the user - now it is always ``null`` + + .. deprecated:: 4.0 + This field is serves no known purpose, and shouldn't be used for anything so it can be removed in the future. + +:id: An integral, unique identifier for this user +:lastAuthenticated: The date and time at which the user was last authenticated, in :rfc:`3339` +:lastUpdated: The date and time at which the user was last modified, in :ref:`non-rfc-datetime` +:newUser: A meta field with no apparent purpose that is usually ``null`` unless explicitly set during creation or modification of a user via some API endpoint +:phoneNumber: The user's phone number +:postalCode: The postal code of the area in which the user resides +:publicSshKey: The user's public key used for the SSH protocol +:registrationSent: If the user was created using the :ref:`to-api-users-register` endpoint, this will be the date and time at which the registration email was sent - otherwise it will be ``null`` +:role: The name of the role assigned to this user +:stateOrProvince: The name of the state or province where this user resides +:tenant: The name of the tenant to which this user belongs +:tenantId: The integral, unique identifier of the tenant to which this user belongs +:ucdn: The name of the :abbr:`uCDN (Upstream Content Delivery Network)` to which the user belongs + + .. versionadded:: 4.0 + +:uid: A deprecated field only kept for legacy compatibility reasons that used to contain the UNIX user ID of the user - now it is always ``null`` + + .. deprecated:: 4.0 + This field is serves no known purpose, and shouldn't be used for anything so it can be removed in the future. + + +:username: The user's username .. code-block:: http :caption: Response Example HTTP/1.1 201 Created - Access-Control-Allow-Credentials: true - Access-Control-Allow-Headers: Origin, X-Requested-With, Content-Type, Accept - Access-Control-Allow-Methods: POST,GET,OPTIONS,PUT,DELETE - Access-Control-Allow-Origin: * - Cache-Control: no-cache, no-store, max-age=0, must-revalidate + Content-Encoding: gzip Content-Type: application/json - Location: /api/4.0/users?id=44 - Date: Thu, 13 Dec 2018 02:28:27 GMT - X-Server-Name: traffic_ops_golang/ - Set-Cookie: mojolicious=...; Path=/; Expires=Mon, 18 Nov 2019 17:40:54 GMT; Max-Age=3600; HttpOnly + Location: /api/4.0/users?id=3 + Permissions-Policy: interest-cohort=() + Set-Cookie: mojolicious=...; Path=/; Expires=Fri, 13 May 2022 23:13:54 GMT; Max-Age=3600; HttpOnly Vary: Accept-Encoding - Whole-Content-Sha512: vDqbaMvgeeoIds1czqvIWlyDG8WLnCCJdF14Ub05nsE+oJOakkyeZ8odf4d0Zjtqpk01hoVo14H2tjuWPdqwgw== - Content-Length: 623 + X-Server-Name: traffic_ops_golang/ + Date: Fri, 13 May 2022 22:13:54 GMT + Content-Length: 382 { "alerts": [ { - "level": "success", - "text": "user was created." + "text": "user was created.", + "level": "success" } ], "response": { @@ -251,14 +281,13 @@ Response Structure "changeLogCount": null, "city": "Monstropolis", "company": null, - "confirmLocalPasswd": "BFFsully", "country": null, "email": "mwazowski@minc.biz", "fullName": "Mike Wazowski", "gid": null, - "id": 26, - "lastAuthenticated": "2021-07-09T14:44:10.371708-06:00", - "lastUpdated": "2021-08-25T14:43:10.466412-06:00", + "id": 3, + "lastAuthenticated": null, + "lastUpdated": "2022-05-13T22:13:54.605052Z", "newUser": true, "phoneNumber": null, "postalCode": null, @@ -268,6 +297,9 @@ Response Structure "stateOrProvince": null, "tenant": "root", "tenantId": 1, + "ucdn": "", "uid": null, "username": "mike" }} + +.. [tenancy] While no roles are required, this endpoint does respect tenancy. A user will only be able to see, create, delete or modify other users belonging to the same tenant, or its descendants. diff --git a/docs/source/api/v4/users_id.rst b/docs/source/api/v4/users_id.rst index b9a0ffb4bd..be49381bf1 100644 --- a/docs/source/api/v4/users_id.rst +++ b/docs/source/api/v4/users_id.rst @@ -41,22 +41,29 @@ Request Structure .. code-block:: http :caption: Request Example - GET /api/4.0/users/2 HTTP/1.1 - Host: trafficops.infra.ciab.test - User-Agent: curl/7.47.0 + GET /api/4.0/users/3 HTTP/1.1 + User-Agent: python-requests/2.25.1 + Accept-Encoding: gzip, deflate Accept: */* + Connection: keep-alive Cookie: mojolicious=... + Response Structure ------------------ -:addressLine1: The user's address - including street name and number -:addressLine2: An additional address field for e.g. apartment number -:city: The name of the city wherein the user resides -:company: The name of the company for which the user works -:country: The name of the country wherein the user resides -:email: The user's email address -:fullName: The user's full name, e.g. "John Quincy Adams" -:gid: A deprecated field only kept for legacy compatibility reasons that used to contain the UNIX group ID of the user - now it is always ``null`` +:addressLine1: The user's address - including street name and number +:addressLine2: An additional address field for e.g. apartment number +:changeLogCount: The number of change log entries created by the user +:city: The name of the city wherein the user resides +:company: The name of the company for which the user works +:country: The name of the country wherein the user resides +:email: The user's email address +:fullName: The user's full name, e.g. "John Quincy Adams" +:gid: A deprecated field only kept for legacy compatibility reasons that used to contain the UNIX group ID of the user - now it is always ``null`` + + .. deprecated:: 4.0 + This field is serves no known purpose, and shouldn't be used for anything so it can be removed in the future. + :id: An integral, unique identifier for this user :lastAuthenticated: The date and time at which the user was last authenticated, in :rfc:`3339` :lastUpdated: The date and time at which the user was last modified, in :ref:`non-rfc-datetime` @@ -71,43 +78,43 @@ Response Structure :tenantId: The integral, unique identifier of the tenant to which this user belongs :ucdn: The name of the :abbr:`uCDN (Upstream Content Delivery Network)` to which the user belongs - .. versionadded:: 6.2 + .. versionadded:: 4.0 - .. note:: This field is optional and only used if :abbr:`CDNi (Content Delivery Network Interconnect)` is in use. +:uid: A deprecated field only kept for legacy compatibility reasons that used to contain the UNIX user ID of the user - now it is always ``null`` -:uid: A deprecated field only kept for legacy compatibility reasons that used to contain the UNIX user ID of the user - now it is always ``null`` -:username: The user's username + .. deprecated:: 4.0 + This field is serves no known purpose, and shouldn't be used for anything so it can be removed in the future. + +:username: The user's username .. code-block:: http :caption: Response Example HTTP/1.1 200 OK - Access-Control-Allow-Credentials: true - Access-Control-Allow-Headers: Origin, X-Requested-With, Content-Type, Accept, Set-Cookie, Cookie - Access-Control-Allow-Methods: POST,GET,OPTIONS,PUT,DELETE - Access-Control-Allow-Origin: * + Content-Encoding: gzip Content-Type: application/json - Set-Cookie: mojolicious=...; Path=/; Expires=Mon, 18 Nov 2019 17:40:54 GMT; Max-Age=3600; HttpOnly - Whole-Content-Sha512: 9vqUmt8fWEuDb+9LQJ4sGbbF4Z0a7uNyBNSWhyzAi3fBUZ5mGhd4Jx5IuSlEqiLZnYeViJJL8mpRortkHCgp5Q== + Permissions-Policy: interest-cohort=() + Set-Cookie: mojolicious=...; Path=/; Expires=Fri, 13 May 2022 23:48:14 GMT; Max-Age=3600; HttpOnly + Vary: Accept-Encoding X-Server-Name: traffic_ops_golang/ - Date: Thu, 13 Dec 2018 17:46:00 GMT - Content-Length: 454 + Date: Fri, 13 May 2022 22:48:14 GMT + Content-Length: 350 { "response": [ { - "addressLine1": null, + "addressLine1": "22 Mike Wazowski You've Got Your Life Back Lane", "addressLine2": null, - "changeLogCount": null, - "city": null, + "changeLogCount": 0, + "city": "Monstropolis", "company": null, "country": null, - "email": null, - "fullName": null, + "email": "mwazowski@minc.biz", + "fullName": "Mike Wazowski", "gid": null, - "id": 2, - "lastAuthenticated": "0001-01-01T00:00:00Z", - "lastUpdated": "2021-08-25T14:08:13.974447-06:00", - "newUser": false, + "id": 3, + "lastAuthenticated": null, + "lastUpdated": "2022-05-13T22:13:54.605052Z", + "newUser": true, "phoneNumber": null, "postalCode": null, "publicSshKey": null, @@ -116,8 +123,9 @@ Response Structure "stateOrProvince": null, "tenant": "root", "tenantId": 1, + "ucdn": "", "uid": null, - "username": "admin" + "username": "mike" } ]} @@ -143,39 +151,44 @@ Request Structure :addressLine2: An optional field which should contain an additional address field for e.g. apartment number :city: An optional field which should contain the name of the city wherein the user resides :company: An optional field which should contain the name of the company for which the user works -:confirmLocalPasswd: The 'confirm' field in a new user's password specification - must match ``localPasswd`` :country: An optional field which should contain the name of the country wherein the user resides :email: The user's email address The given email is validated (circuitously) by `GitHub user asaskevich's regular expression `_ . Note that it can't actually distinguish a valid, deliverable, email address but merely ensure the email is in a commonly-found format. :fullName: The user's full name, e.g. "John Quincy Adams" -:localPasswd: The user's password -:newUser: An optional meta field with no apparent purpose - don't use this -:phoneNumber: An optional field which should contain the user's phone number -:postalCode: An optional field which should contain the user's postal code -:publicSshKey: An optional field which should contain the user's public encryption key used for the SSH protocol -:role: The number that corresponds to the highest permission role which will be permitted to the user -:stateOrProvince: An optional field which should contain the name of the state or province in which the user resides -:tenantId: The integral, unique identifier of the tenant to which the new user shall belong +:gid: A deprecated field only kept for legacy compatibility reasons that used to contain the UNIX group ID of the user - now it is always ``null`` - .. note:: This field is optional if and only if tenancy is not enabled in Traffic Control + .. deprecated:: 4.0 + This field is serves no known purpose, and shouldn't be used for anything so it can be removed in the future. -:ucdn: The name of the :abbr:`uCDN (Upstream Content Delivery Network)` to which the user belongs +:id: This field *may* optionally be given, but **must** match the user's existing ID as IDs are immutable +:localPasswd: The user's password +:newUser: An optional meta field with no apparent purpose - don't use this +:phoneNumber: An optional field which should contain the user's phone number +:postalCode: An optional field which should contain the user's postal code +:publicSshKey: An optional field which should contain the user's public encryption key used for the SSH protocol +:role: The name of the Role which will be granted to the user +:stateOrProvince: An optional field which should contain the name of the state or province in which the user resides +:tenantId: The integral, unique identifier of the tenant to which the new user shall belong +:ucdn: The name of the :abbr:`uCDN (Upstream Content Delivery Network)` to which the user belongs - .. versionadded:: 6.2 + .. versionadded:: 4.0 - .. note:: This field is optional and only used if :abbr:`CDNi (Content Delivery Network Interconnect)` is in use. +:uid: A deprecated field only kept for legacy compatibility reasons that used to contain the UNIX user ID of the user - now it is always ``null`` + + .. deprecated:: 4.0 + This field is serves no known purpose, and shouldn't be used for anything so it can be removed in the future. :username: The user's username .. code-block:: http :caption: Request Structure - PUT /api/4.0/users/2 HTTP/1.1 - Host: trafficops.infra.ciab.test - User-Agent: curl/7.47.0 + PUT /api/4.0/users/3 HTTP/1.1 + User-Agent: python-requests/2.25.1 + Accept-Encoding: gzip, deflate Accept: */* + Connection: keep-alive Cookie: mojolicious=... - Content-Length: 458 - Content-Type: application/json + Content-Length: 476 { "addressLine1": "not a real address", @@ -183,28 +196,35 @@ Request Structure "city": "not a real city", "company": "not a real company", "country": "not a real country", - "email": "not@real.email", - "fullName": "Not a real fullName", + "email": "mwazowski@minc.biz", + "fullName": "Mike Wazowski", "phoneNumber": "not a real phone number", "postalCode": "not a real postal code", "publicSshKey": "not a real ssh key", "stateOrProvince": "not a real state or province", "tenantId": 1, "role": "admin", - "username": "admin" + "username": "mike" } + Response Structure ------------------ -:addressLine1: The user's address - including street name and number -:addressLine2: An additional address field for e.g. apartment number -:city: The name of the city wherein the user resides -:company: The name of the company for which the user works -:country: The name of the country wherein the user resides -:email: The user's email address -:fullName: The user's full name, e.g. "John Quincy Adams" -:gid: A deprecated field only kept for legacy compatibility reasons that used to contain the UNIX group ID of the user - now it is always ``null`` +:addressLine1: The user's address - including street name and number +:addressLine2: An additional address field for e.g. apartment number +:changeLogCount: The number of change log entries created by the user +:city: The name of the city wherein the user resides +:company: The name of the company for which the user works +:country: The name of the country wherein the user resides +:email: The user's email address +:fullName: The user's full name, e.g. "John Quincy Adams" +:gid: A deprecated field only kept for legacy compatibility reasons that used to contain the UNIX group ID of the user - now it is always ``null`` + + .. deprecated:: 4.0 + This field is serves no known purpose, and shouldn't be used for anything so it can be removed in the future. + :id: An integral, unique identifier for this user +:lastAuthenticated: The date and time at which the user was last authenticated, in :rfc:`3339` :lastUpdated: The date and time at which the user was last modified, in :ref:`non-rfc-datetime` :newUser: A meta field with no apparent purpose that is usually ``null`` unless explicitly set during creation or modification of a user via some API endpoint :phoneNumber: The user's phone number @@ -217,49 +237,47 @@ Response Structure :tenantId: The integral, unique identifier of the tenant to which this user belongs :ucdn: The name of the :abbr:`uCDN (Upstream Content Delivery Network)` to which the user belongs - .. versionadded:: 6.2 + .. versionadded:: 4.0 - .. note:: This field is optional and only used if :abbr:`CDNi (Content Delivery Network Interconnect)` is in use. +:uid: A deprecated field only kept for legacy compatibility reasons that used to contain the UNIX user ID of the user - now it is always ``null`` -:uid: A deprecated field only kept for legacy compatibility reasons that used to contain the UNIX user ID of the user - now it is always ``null`` -:username: The user's username + .. deprecated:: 4.0 + This field is serves no known purpose, and shouldn't be used for anything so it can be removed in the future. + +:username: The user's username .. code-block:: http :caption: Response Example HTTP/1.1 200 OK - Access-Control-Allow-Credentials: true - Access-Control-Allow-Headers: Origin, X-Requested-With, Content-Type, Accept - Access-Control-Allow-Methods: POST,GET,OPTIONS,PUT,DELETE - Access-Control-Allow-Origin: * - Cache-Control: no-cache, no-store, max-age=0, must-revalidate + Content-Encoding: gzip Content-Type: application/json - Date: Thu, 13 Dec 2018 17:24:23 GMT - X-Server-Name: traffic_ops_golang/ - Set-Cookie: mojolicious=...; Path=/; Expires=Mon, 18 Nov 2019 17:40:54 GMT; Max-Age=3600; HttpOnly + Permissions-Policy: interest-cohort=() + Set-Cookie: mojolicious=...; Path=/; Expires=Fri, 13 May 2022 23:50:25 GMT; Max-Age=3600; HttpOnly Vary: Accept-Encoding - Whole-Content-Sha512: QKvGSIwSdreMI/OdgWv9WQfI/C1JbXSoQGGospTGfCVUJ32XNWMhmREGzojWsilW8os8b14TGYeyMLUWunf2Ug== - Content-Length: 478 + X-Server-Name: traffic_ops_golang/ + Date: Fri, 13 May 2022 22:50:25 GMT + Content-Length: 399 { "alerts": [ { - "level": "success", - "text": "user was updated." + "text": "user was updated.", + "level": "success" } ], "response": { "addressLine1": "not a real address", "addressLine2": "not a real address either", - "changeLogCount": null, + "changeLogCount": 0, "city": "not a real city", "company": "not a real company", "country": "not a real country", - "email": "not@real.email", - "fullName": "Not a real fullName", + "email": "mwazowski@minc.biz", + "fullName": "Mike Wazowski", "gid": null, - "id": 2, - "lastAuthenticated": "2021-07-09T14:44:10.371708-06:00", - "lastUpdated": "2021-08-25T15:05:16.32163-06:00", + "id": 3, + "lastAuthenticated": null, + "lastUpdated": "2022-05-13T22:50:25.965004Z", "newUser": false, "phoneNumber": "not a real phone number", "postalCode": "not a real postal code", @@ -269,6 +287,7 @@ Response Structure "stateOrProvince": "not a real state or province", "tenant": "root", "tenantId": 1, + "ucdn": "", "uid": null, - "username": "admin" + "username": "mike" }} From 8b2ea0357ebe88d5271e2eaa67d91c16e9546efa Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Fri, 13 May 2022 21:42:51 -0600 Subject: [PATCH 11/14] Update Traffic Portal to account for the new user structure --- .../app/src/common/api/UserService.js | 76 +++++++++++++++---- 1 file changed, 61 insertions(+), 15 deletions(-) diff --git a/traffic_portal/app/src/common/api/UserService.js b/traffic_portal/app/src/common/api/UserService.js index 4e7fe31fb6..44b21548f5 100644 --- a/traffic_portal/app/src/common/api/UserService.js +++ b/traffic_portal/app/src/common/api/UserService.js @@ -17,6 +17,46 @@ * under the License. */ +/** + * @typedef Alert + * @property {"error" | "info" | "success" | "warning"} level + * @property {string} text + */ + +/** + * @typedef User + * @property {string | null | undefined} addressLine1 + * @property {string | null | undefined} addressLine2 + * @property {number | null | undefined} changeLogCount + * @property {string | null | undefined} city + * @property {string | null | undefined} company + * @property {string | null | undefined} country + * @property {string} email + * @property {string} fullName + * @property {number | null | undefined} gid + * @property {number | null | undefined} id + * @property {string | null | undefined} lastAuthenticated + * @property {string | null | undefined} lastUpdated + * @property {boolean} newUser + * @property {string | null | undefined} postalCode + * @property {string | null | undefined} phoneNumber + * @property {string | null | undefined} publicSshKey + * @property {string | null | undefined} registrationSent + * @property {string} role + * @property {string | null | undefined} stateOrProvince + * @property {string | null | undefined} tenant + * @property {number} tenantId + * @property {string} ucdn + * @property {number | null | undefined} uid + * @property {string} username + */ + +/** + * @typedef UserResponse + * @property {User} response + * @property {Alert[] | undefined} alerts + */ + var UserService = function($http, locationUtils, userModel, messageModel, ENV) { this.getCurrentUser = function() { @@ -82,21 +122,27 @@ var UserService = function($http, locationUtils, userModel, messageModel, ENV) { ); }; - this.updateCurrentUser = function(user) { - // We should be using PUT 'user/current' to update the current user - const currUser = { user }; - return $http.put(ENV.api.unstable + 'user/current', currUser).then( - function(result) { - userModel.setUser(user); - messageModel.setMessages(result.data.alerts, false); - return result; - }, - function(err) { - messageModel.setMessages(err.data.alerts, false); - throw err; - } - ); - }; + /** + * Updates the current user to match the one passed in. + * + * @param {User} user + * @returns {Promise<{data: UserResponse & {changeLogCount: number; id: number; lastUpdated: string}}>} + */ + async function updateCurrentUser(user) { + let result; + try { + result = await $http.put(`${ENV.api.unstable}user/current`, user); + } catch (err) { + messageModel.setMessages(err.data.alerts, false); + throw err; + } + userModel.setUser(user); + messageModel.setMessages(result.data.alerts, false); + return result; + } + + /** @type {typeof updateCurrentUser} */ + this.updateCurrentUser = updateCurrentUser; // todo: change to use query param when it is supported this.updateUser = function(user) { From 040dbadfb8ed827bd2b309b72eb122b2c7302bb7 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Fri, 13 May 2022 22:11:13 -0600 Subject: [PATCH 12/14] Update changelog --- CHANGELOG.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ad4e78efe..da7fc76b6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,10 +47,12 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - Fixed TO API `PUT /servers/:id/status` to only queue updates on the same CDN as the updated server - t3c-generate fix for combining remapconfig and cachekeyconfig parameters for MakeRemapDotConfig call. - [#6780](https://github.com/apache/trafficcontrol/issues/6780) Fixed t3c to use secondary parents when there are no primary parents available. -- Correction where using the placeholder __HOSTNAME__ in "unknown" files (others than the defaults ones), was being replaced by the full FQDN instead of the shot hostname. +- Correction where using the placeholder `__HOSTNAME__` in "unknown" files (others than the defaults ones), was being replaced by the full FQDN instead of the shot hostname. - [#6800](https://github.com/apache/trafficcontrol/issues/6800) Fixed incorrect error message for `/server/details` associated with query parameters. - [#6712](https://github.com/apache/trafficcontrol/issues/6712) - Fixed error when loading the Traffic Vault schema from `create_tables.sql` more than once. -- [#6834](https://github.com/apache/trafficcontrol/issues/6834) - In API 4.0, fixed `GET` for `/servers` to display all profiles irrespective of the index position. Also, replaced query param `profileId` with `profileName`. +- [#6834](https://github.com/apache/trafficcontrol/issues/6834) - In API 4.0, fixed `GET` for `/servers` to display all profiles irrespective of the index position. Also, replaced query param `profileId` with `profileName`. +- [#6299](https://github.com/apache/trafficcontrol/issues/6299) User representations don't match +- [#6776](https://github.com/apache/trafficcontrol/issues/6776) User properties only required sometimes ### Removed - Remove traffic\_portal dependencies to mitigate `npm audit` issues, specifically `grunt-concurrent`, `grunt-contrib-concat`, `grunt-contrib-cssmin`, `grunt-contrib-jsmin`, `grunt-contrib-uglify`, `grunt-contrib-htmlmin`, `grunt-newer`, and `grunt-wiredep` From e8a93e202504823bc0d9121e20c8c703c6b9d41d Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Fri, 20 May 2022 12:42:50 -0600 Subject: [PATCH 13/14] Fix broken footnote linking --- docs/source/api/v4/users.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/source/api/v4/users.rst b/docs/source/api/v4/users.rst index 2410b21d79..aa8f8cd40a 100644 --- a/docs/source/api/v4/users.rst +++ b/docs/source/api/v4/users.rst @@ -154,7 +154,7 @@ Response Structure Creates a new user. :Auth. Required: Yes -:Roles Required: "admin" or "operations"\ [1]_ +:Roles Required: "admin" or "operations"\ [#tenancy]_ :Permissions Required: USER:CREATE, USER:READ :Response Type: Object @@ -302,4 +302,4 @@ Response Structure "username": "mike" }} -.. [tenancy] While no roles are required, this endpoint does respect tenancy. A user will only be able to see, create, delete or modify other users belonging to the same tenant, or its descendants. +.. [#tenancy] While no roles are required, this endpoint does respect tenancy. A user will only be able to see, create, delete or modify other users belonging to the same tenant, or its descendants. From b1a209c6e6385ec87d8ff8b503a34151bf242143 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Fri, 20 May 2022 14:46:42 -0600 Subject: [PATCH 14/14] Fix unnecessarily duplicated expression --- lib/go-tc/tovalidate/validate.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/go-tc/tovalidate/validate.go b/lib/go-tc/tovalidate/validate.go index eb721c6920..7ee75dcc2c 100644 --- a/lib/go-tc/tovalidate/validate.go +++ b/lib/go-tc/tovalidate/validate.go @@ -39,7 +39,7 @@ func ToErrors(err map[string]error) []error { vErrors := []error{} for key, value := range err { if value != nil { - errMsg := fmt.Errorf("'%v' %v", key, value) + errMsg := fmt.Errorf("'%v' %w", key, value) vErrors = append(vErrors, errMsg) } } @@ -68,5 +68,5 @@ func ToError(err map[string]error) error { if msg == "" { return nil } - return errors.New(strings.TrimSuffix(b.String(), ", ")) + return errors.New(msg) }