Upgrade Guacamole to version 1.6.0 (required extension changes) and OAuth Proxy to 7.13.0#4754
Conversation
Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
…nection Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
…rror handling tests Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
…SSL issues Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
…re with Docker Compose and Playwright Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
… Playwright Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
…ficates in both build and runtime stages Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
… chain into Java keystore Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
… and runtime stages Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
… support on Debian base Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
… prevent /bin/sh deletion Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
…kerfile - all tests passing Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
…nshots Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
… 502 errors, add proper error checking Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR upgrades the Guacamole workspace service to version 1.6.0 and implements significant security improvements and test coverage enhancements. The upgrade addresses CVEs by updating Guacamole components, migrating from Java 11 to Java 17, upgrading Tomcat to 9.0.111, and updating OAuth2-proxy to 7.13.0.
Key changes include:
- Security enhancements: Automatic credential cleanup after RDP connections, sanitized exception messages to prevent information leakage, and improved JSON construction methods
- Test coverage expansion: Added 51 new tests (unit, integration, and E2E) using JUnit 5, Mockito, and Playwright
- Infrastructure modernization: Java 17 migration, updated dependency versions, and comprehensive E2E test infrastructure with Docker Compose
Reviewed Changes
Copilot reviewed 37 out of 40 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| porter.yaml | Version bump to 0.14.0 for the Guacamole service bundle |
| pom.xml | Updated Maven compiler to Java 17 and upgraded all dependencies (Auth0 JWT, Azure SDK, etc.) |
| guac-manifest.json | Updated Guacamole version requirement from 1.4.0 to 1.6.0 |
| TreUserContextTest.java | New comprehensive unit tests for user context initialization and permissions |
| AzureTREAuthenticatedUserTest.java | Enhanced tests with Mockito extensions and additional edge case coverage |
| RDPCredentialIntegrationTest.java | New integration tests for RDP credential injection and security parameters |
| LocalIntegrationTest.java | New integration tests for authentication flow without cloud dependencies |
| TokenInjectingConnectionTest.java | New unit tests for connection creation and configuration |
| ConnectionServiceTest.java | Added tests for null handling and exception scenarios |
| AzureTREAuthenticationProvider.java | Migrated from deprecated getRequest() to RequestDetails API and refactored token validation |
| AuthenticationProviderService.java | Improved error handling with sanitized exception messages |
| TokenInjectingConnection.java | Added credential cleanup in finally block and improved logging |
| Dockerfile | Upgraded to Java 17, Tomcat 9.0.111, removed SSH service, added non-root user execution |
| Service run scripts | Added finish handlers for graceful shutdown, removed SSH service, updated Application Insights logic |
| logback.xml | Reduced default log level to INFO and added logger-specific configurations |
| E2E test infrastructure | Complete new test suite with Docker Compose, Playwright, mock API, and Azure AD provisioning scripts |
| CHANGELOG.md | Added detailed entry for Guacamole upgrade with component versions and improvements |
…ps://github.com/marrobi/AzureTRE into copilot/update-guacamole-auth-azure-credentials
james-annages
left a comment
There was a problem hiding this comment.
LGTM
tested the deploy and it worked.
…ilot/update-guacamole-auth-azure-credentials
|
/test-extended 7b5dd8d |
|
🤖 pr-bot 🤖 🏃 Running extended tests: https://github.com/microsoft/AzureTRE/actions/runs/19476107496 (with refid (in response to this comment from @marrobi) |
|
/test-extended 7b5dd8d |
|
🤖 pr-bot 🤖 🏃 Running extended tests: https://github.com/microsoft/AzureTRE/actions/runs/19494344467 (with refid (in response to this comment from @marrobi) |
|
/test-extended 7b5dd8d |
|
🤖 pr-bot 🤖 🏃 Running extended tests: https://github.com/microsoft/AzureTRE/actions/runs/19534090501 (with refid (in response to this comment from @marrobi) |
|
/test-force-approve 791418d Passed: https://github.com/microsoft/AzureTRE/actions/runs/19534090501 |
|
🤖 pr-bot 🤖 ✅ Marking tests as complete (for commit 791418d) (in response to this comment from @marrobi) |
|
/test-force-approve fd3aff5 Passed: https://github.com/microsoft/AzureTRE/actions/runs/19534090501 |
|
🤖 pr-bot 🤖 ✅ Marking tests as complete (for commit fd3aff5) (in response to this comment from @marrobi) |
|
/test-force-approve ef117e7 Passed: https://github.com/microsoft/AzureTRE/actions/runs/19534090501 |
|
🤖 pr-bot 🤖 ✅ Marking tests as complete (for commit ef117e7) (in response to this comment from @marrobi) |
|
/test-force-approve 6e8b056 Passed: https://github.com/microsoft/AzureTRE/actions/runs/19534090501 |
|
🤖 pr-bot 🤖 ✅ Marking tests as complete (for commit 6e8b056) (in response to this comment from @marrobi) |
|
/test-force-approve 6070eaf |
|
🤖 pr-bot 🤖 ✅ Marking tests as complete (for commit 6070eaf) (in response to this comment from @marrobi) |
What is being addressed
Upgrade Guacamole to version 1.6.0 (required extension changes) and OAuth Proxy to 7.13.0 and other components to address open CVEs. Also set s6 services to run under non root accounts.
How is this addressed
Upgraded Guacamole components and Tomcat, migrated to Java 17 for better compatibility and security.
Implemented comprehensive unit and integration tests, increasing test coverage significantly.
Added end-to-end test infrastructure using Docker Compose and Playwright for thorough validation.
This has been coauthered with copilot, I've done a fair bit of testing/optimization. Icreated e2e tests so I could iterate better locally.
I still need to fix a few tests.