feat: add authentication integration tests (Phase 2)

.claude/commands/review-bot-comments.md

@@ -21,8 +21,11 @@ For each bot comment, determine verdict and rationale:
 |---------|---------|
 | **Valid** | Bot is correct, code should be changed |
 | **False Positive** | Bot is wrong, explain why |
+| **Duplicate** | Same issue reported by another bot - still needs reply |
 | **Unclear** | Need to investigate before deciding |
 
+**IMPORTANT:** Every comment needs a reply, including duplicates. Track all comment IDs to ensure none are missed.
+
 ### 3. Present Summary and WAIT FOR APPROVAL
 
 **CRITICAL: Do NOT implement fixes automatically.**
@@ -69,6 +72,7 @@ gh api repos/joshsmithxrm/ppds-sdk/pulls/{pr}/comments \
 | Valid (fixed) | `Fixed in {commit_sha} - {brief description}` |
 | Declined | `Declining - {reason}` |
 | False positive | `False positive - {explanation}` |
+| Duplicate | Same reply as original (reference the fix commit) |
 
 ## Common False Positives
 
@@ -89,6 +93,17 @@ gh api repos/joshsmithxrm/ppds-sdk/pulls/{pr}/comments \
 | Resource not disposed | Yes - but verify interface first |
 | Generic catch clause | Context-dependent |
 
+### 6. Verify All Comments Addressed
+
+Before completing, verify every comment has a reply:
+
+```bash
+# Count original comments vs replies
+gh api repos/joshsmithxrm/ppds-sdk/pulls/{pr}/comments --jq "length"
+```
+
+If any comments are missing replies, address them before marking complete.
+
 ## When to Use
 
 - After opening a PR (before requesting review)

.github/workflows/integration-tests.yml

@@ -40,6 +40,9 @@ jobs:
     if: github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository
     environment: test-dataverse
     needs: unit-tests
+    permissions:
+      contents: read
+      id-token: write  # Required for GitHub OIDC federated authentication
 
     steps:
     - uses: actions/checkout@v6
@@ -62,10 +65,12 @@ jobs:
 
     - name: Run Integration Tests
       env:
-        DATAVERSE_URL: ${{ secrets.DATAVERSE_URL }}
-        PPDS_TEST_APP_ID: ${{ secrets.PPDS_TEST_APP_ID }}
+        # Repository variables (non-sensitive)
+        DATAVERSE_URL: ${{ vars.DATAVERSE_URL }}
+        PPDS_TEST_APP_ID: ${{ vars.PPDS_TEST_APP_ID }}
+        PPDS_TEST_TENANT_ID: ${{ vars.PPDS_TEST_TENANT_ID }}
+        # Repository secrets (sensitive)
         PPDS_TEST_CLIENT_SECRET: ${{ secrets.PPDS_TEST_CLIENT_SECRET }}
-        PPDS_TEST_TENANT_ID: ${{ secrets.PPDS_TEST_TENANT_ID }}
         PPDS_TEST_CERT_BASE64: ${{ secrets.PPDS_TEST_CERT_BASE64 }}
         PPDS_TEST_CERT_PASSWORD: ${{ secrets.PPDS_TEST_CERT_PASSWORD }}
       run: dotnet test --configuration Release --no-build --verbosity normal --filter "Category=Integration"

src/PPDS.Auth/CHANGELOG.md

@@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- **Integration tests for credential providers** - Live tests for `ClientSecretCredentialProvider`, `CertificateFileCredentialProvider`, and `GitHubFederatedCredentialProvider` ([#55](https://github.com/joshsmithxrm/ppds-sdk/issues/55))
+- Manual test procedures documentation for interactive browser and device code authentication
+
+
 ## [1.0.0-beta.3] - 2026-01-02
 
 ### Fixed

tests/PPDS.LiveTests/Authentication/CertificateAuthenticationTests.cs

@@ -0,0 +1,142 @@
+using FluentAssertions;
+using PPDS.Auth.Credentials;
+using PPDS.LiveTests.Infrastructure;
+using Xunit;
+
+namespace PPDS.LiveTests.Authentication;
+
+/// <summary>
+/// Integration tests for certificate-based (service principal) authentication.
+/// These tests verify that CertificateFileCredentialProvider can successfully
+/// authenticate to Dataverse using a certificate file.
+/// </summary>
+[Trait("Category", "Integration")]
+public class CertificateAuthenticationTests : LiveTestBase, IDisposable
+{
+    [SkipIfNoCertificate]
+    public async Task CertificateFileCredentialProvider_CreatesWorkingServiceClient()
+    {
+        // Arrange
+        var certPath = Configuration.GetCertificatePath();
+        using var provider = new CertificateFileCredentialProvider(
+            Configuration.ApplicationId!,
+            certPath,
+            Configuration.CertificatePassword,
+            Configuration.TenantId!);
+
+        // Act
+        using var client = await provider.CreateServiceClientAsync(Configuration.DataverseUrl!);
+
+        // Assert
+        client.Should().NotBeNull();
+        client.IsReady.Should().BeTrue("ServiceClient should be ready after successful authentication");
+    }
+
+    [SkipIfNoCertificate]
+    public async Task CertificateFileCredentialProvider_CanExecuteWhoAmI()
+    {
+        // Arrange
+        var certPath = Configuration.GetCertificatePath();
+        using var provider = new CertificateFileCredentialProvider(
+            Configuration.ApplicationId!,
+            certPath,
+            Configuration.CertificatePassword,
+            Configuration.TenantId!);
+
+        using var client = await provider.CreateServiceClientAsync(Configuration.DataverseUrl!);
+
+        // Act
+        var response = (Microsoft.Crm.Sdk.Messages.WhoAmIResponse)client.Execute(
+            new Microsoft.Crm.Sdk.Messages.WhoAmIRequest());
+
+        // Assert
+        response.Should().NotBeNull();
+        response.UserId.Should().NotBeEmpty("WhoAmI should return a valid user ID");
+        response.OrganizationId.Should().NotBeEmpty("WhoAmI should return a valid organization ID");
+    }
+
+    [SkipIfNoCertificate]
+    public void CertificateFileCredentialProvider_SetsIdentityProperty()
+    {
+        // Arrange
+        var certPath = Configuration.GetCertificatePath();
+        using var provider = new CertificateFileCredentialProvider(
+            Configuration.ApplicationId!,
+            certPath,
+            Configuration.CertificatePassword,
+            Configuration.TenantId!);
+
+        // Assert - Identity is set before authentication
+        provider.Identity.Should().NotBeNullOrWhiteSpace();
+        provider.Identity.Should().StartWith("app:");
+        provider.AuthMethod.Should().Be(PPDS.Auth.Profiles.AuthMethod.CertificateFile);
+    }
+
+    [SkipIfNoCertificate]
+    public async Task CertificateFileCredentialProvider_SetsTokenExpirationAfterAuth()
+    {
+        // Arrange
+        var certPath = Configuration.GetCertificatePath();
+        using var provider = new CertificateFileCredentialProvider(
+            Configuration.ApplicationId!,
+            certPath,
+            Configuration.CertificatePassword,
+            Configuration.TenantId!);
+
+        // Token expiration is null before authentication
+        provider.TokenExpiresAt.Should().BeNull();
+
+        // Act
+        using var client = await provider.CreateServiceClientAsync(Configuration.DataverseUrl!);
+
+        // Assert - Token expiration is set after authentication
+        provider.TokenExpiresAt.Should().NotBeNull();
+        provider.TokenExpiresAt.Should().BeAfter(DateTimeOffset.UtcNow);
+    }
+
+    [SkipIfNoCertificate]
+    public void LiveTestConfiguration_CanLoadCertificate()
+    {
+        // Arrange & Act
+        using var cert = Configuration.LoadCertificate();
+
+        // Assert
+        cert.Should().NotBeNull();
+        cert.HasPrivateKey.Should().BeTrue("Certificate must have private key for authentication");
+        cert.Thumbprint.Should().NotBeNullOrWhiteSpace();
+    }
+
+    [Fact]
+    public void CertificateFileCredentialProvider_ThrowsOnNullArguments()
+    {
+        // Act & Assert
+        var act1 = () => new CertificateFileCredentialProvider(null!, "path", null, "tenant");
+        var act2 = () => new CertificateFileCredentialProvider("app", null!, null, "tenant");
+        var act3 = () => new CertificateFileCredentialProvider("app", "path", null, null!);
+
+        act1.Should().Throw<ArgumentNullException>().WithParameterName("applicationId");
+        act2.Should().Throw<ArgumentNullException>().WithParameterName("certificatePath");
+        act3.Should().Throw<ArgumentNullException>().WithParameterName("tenantId");
+    }
+
+    [Fact]
+    public async Task CertificateFileCredentialProvider_ThrowsOnMissingFile()
+    {
+        // Arrange
+        using var provider = new CertificateFileCredentialProvider(
+            "00000000-0000-0000-0000-000000000000",
+            "/nonexistent/path/cert.pfx",
+            null,
+            "00000000-0000-0000-0000-000000000000");
+
+        // Act & Assert
+        var act = () => provider.CreateServiceClientAsync("https://fake.crm.dynamics.com");
+        await act.Should().ThrowAsync<AuthenticationException>()
+            .WithMessage("*not found*");
+    }
+
+    public void Dispose()
+    {
+        Configuration.Dispose();
+    }
+}

tests/PPDS.LiveTests/Authentication/ClientSecretAuthenticationTests.cs

@@ -0,0 +1,120 @@
+using FluentAssertions;
+using PPDS.Auth.Credentials;
+using PPDS.LiveTests.Infrastructure;
+using Xunit;
+
+namespace PPDS.LiveTests.Authentication;
+
+/// <summary>
+/// Integration tests for client secret (service principal) authentication.
+/// These tests verify that ClientSecretCredentialProvider can successfully
+/// authenticate to Dataverse using a client ID and secret.
+/// </summary>
+[Trait("Category", "Integration")]
+public class ClientSecretAuthenticationTests : LiveTestBase, IDisposable
+{
+    [SkipIfNoClientSecret]
+    public async Task ClientSecretCredentialProvider_CreatesWorkingServiceClient()
+    {
+        // Arrange
+        using var provider = new ClientSecretCredentialProvider(
+            Configuration.ApplicationId!,
+            Configuration.ClientSecret!,
+            Configuration.TenantId!);
+
+        // Act
+        using var client = await provider.CreateServiceClientAsync(Configuration.DataverseUrl!);
+
+        // Assert
+        client.Should().NotBeNull();
+        client.IsReady.Should().BeTrue("ServiceClient should be ready after successful authentication");
+    }
+
+    [SkipIfNoClientSecret]
+    public async Task ClientSecretCredentialProvider_CanExecuteWhoAmI()
+    {
+        // Arrange
+        using var provider = new ClientSecretCredentialProvider(
+            Configuration.ApplicationId!,
+            Configuration.ClientSecret!,
+            Configuration.TenantId!);
+
+        using var client = await provider.CreateServiceClientAsync(Configuration.DataverseUrl!);
+
+        // Act
+        var response = (Microsoft.Crm.Sdk.Messages.WhoAmIResponse)client.Execute(
+            new Microsoft.Crm.Sdk.Messages.WhoAmIRequest());
+
+        // Assert
+        response.Should().NotBeNull();
+        response.UserId.Should().NotBeEmpty("WhoAmI should return a valid user ID");
+        response.OrganizationId.Should().NotBeEmpty("WhoAmI should return a valid organization ID");
+    }
+
+    [SkipIfNoClientSecret]
+    public void ClientSecretCredentialProvider_SetsIdentityProperty()
+    {
+        // Arrange
+        using var provider = new ClientSecretCredentialProvider(
+            Configuration.ApplicationId!,
+            Configuration.ClientSecret!,
+            Configuration.TenantId!);
+
+        // Assert - Identity is set before authentication
+        provider.Identity.Should().NotBeNullOrWhiteSpace();
+        provider.Identity.Should().StartWith("app:");
+        provider.AuthMethod.Should().Be(PPDS.Auth.Profiles.AuthMethod.ClientSecret);
+    }
+
+    [SkipIfNoClientSecret]
+    public async Task ClientSecretCredentialProvider_SetsTokenExpirationAfterAuth()
+    {
+        // Arrange
+        using var provider = new ClientSecretCredentialProvider(
+            Configuration.ApplicationId!,
+            Configuration.ClientSecret!,
+            Configuration.TenantId!);
+
+        // Token expiration is null before authentication
+        provider.TokenExpiresAt.Should().BeNull();
+
+        // Act
+        using var client = await provider.CreateServiceClientAsync(Configuration.DataverseUrl!);
+
+        // Assert - Token expiration is set after authentication
+        provider.TokenExpiresAt.Should().NotBeNull();
+        provider.TokenExpiresAt.Should().BeAfter(DateTimeOffset.UtcNow);
+    }
+
+    [Fact]
+    public void ClientSecretCredentialProvider_ThrowsOnNullArguments()
+    {
+        // Act & Assert
+        var act1 = () => new ClientSecretCredentialProvider(null!, "secret", "tenant");
+        var act2 = () => new ClientSecretCredentialProvider("app", null!, "tenant");
+        var act3 = () => new ClientSecretCredentialProvider("app", "secret", null!);
+
+        act1.Should().Throw<ArgumentNullException>().WithParameterName("applicationId");
+        act2.Should().Throw<ArgumentNullException>().WithParameterName("clientSecret");
+        act3.Should().Throw<ArgumentNullException>().WithParameterName("tenantId");
+    }
+
+    [Fact]
+    public async Task ClientSecretCredentialProvider_ThrowsOnInvalidCredentials()
+    {
+        // Arrange - Use fake credentials that will fail
+        using var provider = new ClientSecretCredentialProvider(
+            "00000000-0000-0000-0000-000000000000",
+            "invalid-secret",
+            "00000000-0000-0000-0000-000000000000");
+
+        // Act & Assert
+        var act = () => provider.CreateServiceClientAsync("https://fake.crm.dynamics.com");
+        await act.Should().ThrowAsync<AuthenticationException>();
+    }
+
+    public void Dispose()
+    {
+        Configuration.Dispose();
+    }
+}