From b2ba64825ea03938b42a240725d9de387bad165f Mon Sep 17 00:00:00 2001 From: Taylor Frey Date: Wed, 1 Dec 2021 14:01:18 -0700 Subject: [PATCH 1/3] Update PUT user/current to work with v4 User Roles --- CHANGELOG.md | 1 + lib/go-tc/users.go | 2 +- .../traffic_ops_golang/user/current.go | 40 +++++++++++++++++-- 3 files changed, 38 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index da785de5d3..d1ac3a1f9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - [#6285](https://github.com/apache/trafficcontrol/issues/6285) - The Traffic Ops Postinstall script will work in CentOS 7, even if Python 3 is installed - [#5373](https://github.com/apache/trafficcontrol/issues/5373) - Traffic Monitor logs not consistent - Traffic Ops: Sanitize username before executing LDAP query +- [#6367](https://github.com/apache/trafficcontrol/issues/6367) - Fix PUT `user/current` to work with v4 User Roles and Permissions ### Changed - Updated `t3c` to request less unnecessary deliveryservice-server assignment and invalidation jobs data via new query params supported by Traffic Ops diff --git a/lib/go-tc/users.go b/lib/go-tc/users.go index 1852c3b76d..2b2b9537f9 100644 --- a/lib/go-tc/users.go +++ b/lib/go-tc/users.go @@ -317,7 +317,7 @@ type UserResponseV4 struct { // "undefined" values. type CurrentUserUpdateRequest struct { // User, for whatever reason, contains all of the actual data. - User CurrentUserUpdateRequestUser `json:"user"` + User *CurrentUserUpdateRequestUser `json:"user"` } // CurrentUserUpdateRequestUser holds all of the actual data in a request to update the current user. diff --git a/traffic_ops/traffic_ops_golang/user/current.go b/traffic_ops/traffic_ops_golang/user/current.go index ea6e1eac31..2c06e86397 100644 --- a/traffic_ops/traffic_ops_golang/user/current.go +++ b/traffic_ops/traffic_ops_golang/user/current.go @@ -28,11 +28,13 @@ import ( "github.com/apache/trafficcontrol/lib/go-tc" "github.com/apache/trafficcontrol/lib/go-util" + "github.com/jmoiron/sqlx" "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api" "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/auth" "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/dbhelpers" "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/tenant" + //"github.com/jmoiron/sqlx" ) const replacePasswordQuery = ` @@ -183,13 +185,18 @@ func ReplaceCurrent(w http.ResponseWriter, r *http.Request) { api.HandleErr(w, r, tx, errCode, userErr, nil) return } + + if userRequest.User == nil { + errCode = http.StatusBadRequest + userErr = fmt.Errorf("missing required 'user' object") + api.HandleErr(w, r, tx, errCode, userErr, nil) + return + } + if inf.Version.Major >= 4 { useV4User = true } user, exists, err := dbhelpers.GetUserByID(inf.User.ID, tx) - if useV4User { - userV4 = user.Upgrade() - } if err != nil { sysErr = fmt.Errorf("getting user by ID %d: %v", inf.User.ID, err) errCode = http.StatusInternalServerError @@ -309,7 +316,16 @@ func ReplaceCurrent(w http.ResponseWriter, r *http.Request) { } } - if err = updateUser(&user, tx, changePasswd, changeConfirmPasswd); err != nil { + if useV4User { + userV4 = user.Upgrade() + } + + if useV4User { + err = updateUserV4(userV4, inf.Tx) + } else { + err = updateUser(&user, tx, changePasswd, changeConfirmPasswd) + } + if err != nil { errCode = http.StatusInternalServerError sysErr = fmt.Errorf("updating user: %v", err) api.HandleErr(w, r, tx, errCode, nil, sysErr) @@ -323,6 +339,22 @@ func ReplaceCurrent(w http.ResponseWriter, r *http.Request) { } } +func updateUserV4(u tc.UserV4, tx *sqlx.Tx) error { + resultRows, err := tx.NamedQuery(UpdateQueryV40(), u) + if err != nil { + return err + } + defer resultRows.Close() + + for resultRows.Next() { + if err := resultRows.Scan(&u.LastUpdated, &u.Tenant, &u.Role); err != nil { + return err + } + } + + return nil +} + func updateUser(u *tc.User, tx *sql.Tx, changePassword bool, changeConfirmPasswd bool) error { row := tx.QueryRow(replaceCurrentQuery, u.AddressLine1, From b5310e21c68bbfe79ed33c12765388da60ce3c2f Mon Sep 17 00:00:00 2001 From: Taylor Frey Date: Wed, 1 Dec 2021 14:08:04 -0700 Subject: [PATCH 2/3] Move import to correct location per ordering --- traffic_ops/traffic_ops_golang/user/current.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/traffic_ops/traffic_ops_golang/user/current.go b/traffic_ops/traffic_ops_golang/user/current.go index 2c06e86397..8f7933a789 100644 --- a/traffic_ops/traffic_ops_golang/user/current.go +++ b/traffic_ops/traffic_ops_golang/user/current.go @@ -28,13 +28,13 @@ import ( "github.com/apache/trafficcontrol/lib/go-tc" "github.com/apache/trafficcontrol/lib/go-util" - "github.com/jmoiron/sqlx" "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api" "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/auth" "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/dbhelpers" "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/tenant" - //"github.com/jmoiron/sqlx" + + "github.com/jmoiron/sqlx" ) const replacePasswordQuery = ` From f631ae43d3f3df1089dc3be2ac97ad47ae5f7005 Mon Sep 17 00:00:00 2001 From: Taylor Frey Date: Wed, 1 Dec 2021 17:16:28 -0700 Subject: [PATCH 3/3] Remove 'UpdateQuery40' implementation --- .../traffic_ops_golang/user/current.go | 32 ++----------------- 1 file changed, 2 insertions(+), 30 deletions(-) diff --git a/traffic_ops/traffic_ops_golang/user/current.go b/traffic_ops/traffic_ops_golang/user/current.go index 8f7933a789..d2f2365bc8 100644 --- a/traffic_ops/traffic_ops_golang/user/current.go +++ b/traffic_ops/traffic_ops_golang/user/current.go @@ -33,8 +33,6 @@ import ( "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/auth" "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/dbhelpers" "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/tenant" - - "github.com/jmoiron/sqlx" ) const replacePasswordQuery = ` @@ -169,7 +167,6 @@ WHERE u.id=$1 func ReplaceCurrent(w http.ResponseWriter, r *http.Request) { var useV4User bool - var userV4 tc.UserV4 inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil) tx := inf.Tx.Tx if userErr != nil || sysErr != nil { @@ -316,16 +313,7 @@ func ReplaceCurrent(w http.ResponseWriter, r *http.Request) { } } - if useV4User { - userV4 = user.Upgrade() - } - - if useV4User { - err = updateUserV4(userV4, inf.Tx) - } else { - err = updateUser(&user, tx, changePasswd, changeConfirmPasswd) - } - if err != nil { + if err = updateUser(&user, tx, changePasswd, changeConfirmPasswd); err != nil { errCode = http.StatusInternalServerError sysErr = fmt.Errorf("updating user: %v", err) api.HandleErr(w, r, tx, errCode, nil, sysErr) @@ -333,28 +321,12 @@ func ReplaceCurrent(w http.ResponseWriter, r *http.Request) { } if useV4User { - api.WriteRespAlertObj(w, r, tc.SuccessLevel, "User profile was successfully updated", userV4) + 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) } } -func updateUserV4(u tc.UserV4, tx *sqlx.Tx) error { - resultRows, err := tx.NamedQuery(UpdateQueryV40(), u) - if err != nil { - return err - } - defer resultRows.Close() - - for resultRows.Next() { - if err := resultRows.Scan(&u.LastUpdated, &u.Tenant, &u.Role); err != nil { - return err - } - } - - return nil -} - func updateUser(u *tc.User, tx *sql.Tx, changePassword bool, changeConfirmPasswd bool) error { row := tx.QueryRow(replaceCurrentQuery, u.AddressLine1,