Merge pull request #2462 from Neet-Nestor/main

Fix Azure OAuth token refresh with unprefixed scopes

src/fastmcp/server/auth/oauth_proxy.py

@@ -1273,6 +1273,23 @@ class OAuthProxy(OAuthProvider):
     # Refresh Token Flow
     # -------------------------------------------------------------------------
 
+    def _prepare_scopes_for_upstream_refresh(self, scopes: list[str]) -> list[str]:
+        """Prepare scopes for upstream token refresh request.
+
+        Override this method to transform scopes before sending to upstream provider.
+        For example, Azure needs to prefix scopes and add additional Graph scopes.
+
+        The scopes parameter represents what should be stored in the RefreshToken.
+        This method returns what should be sent to the upstream provider.
+
+        Args:
+            scopes: Base scopes that will be stored in RefreshToken
+
+        Returns:
+            Scopes to send to upstream provider (may be transformed/augmented)
+        """
+        return scopes
+
     async def load_refresh_token(
         self,
         client: OAuthClientInformationFull,
@@ -1333,12 +1350,17 @@ class OAuthProxy(OAuthProvider):
             timeout=HTTP_TIMEOUT_SECONDS,
         )
 
+        # Allow child classes to transform scopes before sending to upstream
+        # This enables provider-specific scope formatting (e.g., Azure prefixing)
+        # while keeping original scopes in storage
+        upstream_scopes = self._prepare_scopes_for_upstream_refresh(scopes)
+
         try:
             logger.debug("Refreshing upstream token (jti=%s)", refresh_jti[:8])
             token_response: dict[str, Any] = await oauth_client.refresh_token(  # type: ignore[misc]
                 url=self._upstream_token_endpoint,
                 refresh_token=upstream_token_set.refresh_token,
-                scope=" ".join(scopes) if scopes else None,
+                scope=" ".join(upstream_scopes) if upstream_scopes else None,
                 **self._extra_token_params,
             )
             logger.debug("Successfully refreshed upstream token")

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:
@@ -358,3 +373,42 @@ class AzureProvider(OAuthProxy):
 
         # Let parent build the URL with prefixed scopes
         return super()._build_upstream_authorize_url(txn_id, modified_transaction)
+
+    def _prepare_scopes_for_upstream_refresh(self, scopes: list[str]) -> list[str]:
+        """Prepare scopes for Azure token refresh.
+
+        Azure requires:
+        1. Fully-qualified custom scopes (e.g., "api://xxx/read" not "read")
+        2. Microsoft Graph scopes (e.g., "User.Read", "openid") sent as-is
+        3. Additional scopes from provider config (additional_authorize_scopes)
+
+        This method transforms base client scopes for Azure while keeping them
+        unprefixed in storage to prevent accumulation.
+
+        Args:
+            scopes: Base scopes from RefreshToken (unprefixed, e.g., ["read"])
+
+        Returns:
+            Deduplicated list of scopes formatted for Azure token endpoint
+        """
+        logger.debug("Base scopes from storage: %s", scopes)
+
+        # Filter out any additional_authorize_scopes that may have been stored
+        # (they shouldn't be in storage, but clean them up if they are)
+        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 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)
+
+        # 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("Scopes for Azure token endpoint: %s", deduplicated_scopes)
+        return deduplicated_scopes

tests/server/auth/providers/test_azure.py

@@ -487,3 +487,225 @@ class TestAzureProvider:
         parsed = urlparse(provider._upstream_authorization_endpoint)
         assert parsed.netloc == "login.microsoftonline.us"
         assert "/organizations/" in parsed.path
+
+    def test_prepare_scopes_for_upstream_refresh_basic_prefixing(self):
+        """Test that unprefixed scopes are correctly prefixed for Azure token refresh."""
+        provider = AzureProvider(
+            client_id="test_client",
+            client_secret="test_secret",
+            tenant_id="test-tenant",
+            identifier_uri="api://my-api",
+            required_scopes=["read", "write"],
+            jwt_signing_key="test-secret",
+        )
+
+        # Unprefixed scopes from storage should be prefixed
+        result = provider._prepare_scopes_for_upstream_refresh(["read", "write"])
+
+        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_already_prefixed(self):
+        """Test that already-prefixed scopes remain unchanged."""
+        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",
+        )
+
+        # Already prefixed scopes should pass through unchanged
+        result = provider._prepare_scopes_for_upstream_refresh(
+            ["api://my-api/read", "api://other-api/admin"]
+        )
+
+        assert "api://my-api/read" in result
+        assert "api://other-api/admin" in result
+        assert len(result) == 2
+
+    def test_prepare_scopes_for_upstream_refresh_with_additional_scopes(self):
+        """Test that additional_authorize_scopes are added during token refresh."""
+        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",
+                "profile",
+                "offline_access",
+            ],
+            jwt_signing_key="test-secret",
+        )
+
+        # Base scopes should be prefixed, additional scopes appended
+        result = provider._prepare_scopes_for_upstream_refresh(["read", "write"])
+
+        assert "api://my-api/read" in result
+        assert "api://my-api/write" in result
+        assert "User.Read" in result
+        assert "openid" in result
+        assert "profile" in result
+        assert "offline_access" in result
+        assert len(result) == 6
+
+    def test_prepare_scopes_for_upstream_refresh_filters_duplicate_additional_scopes(
+        self,
+    ):
+        """Test that accidentally stored additional_authorize_scopes are filtered out."""
+        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",
+        )
+
+        # If additional scopes were accidentally stored, they should be filtered
+        # to prevent accumulation
+        result = provider._prepare_scopes_for_upstream_refresh(
+            ["read", "User.Read", "openid"]
+        )
+
+        # Should have: api://my-api/read (prefixed) + User.Read + openid (added once)
+        assert "api://my-api/read" in result
+        assert result.count("User.Read") == 1
+        assert result.count("openid") == 1
+        assert len(result) == 3
+
+    def test_prepare_scopes_for_upstream_refresh_mixed_scopes(self):
+        """Test mixed scenario with both prefixed and unprefixed scopes."""
+        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"],
+            jwt_signing_key="test-secret",
+        )
+
+        # Mix of prefixed and unprefixed scopes
+        result = provider._prepare_scopes_for_upstream_refresh(
+            ["read", "api://other-api/admin", "write"]
+        )
+
+        assert "api://my-api/read" in result
+        assert "api://other-api/admin" in result  # Already prefixed, unchanged
+        assert "api://my-api/write" in result
+        assert "User.Read" in result
+        assert len(result) == 4
+
+    def test_prepare_scopes_for_upstream_refresh_scope_with_slash(self):
+        """Test that scopes containing '/' are not prefixed."""
+        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",
+        )
+
+        # Scopes with "/" should not be prefixed (already fully qualified)
+        result = provider._prepare_scopes_for_upstream_refresh(
+            ["read", "https://graph.microsoft.com/.default"]
+        )
+
+        assert "api://my-api/read" in result
+        assert (
+            "https://graph.microsoft.com/.default" in result
+        )  # Not prefixed (contains ://)
+
+    def test_prepare_scopes_for_upstream_refresh_empty_scopes(self):
+        """Test behavior with empty scopes list."""
+        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",
+        )
+
+        # Empty scopes should still add additional_authorize_scopes
+        result = provider._prepare_scopes_for_upstream_refresh([])
+
+        assert "User.Read" in result
+        assert "openid" in result
+        assert len(result) == 2
+
+    def test_prepare_scopes_for_upstream_refresh_no_additional_scopes(self):
+        """Test behavior when no additional_authorize_scopes are configured."""
+        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",
+        )
+
+        # Should only prefix base scopes, no additional scopes added
+        result = provider._prepare_scopes_for_upstream_refresh(["read", "write"])
+
+        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