Add configurable base URL for SSO authenticators #2984
Conversation
Replace dynamic IPv6/localhost detection with configurable base URL properties for OIDC and SAML authenticators: - Add oic.base.url property for OpenIdConnectAuthenticator - Add saml.sp.base.url property for SamlAuthenticator - Default to http://localhost:8080 for compatibility - Add comprehensive Javadoc for SAML configuration - Update tests for new configuration approach This simplifies SSO setup by allowing explicit base URL configuration instead of relying on network address detection. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was a problem hiding this comment.
Pull request overview
This PR replaces dynamic IPv6/localhost detection with configurable base URL properties for SSO authenticators to improve deployment flexibility and configuration clarity.
Key Changes:
- Introduced
saml.sp.base.urlandoic.base.urlconfiguration properties withhttp://localhost:8080as the default fallback - Removed dependency on
IpAddressUtiland dynamic IP resolution in favor of explicit configuration - Added comprehensive Javadoc documentation for SAML configuration including setup examples for popular IdPs
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/main/java/org/codelibs/fess/sso/saml/SamlAuthenticator.java | Replaced dynamic URL building with configurable base URL property saml.sp.base.url and added extensive Javadoc documentation |
| src/main/java/org/codelibs/fess/sso/oic/OpenIdConnectAuthenticator.java | Replaced dynamic URL building with configurable base URL property oic.base.url |
| src/test/java/org/codelibs/fess/sso/saml/SamlAuthenticatorTest.java | Updated tests to verify new configuration-based URL building with comprehensive edge case coverage |
| src/test/java/org/codelibs/fess/sso/oic/OpenIdConnectAuthenticatorTest.java | Updated tests to verify new configuration-based URL building with comprehensive edge case coverage |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| DynamicProperties systemProperties = ComponentUtil.getSystemProperties(); | ||
| systemProperties.remove("saml.sp.base.url"); |
There was a problem hiding this comment.
The property removal at the start of the test assumes a clean state, but if a previous test failed without cleanup, this property might still be set. Consider adding a @before method or test fixture to ensure consistent test isolation by clearing this property before each test runs.
| DynamicProperties systemProperties = ComponentUtil.getSystemProperties(); | ||
| systemProperties.remove("oic.base.url"); |
There was a problem hiding this comment.
The property removal at the start of the test assumes a clean state, but if a previous test failed without cleanup, this property might still be set. Consider adding a @before method or test fixture to ensure consistent test isolation by clearing this property before each test runs.
src/main/java/org/codelibs/fess/sso/saml/SamlAuthenticator.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
Summary
oic.base.urlproperty for OpenIdConnectAuthenticatorsaml.sp.base.urlproperty for SamlAuthenticatorhttp://localhost:8080for backward compatibilityTest plan
🤖 Generated with Claude Code