Skip to content

Conversation

@Robbie-Microsoft
Copy link
Contributor

Implements #5591

@Robbie-Microsoft Robbie-Microsoft marked this pull request as ready for review December 8, 2025 18:22
@Robbie-Microsoft Robbie-Microsoft requested a review from a team as a code owner December 8, 2025 18:22
}
}
}
// This file intentionally left empty.
Copy link
Contributor

Choose a reason for hiding this comment

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

can we delete this project? do we need this?

@@ -0,0 +1,44 @@
<Project Sdk="Microsoft.NET.Sdk">
Copy link
Contributor

Choose a reason for hiding this comment

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

should we update the name to Microsoft.Identity.Client.Attestation or Microsoft.Identity.Client.KeyAttestation

CsrMetadata csrMetadata = await GetCsrMetadataAsync(_requestContext).ConfigureAwait(false);

// Validate that mTLS PoP requires KeyGuard - fail fast before network calls
if (_isMtlsPopRequested)
Copy link
Contributor

Choose a reason for hiding this comment

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

Wrong condition. Your comment is very clear,

  • Attempt attestation only for KeyGuard keys when provider is available
  • For non-KeyGuard keys (Hardware, InMemory), proceed with non-attested flow

if (provider == null)
{
_requestContext.Logger.Info("[ImdsV2] No attestation provider configured. Proceeding with non-attested flow.");
return string.Empty; // Empty attestation token indicates non-attested flow
Copy link
Contributor

Choose a reason for hiding this comment

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

Will MIRP accept an empty attestation string? has this been tested?


// CreateManagedIdentityAsync does a probe; Add one more CSR response for the actual acquire.
httpManager.AddMockHandler(MockHelpers.MockCsrResponse());
// Add mocks for successful non-attested flow (CSR + issuecredential + token)
Copy link
Contributor

Choose a reason for hiding this comment

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

don't we need to update existing POP tests to use WithAttestationSupport()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants