Don't update identity if it hasn't changed. Fix change attribution fo…

…r edge client updates. Fixes #1610. Fixes #1611
openziti · Sep 13, 2023 · 74006ee · 74006ee
1 parent 02b39f5
commit 74006ee
Show file tree

Hide file tree

Showing 3 changed files with 67 additions and 23 deletions.
diff --git a/controller/handler_edge_ctrl/common_tunnel.go b/controller/handler_edge_ctrl/common_tunnel.go
@@ -341,25 +341,36 @@ func (self *baseTunnelRequestContext) getCreateSessionResponse() *edge_ctrl_pb.C
 
 func (self *baseTunnelRequestContext) updateIdentityInfo(envInfo *edge_ctrl_pb.EnvInfo, sdkInfo *edge_ctrl_pb.SdkInfo) {
 	if self.err == nil {
+		updateIdentity := false
 		if envInfo != nil {
-			self.identity.EnvInfo = &model.EnvInfo{}
-			self.identity.EnvInfo.Arch = envInfo.Arch
-			self.identity.EnvInfo.Os = envInfo.Os
-			self.identity.EnvInfo.OsRelease = envInfo.OsRelease
-			self.identity.EnvInfo.OsVersion = envInfo.OsVersion
+			newEnvInfo := &model.EnvInfo{
+				Arch:      envInfo.Arch,
+				Os:        envInfo.Os,
+				OsRelease: envInfo.OsRelease,
+				OsVersion: envInfo.OsVersion,
+			}
+			if !self.identity.EnvInfo.Equals(newEnvInfo) {
+				self.identity.EnvInfo = newEnvInfo
+				updateIdentity = true
+			}
 		}
 
 		if sdkInfo != nil {
-			self.identity.SdkInfo = &model.SdkInfo{}
-			self.identity.SdkInfo.AppId = sdkInfo.AppId
-			self.identity.SdkInfo.AppVersion = sdkInfo.AppVersion
-			self.identity.SdkInfo.Branch = sdkInfo.Branch
-			self.identity.SdkInfo.Revision = sdkInfo.Revision
-			self.identity.SdkInfo.Type = sdkInfo.Type
-			self.identity.SdkInfo.Version = sdkInfo.Version
+			newSdkInfo := &model.SdkInfo{
+				AppId:      sdkInfo.AppId,
+				AppVersion: sdkInfo.AppVersion,
+				Branch:     sdkInfo.Branch,
+				Revision:   sdkInfo.Revision,
+				Type:       sdkInfo.Type,
+				Version:    sdkInfo.Version,
+			}
+			if !self.identity.SdkInfo.Equals(newSdkInfo) {
+				self.identity.SdkInfo = newSdkInfo
+				updateIdentity = true
+			}
 		}
 
-		if envInfo != nil || sdkInfo != nil {
+		if updateIdentity {
 			self.err = internalError(self.handler.getAppEnv().GetManagers().Identity.PatchInfo(self.identity, self.newTunnelChangeContext()))
 		}
 	}

diff --git a/controller/internal/routes/authenticate_router.go b/controller/internal/routes/authenticate_router.go
@@ -24,11 +24,11 @@ import (
 	clientApiAuthentication "github.com/openziti/edge-api/rest_client_api_server/operations/authentication"
 	managementApiAuthentication "github.com/openziti/edge-api/rest_management_api_server/operations/authentication"
 	"github.com/openziti/edge-api/rest_model"
-	"github.com/openziti/fabric/controller/apierror"
 	"github.com/openziti/edge/controller/env"
 	"github.com/openziti/edge/controller/internal/permissions"
 	"github.com/openziti/edge/controller/model"
 	"github.com/openziti/edge/controller/response"
+	"github.com/openziti/fabric/controller/apierror"
 	"github.com/openziti/foundation/v2/errorz"
 	"github.com/openziti/metrics"
 	"net"
@@ -79,8 +79,6 @@ func (ro *AuthRouter) authHandler(ae *env.AppEnv, rc *response.RequestContext, h
 
 	authResult, err := ae.Managers.Authenticator.Authorize(authContext)
 
-	changeCtx := rc.NewChangeContext()
-
 	if err != nil {
 		rc.RespondWithError(err)
 		return
@@ -94,6 +92,8 @@ func (ro *AuthRouter) authHandler(ae *env.AppEnv, rc *response.RequestContext, h
 	rc.AuthPolicy = authResult.AuthPolicy()
 
 	identity := authResult.Identity()
+	rc.Identity = identity
+
 	if identity.EnvInfo == nil {
 		identity.EnvInfo = &model.EnvInfo{}
 	}
@@ -102,25 +102,30 @@ func (ro *AuthRouter) authHandler(ae *env.AppEnv, rc *response.RequestContext, h
 		identity.SdkInfo = &model.SdkInfo{}
 	}
 
+	changeCtx := rc.NewChangeContext()
+
 	if dataMap := authContext.GetData(); dataMap != nil {
 		shouldUpdate := false
 
 		if envInfoInterface := dataMap["envInfo"]; envInfoInterface != nil {
-			if envInfo := envInfoInterface.(map[string]interface{}); envInfo != nil {
-				if err := mapstructure.Decode(envInfo, &identity.EnvInfo); err != nil {
+			if envInfoMap := envInfoInterface.(map[string]interface{}); envInfoMap != nil {
+				var envInfo *model.EnvInfo
+				if err := mapstructure.Decode(envInfoMap, &envInfo); err != nil {
 					logger.WithError(err).Error("error processing env info")
-				} else {
+				} else if !identity.EnvInfo.Equals(envInfo) {
+					identity.EnvInfo = envInfo
 					shouldUpdate = true
 				}
-
 			}
 		}
 
 		if sdkInfoInterface := dataMap["sdkInfo"]; sdkInfoInterface != nil {
-			if sdkInfo := sdkInfoInterface.(map[string]interface{}); sdkInfo != nil {
-				if err := mapstructure.Decode(sdkInfo, &identity.SdkInfo); err != nil {
+			if sdkInfoMap := sdkInfoInterface.(map[string]interface{}); sdkInfoMap != nil {
+				var sdkInfo *model.SdkInfo
+				if err := mapstructure.Decode(sdkInfoMap, &sdkInfo); err != nil {
 					logger.WithError(err).Error("error processing sdk info")
-				} else {
+				} else if !identity.SdkInfo.Equals(sdkInfo) {
+					identity.SdkInfo = sdkInfo
 					shouldUpdate = true
 				}
 			}

diff --git a/controller/model/identity_model.go b/controller/model/identity_model.go
@@ -33,6 +33,19 @@ type EnvInfo struct {
 	OsVersion string
 }
 
+func (self *EnvInfo) Equals(other *EnvInfo) bool {
+	if self == nil {
+		return other == nil
+	}
+	if other == nil {
+		return false
+	}
+	return self.Arch == other.Arch &&
+		self.Os == other.Os &&
+		self.OsRelease == other.OsRelease &&
+		self.OsVersion == other.OsVersion
+}
+
 type SdkInfo struct {
 	AppId      string
 	AppVersion string
@@ -42,6 +55,21 @@ type SdkInfo struct {
 	Version    string
 }
 
+func (self *SdkInfo) Equals(other *SdkInfo) bool {
+	if self == nil {
+		return other == nil
+	}
+	if other == nil {
+		return false
+	}
+	return self.AppId == other.AppId &&
+		self.AppVersion == other.AppVersion &&
+		self.Branch == other.Branch &&
+		self.Revision == other.Revision &&
+		self.Type == other.Type &&
+		self.Version == other.Version
+}
+
 type Identity struct {
 	models.BaseEntity
 	Name                      string