-
Notifications
You must be signed in to change notification settings - Fork 0
✨ Fix certificate export and improve SFA certificate management workflow #81
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Removed duplicate successList entry that caused JSF to appear twice in summary - Consolidated to single success record with complete information - Removed unused successCount variable
…cates - Added pre-flight connectivity check to main script - Validates all branch paths are accessible before processing - Displays accessible/inaccessible branch summary upfront - Removed standalone Test-BranchMappings.ps1 (functionality now built-in)
- Updated feat_Publish-SFACertificates requirements.md with new FR requirements - Documented pre-flight connectivity check functionality - Updated README.html with workflow details - Updated Publish-SFACertificates.ps1 script header - Documented user prompt behavior for inaccessible branches - Noted removal of standalone Test-BranchMappings.ps1
* test: Add integration tests for pre-flight connectivity check - Tests for pre-flight check existence and structure - Tests for user prompt behavior (all accessible, partial, abort) - Tests for display output and emoji indicators - Tests for integration with branch mappings - Tests for edge cases and error handling - Tests for abort behavior and state preservation - Tests for user experience and messaging clarity * test: Fix pre-flight check test assertions * test: Add integration tests for certificate copy operations * docs: Add test implementation summary for SFA integration tests * test: Add Phase 2 network integration tests for real file operations * test: Update Phase 2 tests and completion summary with current status
- Run all SFA tests on push and pull requests to main/testing - Test on PowerShell 7.2, 7.3, and 7.4 - Generate NUnit XML test reports - Upload artifacts and publish results - Fail build on test failures
- Changed test path from Tests/SFA/ to Tests/ for comprehensive coverage - Updated path triggers to watch all Scripts, Tests, and Modules - Renamed job to reflect full test suite execution - Ensures all modules and utilities are validated on every push/PR
- Fixed improper indentation on Run All Tests step - Aligns with GitHub Actions workflow syntax requirements
- Export-PfxCertificate fails with 'Cannot export non-exportable private key' error on some certificates - Windows certutil.exe can successfully export these certificates where PowerShell cmdlets fail - Added automatic fallback: when Export-PfxCertificate fails, script attempts export using certutil.exe - Certutil fallback preserves PFX password protection using -p parameter - Script reports status as 'Exported (via certutil)' when fallback method is used - Resolves issue where valid certificates with non-exportable flags could not be distributed Testing verified: - PowerShell method used first (preferred when available) - Certutil fallback triggers only on 'non-exportable' errors - Both methods preserve password protection - Certificate file successfully created and accessible
…llback - certutil.exe proved to be more reliable than PowerShell's Export-PfxCertificate - certutil is the underlying tool used by Windows Certificate Manager (MMC) - Switched method priority: certutil first, PowerShell as fallback - Both methods support password-protected PFX export - Reduces warnings in output when certutil succeeds (no unnecessary fallback attempts) - Maintains backward compatibility by keeping PowerShell fallback Rationale: - PowerShell Export-PfxCertificate has undocumented limitations with certain passwords - certutil handles edge cases and special characters in passwords more robustly - Users familiar with MMC will recognize certutil as the trusted method - No behavioral change for end users - exports still work the same way Testing verified: - Direct certutil export succeeds without fallback - PFX file created with correct naming and size - Password protection preserved correctly
- Changed default OutputDirectory parameter from script root to Scripts/Output/SFA - Centralizes certificate exports with other SFA outputs instead of cluttering Scripts/SFA - Path now resolves to: Scripts/Output/SFA/exports_[timestamp]/ - Users can still override with -OutputDirectory parameter if needed - Updated help documentation to reflect new default location
- Changed output directory path from inconsistent locations to unified Scripts/Output/SFA - Fixed path calculation to use correct relative path (one level up from Scripts/SFA) - All reports (failures, successes, cleanup warnings) now export to Scripts/Output/SFA/ - Consistent with Export-UserCertificates.ps1 output location - Centralizes all SFA operation reports in one organized location
- Changed output directory from Scripts/Output/SFA to Output/SFA (root level) - Export-UserCertificates.ps1 now outputs to Output/SFA/exports_[timestamp]/ - Publish-SFACertificates.ps1 reports now export to Output/SFA/ - Both scripts now use correct relative paths (two levels up from Scripts/SFA) - Centralizes all SFA reports at project root Output folder - Verified with test export - certificate created in correct location
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
bug
Something isn't working
certificates
enhancement
New feature or request
refactoring
sfa-certificates
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Export-UserCertificates.ps1 Improvements
Publish-SFACertificates.ps1 Updates
Why are we doing this?
How should this be tested?
Certificate export test:
Output directory test:
Edge case testing:
Deployment notes
Closes #77