From 8f3a0f2c38bfeb20b3122dedbad90d0ae6d31d2f Mon Sep 17 00:00:00 2001
From: pascal-fischer <32096965+pascal-fischer@users.noreply.github.com>
Date: Tue, 23 Apr 2024 19:23:43 +0200
Subject: [PATCH] Add retry to IdP cache lookup (#1882)

---
 .github/workflows/golangci-lint.yml |  2 +-
 management/server/account.go        | 53 ++++++++++++++++++++---------
 management/server/peer.go           |  2 +-
 3 files changed, 39 insertions(+), 18 deletions(-)

diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml
index 50cb4e2af..78b9f504f 100644
--- a/.github/workflows/golangci-lint.yml
+++ b/.github/workflows/golangci-lint.yml
@@ -19,7 +19,7 @@ jobs:
       - name: codespell
         uses: codespell-project/actions-codespell@v2
         with:
-          ignore_words_list: erro,clienta
+          ignore_words_list: erro,clienta,hastable,
           skip: go.mod,go.sum
           only_warn: 1
   golangci:
diff --git a/management/server/account.go b/management/server/account.go
index 9c6da05bd..aac136657 100644
--- a/management/server/account.go
+++ b/management/server/account.go
@@ -1350,18 +1350,46 @@ func (am *DefaultAccountManager) getAccountFromCache(accountID string, forceRelo
 }
 
 func (am *DefaultAccountManager) lookupCache(accountUsers map[string]userLoggedInOnce, accountID string) ([]*idp.UserData, error) {
-	data, err := am.getAccountFromCache(accountID, false)
+	var data []*idp.UserData
+	var err error
+
+	maxAttempts := 2
+
+	data, err = am.getAccountFromCache(accountID, false)
 	if err != nil {
 		return nil, err
 	}
 
+	for attempt := 1; attempt <= maxAttempts; attempt++ {
+		if am.isCacheFresh(accountUsers, data) {
+			return data, nil
+		}
+
+		if attempt > 1 {
+			time.Sleep(200 * time.Millisecond)
+		}
+
+		log.Infof("refreshing cache for account %s", accountID)
+		data, err = am.refreshCache(accountID)
+		if err != nil {
+			return nil, err
+		}
+
+		if attempt == maxAttempts {
+			log.Warnf("cache for account %s reached maximum refresh attempts (%d)", accountID, maxAttempts)
+		}
+	}
+
+	return data, nil
+}
+
+// isCacheFresh checks if the cache is refreshed already by comparing the accountUsers with the cache data by user count and user invite status
+func (am *DefaultAccountManager) isCacheFresh(accountUsers map[string]userLoggedInOnce, data []*idp.UserData) bool {
 	userDataMap := make(map[string]*idp.UserData, len(data))
 	for _, datum := range data {
 		userDataMap[datum.ID] = datum
 	}
 
-	mustRefreshInviteStatus := false
-
 	// the accountUsers ID list of non integration users from store, we check if cache has all of them
 	// as result of for loop knownUsersCount will have number of users are not presented in the cashed
 	knownUsersCount := len(accountUsers)
@@ -1369,9 +1397,8 @@ func (am *DefaultAccountManager) lookupCache(accountUsers map[string]userLoggedI
 		if datum, ok := userDataMap[user]; ok {
 			// check if the matching user data has a pending invite and if the user has logged in once, forcing the cache to be refreshed
 			if datum.AppMetadata.WTPendingInvite != nil && *datum.AppMetadata.WTPendingInvite && loggedInOnce == true { //nolint:gosimple
-				mustRefreshInviteStatus = true
-				log.Infof("user %s has a pending invite and has logged in once, forcing cache refresh", user)
-				break
+				log.Infof("user %s has a pending invite and has logged in once, cache invalid", user)
+				return false
 			}
 			knownUsersCount--
 			continue
@@ -1380,18 +1407,12 @@ func (am *DefaultAccountManager) lookupCache(accountUsers map[string]userLoggedI
 	}
 
 	// if we know users that are not yet in cache more likely cache is outdated
-	if knownUsersCount > 0 || mustRefreshInviteStatus {
-		if !mustRefreshInviteStatus {
-			log.Infof("reloading cache with IDP manager. Users unknown to the cache: %d", knownUsersCount)
-		}
-		// reload cache once avoiding loops
-		data, err = am.refreshCache(accountID)
-		if err != nil {
-			return nil, err
-		}
+	if knownUsersCount > 0 {
+		log.Infof("cache invalid. Users unknown to the cache: %d", knownUsersCount)
+		return false
 	}
 
-	return data, err
+	return true
 }
 
 func (am *DefaultAccountManager) removeUserFromCache(accountID, userID string) error {
diff --git a/management/server/peer.go b/management/server/peer.go
index 1448e3011..1a8b183ed 100644
--- a/management/server/peer.go
+++ b/management/server/peer.go
@@ -374,7 +374,7 @@ func (am *DefaultAccountManager) AddPeer(setupKey, userID string, peer *nbpeer.P
 	if strings.ToLower(peer.Meta.Hostname) == "iphone" || strings.ToLower(peer.Meta.Hostname) == "ipad" && userID != "" {
 		if am.idpManager != nil {
 			userdata, err := am.lookupUserInCache(userID, account)
-			if err == nil {
+			if err == nil && userdata != nil {
 				peer.Meta.Hostname = fmt.Sprintf("%s-%s", peer.Meta.Hostname, strings.Split(userdata.Email, "@")[0])
 			}
 		}