Repository: supabase/gotrue
Pull request #1020 (2023-04-06 09:54:45)

feat: add saml metadata force update every 24 hours
If the SAML Metadata defined via a URL does not publish validity or cache duration information, forcefully try to update it every 24 hours.


Patch:
diff --git a/internal/api/samlacs.go b/internal/api/samlacs.go
index 7f2d341c..30d9c69a 100644
--- a/internal/api/samlacs.go
+++ b/internal/api/samlacs.go
@@ -31,10 +31,16 @@ func (a *API) samlDestroyRelayState(ctx context.Context, relayState *models.SAML
 	})
 }
 
-func IsMetadataStale(idpMetadata *saml.EntityDescriptor, samlProvider models.SAMLProvider) bool {
-	hasIDPMetadataExpired := !idpMetadata.ValidUntil.IsZero() && idpMetadata.ValidUntil.Before(time.Now())
-	hasCacheDurationExceeded := idpMetadata.CacheDuration != 0 && samlProvider.UpdatedAt.Add(idpMetadata.CacheDuration).Before(time.Now())
-	return hasIDPMetadataExpired || hasCacheDurationExceeded
+func IsSAMLMetadataStale(idpMetadata *saml.EntityDescriptor, samlProvider models.SAMLProvider) bool {
+	now := time.Now()
+
+	hasValidityExpired := !idpMetadata.ValidUntil.IsZero() && now.After(idpMetadata.ValidUntil)
+	hasCacheDurationExceeded := idpMetadata.CacheDuration != 0 && now.After(samlProvider.UpdatedAt.Add(idpMetadata.CacheDuration))
+
+	// if metadata XML does not publish validity or caching information, update once in 24 hours
+	needsForceUpdate := idpMetadata.ValidUntil.IsZero() && idpMetadata.CacheDuration == 0 && now.After(samlProvider.UpdatedAt.Add(24*time.Hour))
+
+	return hasValidityExpired || hasCacheDurationExceeded || needsForceUpdate
 }
 
 // SAMLACS implements the main Assertion Consumer Service endpoint behavior.
@@ -154,7 +160,7 @@ func (a *API) SAMLACS(w http.ResponseWriter, r *http.Request) error {
 
 			logentry.Warn("SAML Metadata for identity provider will expire soon! Update its metadata_xml!")
 		}
-	} else if *ssoProvider.SAMLProvider.MetadataURL != "" && IsMetadataStale(idpMetadata, ssoProvider.SAMLProvider) {
+	} else if *ssoProvider.SAMLProvider.MetadataURL != "" && IsSAMLMetadataStale(idpMetadata, ssoProvider.SAMLProvider) {
 		rawMetadata, err := fetchSAMLMetadata(ctx, *ssoProvider.SAMLProvider.MetadataURL)
 		if err != nil {
 			// Fail silently but raise warning and continue with existing metadata
diff --git a/internal/api/token.go b/internal/api/token.go
index 491830b2..5149970c 100644
--- a/internal/api/token.go
+++ b/internal/api/token.go
@@ -618,6 +618,11 @@ func (a *API) PKCE(ctx context.Context, w http.ResponseWriter, r *http.Request)
 		if err != nil {
 			return err
 		}
+		if terr := models.NewAuditLogEntry(r, tx, user, models.LoginAction, "", map[string]interface{}{
+			"provider_type": flowState.ProviderType,
+		}); terr != nil {
+			return terr
+		}
 		token, terr = a.issueRefreshToken(ctx, tx, user, authMethod, grantParams)
 		if terr != nil {
 			return oauthError("server_error", terr.Error())
diff --git a/internal/models/linking.go b/internal/models/linking.go
index 2696682f..16bffab1 100644
--- a/internal/models/linking.go
+++ b/internal/models/linking.go
@@ -78,8 +78,12 @@ func DetermineAccountLinking(tx *storage.Connection, provider, sub string, email
 			return AccountLinkingResult{}, terr
 		}
 
-		if terr := tx.Q().Eager().Where("email in (?) and is_sso_user is false", emails).All(&similarUsers); terr != nil {
-			return AccountLinkingResult{}, terr
+		if !strings.HasPrefix(provider, "sso:") {
+			// there can be multiple user accounts with the same email when is_sso_user is true
+			// so we just do not consider those similar user accounts
+			if terr := tx.Q().Eager().Where("email in (?) and is_sso_user is false", emails).All(&similarUsers); terr != nil {
+				return AccountLinkingResult{}, terr
+			}
 		}
 	}
 
diff --git a/migrations/20230411005111_remove_duplicate_idx.up.sql b/migrations/20230411005111_remove_duplicate_idx.up.sql
index 543b3492..dc23931c 100644
--- a/migrations/20230411005111_remove_duplicate_idx.up.sql
+++ b/migrations/20230411005111_remove_duplicate_idx.up.sql
@@ -1,1 +1,1 @@
-drop index {{index .Options "Namespace" }}.refresh_tokens_token_idx;
+drop index if exists {{index .Options "Namespace" }}.refresh_tokens_token_idx;


Test patch:
diff --git a/internal/api/sso_test.go b/internal/api/sso_test.go
index 0ee05902..beac825e 100644
--- a/internal/api/sso_test.go
+++ b/internal/api/sso_test.go
@@ -183,7 +183,7 @@ func (ts *SSOTestSuite) TestIsStaleSAMLMetadata() {
 			provider.UpdatedAt = currentTime.Add(-time.Minute * 59)
 		}
 
-		require.Equal(ts.T(), example.IsStale, IsMetadataStale(metadata, provider))
+		require.Equal(ts.T(), example.IsStale, IsSAMLMetadataStale(metadata, provider))
 	}
 
 }