From 74006eeb9d18be8cadb21136585d6a71e85eed06 Mon Sep 17 00:00:00 2001 From: Paul Lorenz Date: Wed, 13 Sep 2023 11:42:35 -0400 Subject: [PATCH] Don't update identity if it hasn't changed. Fix change attribution for edge client updates. Fixes #1610. Fixes #1611 --- controller/handler_edge_ctrl/common_tunnel.go | 37 ++++++++++++------- .../internal/routes/authenticate_router.go | 25 ++++++++----- controller/model/identity_model.go | 28 ++++++++++++++ 3 files changed, 67 insertions(+), 23 deletions(-) diff --git a/controller/handler_edge_ctrl/common_tunnel.go b/controller/handler_edge_ctrl/common_tunnel.go index bc7a1ddee..c3b9e91cb 100644 --- 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 index 2a224e395..25858b06d 100644 --- 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 index bfbe5d1e9..f5337bf86 100644 --- 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