Extract generic OAuth2UserProvider from KeycloakUserProvider and refactor AdfsUserProvider
KeycloakUserProvider and AdfsUserProvider contained significant code duplication. Both are OAuth2 implementations with ADFS having minor extensions for domain handling, password login, and technical users.
Changes
-
OAuth2UserProvider - Renamed
KeycloakUserProvidertoOAuth2UserProvideras the generic OAuth2/SSO implementation. Protected fields (userIdProperty,userNameProperty,flowParser, configuration fields) enable subclass customization. -
KeycloakUserProvider - Now an empty deprecated wrapper extending
OAuth2UserProvider. Maintains backward compatibility. -
AdfsUserProvider - Refactored to extend
OAuth2UserProvider. Overrides:-
init()- passesnullforuserEndpointandintrospectionEndpoint(ADFS doesn't use these endpoints) -
login()- adds domain to username, supports password flow viassoHelper.passwordLogin(), usesinfoInIdTokenproperty to select ID vs access token claims -
parse()- handles technical users viaappidclaim andtechnicalUserIdproperty
-
Eliminates ~270 lines of duplicate code. Both deprecated classes remain functional for existing configurations.
// Before: Two independent implementations with duplication
public class KeycloakUserProvider extends BaseUserProvider<SSOLogin> { /* 300+ lines */ }
public class AdfsUserProvider extends BaseUserProvider<SSOLogin> { /* 180+ lines of mostly duplicate code */ }
// After: Clear hierarchy with isolated ADFS-specific behavior
public class OAuth2UserProvider extends BaseUserProvider<SSOLogin> { /* generic OAuth2 */ }
public class KeycloakUserProvider extends OAuth2UserProvider { /* empty wrapper */ }
public class AdfsUserProvider extends OAuth2UserProvider {
// Only ADFS-specific: domain, infoInIdToken, technicalUserId, password login
}
Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
-
artifacts.cibseven.org- Triggering command:
/home/REDACTED/work/cibseven-webclient/cibseven-webclient/frontend/node/node /home/REDACTED/work/cibseven-webclient/cibseven-webclient/frontend/node/node /home/REDACTED/work/cibseven-webclient/cibseven-webclient/frontend/node/node_modules/npm/bin/npm-cli.js install --no-audit --no-fund(dns block) - Triggering command:
/usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --enable-native-access=ALL-UNNAMED -classpath /usr/share/apache-maven-3.9.11/boot/plexus-classworlds-2.9.0.jar -Dclassworlds.conf=/usr/share/apache-maven-3.9.11/bin/m2.conf -Dmaven.home=/usr/share/apache-maven-3.9.11 -Dlibrary.jansi.path=/usr/share/apache-maven-3.9.11/lib/jansi-native -Dmaven.multiModuleProjectDirectory=/home/REDACTED/work/cibseven-webclient/cibseven-webclient org.codehaus.plexus.classworlds.launcher.Launcher -f pom.xml -B -V -e -Dfindbugs.skip -Dcheckstyle.skip -Dpmd.skip=true -Dspotbugs.skip -Denforcer.skip -Dmaven.javadoc.skip(dns block)
- Triggering command:
If you need me to access, download, or install something from one of these locations, you can either:
- Configure Actions setup steps to set up my environment, which run before the firewall is enabled
- Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)
Original prompt
Problem
KeycloakUserProvider.javaandAdfsUserProvider.javaincibseven-webclient-core/src/main/java/org/cibseven/webapp/auth/are both OAuth2 implementations with significant code duplication. KeycloakUserProvider is essentially a generic OAuth2 provider, while AdfsUserProvider has some ADFS-specific extensions.Required Changes
1. Create
OAuth2UserProvider.java
- Rename/move the current
KeycloakUserProvider.javatoOAuth2UserProvider.javain the same directory- This should be the generic OAuth2/SSO implementation that other providers extend
- Keep all the existing functionality (token caching, user info endpoint, introspection, etc.)
2. Create new
KeycloakUserProvider.java
- Create a new
KeycloakUserProvider.javathat extendsOAuth2UserProvider- The class should be essentially empty (just extends the parent)
- Add
@Deprecatedannotation with the message:"Use OAuth2UserProvider instead"3. Refactor
AdfsUserProvider.java
- Make it extend
OAuth2UserProviderinstead ofBaseUserProvider<SSOLogin>- Keep the ADFS-specific functionality that differs from the generic OAuth2 implementation:
domainproperty for appending domain to usernameinfoInIdTokenpropertytechnicalUserIdproperty- Password login flow (
ssoHelper.passwordLogin())- Technical user handling in
parse()method usingappidclaim- Remove all duplicate code that is now inherited from
OAuth2UserProvider- Add
@Deprecatedannotation with the message:"Use OAuth2UserProvider instead"- Override only the methods that have ADFS-specific behavior
Files to modify
cibseven-webclient-core/src/main/java/org/cibseven/webapp/auth/KeycloakUserProvider.java→ rename toOAuth2UserProvider.java- Create new
cibseven-webclient-core/src/main/java/org/cibseven/webapp/auth/KeycloakUserProvider.java(deprecated wrapper)cibseven-webclient-core/src/main/java/org/cibseven/webapp/auth/AdfsUserProvider.java→ refactor to extendOAuth2UserProvider
This pull request was created as a result of the following prompt from Copilot chat.
Problem
KeycloakUserProvider.javaandAdfsUserProvider.javaincibseven-webclient-core/src/main/java/org/cibseven/webapp/auth/are both OAuth2 implementations with significant code duplication. KeycloakUserProvider is essentially a generic OAuth2 provider, while AdfsUserProvider has some ADFS-specific extensions.Required Changes
1. Create
OAuth2UserProvider.java
- Rename/move the current
KeycloakUserProvider.javatoOAuth2UserProvider.javain the same directory- This should be the generic OAuth2/SSO implementation that other providers extend
- Keep all the existing functionality (token caching, user info endpoint, introspection, etc.)
2. Create new
KeycloakUserProvider.java
- Create a new
KeycloakUserProvider.javathat extendsOAuth2UserProvider- The class should be essentially empty (just extends the parent)
- Add
@Deprecatedannotation with the message:"Use OAuth2UserProvider instead"3. Refactor
AdfsUserProvider.java
- Make it extend
OAuth2UserProviderinstead ofBaseUserProvider<SSOLogin>- Keep the ADFS-specific functionality that differs from the generic OAuth2 implementation:
domainproperty for appending domain to usernameinfoInIdTokenpropertytechnicalUserIdproperty- Password login flow (
ssoHelper.passwordLogin())- Technical user handling in
parse()method usingappidclaim- Remove all duplicate code that is now inherited from
OAuth2UserProvider- Add
@Deprecatedannotation with the message:"Use OAuth2UserProvider instead"- Override only the methods that have ADFS-specific behavior
Files to modify
cibseven-webclient-core/src/main/java/org/cibseven/webapp/auth/KeycloakUserProvider.java→ rename toOAuth2UserProvider.java- Create new
cibseven-webclient-core/src/main/java/org/cibseven/webapp/auth/KeycloakUserProvider.java(deprecated wrapper)cibseven-webclient-core/src/main/java/org/cibseven/webapp/auth/AdfsUserProvider.java→ refactor to extendOAuth2UserProvider