TO: Only display Server ILO/XMPP Passwords Based on New Permission (#…

…7697)

* Remove priv checks for secure server fields

* Handle api version 4 also

* Forgot a file + changelog

---------

Co-authored-by: Steve Hamrick <[email protected]>

CHANGELOG.md

@@ -145,6 +145,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - [#7628](https://github.com/apache/trafficcontrol/pull/7628) *Traffic Ops* Fixes an issue where certificate chain validation failed based on leading or trailing whitespace.
 - [#7596](https://github.com/apache/trafficcontrol/pull/7596) *Traffic Ops* Fixes `federation_resolvers` v5 apis to respond with `RFC3339` date/time Format.
 - [#7660](https://github.com/apache/trafficcontrol/pull/7660) *Traffic Ops* Fixes `deliveryServices` v5 apis to respond with `RFC3339` date/time Format.
+- [#7697](https://github.com/apache/trafficcontrol/pull/7697) *Traffic Ops* Fixes `iloPassword` and `xmppPassword` checking for priv-level instead of using permissions.
 
 ### Removed
 - [#7271](https://github.com/apache/trafficcontrol/pull/7271) Remove components in `infrastructre/docker/`, not in use as cdn-in-a-box performs the same functionality.

docs/source/api/v5/servers.rst

@@ -98,7 +98,7 @@ Response Structure
 :iloIpAddress: The IPv4 address of the server's :abbr:`ILO (Integrated Lights-Out)` service\ [#ilo]_
 :iloIpGateway: The IPv4 gateway address of the server's :abbr:`ILO (Integrated Lights-Out)` service\ [#ilo]_
 :iloIpNetmask: The IPv4 subnet mask of the server's :abbr:`ILO (Integrated Lights-Out)` service\ [#ilo]_
-:iloPassword:  The password of the of the server's :abbr:`ILO (Integrated Lights-Out)` service user\ [#ilo]_ - displays as simply ``******`` if the currently logged-in user does not have the 'admin' or 'operations' :term:`Role(s) <Role>`
+:iloPassword:  The password of the of the server's :abbr:`ILO (Integrated Lights-Out)` service user\ [#ilo]_ - displays as simply ``******`` if the currently logged-in user does not have the SECURE-SERVER:READ permission.
 :iloUsername:  The user name for the server's :abbr:`ILO (Integrated Lights-Out)` service\ [#ilo]_
 :interfaces:   A set of the network interfaces in use by the server. In most scenarios, only one will be present, but it is illegal for this set to be an empty collection.
 
@@ -158,7 +158,7 @@ Response Structure
 :typeId:     The integral, unique identifier of the 'type' of this server
 :updPending: A boolean value which, if ``true``, indicates that the server has updates of some kind pending, typically to be acted upon by Traffic Control Cache Config (:term:`t3c`, formerly ORT)
 :xmppId:     A system-generated UUID used to generate a server hashId for use in Traffic Router's consistent hashing algorithm. This value is set when a server is created and cannot be changed afterwards.
-:xmppPasswd: The password used in XMPP communications with the server
+:xmppPasswd: The password used in XMPP communications with the server - displays as simply ``******`` if the currently logged-in user does not have the SECURE-SERVER:READ permission.
 
 .. code-block:: http
 	:caption: Response Example
@@ -254,7 +254,7 @@ Request Structure
 :iloIpAddress: An optional IPv4 address of the server's :abbr:`ILO (Integrated Lights-Out)` service\ [#ilo]_
 :iloIpGateway: An optional IPv4 gateway address of the server's :abbr:`ILO (Integrated Lights-Out)` service\ [#ilo]_
 :iloIpNetmask: An optional IPv4 subnet mask of the server's :abbr:`ILO (Integrated Lights-Out)` service\ [#ilo]_
-:iloPassword:  An optional string containing the password of the of the server's :abbr:`ILO (Integrated Lights-Out)` service user\ [#ilo]_ - displays as simply ``******`` if the currently logged-in user does not have the 'admin' or 'operations' :term:`Role(s) <Role>`
+:iloPassword:  An optional string containing the password of the of the server's :abbr:`ILO (Integrated Lights-Out)` service user\ [#ilo]_ - displays as simply ``******`` if the currently logged-in user does not have the SECURE-SERVER:READ permission.
 :iloUsername:  An optional string containing the user name for the server's :abbr:`ILO (Integrated Lights-Out)` service\ [#ilo]_
 :interfaces:   A set of the network interfaces in use by the server. In most scenarios, only one will be necessary, but it is illegal for this set to be an empty collection.
 
@@ -302,7 +302,7 @@ Request Structure
 
 :typeId:     The integral, unique identifier of the 'type' of this server
 :xmppId:     A system-generated UUID used to generate a server hashId for use in Traffic Router's consistent hashing algorithm. This value is set when a server is created and cannot be changed afterwards.
-:xmppPasswd: An optional password used in XMPP communications with the server
+:xmppPasswd: An optional password used in XMPP communications with the server - displays as simply ``******`` if the currently logged-in user does not have the SECURE-SERVER:READ permission.
 
 .. code-block:: http
 	:caption: Request Example
@@ -385,7 +385,7 @@ Response Structure
 :iloIpAddress: The IPv4 address of the server's :abbr:`ILO (Integrated Lights-Out)` service\ [#ilo]_
 :iloIpGateway: The IPv4 gateway address of the server's :abbr:`ILO (Integrated Lights-Out)` service\ [#ilo]_
 :iloIpNetmask: The IPv4 subnet mask of the server's :abbr:`ILO (Integrated Lights-Out)` service\ [#ilo]_
-:iloPassword:  The password of the of the server's :abbr:`ILO (Integrated Lights-Out)` service user\ [#ilo]_ - displays as simply ``******`` if the currently logged-in user does not have the 'admin' or 'operations' :abbr:`Role(s) <Role>`
+:iloPassword:  The password of the of the server's :abbr:`ILO (Integrated Lights-Out)` service user\ [#ilo]_ - displays as simply ``******`` if the currently logged-in user does not have the SECURE-SERVER:READ permission.
 :iloUsername:  The user name for the server's :abbr:`ILO (Integrated Lights-Out)` service\ [#ilo]_
 :interfaces:   A set of the network interfaces in use by the server. In most scenarios, only one will be present, but it is illegal for this set to be an empty collection.
 
@@ -445,7 +445,7 @@ Response Structure
 :typeId:     The integral, unique identifier of the 'type' of this server
 :updPending: A boolean value which, if ``true``, indicates that the server has updates of some kind pending, typically to be acted upon by Traffic Control Cache Config (T3C, formerly ORT)
 :xmppId:     A system-generated UUID used to generate a server hashId for use in Traffic Router's consistent hashing algorithm. This value is set when a server is created and cannot be changed afterwards.
-:xmppPasswd: The password used in XMPP communications with the server
+:xmppPasswd: The password used in XMPP communications with the server - displays as simply ``******`` if the currently logged-in user does not have the SECURE-SERVER:READ permission.
 
 .. code-block:: http
 	:caption: Response Example

traffic_ops/testing/api/v5/servers_test.go

@@ -39,6 +39,8 @@ func TestServers(t *testing.T) {
 		currentTimeRFC := currentTime.Format(time.RFC1123)
 		tomorrow := currentTime.AddDate(0, 0, 1).Format(time.RFC1123)
 
+		opsUserSession := utils.CreateV5Session(t, Config.TrafficOps.URL, "operations", Config.TrafficOps.UserPassword, Config.Default.Session.TimeoutInSecs)
+
 		methodTests := utils.V5TestCase{
 			"GET": {
 				"NOT MODIFIED when NO CHANGES made": {
@@ -117,6 +119,12 @@ func TestServers(t *testing.T) {
 					RequestOpts:   client.RequestOptions{QueryParameters: url.Values{"dsId": {strconv.Itoa(GetDeliveryServiceId(t, "ds-based-top-with-no-mids")())}}},
 					Expectations:  utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), utils.ResponseLengthGreaterOrEqual(1), validateServerTypeIsNotMid()),
 				},
+				"VALUE HIDDEN when OPERATIONS USER views SERVER": {
+					ClientSession: opsUserSession,
+					RequestOpts:   client.RequestOptions{QueryParameters: url.Values{"iloPassword": {"testSecure"}, "xmppPasswd": {"testSecure"}}},
+					Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), utils.ResponseLengthGreaterOrEqual(1),
+						validateServerFields(map[string]interface{}{"ILOPassword": "********", "XMPPPasswd": "********"})),
+				},
 				"EMPTY RESPONSE when INVALID DSID parameter": {
 					ClientSession: TOSession,
 					RequestOpts:   client.RequestOptions{QueryParameters: url.Values{"dsId": {"999999"}}},
@@ -438,6 +446,12 @@ func validateServerFields(expectedResp map[string]interface{}) utils.CkReqFunc {
 				case "TypeID":
 					assert.RequireNotNil(t, server.TypeID, "Expected TypeID to not be nil")
 					assert.Equal(t, expected, *server.TypeID, "Expected Type to be %d, but got %d", expected, *server.TypeID)
+				case "XMPPPasswd":
+					assert.RequireNotNil(t, server.XMPPPasswd, "Expected XMPPPasswd to not be nil")
+					assert.Equal(t, expected, *server.XMPPPasswd, "Expected XMPPPasswd to be %s, but got %s", expected, *server.XMPPPasswd)
+				case "ILOPassword":
+					assert.RequireNotNil(t, server.ILOPassword, "Expected ILOPassword to not be nil")
+					assert.Equal(t, expected, *server.ILOPassword, "Expected ILOPassword to be %s, but got %s", expected, *server.ILOPassword)
 				default:
 					t.Errorf("Expected field: %v, does not exist in response", field)
 				}

traffic_ops/traffic_ops_golang/server/servers.go

@@ -698,7 +698,7 @@ func Read(w http.ResponseWriter, r *http.Request) {
 		log.Warnf("Couldn't get config %v", e)
 	}
 
-	servers, serverCount, userErr, sysErr, errCode, maxTime = getServers(r.Header, inf.Params, inf.Tx, inf.User, useIMS, *version)
+	servers, serverCount, userErr, sysErr, errCode, maxTime = getServers(r.Header, inf.Params, inf.Tx, inf.User, useIMS, *version, inf.Config.RoleBasedPermissions)
 	if maxTime != nil && api.SetLastModifiedHeader(r, useIMS) {
 		api.AddLastModifiedHdr(w, *maxTime)
 	}
@@ -767,7 +767,7 @@ func getServerCount(tx *sqlx.Tx, query string, queryValues map[string]interface{
 	return serverCount, nil
 }
 
-func getServers(h http.Header, params map[string]string, tx *sqlx.Tx, user *auth.CurrentUser, useIMS bool, version api.Version) ([]tc.ServerV40, uint64, error, error, int, *time.Time) {
+func getServers(h http.Header, params map[string]string, tx *sqlx.Tx, user *auth.CurrentUser, useIMS bool, version api.Version, roleBasedPerms bool) ([]tc.ServerV40, uint64, error, error, int, *time.Time) {
 	var maxTime time.Time
 	var runSecond bool
 	// Query Parameters to Database Query column mappings
@@ -956,7 +956,12 @@ JOIN server_profile sp ON s.id = sp.server`
 		if err != nil {
 			return nil, serverCount, nil, errors.New("getting servers: " + err.Error()), http.StatusInternalServerError, nil
 		}
-		if user.PrivLevel < auth.PrivLevelOperations {
+		if (version.GreaterThanOrEqualTo(&api.Version{Major: 4}) && roleBasedPerms) || version.GreaterThanOrEqualTo(&api.Version{Major: 5}) {
+			if !user.Can("SECURE-SERVER:READ") {
+				s.ILOPassword = &HiddenField
+				s.XMPPPasswd = &HiddenField
+			}
+		} else if user.PrivLevel < auth.PrivLevelOperations {
 			s.ILOPassword = &HiddenField
 			s.XMPPPasswd = &HiddenField
 		}
@@ -1322,7 +1327,7 @@ func Update(w http.ResponseWriter, r *http.Request) {
 	id := inf.IntParams["id"]
 
 	// Get original server
-	originals, _, userErr, sysErr, errCode, _ := getServers(r.Header, inf.Params, inf.Tx, inf.User, false, *version)
+	originals, _, userErr, sysErr, errCode, _ := getServers(r.Header, inf.Params, inf.Tx, inf.User, false, *version, inf.Config.RoleBasedPermissions)
 	if userErr != nil || sysErr != nil {
 		api.HandleErr(w, r, tx, errCode, userErr, sysErr)
 		return
@@ -2185,7 +2190,7 @@ func Delete(w http.ResponseWriter, r *http.Request) {
 	}
 
 	var servers []tc.ServerV40
-	servers, _, userErr, sysErr, errCode, _ = getServers(r.Header, map[string]string{"id": inf.Params["id"]}, inf.Tx, inf.User, false, *version)
+	servers, _, userErr, sysErr, errCode, _ = getServers(r.Header, map[string]string{"id": inf.Params["id"]}, inf.Tx, inf.User, false, *version, inf.Config.RoleBasedPermissions)
 	if userErr != nil || sysErr != nil {
 		api.HandleErr(w, r, tx, errCode, userErr, sysErr)
 		return

traffic_ops/traffic_ops_golang/server/servers_test.go

@@ -265,7 +265,7 @@ func TestGetServersByCachegroup(t *testing.T) {
 
 	version := api.Version{Major: 4, Minor: 0}
 
-	servers, _, userErr, sysErr, errCode, _ := getServers(nil, v, db.MustBegin(), &user, false, version)
+	servers, _, userErr, sysErr, errCode, _ := getServers(nil, v, db.MustBegin(), &user, false, version, false)
 	if userErr != nil || sysErr != nil {
 		t.Errorf("getServers expected: no errors, actual: %v %v with status: %s", userErr, sysErr, http.StatusText(errCode))
 	}
@@ -380,7 +380,7 @@ func TestGetMidServers(t *testing.T) {
 
 	user := auth.CurrentUser{}
 	version := api.Version{Major: 4, Minor: 0}
-	servers, _, userErr, sysErr, errCode, _ := getServers(nil, v, db.MustBegin(), &user, false, version)
+	servers, _, userErr, sysErr, errCode, _ := getServers(nil, v, db.MustBegin(), &user, false, version, false)
 
 	if userErr != nil || sysErr != nil {
 		t.Errorf("getServers expected: no errors, actual: %v %v with status: %s", userErr, sysErr, http.StatusText(errCode))