Skip to content

Conversation

chlowell
Copy link
Member

@chlowell chlowell commented Apr 18, 2025

Closes #2282 by moving the client_assertion implementation into ClientAssertionCredential

@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.

@chlowell chlowell marked this pull request as ready for review April 18, 2025 18:03
@Copilot Copilot AI review requested due to automatic review settings April 18, 2025 18:03
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR removes the federated_credentials_flow module and moves its client assertion logic into the ClientAssertionCredential implementation, while updating related code in the SDK.

  • Removed the federated_credentials_flow module and its associated files.
  • Integrated client assertion token retrieval into ClientAssertionCredential with updated request and error handling logic.
  • Updated client_secret_credential to use the new deserialize API and improved error messaging.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
sdk/identity/azure_identity/src/lib.rs Removed federated_credentials_flow module import; added a new deserialize helper function with enhanced error context.
sdk/identity/azure_identity/src/federated_credentials_flow/response.rs Removed as part of the module deprecation.
sdk/identity/azure_identity/src/federated_credentials_flow/mod.rs Removed the entire federated_credentials_flow module implementation.
sdk/identity/azure_identity/src/credentials/client_assertion_credentials.rs Migrated token retrieval logic into a new get_token_impl method; updated endpoint construction, error handling, and tests.
sdk/identity/azure_identity/src/client_secret_credential.rs Updated usage of deserialize function and error message construction for consistency.
Comments suppressed due to low confidence (1)

sdk/identity/azure_identity/src/credentials/client_assertion_credentials.rs:86

  • [nitpick] Consider renaming the 'endpoint' variable to 'token_endpoint' to more clearly indicate that it represents the URL for token acquisition.
let endpoint = options.authority_host()?.join(&format!("/{tenant_id}/oauth2/v2.0/token"))

@github-project-automation github-project-automation bot moved this from Untriaged to In Progress in Azure Identity SDK Improvements Apr 22, 2025
@chlowell chlowell merged commit 98638b1 into Azure:main Apr 22, 2025
18 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in Azure Identity SDK Improvements Apr 22, 2025
@chlowell chlowell deleted the fed-flow branch April 22, 2025 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Azure.Identity The azure_identity crate
Projects
Development

Successfully merging this pull request may close these issues.

Federated credential flow should be renamed/repurposed as client assertion credential
3 participants