Resolve comments

src/fastmcp/server/auth/providers/azure.py

@@ -327,6 +327,28 @@ class AzureProvider(OAuthProxy):
         separator = "&" if "?" in auth_url else "?"
         return f"{auth_url}{separator}prompt=select_account"
 
+    def _prefix_scopes_for_azure(self, scopes: list[str]) -> list[str]:
+        """Prefix unprefixed scopes with identifier_uri for Azure.
+
+        This helper centralizes the scope prefixing logic used in both
+        authorization and token refresh flows.
+
+        Args:
+            scopes: List of scopes, may be prefixed or unprefixed
+
+        Returns:
+            List of scopes with identifier_uri prefix applied where needed
+        """
+        prefixed = []
+        for scope in scopes:
+            if "://" in scope or "/" in scope:
+                # Already fully-qualified (e.g., "api://xxx/read" or "User.Read")
+                prefixed.append(scope)
+            else:
+                # Unprefixed client scope - prefix with identifier_uri
+                prefixed.append(f"{self.identifier_uri}/{scope}")
+        return prefixed
+
     def _build_upstream_authorize_url(
         self, txn_id: str, transaction: dict[str, Any]
     ) -> str:
@@ -339,14 +361,7 @@ class AzureProvider(OAuthProxy):
         unprefixed_scopes = transaction.get("scopes") or self.required_scopes or []
 
         # Prefix scopes for Azure authorization request
-        prefixed_scopes = []
-        for scope in unprefixed_scopes:
-            if "://" in scope or "/" in scope:
-                # Already a full URI or path (e.g., "api://xxx/read" or "User.Read")
-                prefixed_scopes.append(scope)
-            else:
-                # Unprefixed scope name - prefix it with identifier_uri
-                prefixed_scopes.append(f"{self.identifier_uri}/{scope}")
+        prefixed_scopes = self._prefix_scopes_for_azure(unprefixed_scopes)
 
         # Add Microsoft Graph scopes (not validated, not prefixed)
         if self.additional_authorize_scopes:
@@ -374,7 +389,7 @@ class AzureProvider(OAuthProxy):
             scopes: Base scopes from RefreshToken (unprefixed, e.g., ["read"])
 
         Returns:
-            Scopes formatted for Azure token endpoint
+            Deduplicated list of scopes formatted for Azure token endpoint
         """
         logger.debug(f"Base scopes from storage: {scopes}")
 
@@ -383,20 +398,17 @@ class AzureProvider(OAuthProxy):
         additional_scopes_set = set(self.additional_authorize_scopes or [])
         base_scopes = [s for s in scopes if s not in additional_scopes_set]
 
-        # Prefix base scopes with identifier_uri for Azure
-        prefixed_scopes = []
-        for scope in base_scopes:
-            if "://" in scope or "/" in scope:
-                # Already fully-qualified (e.g., "api://xxx/read")
-                prefixed_scopes.append(scope)
-            else:
-                # Unprefixed client scope - prefix with identifier_uri
-                prefixed_scopes.append(f"{self.identifier_uri}/{scope}")
+        # Prefix base scopes with identifier_uri for Azure using shared helper
+        prefixed_scopes = self._prefix_scopes_for_azure(base_scopes)
 
         # Add additional scopes (Graph + OIDC) for the Azure request
         # These are NOT stored in RefreshToken, only sent to Azure
         if self.additional_authorize_scopes:
             prefixed_scopes.extend(self.additional_authorize_scopes)
 
-        logger.debug(f"Scopes for Azure token endpoint: {prefixed_scopes}")
-        return prefixed_scopes
+        # Deduplicate while preserving order (in case older tokens have duplicates)
+        # Use dict.fromkeys() for O(n) deduplication with order preservation
+        deduplicated_scopes = list(dict.fromkeys(prefixed_scopes))
+
+        logger.debug(f"Scopes for Azure token endpoint: {deduplicated_scopes}")
+        return deduplicated_scopes

tests/server/auth/providers/test_azure.py

@@ -660,3 +660,52 @@ class TestAzureProvider:
         assert "api://my-api/read" in result
         assert "api://my-api/write" in result
         assert len(result) == 2
+
+    def test_prepare_scopes_for_upstream_refresh_deduplicates_scopes(self):
+        """Test that duplicate scopes are deduplicated while preserving order."""
+        provider = AzureProvider(
+            client_id="test_client",
+            client_secret="test_secret",
+            tenant_id="test-tenant",
+            identifier_uri="api://my-api",
+            required_scopes=["read"],
+            additional_authorize_scopes=["User.Read", "openid"],
+            jwt_signing_key="test-secret",
+        )
+
+        # Test with duplicate base scopes and duplicate additional scopes
+        result = provider._prepare_scopes_for_upstream_refresh(
+            ["read", "write", "read", "User.Read", "openid"]
+        )
+
+        # Should have deduplicated results in order
+        assert result == [
+            "api://my-api/read",
+            "api://my-api/write",
+            "User.Read",
+            "openid",
+        ]
+        assert len(result) == 4
+
+    def test_prepare_scopes_for_upstream_refresh_deduplicates_prefixed_variants(self):
+        """Test that both prefixed and unprefixed variants are deduplicated."""
+        provider = AzureProvider(
+            client_id="test_client",
+            client_secret="test_secret",
+            tenant_id="test-tenant",
+            identifier_uri="api://my-api",
+            required_scopes=["read"],
+            jwt_signing_key="test-secret",
+        )
+
+        # Test with both prefixed and unprefixed variants of same scope
+        result = provider._prepare_scopes_for_upstream_refresh(
+            ["read", "api://my-api/read", "write"]
+        )
+
+        # Should deduplicate - first occurrence wins (api://my-api/read from "read")
+        assert "api://my-api/read" in result
+        assert "api://my-api/write" in result
+        # Should only have 2 items (read processed twice, but deduplicated)
+        assert len(result) == 2
+        assert result.count("api://my-api/read") == 1