feat: add authentication integration tests (Phase 2)#87
Conversation
Add live integration tests for credential providers: - ClientSecretCredentialProvider tests (6 tests) - CertificateFileCredentialProvider tests (7 tests) - GitHubFederatedCredentialProvider tests (6 tests) Infrastructure improvements: - LiveTestConfiguration: GitHub OIDC detection, certificate temp file handling - SkipIfNoGitHubOidcAttribute for OIDC-only tests - integration-tests.yml: id-token permission for OIDC, vars for non-secrets Documentation: - MANUAL_TESTING.md for interactive/device code auth procedures Closes #55 (Phase 2) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary of ChangesHello @joshsmithxrm, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the testing capabilities for authentication providers by introducing live integration tests for client secret, certificate, and GitHub OIDC methods. It also includes the necessary infrastructure for GitHub OIDC testing within CI environments and comprehensive documentation for manually testing interactive and device code authentication flows, ensuring robust and verifiable authentication mechanisms. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request is a great addition, providing valuable integration tests for the core authentication providers (Client Secret, Certificate, and GitHub OIDC). The tests are well-structured and cover success cases, property assertions, and error handling. The infrastructure for handling certificates from base64 strings and for skipping tests based on available credentials is also well-implemented. My feedback focuses on improving consistency in the test setup, particularly around resource management (IDisposable) and configuration handling, to make the test suite more robust and maintainable. I've also identified a couple of minor issues, such as a test method that can be simplified and a small bug in a test's verification logic.
tests/PPDS.LiveTests/Authentication/CertificateAuthenticationTests.cs
Outdated
Show resolved
Hide resolved
tests/PPDS.LiveTests/Authentication/CertificateAuthenticationTests.cs
Outdated
Show resolved
Hide resolved
tests/PPDS.LiveTests/Authentication/ClientSecretAuthenticationTests.cs
Outdated
Show resolved
Hide resolved
tests/PPDS.LiveTests/Authentication/ClientSecretAuthenticationTests.cs
Outdated
Show resolved
Hide resolved
tests/PPDS.LiveTests/Authentication/GitHubFederatedAuthenticationTests.cs
Outdated
Show resolved
Hide resolved
tests/PPDS.LiveTests/Authentication/GitHubFederatedAuthenticationTests.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
This PR implements Phase 2 of the integration testing strategy (issue #55) by adding live authentication tests for three credential providers: Client Secret, Certificate, and GitHub OIDC. The changes include comprehensive test coverage, infrastructure for OIDC detection and certificate handling, documentation for manual testing of interactive methods, and CI workflow updates to enable GitHub OIDC federated authentication.
Key changes:
- Added live integration tests for Client Secret, Certificate, and GitHub OIDC credential providers with WhoAmI validation
- Enhanced LiveTestConfiguration with certificate loading, GitHub OIDC detection, and IDisposable pattern for temp file cleanup
- Updated CI workflow to support GitHub OIDC federated authentication with proper permissions and environment variables
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
tests/PPDS.LiveTests/Infrastructure/LiveTestConfiguration.cs |
Added IDisposable pattern, certificate path resolution from base64, GitHub OIDC environment variable detection, and helper methods for certificate loading |
tests/PPDS.LiveTests/Infrastructure/SkipIfNoCredentialsAttribute.cs |
Added SkipIfNoGitHubOidcAttribute for conditional test execution based on GitHub Actions OIDC availability |
tests/PPDS.LiveTests/Authentication/ClientSecretAuthenticationTests.cs |
New test file with 6 integration tests validating client secret authentication flow, token expiration, and error handling |
tests/PPDS.LiveTests/Authentication/CertificateAuthenticationTests.cs |
New test file with 7 integration tests for certificate-based authentication including file loading and certificate validation |
tests/PPDS.LiveTests/Authentication/GitHubFederatedAuthenticationTests.cs |
New test file with 6 integration tests for GitHub OIDC federated authentication specific to GitHub Actions environment |
tests/PPDS.LiveTests/Authentication/MANUAL_TESTING.md |
Documentation for manual testing procedures covering interactive browser and device code authentication methods with troubleshooting guidance |
src/PPDS.Auth/CHANGELOG.md |
Updated unreleased section documenting new integration tests and manual testing documentation |
.github/workflows/integration-tests.yml |
Added id-token: write permission for GitHub OIDC and moved non-sensitive configuration to repository variables |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/PPDS.LiveTests/Authentication/CertificateAuthenticationTests.cs
Outdated
Show resolved
Hide resolved
tests/PPDS.LiveTests/Authentication/GitHubFederatedAuthenticationTests.cs
Show resolved
Hide resolved
- Remove redundant _config field in CertificateAuthenticationTests, use inherited Configuration - Change async methods without await to synchronous (SetsIdentityProperty tests) - Add IDisposable to ClientSecretAuthenticationTests and GitHubFederatedAuthenticationTests - Add missing GitHubOidcRequestToken check in Configuration_DetectsGitHubOidcEnvironment test - Add XML doc remarks about caller disposal responsibility for LoadCertificate 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add "Duplicate" verdict for same issues reported by multiple bots - Add note that ALL comments need replies, including duplicates - Add reply template for duplicate findings - Add verification step to confirm all comments addressed 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
Changes
New Test Files
ClientSecretAuthenticationTests.cs- 6 testsCertificateAuthenticationTests.cs- 7 testsGitHubFederatedAuthenticationTests.cs- 6 testsMANUAL_TESTING.md- Manual procedures for interactive/device code authInfrastructure
LiveTestConfiguration- GitHub OIDC detection, certificate temp file handling from base64SkipIfNoGitHubOidcAttribute- Skip tests when not in GitHub Actions with OIDCintegration-tests.yml- Addedid-token: writepermission, usevars.*for non-secretsTest Plan
Configuration Required
Repository Variables:
DATAVERSE_URLPPDS_TEST_APP_IDPPDS_TEST_TENANT_IDRepository Secrets:
PPDS_TEST_CLIENT_SECRETPPDS_TEST_CERT_BASE64PPDS_TEST_CERT_PASSWORDAzure:
Closes #55 (Phase 2)
🤖 Generated with Claude Code