From ecaf730eb1617d4a6331a219631dd37df18b503a Mon Sep 17 00:00:00 2001 From: Matt Raible Date: Tue, 11 Aug 2026 09:34:16 -0600 Subject: [PATCH 1/7] Replace OpenID 2.0 with OAuth 2.0/OIDC login via Spring Security Replace the obsolete OpenID 2.0 authentication (removed in Spring Security 6) with modern OAuth 2.0/OIDC login using spring-security-oauth2-client. OIDC providers are configured via roller-custom.properties with issuer discovery. The existing openIdUrl column is reused to store OIDC subjects (formatted as issuer#sub), avoiding schema changes. New OIDC users are redirected to registration with claims pre-populated; returning users go straight to the menu. AuthMethod enum values updated from OPENID/DB_OPENID to OIDC/DB_OIDC. --- app/pom.xml | 12 ++ .../roller/weblogger/config/AuthMethod.java | 4 +- .../weblogger/ui/core/RollerSession.java | 12 ++ .../RollerClientRegistrationRepository.java | 121 ++++++++++++++++ .../security/RollerOAuth2SuccessHandler.java | 76 ++++++++++ .../core/security/RollerOidcUserService.java | 84 +++++++++++ .../weblogger/ui/struts2/admin/UserEdit.java | 10 +- .../weblogger/ui/struts2/core/Login.java | 44 ++++-- .../weblogger/ui/struts2/core/Profile.java | 8 +- .../weblogger/ui/struts2/core/Register.java | 32 ++++- .../resources/ApplicationResources.properties | 30 ++-- .../roller/weblogger/config/roller.properties | 33 +++-- .../main/webapp/WEB-INF/jsps/core/Login.jsp | 72 +++------- app/src/main/webapp/WEB-INF/security.xml | 16 +++ ...ollerClientRegistrationRepositoryTest.java | 111 +++++++++++++++ .../RollerOAuth2SuccessHandlerTest.java | 134 ++++++++++++++++++ .../security/RollerOidcUserServiceTest.java | 69 +++++++++ 17 files changed, 758 insertions(+), 110 deletions(-) create mode 100644 app/src/main/java/org/apache/roller/weblogger/ui/core/security/RollerClientRegistrationRepository.java create mode 100644 app/src/main/java/org/apache/roller/weblogger/ui/core/security/RollerOAuth2SuccessHandler.java create mode 100644 app/src/main/java/org/apache/roller/weblogger/ui/core/security/RollerOidcUserService.java create mode 100644 app/src/test/java/org/apache/roller/weblogger/ui/core/security/RollerClientRegistrationRepositoryTest.java create mode 100644 app/src/test/java/org/apache/roller/weblogger/ui/core/security/RollerOAuth2SuccessHandlerTest.java create mode 100644 app/src/test/java/org/apache/roller/weblogger/ui/core/security/RollerOidcUserServiceTest.java diff --git a/app/pom.xml b/app/pom.xml index 357120e671..3eef4a04de 100644 --- a/app/pom.xml +++ b/app/pom.xml @@ -485,6 +485,18 @@ limitations under the License. + + org.springframework.security + spring-security-oauth2-client + ${spring.security.version} + + + + org.springframework.security + spring-security-oauth2-jose + ${spring.security.version} + + diff --git a/app/src/main/java/org/apache/roller/weblogger/config/AuthMethod.java b/app/src/main/java/org/apache/roller/weblogger/config/AuthMethod.java index 3c4fa58af1..4a46deba76 100644 --- a/app/src/main/java/org/apache/roller/weblogger/config/AuthMethod.java +++ b/app/src/main/java/org/apache/roller/weblogger/config/AuthMethod.java @@ -20,8 +20,8 @@ public enum AuthMethod { ROLLERDB("db"), LDAP("ldap"), - OPENID("openid"), - DB_OPENID("db-openid"), + OIDC("oidc"), + DB_OIDC("db-oidc"), CMA("cma"); private final String propertyName; diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerSession.java b/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerSession.java index 959cc3e908..f02fba54b6 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerSession.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerSession.java @@ -34,6 +34,9 @@ import org.apache.roller.weblogger.business.UserManager; import org.apache.roller.weblogger.pojos.User; import org.apache.roller.weblogger.ui.core.security.AutoProvision; +import org.apache.roller.weblogger.ui.core.security.RollerOidcUserService; +import org.springframework.security.oauth2.client.authentication.OAuth2AuthenticationToken; +import org.springframework.security.oauth2.core.oidc.user.OidcUser; /** @@ -90,6 +93,15 @@ public static RollerSession getRollerSession(HttpServletRequest request) { UserManager umgr = WebloggerFactory.getWeblogger().getUserManager(); User user = umgr.getUserByUserName(principal.getName()); + // For OIDC-authenticated users, look up by OIDC subject + if (user == null && principal instanceof OAuth2AuthenticationToken oauthToken) { + Object oauthPrincipal = oauthToken.getPrincipal(); + if (oauthPrincipal instanceof OidcUser oidcUser) { + String oidcSubject = RollerOidcUserService.toOidcSubject(oidcUser); + user = umgr.getUserByOpenIdUrl(oidcSubject); + } + } + // try one time to auto-provision, only happens if user==null // which means installation has LDAP enabled in security.xml if (user == null && WebloggerConfig.getBooleanProperty("users.ldap.autoProvision.enabled")) { diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/core/security/RollerClientRegistrationRepository.java b/app/src/main/java/org/apache/roller/weblogger/ui/core/security/RollerClientRegistrationRepository.java new file mode 100644 index 0000000000..6f55ddbcc5 --- /dev/null +++ b/app/src/main/java/org/apache/roller/weblogger/ui/core/security/RollerClientRegistrationRepository.java @@ -0,0 +1,121 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. The ASF licenses this file to You + * under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. For additional information regarding + * copyright in this work, please see the NOTICE file in the top level + * directory of this distribution. + */ +package org.apache.roller.weblogger.ui.core.security; + +import java.util.Collections; +import java.util.Enumeration; +import java.util.Iterator; +import java.util.LinkedHashMap; +import java.util.Map; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.apache.roller.weblogger.config.WebloggerConfig; +import org.springframework.security.oauth2.client.registration.ClientRegistration; +import org.springframework.security.oauth2.client.registration.ClientRegistrationRepository; +import org.springframework.security.oauth2.client.registration.ClientRegistrations; +import org.springframework.security.oauth2.core.AuthorizationGrantType; + +/** + * Builds OAuth2/OIDC client registrations from Roller properties. + * + *

Properties follow the pattern: + * 

+ * oidc.{registrationId}.client-id=...
+ * oidc.{registrationId}.client-secret=...
+ * oidc.{registrationId}.issuer-uri=...
+ * oidc.{registrationId}.client-name=...  (optional, defaults to registrationId)
+ * oidc.{registrationId}.scope=openid,profile,email  (optional)
+ *
+ */ +public class RollerClientRegistrationRepository implements ClientRegistrationRepository, Iterable { + + private static final Log log = LogFactory.getLog(RollerClientRegistrationRepository.class); + private static final String PREFIX = "oidc."; + + private final Map registrations; + + public RollerClientRegistrationRepository() { + this.registrations = buildRegistrations(); + if (!registrations.isEmpty()) { + log.info("Configured OIDC providers: " + registrations.keySet()); + } + } + + @Override + public ClientRegistration findByRegistrationId(String registrationId) { + return registrations.get(registrationId); + } + + @Override + public Iterator iterator() { + return registrations.values().iterator(); + } + + public Map getRegistrations() { + return Collections.unmodifiableMap(registrations); + } + + private Map buildRegistrations() { + Map registrationIds = new LinkedHashMap<>(); + + Enumeration keys = WebloggerConfig.keys(); + while (keys.hasMoreElements()) { + String key = (String) keys.nextElement(); + if (key.startsWith(PREFIX) && key.endsWith(".client-id")) { + String id = key.substring(PREFIX.length(), key.length() - ".client-id".length()); + registrationIds.put(id, WebloggerConfig.getProperty(key)); + } + } + + Map result = new LinkedHashMap<>(); + for (Map.Entry entry : registrationIds.entrySet()) { + String id = entry.getKey(); + String clientId = entry.getValue(); + String clientSecret = WebloggerConfig.getProperty(PREFIX + id + ".client-secret"); + String issuerUri = WebloggerConfig.getProperty(PREFIX + id + ".issuer-uri"); + String clientName = WebloggerConfig.getProperty(PREFIX + id + ".client-name", id); + String scopeStr = WebloggerConfig.getProperty(PREFIX + id + ".scope", "openid,profile,email"); + + if (clientId == null || clientId.isBlank() || issuerUri == null || issuerUri.isBlank()) { + log.warn("Skipping OIDC registration '" + id + "': client-id and issuer-uri are required"); + continue; + } + + try { + ClientRegistration.Builder builder = ClientRegistrations.fromIssuerLocation(issuerUri) + .registrationId(id) + .clientId(clientId) + .clientName(clientName) + .authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE) + .scope(scopeStr.split(",")); + + if (clientSecret != null && !clientSecret.isBlank()) { + builder.clientSecret(clientSecret); + } + + result.put(id, builder.build()); + log.info("Registered OIDC provider: " + id + " (issuer: " + issuerUri + ")"); + } catch (Exception e) { + log.error("Failed to configure OIDC provider '" + id + "' (issuer: " + issuerUri + "): " + e.getMessage()); + } + } + + return result; + } +} diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/core/security/RollerOAuth2SuccessHandler.java b/app/src/main/java/org/apache/roller/weblogger/ui/core/security/RollerOAuth2SuccessHandler.java new file mode 100644 index 0000000000..4ea6e76f94 --- /dev/null +++ b/app/src/main/java/org/apache/roller/weblogger/ui/core/security/RollerOAuth2SuccessHandler.java @@ -0,0 +1,76 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. The ASF licenses this file to You + * under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. For additional information regarding + * copyright in this work, please see the NOTICE file in the top level + * directory of this distribution. + */ +package org.apache.roller.weblogger.ui.core.security; + +import java.io.IOException; + +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import jakarta.servlet.http.HttpSession; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.apache.roller.weblogger.business.WebloggerFactory; +import org.apache.roller.weblogger.pojos.User; +import org.springframework.security.core.Authentication; +import org.springframework.security.oauth2.core.oidc.user.OidcUser; +import org.springframework.security.web.authentication.AuthenticationSuccessHandler; + +/** + * After a successful OIDC login, checks whether the authenticated user already + * has a Roller account. Existing users are sent to the main menu; new users + * are redirected to the registration page with their OIDC claims stored in + * the session for pre-population. + */ +public class RollerOAuth2SuccessHandler implements AuthenticationSuccessHandler { + + private static final Log log = LogFactory.getLog(RollerOAuth2SuccessHandler.class); + + public static final String OIDC_USER_ATTR = "oidcUser"; + public static final String OIDC_SUBJECT_ATTR = "oidcSubject"; + + @Override + public void onAuthenticationSuccess(HttpServletRequest request, + HttpServletResponse response, + Authentication authentication) throws IOException, ServletException { + + OidcUser oidcUser = (OidcUser) authentication.getPrincipal(); + String oidcSubject = RollerOidcUserService.toOidcSubject(oidcUser); + String contextPath = request.getContextPath(); + + try { + if (WebloggerFactory.isBootstrapped()) { + User rollerUser = WebloggerFactory.getWeblogger().getUserManager() + .getUserByOpenIdUrl(oidcSubject); + + if (rollerUser != null) { + response.sendRedirect(contextPath + "/roller-ui/menu.rol"); + return; + } + } + } catch (Exception e) { + log.error("Error checking OIDC user in Roller", e); + } + + HttpSession session = request.getSession(); + session.setAttribute(OIDC_USER_ATTR, oidcUser); + session.setAttribute(OIDC_SUBJECT_ATTR, oidcSubject); + response.sendRedirect(contextPath + "/roller-ui/register.rol"); + } +} diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/core/security/RollerOidcUserService.java b/app/src/main/java/org/apache/roller/weblogger/ui/core/security/RollerOidcUserService.java new file mode 100644 index 0000000000..380b9be002 --- /dev/null +++ b/app/src/main/java/org/apache/roller/weblogger/ui/core/security/RollerOidcUserService.java @@ -0,0 +1,84 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. The ASF licenses this file to You + * under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. For additional information regarding + * copyright in this work, please see the NOTICE file in the top level + * directory of this distribution. + */ +package org.apache.roller.weblogger.ui.core.security; + +import java.util.ArrayList; +import java.util.List; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.apache.roller.weblogger.business.UserManager; +import org.apache.roller.weblogger.business.WebloggerFactory; +import org.apache.roller.weblogger.pojos.User; +import org.springframework.security.core.GrantedAuthority; +import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.security.oauth2.client.oidc.userinfo.OidcUserRequest; +import org.springframework.security.oauth2.client.oidc.userinfo.OidcUserService; +import org.springframework.security.oauth2.client.userinfo.OAuth2UserService; +import org.springframework.security.oauth2.core.OAuth2AuthenticationException; +import org.springframework.security.oauth2.core.oidc.user.DefaultOidcUser; +import org.springframework.security.oauth2.core.oidc.user.OidcUser; + +/** + * Bridges OIDC-authenticated users to Roller's user store. If the OIDC subject + * matches an existing Roller user (via the openIdUrl/openid_url column), the + * returned OidcUser carries that user's Roller authorities. Otherwise, the + * default OIDC scopes are returned and the success handler redirects to + * registration. + */ +public class RollerOidcUserService implements OAuth2UserService { + + private static final Log log = LogFactory.getLog(RollerOidcUserService.class); + private final OidcUserService delegate = new OidcUserService(); + + @Override + public OidcUser loadUser(OidcUserRequest userRequest) throws OAuth2AuthenticationException { + OidcUser oidcUser = delegate.loadUser(userRequest); + + if (!WebloggerFactory.isBootstrapped()) { + return oidcUser; + } + + String oidcSubject = toOidcSubject(oidcUser); + + try { + UserManager umgr = WebloggerFactory.getWeblogger().getUserManager(); + User rollerUser = umgr.getUserByOpenIdUrl(oidcSubject); + + if (rollerUser != null && rollerUser.getEnabled()) { + List authorities = new ArrayList<>(); + for (String role : umgr.getRoles(rollerUser)) { + authorities.add(new SimpleGrantedAuthority(role)); + } + return new DefaultOidcUser(authorities, oidcUser.getIdToken(), oidcUser.getUserInfo()); + } + } catch (Exception e) { + log.error("Error looking up Roller user for OIDC subject: " + oidcSubject, e); + } + + return oidcUser; + } + + /** + * Formats the OIDC issuer and subject as {@code issuer#sub} for storage + * in the User.openIdUrl column. + */ + public static String toOidcSubject(OidcUser oidcUser) { + return oidcUser.getIssuer().toString() + "#" + oidcUser.getSubject(); + } +} diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/admin/UserEdit.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/admin/UserEdit.java index 70878ecf82..46338f5fe9 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/admin/UserEdit.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/admin/UserEdit.java @@ -143,7 +143,7 @@ public String save() { if (!hasActionErrors()) { getBean().copyTo(user); - if (authMethod == AuthMethod.DB_OPENID) { + if (authMethod == AuthMethod.DB_OIDC) { if (StringUtils.isEmpty(user.getPassword()) && StringUtils.isEmpty(bean.getPassword()) && StringUtils.isEmpty(bean.getOpenIdUrl())) { @@ -157,8 +157,8 @@ public String save() { } // User.password does not allow null, so generate one - if (authMethod.equals(AuthMethod.OPENID) || - (authMethod.equals(AuthMethod.DB_OPENID) && !StringUtils.isEmpty(bean.getOpenIdUrl()))) { + if (authMethod.equals(AuthMethod.OIDC) || + (authMethod.equals(AuthMethod.DB_OIDC) && !StringUtils.isEmpty(bean.getOpenIdUrl()))) { String randomString = RandomStringUtils.randomAlphanumeric(255); user.resetPassword(randomString); } @@ -243,7 +243,7 @@ private void myValidate() { addError("error.add.user.badUserName"); } if ((authMethod == AuthMethod.ROLLERDB || - (authMethod == AuthMethod.DB_OPENID && StringUtils.isEmpty(getBean().getOpenIdUrl()))) + (authMethod == AuthMethod.DB_OIDC && StringUtils.isEmpty(getBean().getOpenIdUrl()))) && StringUtils.isEmpty(getBean().getPassword())) { addError("error.add.user.missingPassword"); } @@ -253,7 +253,7 @@ private void myValidate() { addError("userAdmin.error.userNotFound"); } } - if ((authMethod == AuthMethod.OPENID) && StringUtils.isEmpty(getBean().getOpenIdUrl())) { + if ((authMethod == AuthMethod.OIDC) && StringUtils.isEmpty(getBean().getOpenIdUrl())) { addError("userRegister.error.missingOpenID"); } diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/Login.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/Login.java index 0cd3a83860..372599b1a9 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/Login.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/Login.java @@ -18,10 +18,15 @@ package org.apache.roller.weblogger.ui.struts2.core; +import java.util.ArrayList; +import java.util.Enumeration; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + import org.apache.roller.weblogger.config.AuthMethod; import org.apache.roller.weblogger.config.WebloggerConfig; import org.apache.roller.weblogger.ui.struts2.util.UIAction; -import org.apache.struts2.convention.annotation.AllowedMethods; /** * Handle user logins. @@ -36,7 +41,7 @@ */ // TODO: make this work @AllowedMethods({"execute"}) public class Login extends UIAction { - + private String error = null; private AuthMethod authMethod = WebloggerConfig.getAuthMethod(); @@ -50,7 +55,7 @@ public Login() { public boolean isUserRequired() { return false; } - + // override default security, we do not require an action weblog @Override public boolean isWeblogRequired() { @@ -61,22 +66,37 @@ public String getAuthMethod() { return authMethod.name(); } + public List> getOidcProviders() { + List> providers = new ArrayList<>(); + Enumeration keys = WebloggerConfig.keys(); + while (keys.hasMoreElements()) { + String key = (String) keys.nextElement(); + if (key.startsWith("oidc.") && key.endsWith(".client-id")) { + String id = key.substring("oidc.".length(), key.length() - ".client-id".length()); + String clientId = WebloggerConfig.getProperty(key); + if (clientId != null && !clientId.isBlank()) { + Map provider = new LinkedHashMap<>(); + provider.put("id", id); + provider.put("name", WebloggerConfig.getProperty("oidc." + id + ".client-name", id)); + providers.add(provider); + } + } + } + return providers; + } + @Override public String execute() { - + // set action error message if there was login error if(getError() != null) { - if (authMethod == AuthMethod.OPENID) { - addError("error.unmatched.openid"); - } else { - addError("error.password.mismatch"); - } + addError("error.password.mismatch"); } - + return SUCCESS; } - + public String getError() { return error; } @@ -84,5 +104,5 @@ public String getError() { public void setError(String error) { this.error = error; } - + } diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/Profile.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/Profile.java index 6f83a0ddb6..9ea0a37686 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/Profile.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/Profile.java @@ -91,7 +91,7 @@ public String save() { } } - if (authMethod == AuthMethod.DB_OPENID) { + if (authMethod == AuthMethod.DB_OIDC) { if (StringUtils.isEmpty(existingUser.getPassword()) && StringUtils.isEmpty(bean.getPasswordText()) && StringUtils.isEmpty(bean.getOpenIdUrl())) { @@ -105,8 +105,8 @@ public String save() { } // User.password does not allow null, so generate one - if (authMethod.equals(AuthMethod.OPENID) || - (authMethod.equals(AuthMethod.DB_OPENID) && !StringUtils.isEmpty(bean.getOpenIdUrl()))) { + if (authMethod.equals(AuthMethod.OIDC) || + (authMethod.equals(AuthMethod.DB_OIDC) && !StringUtils.isEmpty(bean.getOpenIdUrl()))) { String randomString = RandomStringUtils.randomAlphanumeric(255); existingUser.resetPassword(randomString); } @@ -138,7 +138,7 @@ public void myValidate() { if (!StringUtils.equals(getBean().getPasswordText(), getBean().getPasswordConfirm())) { addError("userRegister.error.mismatchedPasswords"); } - if (authMethod == AuthMethod.OPENID) { + if (authMethod == AuthMethod.OIDC) { addError("userRegister.error.missingOpenID"); } } else { diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/Register.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/Register.java index 5ddf04337e..f6e0c415cf 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/Register.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/Register.java @@ -35,7 +35,9 @@ import org.apache.roller.weblogger.pojos.User; import org.apache.roller.weblogger.ui.core.RollerSession; import org.apache.roller.weblogger.ui.core.security.CustomUserRegistry; +import org.apache.roller.weblogger.ui.core.security.RollerOAuth2SuccessHandler; import org.apache.roller.weblogger.ui.struts2.util.UIAction; +import org.springframework.security.oauth2.core.oidc.user.OidcUser; import org.apache.roller.weblogger.util.MailUtil; import org.apache.struts2.ActionContext; import org.apache.struts2.convention.annotation.AllowedMethods; @@ -159,6 +161,27 @@ public String execute() { getBean().setScreenName(getServletRequest().getUserPrincipal().getName()); } } + + // Pre-populate from OIDC claims if arriving from OIDC login + OidcUser oidcUser = (OidcUser) getServletRequest().getSession() + .getAttribute(RollerOAuth2SuccessHandler.OIDC_USER_ATTR); + if (oidcUser != null) { + if (oidcUser.getEmail() != null) { + getBean().setEmailAddress(oidcUser.getEmail()); + } + if (oidcUser.getFullName() != null) { + getBean().setFullName(oidcUser.getFullName()); + } + if (oidcUser.getPreferredUsername() != null) { + getBean().setUserName(oidcUser.getPreferredUsername()); + getBean().setScreenName(oidcUser.getPreferredUsername()); + } + String oidcSubject = (String) getServletRequest().getSession() + .getAttribute(RollerOAuth2SuccessHandler.OIDC_SUBJECT_ATTR); + if (oidcSubject != null) { + getBean().setOpenIdUrl(oidcSubject); + } + } } catch (Exception ex) { log.error("Error reading SSO user data", ex); @@ -336,7 +359,8 @@ public String activate() { public void myValidate() { // if using external auth, we don't want to error on empty password/username from HTML form. - boolean usingSSO = authMethod == AuthMethod.LDAP || authMethod == AuthMethod.CMA; + boolean usingSSO = authMethod == AuthMethod.LDAP || authMethod == AuthMethod.CMA + || authMethod == AuthMethod.OIDC; if (usingSSO) { // store an unused marker in the Roller DB for the passphrase in // the LDAP or CMA cases, as actual passwords are stored externally @@ -364,9 +388,9 @@ public void myValidate() { return; } - // User.password does not allow null, so generate one - if (getAuthMethod().equals(AuthMethod.OPENID.name()) || - (getAuthMethod().equals(AuthMethod.DB_OPENID.name()) && !StringUtils.isEmpty(getBean().getOpenIdUrl()))) { + // User.password does not allow null, so generate one for OIDC users + if (getAuthMethod().equals(AuthMethod.OIDC.name()) || + (getAuthMethod().equals(AuthMethod.DB_OIDC.name()) && !StringUtils.isEmpty(getBean().getOpenIdUrl()))) { String randomString = RandomStringUtils.randomAlphanumeric(255); getBean().setPasswordText(randomString); getBean().setPasswordConfirm(randomString); diff --git a/app/src/main/resources/ApplicationResources.properties b/app/src/main/resources/ApplicationResources.properties index 6a0abebdce..5041ab909a 100644 --- a/app/src/main/resources/ApplicationResources.properties +++ b/app/src/main/resources/ApplicationResources.properties @@ -463,6 +463,7 @@ error.upload.forbiddenFile=File {0} content-type {1} not allowed error.general=ERROR: Unexpected Exception [{0}] has been logged. error.password.mismatch=Wrong username and password combination error.unmatched.openid=Unknown or invalid OpenID URL +error.oidc.login=OIDC authentication failed error.trackback=Error sending trackback. Possible cause: incorrect \ trackback URL. {0} @@ -662,12 +663,11 @@ issued during the upgrade process: loginPage.title=Welcome to Roller loginPage.prompt=Please login -loginPage.openIdPrompt=Login with OpenID -loginPage.openIdHybridPrompt=Or with username +loginPage.oidcPrompt=Sign in with your identity provider +loginPage.dbOidcPrompt=Or sign in with username and password +loginPage.signInWith=Sign in with {0} loginPage.userName=Username loginPage.password=Password -loginPage.openID=OpenID username -loginPage.loginOpenID=Login loginPage.rememberMe=Remember Me (up to two weeks) loginPage.login=Login loginPage.reset=Reset @@ -1347,7 +1347,7 @@ userSettings.passwordConfirm=Password (Confirm) userSettings.email=Email userSettings.locale=Locale userSettings.timeZone=Timezone -userSettings.openIdUrl=OpenID URL +userSettings.openIdUrl=Federated Identity userSettings.tip.username=Usernames can''t be changed. # ----------------------------------------------------------------- Your profile (profile.jsp) @@ -1386,13 +1386,13 @@ userAdmin.newEntry=New Entry userAdmin.editEntries=Edit Entries userAdmin.manage=Manage -userAdmin.noPasswordForOpenID=Leave password field(s) blank if providing an OpenID. +userAdmin.noPasswordForOidc=Leave password field(s) blank for OIDC-authenticated users. userAdmin.tip.screenName=User''s screen name (with no HTML). userAdmin.tip.fullName=User''s full name (with no HTML). userAdmin.tip.userName=A short one-word username for the user account. \ Please limit it to simple ASCII alphanumeric characters (a-z, A-Z and 0-9), \ and do not use HTML. -userAdmin.tip.openIdUrl=Open ID identifier (in the form of a URL). +userAdmin.tip.openIdUrl=Federated identity (OIDC issuer and subject). userAdmin.tip.password=User''s password. Fill in only to change it to what you enter. userAdmin.tip.email=Valid email address needed for automated notification. userAdmin.tip.enabled=Disabled users are unable to login to Roller. @@ -1436,21 +1436,19 @@ may disable your account if he/she cannot reach you via email. userRegister.heading.authentication=How will you be authenticated? -userRegister.tip.openid.disabled=Enter a password to be used when you login \ +userRegister.tip.password.db=Enter a password to be used when you login \ and confirm that password by entering it a second time. -userRegister.tip.openid.hybrid=You can choose to login via username/password or \ -OpenID. If you choose the latter, leave \ +userRegister.tip.password.dbOidc=You can choose to login via username/password or \ +your identity provider. If you authenticated via your identity provider, leave \ the password fields blank. -userRegister.tip.openid.only=This site uses only OpenID for logins, so please \ -specify your OpenID identifier below. For more information about OpenID see \ -http://openid.net. +userRegister.tip.password.oidc=This site uses an external identity provider for logins. \ +Your account will be linked to your identity provider automatically. -userRegister.tip.password=Your password. -userSettings.tip.password=Your password. Fill-in only if you wish to change it. +userRegister.tip.password=Your password. +userSettings.tip.password=Your password. Fill-in only if you wish to change it. userRegister.tip.passwordConfirm=Confirm your password. -userRegister.tip.openIdUrl=Your OpenID identifier (in the form of a URL). userRegister.heading.locale=What are your locale and timezone settings? diff --git a/app/src/main/resources/org/apache/roller/weblogger/config/roller.properties b/app/src/main/resources/org/apache/roller/weblogger/config/roller.properties index d73e7f9ca1..9d6b56ccfe 100644 --- a/app/src/main/resources/org/apache/roller/weblogger/config/roller.properties +++ b/app/src/main/resources/org/apache/roller/weblogger/config/roller.properties @@ -322,21 +322,14 @@ cache.salt.timeout=3600 # User management and security settings #----------------------------------------------------------------------------- -# Top-level authentication declaration for Apache Roller. Introduced in Roller 5.1, -# replaces authentication.cma.enabled, authentication.openid, and users.sso.enabled -# from earlier versions. Must be one of the following values: +# Top-level authentication declaration for Apache Roller. Introduced in Roller 5.1. +# Must be one of the following values: # db: use Roller database to store usernames and passwords # ldap: use external LDAP to authenticate (must configure Roller security.xml, # see Roller Wiki for more details) -# openid: users must use OpenID to authenticate -# db-openid: users may choose to authenticate via Roller DB or OpenID but not both. -# Trickier to implement so may not work as well as above methods, test before using. -# cma: container-managed authentication (e.g., Tomcat tomcat-users.xml file). Currently -# unusable, not implemented. -# Note that if you override this value in your roller-custom.properties file, you will also -# need to re-configure the security.xml file in the Roller WAR (under WEB-INF) to use the -# new security method -- check the comments in that latter file for instructions on -# how to do so. +# oidc: users must use an OIDC provider to authenticate (configure oidc.* properties below) +# db-oidc: users may choose to authenticate via Roller DB or OIDC provider. +# cma: container-managed authentication (e.g., Tomcat tomcat-users.xml file). authentication.method=db # Enables HTTPS for login page only @@ -345,10 +338,22 @@ securelogin.enabled=false # With this settings, all users will have HTML posts sanitized. weblogAdminsUntrusted=true -# Empty value used for passphrase in roller_user table when LDAP or CMA used; -# openid presently generates a random (long) password string instead. +# Empty value used for passphrase in roller_user table when LDAP, CMA, or OIDC used. users.passwords.externalAuthValue= +# OIDC provider configuration. To configure a provider, set all three required +# properties using the pattern oidc.{registrationId}.{property}. +# Multiple providers can be configured with different registration IDs. +# Required: client-id, client-secret, issuer-uri +# Optional: client-name (display name, defaults to registrationId), +# scope (comma-separated, defaults to openid,profile,email) +# +# Example for Keycloak: +# oidc.keycloak.client-id=roller +# oidc.keycloak.client-secret=your-client-secret +# oidc.keycloak.issuer-uri=http://localhost:9080/realms/roller +# oidc.keycloak.client-name=Keycloak + # Password security settings passwds.encryption.enabled=true passwds.encryption.algorithm=bcrypt diff --git a/app/src/main/webapp/WEB-INF/jsps/core/Login.jsp b/app/src/main/webapp/WEB-INF/jsps/core/Login.jsp index 0a3918e990..232ae2cd09 100644 --- a/app/src/main/webapp/WEB-INF/jsps/core/Login.jsp +++ b/app/src/main/webapp/WEB-INF/jsps/core/Login.jsp @@ -33,36 +33,33 @@ } %> - - - -
- + +
- +
- - + + " + class="btn btn-default" style="margin-bottom: 5px;"> + + + + +
- - - - - +
- +
"/>" onsubmit="saveUsername(this)">
- - + + @@ -102,44 +99,14 @@ \ No newline at end of file + diff --git a/app/src/main/webapp/WEB-INF/security.xml b/app/src/main/webapp/WEB-INF/security.xml index 68a6644b23..e0620db5b5 100644 --- a/app/src/main/webapp/WEB-INF/security.xml +++ b/app/src/main/webapp/WEB-INF/security.xml @@ -49,6 +49,12 @@ + + + @@ -105,6 +111,16 @@ + + + + + + + diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerContext.java b/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerContext.java index 9413bc8caf..65f11a1b0c 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerContext.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerContext.java @@ -45,6 +45,7 @@ import org.apache.roller.weblogger.ui.core.plugins.UIPluginManager; import org.apache.roller.weblogger.ui.core.plugins.UIPluginManagerImpl; import org.apache.roller.weblogger.ui.core.security.AutoProvision; +import org.apache.roller.weblogger.ui.core.security.RollerClientRegistrationRepository; import org.apache.roller.weblogger.util.Reflection; import org.apache.roller.weblogger.util.cache.CacheManager; import org.apache.velocity.runtime.RuntimeSingleton; @@ -309,7 +310,10 @@ private DelegatingPasswordEncoder createPasswordEncoder() { // supported encoders encoders.put("bcrypt", new BCryptPasswordEncoder()); - encoders.put("pbkdf2", Pbkdf2PasswordEncoder.defaultsForSpringSecurity_v5_8()); + // pbkdf2 stores only salt+hash, so its parameters must stay as they were + // when existing passwords were encoded or those passwords stop verifying. + // scrypt and argon2 encode their parameters, so they can take v5_8. + encoders.put("pbkdf2", Pbkdf2PasswordEncoder.defaultsForSpringSecurity_v5_5()); // provided by bouncy castle dependency encoders.put("scrypt", SCryptPasswordEncoder.defaultsForSpringSecurity_v5_8()); encoders.put("argon2", Argon2PasswordEncoder.defaultsForSpringSecurity_v5_8()); @@ -340,6 +344,21 @@ private DelegatingPasswordEncoder createPasswordEncoder() { } + /** + * The OIDC client registrations declared in security.xml, or null when + * OIDC is not configured. + */ + public static RollerClientRegistrationRepository getClientRegistrationRepository() { + ApplicationContext ctx = + WebApplicationContextUtils.getRequiredWebApplicationContext(servletContext); + try { + return ctx.getBean("clientRegistrationRepository", RollerClientRegistrationRepository.class); + } catch (NoSuchBeanDefinitionException exc) { + log.debug("No clientRegistrationRepository bean in context", exc); + return null; + } + } + /** * Flush user from any caches maintained by security system. */ diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/core/security/RollerClientRegistrationRepository.java b/app/src/main/java/org/apache/roller/weblogger/ui/core/security/RollerClientRegistrationRepository.java index 6f55ddbcc5..5e3e96b2e9 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/core/security/RollerClientRegistrationRepository.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/core/security/RollerClientRegistrationRepository.java @@ -34,6 +34,9 @@ /** * Builds OAuth2/OIDC client registrations from Roller properties. * + *

OIDC discovery is deferred until first access so the identity provider + * does not need to be reachable during application startup. + * *

Properties follow the pattern: * 

  * oidc.{registrationId}.client-id=...
@@ -48,40 +51,63 @@ public class RollerClientRegistrationRepository implements ClientRegistrationRep
     private static final Log log = LogFactory.getLog(RollerClientRegistrationRepository.class);
     private static final String PREFIX = "oidc.";
 
-    private final Map registrations;
-
-    public RollerClientRegistrationRepository() {
-        this.registrations = buildRegistrations();
-        if (!registrations.isEmpty()) {
-            log.info("Configured OIDC providers: " + registrations.keySet());
-        }
-    }
+    private volatile Map registrations;
 
     @Override
     public ClientRegistration findByRegistrationId(String registrationId) {
-        return registrations.get(registrationId);
+        return getRegistrations().get(registrationId);
     }
 
     @Override
     public Iterator iterator() {
-        return registrations.values().iterator();
+        return getRegistrations().values().iterator();
     }
 
+    /**
+     * Discovery runs on first use and the result is cached, but only once every
+     * configured provider resolved. A provider that was unreachable is retried
+     * on the next call rather than being cached as permanently broken.
+     */
     public Map getRegistrations() {
-        return Collections.unmodifiableMap(registrations);
+        Map cached = registrations;
+        if (cached != null) {
+            return cached;
+        }
+        synchronized (this) {
+            if (registrations != null) {
+                return registrations;
+            }
+            Map built = buildRegistrations();
+            if (built.size() < configuredProviderIds().size()) {
+                return Collections.unmodifiableMap(built);
+            }
+            registrations = Collections.unmodifiableMap(built);
+            if (!registrations.isEmpty()) {
+                log.info("Configured OIDC providers: " + registrations.keySet());
+            }
+            return registrations;
+        }
     }
 
-    private Map buildRegistrations() {
+    /** Registration ids that have an {@code oidc..client-id} property set. */
+    static Map configuredProviderIds() {
         Map registrationIds = new LinkedHashMap<>();
-
         Enumeration keys = WebloggerConfig.keys();
         while (keys.hasMoreElements()) {
             String key = (String) keys.nextElement();
             if (key.startsWith(PREFIX) && key.endsWith(".client-id")) {
                 String id = key.substring(PREFIX.length(), key.length() - ".client-id".length());
-                registrationIds.put(id, WebloggerConfig.getProperty(key));
+                String clientId = WebloggerConfig.getProperty(key);
+                if (clientId != null && !clientId.isBlank()) {
+                    registrationIds.put(id, clientId);
+                }
             }
         }
+        return registrationIds;
+    }
+
+    private Map buildRegistrations() {
+        Map registrationIds = configuredProviderIds();
 
         Map result = new LinkedHashMap<>();
         for (Map.Entry entry : registrationIds.entrySet()) {
@@ -112,7 +138,7 @@ private Map buildRegistrations() {
                 result.put(id, builder.build());
                 log.info("Registered OIDC provider: " + id + " (issuer: " + issuerUri + ")");
             } catch (Exception e) {
-                log.error("Failed to configure OIDC provider '" + id + "' (issuer: " + issuerUri + "): " + e.getMessage());
+                log.error("Failed to configure OIDC provider '" + id + "' (issuer: " + issuerUri + ")", e);
             }
         }
 
diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/core/security/RollerOAuth2SuccessHandler.java b/app/src/main/java/org/apache/roller/weblogger/ui/core/security/RollerOAuth2SuccessHandler.java
index 4ea6e76f94..aa739f5845 100644
--- a/app/src/main/java/org/apache/roller/weblogger/ui/core/security/RollerOAuth2SuccessHandler.java
+++ b/app/src/main/java/org/apache/roller/weblogger/ui/core/security/RollerOAuth2SuccessHandler.java
@@ -22,55 +22,22 @@
 import jakarta.servlet.ServletException;
 import jakarta.servlet.http.HttpServletRequest;
 import jakarta.servlet.http.HttpServletResponse;
-import jakarta.servlet.http.HttpSession;
 
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
-import org.apache.roller.weblogger.business.WebloggerFactory;
-import org.apache.roller.weblogger.pojos.User;
 import org.springframework.security.core.Authentication;
-import org.springframework.security.oauth2.core.oidc.user.OidcUser;
 import org.springframework.security.web.authentication.AuthenticationSuccessHandler;
 
 /**
- * After a successful OIDC login, checks whether the authenticated user already
- * has a Roller account. Existing users are sent to the main menu; new users
- * are redirected to the registration page with their OIDC claims stored in
- * the session for pre-population.
+ * Sends OIDC-authenticated users to the main menu, the same landing page used
+ * by form login. The Roller account itself is resolved (and provisioned when
+ * new) by {@link RollerOidcUserService} before this handler runs.
  */
 public class RollerOAuth2SuccessHandler implements AuthenticationSuccessHandler {
 
-    private static final Log log = LogFactory.getLog(RollerOAuth2SuccessHandler.class);
-
-    public static final String OIDC_USER_ATTR = "oidcUser";
-    public static final String OIDC_SUBJECT_ATTR = "oidcSubject";
-
     @Override
     public void onAuthenticationSuccess(HttpServletRequest request,
                                         HttpServletResponse response,
                                         Authentication authentication) throws IOException, ServletException {
 
-        OidcUser oidcUser = (OidcUser) authentication.getPrincipal();
-        String oidcSubject = RollerOidcUserService.toOidcSubject(oidcUser);
-        String contextPath = request.getContextPath();
-
-        try {
-            if (WebloggerFactory.isBootstrapped()) {
-                User rollerUser = WebloggerFactory.getWeblogger().getUserManager()
-                        .getUserByOpenIdUrl(oidcSubject);
-
-                if (rollerUser != null) {
-                    response.sendRedirect(contextPath + "/roller-ui/menu.rol");
-                    return;
-                }
-            }
-        } catch (Exception e) {
-            log.error("Error checking OIDC user in Roller", e);
-        }
-
-        HttpSession session = request.getSession();
-        session.setAttribute(OIDC_USER_ATTR, oidcUser);
-        session.setAttribute(OIDC_SUBJECT_ATTR, oidcSubject);
-        response.sendRedirect(contextPath + "/roller-ui/register.rol");
+        response.sendRedirect(request.getContextPath() + "/roller-ui/menu.rol");
     }
 }
diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/core/security/RollerOidcUserService.java b/app/src/main/java/org/apache/roller/weblogger/ui/core/security/RollerOidcUserService.java
index 380b9be002..8ba9dd76a8 100644
--- a/app/src/main/java/org/apache/roller/weblogger/ui/core/security/RollerOidcUserService.java
+++ b/app/src/main/java/org/apache/roller/weblogger/ui/core/security/RollerOidcUserService.java
@@ -17,13 +17,21 @@
  */
 package org.apache.roller.weblogger.ui.core.security;
 
+import java.sql.Timestamp;
 import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.TimeZone;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+import org.apache.roller.util.UUIDGenerator;
 import org.apache.roller.weblogger.business.UserManager;
 import org.apache.roller.weblogger.business.WebloggerFactory;
+import org.apache.roller.weblogger.config.WebloggerConfig;
 import org.apache.roller.weblogger.pojos.User;
 import org.springframework.security.core.GrantedAuthority;
 import org.springframework.security.core.authority.SimpleGrantedAuthority;
@@ -31,15 +39,20 @@
 import org.springframework.security.oauth2.client.oidc.userinfo.OidcUserService;
 import org.springframework.security.oauth2.client.userinfo.OAuth2UserService;
 import org.springframework.security.oauth2.core.OAuth2AuthenticationException;
+import org.springframework.security.oauth2.core.OAuth2Error;
+import org.springframework.security.oauth2.core.oidc.OidcIdToken;
+import org.springframework.security.oauth2.core.oidc.OidcUserInfo;
 import org.springframework.security.oauth2.core.oidc.user.DefaultOidcUser;
 import org.springframework.security.oauth2.core.oidc.user.OidcUser;
 
 /**
- * Bridges OIDC-authenticated users to Roller's user store. If the OIDC subject
- * matches an existing Roller user (via the openIdUrl/openid_url column), the
- * returned OidcUser carries that user's Roller authorities. Otherwise, the
- * default OIDC scopes are returned and the success handler redirects to
- * registration.
+ * Bridges OIDC-authenticated users to Roller's user store.
+ *
+ * 
The OIDC subject (formatted as {@code issuer#sub}) is matched against the
+ * User.openIdUrl column. Users who authenticate for the first time are
+ * provisioned just-in-time from their OIDC claims. Either way the returned
+ * OidcUser carries the Roller roles as authorities, so authorization works on
+ * the very first request after login.
  */
 public class RollerOidcUserService implements OAuth2UserService {
 
@@ -48,10 +61,17 @@ public class RollerOidcUserService implements OAuth2UserService authorities = new ArrayList<>();
-                for (String role : umgr.getRoles(rollerUser)) {
-                    authorities.add(new SimpleGrantedAuthority(role));
-                }
-                return new DefaultOidcUser(authorities, oidcUser.getIdToken(), oidcUser.getUserInfo());
+            if (rollerUser == null) {
+                rollerUser = linkExistingUser(umgr, oidcUser, oidcSubject);
             }
+            if (rollerUser == null) {
+                rollerUser = provisionUser(umgr, oidcUser, oidcSubject);
+            }
+            if (!Boolean.TRUE.equals(rollerUser.getEnabled())) {
+                throw new OAuth2AuthenticationException(new OAuth2Error("user_disabled"),
+                        "Roller user is disabled: " + rollerUser.getUserName());
+            }
+
+            List authorities = new ArrayList<>();
+            for (String role : umgr.getRoles(rollerUser)) {
+                authorities.add(new SimpleGrantedAuthority(role));
+            }
+            return new RollerOidcUser(authorities, oidcUser.getIdToken(), oidcUser.getUserInfo(),
+                    rollerUser.getUserName());
+
+        } catch (OAuth2AuthenticationException e) {
+            throw e;
         } catch (Exception e) {
-            log.error("Error looking up Roller user for OIDC subject: " + oidcSubject, e);
+            log.error("Error resolving Roller user for OIDC subject: " + oidcSubject, e);
+            throw new OAuth2AuthenticationException(new OAuth2Error("user_resolution_failed"),
+                    "Could not resolve Roller user for OIDC subject: " + oidcSubject, e);
+        }
+    }
+
+    /**
+     * Adopts a pre-existing Roller account whose username matches the one
+     * asserted by the provider, which is how database users carry over when a
+     * site turns on OIDC. Linking requires the provider to have verified an
+     * email address matching the account, so that control of an unverified
+     * address at the provider cannot be used to take over a Roller account.
+     *
+     * @return the linked user, or null if there is no account to adopt
+     */
+    private User linkExistingUser(UserManager umgr, OidcUser oidcUser, String oidcSubject) throws Exception {
+        String username = usernameOf(oidcUser);
+        User existing = umgr.getUserByUserName(username);
+        if (existing == null) {
+            return null;
+        }
+
+        String email = oidcUser.getEmail();
+        boolean emailVerified = Boolean.TRUE.equals(oidcUser.getEmailVerified())
+                && email != null && email.equalsIgnoreCase(existing.getEmailAddress());
+
+        if (!emailVerified) {
+            throw new OAuth2AuthenticationException(new OAuth2Error("account_link_required"),
+                    "A Roller account named '" + username + "' already exists but is not linked to "
+                            + oidcSubject + ". An administrator must set its federated identity, or the"
+                            + " provider must assert a verified email address matching the account.");
         }
 
-        return oidcUser;
+        existing.setOpenIdUrl(oidcSubject);
+        umgr.saveUser(existing);
+        WebloggerFactory.getWeblogger().flush();
+        log.info("Linked existing Roller user '" + username + "' to OIDC subject " + oidcSubject);
+        return existing;
+    }
+
+    /**
+     * Creates a Roller account from the OIDC claims. The account is linked to
+     * the identity provider by subject, and gets a random password since it is
+     * never used for authentication.
+     */
+    private User provisionUser(UserManager umgr, OidcUser oidcUser, String oidcSubject) throws Exception {
+        String username = usernameOf(oidcUser);
+
+        // the identity provider decides who may sign in, so provisioning is a
+        // static config choice like users.ldap.autoProvision.enabled, not tied
+        // to the runtime form-registration toggle
+        if (!WebloggerConfig.getBooleanProperty("users.oidc.autoProvision.enabled")) {
+            throw new OAuth2AuthenticationException(new OAuth2Error("auto_provision_disabled"),
+                    "OIDC auto-provisioning is disabled; no Roller account exists for " + oidcSubject);
+        }
+
+        User user = new User();
+        user.setId(UUIDGenerator.generateUUID());
+        user.setUserName(username);
+
+        String fullName = oidcUser.getFullName();
+        if (fullName == null || fullName.isBlank()) {
+            fullName = username;
+        }
+        user.setFullName(fullName);
+        user.setScreenName(username);
+
+        String email = oidcUser.getEmail();
+        if (email == null || email.isBlank()) {
+            throw new OAuth2AuthenticationException(new OAuth2Error("missing_email"),
+                    "OIDC provider did not supply an email address for " + username);
+        }
+        user.setEmailAddress(email);
+
+        user.setOpenIdUrl(oidcSubject);
+        user.setPassword(UUIDGenerator.generateUUID());
+        user.setDateCreated(new Timestamp(System.currentTimeMillis()));
+        user.setLocale(Locale.getDefault().toString());
+        user.setTimeZone(TimeZone.getDefault().getID());
+        user.setEnabled(Boolean.TRUE);
+
+        // grants the "editor" role, and "admin" if this is the first user
+        umgr.addUser(user);
+
+        // flush before granting so the roles addUser() just created are visible
+        // to grantRole()'s duplicate check, which queries the database
+        WebloggerFactory.getWeblogger().flush();
+
+        Collection claimRoles = extractRoles(oidcUser);
+        if (claimRoles.contains("admin")) {
+            umgr.grantRole("admin", user);
+            WebloggerFactory.getWeblogger().flush();
+        }
+        log.info("Auto-provisioned OIDC user '" + username + "' from claims (roles: "
+                + claimRoles + ", subject: " + oidcSubject + ")");
+        return user;
+    }
+
+    private String usernameOf(OidcUser oidcUser) {
+        String username = oidcUser.getPreferredUsername();
+        return (username == null || username.isBlank()) ? oidcUser.getSubject() : username;
+    }
+
+    /**
+     * Reads role names from the token. Providers differ in where they put them,
+     * so both a flat "roles" claim and Keycloak's nested "realm_access.roles"
+     * are supported.
+     */
+    @SuppressWarnings("unchecked")
+    private Collection extractRoles(OidcUser oidcUser) {
+        Object roles = oidcUser.getClaim("roles");
+        if (roles instanceof Collection) {
+            return (Collection) roles;
+        }
+        Object realmAccess = oidcUser.getClaim("realm_access");
+        if (realmAccess instanceof Map) {
+            Object realmRoles = ((Map) realmAccess).get("roles");
+            if (realmRoles instanceof Collection) {
+                return (Collection) realmRoles;
+            }
+        }
+        return Collections.emptyList();
     }
 
     /**
@@ -81,4 +232,26 @@ public OidcUser loadUser(OidcUserRequest userRequest) throws OAuth2Authenticatio
     public static String toOidcSubject(OidcUser oidcUser) {
         return oidcUser.getIssuer().toString() + "#" + oidcUser.getSubject();
     }
+
+    /**
+     * An OidcUser named after its Roller account. Roller looks users up by
+     * principal name throughout (ParsedRequest, RoleAssignmentFilter, Struts
+     * actions), and an OidcUser's default name is the "sub" claim: an opaque
+     * provider ID that matches no Roller account.
+     */
+    private static class RollerOidcUser extends DefaultOidcUser {
+
+        private final String rollerUserName;
+
+        RollerOidcUser(Collection authorities,
+                OidcIdToken idToken, OidcUserInfo userInfo, String rollerUserName) {
+            super(authorities, idToken, userInfo);
+            this.rollerUserName = rollerUserName;
+        }
+
+        @Override
+        public String getName() {
+            return rollerUserName;
+        }
+    }
 }
diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/Login.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/Login.java
index 372599b1a9..304f9f95cf 100644
--- a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/Login.java
+++ b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/Login.java
@@ -19,14 +19,16 @@
 package org.apache.roller.weblogger.ui.struts2.core;
 
 import java.util.ArrayList;
-import java.util.Enumeration;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 
 import org.apache.roller.weblogger.config.AuthMethod;
 import org.apache.roller.weblogger.config.WebloggerConfig;
+import org.apache.roller.weblogger.ui.core.RollerContext;
+import org.apache.roller.weblogger.ui.core.security.RollerClientRegistrationRepository;
 import org.apache.roller.weblogger.ui.struts2.util.UIAction;
+import org.springframework.security.oauth2.client.registration.ClientRegistration;
 
 /**
  * Handle user logins.
@@ -66,21 +68,22 @@ public String getAuthMethod() {
         return authMethod.name();
     }
 
+    /**
+     * Providers to offer sign-in buttons for. Only registrations the repository
+     * could actually resolve are listed, so the page never advertises a provider
+     * whose discovery endpoint was unreachable.
+     */
     public List> getOidcProviders() {
         List> providers = new ArrayList<>();
-        Enumeration keys = WebloggerConfig.keys();
-        while (keys.hasMoreElements()) {
-            String key = (String) keys.nextElement();
-            if (key.startsWith("oidc.") && key.endsWith(".client-id")) {
-                String id = key.substring("oidc.".length(), key.length() - ".client-id".length());
-                String clientId = WebloggerConfig.getProperty(key);
-                if (clientId != null && !clientId.isBlank()) {
-                    Map provider = new LinkedHashMap<>();
-                    provider.put("id", id);
-                    provider.put("name", WebloggerConfig.getProperty("oidc." + id + ".client-name", id));
-                    providers.add(provider);
-                }
-            }
+        RollerClientRegistrationRepository repository = RollerContext.getClientRegistrationRepository();
+        if (repository == null) {
+            return providers;
+        }
+        for (ClientRegistration registration : repository.getRegistrations().values()) {
+            Map provider = new LinkedHashMap<>();
+            provider.put("id", registration.getRegistrationId());
+            provider.put("name", registration.getClientName());
+            providers.add(provider);
         }
         return providers;
     }
diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/ProfileBean.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/ProfileBean.java
index 7e7a9703a7..b9cbf032c3 100644
--- a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/ProfileBean.java
+++ b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/ProfileBean.java
@@ -113,7 +113,22 @@ public String getOpenIdUrl() {
     public void setOpenIdUrl(String openIdUrl) {
         this.openIdUrl = openIdUrl;
     }
-    
+
+    /** The issuer half of the {@code issuer#subject} federated identity. */
+    public String getOidcIssuer() {
+        return openIdUrl == null ? null : openIdUrl.split("#", 2)[0];
+    }
+
+    /** The subject half of the {@code issuer#subject} federated identity. */
+    public String getOidcSubject() {
+        if (openIdUrl == null) {
+            return null;
+        }
+        String[] parts = openIdUrl.split("#", 2);
+        return parts.length > 1 ? parts[1] : null;
+    }
+
+
     public String getPasswordText() {
         return passwordText;
     }
diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/Register.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/Register.java
index f6e0c415cf..e992248c7e 100644
--- a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/Register.java
+++ b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/Register.java
@@ -35,9 +35,7 @@
 import org.apache.roller.weblogger.pojos.User;
 import org.apache.roller.weblogger.ui.core.RollerSession;
 import org.apache.roller.weblogger.ui.core.security.CustomUserRegistry;
-import org.apache.roller.weblogger.ui.core.security.RollerOAuth2SuccessHandler;
 import org.apache.roller.weblogger.ui.struts2.util.UIAction;
-import org.springframework.security.oauth2.core.oidc.user.OidcUser;
 import org.apache.roller.weblogger.util.MailUtil;
 import org.apache.struts2.ActionContext;
 import org.apache.struts2.convention.annotation.AllowedMethods;
@@ -162,27 +160,6 @@ public String execute() {
                 }
             }
 
-            // Pre-populate from OIDC claims if arriving from OIDC login
-            OidcUser oidcUser = (OidcUser) getServletRequest().getSession()
-                    .getAttribute(RollerOAuth2SuccessHandler.OIDC_USER_ATTR);
-            if (oidcUser != null) {
-                if (oidcUser.getEmail() != null) {
-                    getBean().setEmailAddress(oidcUser.getEmail());
-                }
-                if (oidcUser.getFullName() != null) {
-                    getBean().setFullName(oidcUser.getFullName());
-                }
-                if (oidcUser.getPreferredUsername() != null) {
-                    getBean().setUserName(oidcUser.getPreferredUsername());
-                    getBean().setScreenName(oidcUser.getPreferredUsername());
-                }
-                String oidcSubject = (String) getServletRequest().getSession()
-                        .getAttribute(RollerOAuth2SuccessHandler.OIDC_SUBJECT_ATTR);
-                if (oidcSubject != null) {
-                    getBean().setOpenIdUrl(oidcSubject);
-                }
-            }
-            
         } catch (Exception ex) {
             log.error("Error reading SSO user data", ex);
             addError("error.editing.user", ex.toString());
diff --git a/app/src/main/resources/org/apache/roller/weblogger/config/roller.properties b/app/src/main/resources/org/apache/roller/weblogger/config/roller.properties
index 9d6b56ccfe..5246ccd742 100644
--- a/app/src/main/resources/org/apache/roller/weblogger/config/roller.properties
+++ b/app/src/main/resources/org/apache/roller/weblogger/config/roller.properties
@@ -354,6 +354,12 @@ users.passwords.externalAuthValue=
 # oidc.keycloak.issuer-uri=http://localhost:9080/realms/roller
 # oidc.keycloak.client-name=Keycloak
 
+# Create a Roller account automatically the first time someone signs in
+# through an OIDC provider. The provider controls who can sign in, so this
+# defaults to true; set false to only accept accounts an administrator has
+# linked, or that carry a verified email matching an existing account.
+users.oidc.autoProvision.enabled=true
+
 # Password security settings
 passwds.encryption.enabled=true
 passwds.encryption.algorithm=bcrypt
diff --git a/app/src/test/java/org/apache/roller/weblogger/ui/core/security/RollerOAuth2SuccessHandlerTest.java b/app/src/test/java/org/apache/roller/weblogger/ui/core/security/RollerOAuth2SuccessHandlerTest.java
index be4fcba116..9322cdabd2 100644
--- a/app/src/test/java/org/apache/roller/weblogger/ui/core/security/RollerOAuth2SuccessHandlerTest.java
+++ b/app/src/test/java/org/apache/roller/weblogger/ui/core/security/RollerOAuth2SuccessHandlerTest.java
@@ -17,28 +17,14 @@
  */
 package org.apache.roller.weblogger.ui.core.security;
 
-import java.time.Instant;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-
 import jakarta.servlet.http.HttpServletRequest;
 import jakarta.servlet.http.HttpServletResponse;
-import jakarta.servlet.http.HttpSession;
 
-import org.apache.roller.weblogger.business.UserManager;
-import org.apache.roller.weblogger.business.Weblogger;
-import org.apache.roller.weblogger.business.WebloggerFactory;
-import org.apache.roller.weblogger.pojos.User;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.mockito.Mock;
-import org.mockito.MockedStatic;
 import org.mockito.MockitoAnnotations;
 import org.springframework.security.core.Authentication;
-import org.springframework.security.oauth2.core.oidc.OidcIdToken;
-import org.springframework.security.oauth2.core.oidc.user.DefaultOidcUser;
-import org.springframework.security.oauth2.core.oidc.user.OidcUser;
 
 import static org.mockito.Mockito.*;
 
@@ -50,85 +36,32 @@ class RollerOAuth2SuccessHandlerTest {
     @Mock
     private HttpServletResponse response;
 
-    @Mock
-    private HttpSession session;
-
     @Mock
     private Authentication authentication;
 
-    @Mock
-    private Weblogger roller;
-
-    @Mock
-    private UserManager userManager;
-
-    @Mock
-    private User rollerUser;
-
     private RollerOAuth2SuccessHandler handler;
 
     @BeforeEach
     void setUp() {
         MockitoAnnotations.openMocks(this);
         handler = new RollerOAuth2SuccessHandler();
-        when(request.getContextPath()).thenReturn("/roller");
-        when(request.getSession()).thenReturn(session);
-        when(authentication.getPrincipal()).thenReturn(createOidcUser());
     }
 
     @Test
-    void existingUserRedirectsToMenu() throws Exception {
-        try (MockedStatic factory = mockStatic(WebloggerFactory.class)) {
-            factory.when(WebloggerFactory::isBootstrapped).thenReturn(true);
-            factory.when(WebloggerFactory::getWeblogger).thenReturn(roller);
-            when(roller.getUserManager()).thenReturn(userManager);
-            when(userManager.getUserByOpenIdUrl("https://accounts.example.com#user123")).thenReturn(rollerUser);
-
-            handler.onAuthenticationSuccess(request, response, authentication);
-
-            verify(response).sendRedirect("/roller/roller-ui/menu.rol");
-            verify(session, never()).setAttribute(anyString(), any());
-        }
-    }
-
-    @Test
-    void newUserRedirectsToRegistration() throws Exception {
-        try (MockedStatic factory = mockStatic(WebloggerFactory.class)) {
-            factory.when(WebloggerFactory::isBootstrapped).thenReturn(true);
-            factory.when(WebloggerFactory::getWeblogger).thenReturn(roller);
-            when(roller.getUserManager()).thenReturn(userManager);
-            when(userManager.getUserByOpenIdUrl("https://accounts.example.com#user123")).thenReturn(null);
+    void redirectsToMenuUnderContextPath() throws Exception {
+        when(request.getContextPath()).thenReturn("/roller");
 
-            handler.onAuthenticationSuccess(request, response, authentication);
+        handler.onAuthenticationSuccess(request, response, authentication);
 
-            verify(response).sendRedirect("/roller/roller-ui/register.rol");
-            verify(session).setAttribute(eq(RollerOAuth2SuccessHandler.OIDC_USER_ATTR), any(OidcUser.class));
-            verify(session).setAttribute(eq(RollerOAuth2SuccessHandler.OIDC_SUBJECT_ATTR), eq("https://accounts.example.com#user123"));
-        }
+        verify(response).sendRedirect("/roller/roller-ui/menu.rol");
     }
 
     @Test
-    void notBootstrappedRedirectsToRegistration() throws Exception {
-        try (MockedStatic factory = mockStatic(WebloggerFactory.class)) {
-            factory.when(WebloggerFactory::isBootstrapped).thenReturn(false);
-
-            handler.onAuthenticationSuccess(request, response, authentication);
-
-            verify(response).sendRedirect("/roller/roller-ui/register.rol");
-            verify(session).setAttribute(eq(RollerOAuth2SuccessHandler.OIDC_USER_ATTR), any(OidcUser.class));
-        }
-    }
+    void redirectsToMenuAtRootContext() throws Exception {
+        when(request.getContextPath()).thenReturn("");
 
-    private OidcUser createOidcUser() {
-        Map claims = new HashMap<>();
-        claims.put("sub", "user123");
-        claims.put("iss", "https://accounts.example.com");
-        claims.put("aud", List.of("client-id"));
-        claims.put("iat", Instant.now());
-        claims.put("exp", Instant.now().plusSeconds(3600));
+        handler.onAuthenticationSuccess(request, response, authentication);
 
-        OidcIdToken idToken = new OidcIdToken("token-value", Instant.now(),
-                Instant.now().plusSeconds(3600), claims);
-        return new DefaultOidcUser(List.of(), idToken);
+        verify(response).sendRedirect("/roller-ui/menu.rol");
     }
 }
diff --git a/app/src/test/java/org/apache/roller/weblogger/ui/core/security/RollerOidcUserServiceTest.java b/app/src/test/java/org/apache/roller/weblogger/ui/core/security/RollerOidcUserServiceTest.java
index 1bbe3579ef..31549f52fc 100644
--- a/app/src/test/java/org/apache/roller/weblogger/ui/core/security/RollerOidcUserServiceTest.java
+++ b/app/src/test/java/org/apache/roller/weblogger/ui/core/security/RollerOidcUserServiceTest.java
@@ -21,49 +21,394 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
 
+import org.apache.roller.weblogger.business.UserManager;
+import org.apache.roller.weblogger.business.Weblogger;
+import org.apache.roller.weblogger.business.WebloggerFactory;
+import org.apache.roller.weblogger.config.WebloggerConfig;
+import org.apache.roller.weblogger.config.WebloggerRuntimeConfig;
+import org.apache.roller.weblogger.pojos.User;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentCaptor;
+import org.mockito.InOrder;
+import org.mockito.Mock;
+import org.mockito.MockedStatic;
+import org.mockito.MockitoAnnotations;
+import org.springframework.security.core.GrantedAuthority;
+import org.springframework.security.oauth2.core.OAuth2AuthenticationException;
 import org.springframework.security.oauth2.core.oidc.OidcIdToken;
 import org.springframework.security.oauth2.core.oidc.user.DefaultOidcUser;
 import org.springframework.security.oauth2.core.oidc.user.OidcUser;
 
 import static org.junit.jupiter.api.Assertions.*;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.*;
 
 class RollerOidcUserServiceTest {
 
+    private static final String ISSUER = "https://accounts.example.com";
+    private static final String SUBJECT = ISSUER + "#user123";
+
+    @Mock
+    private Weblogger roller;
+
+    @Mock
+    private UserManager userManager;
+
+    @Mock
+    private User existingUser;
+
+    private RollerOidcUserService service;
+
+    @BeforeEach
+    void setUp() {
+        MockitoAnnotations.openMocks(this);
+        service = new RollerOidcUserService();
+    }
+
     @Test
     void toOidcSubjectFormatsIssuerAndSub() {
-        Map claims = new HashMap<>();
-        claims.put("sub", "user123");
-        claims.put("iss", "https://accounts.example.com");
-        claims.put("aud", List.of("client-id"));
-        claims.put("iat", Instant.now());
-        claims.put("exp", Instant.now().plusSeconds(3600));
+        assertEquals(SUBJECT, RollerOidcUserService.toOidcSubject(oidcUser(Map.of())));
+    }
 
-        OidcIdToken idToken = new OidcIdToken("token-value", Instant.now(),
-                Instant.now().plusSeconds(3600), claims);
-        OidcUser oidcUser = new DefaultOidcUser(List.of(), idToken);
+    @Test
+    void toOidcSubjectHandlesTrailingSlashInIssuer() {
+        OidcUser user = oidcUser(Map.of("iss", "https://provider.example.com/", "sub", "abc"));
+        assertEquals("https://provider.example.com/#abc", RollerOidcUserService.toOidcSubject(user));
+    }
 
-        String result = RollerOidcUserService.toOidcSubject(oidcUser);
+    /**
+     * Regression: the returned principal must carry the Roller roles, otherwise
+     * the very first request after login is denied by the authorization rules.
+     */
+    @Test
+    void existingUserPrincipalCarriesRollerAuthorities() throws Exception {
+        try (MockedStatic factory = bootstrappedRoller()) {
+            when(userManager.getUserByOpenIdUrl(SUBJECT)).thenReturn(existingUser);
+            when(existingUser.getEnabled()).thenReturn(Boolean.TRUE);
+            when(userManager.getRoles(existingUser)).thenReturn(List.of("editor", "admin"));
+
+            OidcUser result = service.resolveUser(oidcUser(Map.of()));
 
-        assertEquals("https://accounts.example.com#user123", result);
+            assertEquals(Set.of("editor", "admin"), authorityNames(result));
+            verify(userManager, never()).addUser(any(User.class));
+        }
     }
 
+    /**
+     * Regression: Roller looks users up by principal.getName() throughout the
+     * rendering layer and servlet filters (ParsedRequest, RoleAssignmentFilter,
+     * Register). An OidcUser's default name is the "sub" claim, an opaque
+     * provider ID that matches no Roller account, which made every rendered
+     * weblog page throw an NPE for a signed-in OIDC user. The principal's name
+     * must be the resolved Roller username.
+     */
     @Test
-    void toOidcSubjectHandlesTrailingSlashInIssuer() {
+    void existingUserPrincipalNameIsRollerUsername() throws Exception {
+        try (MockedStatic factory = bootstrappedRoller()) {
+            when(userManager.getUserByOpenIdUrl(SUBJECT)).thenReturn(existingUser);
+            when(existingUser.getEnabled()).thenReturn(Boolean.TRUE);
+            when(existingUser.getUserName()).thenReturn("bob");
+            when(userManager.getRoles(existingUser)).thenReturn(List.of("editor"));
+
+            OidcUser result = service.resolveUser(oidcUser(Map.of()));
+
+            assertEquals("bob", result.getName());
+        }
+    }
+
+    @Test
+    void provisionedUserPrincipalNameIsRollerUsername() throws Exception {
+        try (MockedStatic factory = bootstrappedRoller()) {
+            when(userManager.getUserByOpenIdUrl(SUBJECT)).thenReturn(null);
+            when(userManager.getRoles(any(User.class))).thenReturn(List.of("editor"));
+
+            OidcUser result = service.resolveUser(oidcUser(Map.of(
+                    "preferred_username", "jsmith",
+                    "email", "jsmith@example.com")));
+
+            assertEquals("jsmith", result.getName());
+        }
+    }
+
+    @Test
+    void newUserPrincipalCarriesRollerAuthorities() throws Exception {
+        try (MockedStatic factory = bootstrappedRoller()) {
+            when(userManager.getUserByOpenIdUrl(SUBJECT)).thenReturn(null);
+            when(userManager.getRoles(any(User.class))).thenReturn(List.of("editor"));
+
+            OidcUser result = service.resolveUser(oidcUser(Map.of(
+                    "preferred_username", "jsmith",
+                    "email", "jsmith@example.com")));
+
+            assertEquals(Set.of("editor"), authorityNames(result));
+        }
+    }
+
+    @Test
+    void newUserIsProvisionedFromClaims() throws Exception {
+        try (MockedStatic factory = bootstrappedRoller()) {
+            when(userManager.getUserByOpenIdUrl(SUBJECT)).thenReturn(null);
+            when(userManager.getRoles(any(User.class))).thenReturn(List.of("editor"));
+
+            service.resolveUser(oidcUser(Map.of(
+                    "preferred_username", "jsmith",
+                    "name", "Jane Smith",
+                    "email", "jsmith@example.com")));
+
+            ArgumentCaptor captor = ArgumentCaptor.forClass(User.class);
+            verify(userManager).addUser(captor.capture());
+            User created = captor.getValue();
+
+            assertEquals("jsmith", created.getUserName());
+            assertEquals("Jane Smith", created.getFullName());
+            assertEquals("jsmith@example.com", created.getEmailAddress());
+            assertEquals(SUBJECT, created.getOpenIdUrl());
+            assertEquals(Boolean.TRUE, created.getEnabled());
+            assertNotNull(created.getId());
+            assertNotNull(created.getPassword(), "password is NOT NULL in the schema");
+            assertNotNull(created.getDateCreated(), "datecreated is NOT NULL in the schema");
+        }
+    }
+
+    @Test
+    void adminRoleIsGrantedFromFlatRolesClaim() throws Exception {
+        try (MockedStatic factory = bootstrappedRoller()) {
+            when(userManager.getUserByOpenIdUrl(SUBJECT)).thenReturn(null);
+            when(userManager.getRoles(any(User.class))).thenReturn(List.of("editor", "admin"));
+
+            service.resolveUser(oidcUser(Map.of(
+                    "preferred_username", "boss",
+                    "email", "boss@example.com",
+                    "roles", List.of("editor", "admin"))));
+
+            verify(userManager).grantRole(eq("admin"), any(User.class));
+        }
+    }
+
+    @Test
+    void adminRoleIsGrantedFromKeycloakRealmAccessClaim() throws Exception {
+        try (MockedStatic factory = bootstrappedRoller()) {
+            when(userManager.getUserByOpenIdUrl(SUBJECT)).thenReturn(null);
+            when(userManager.getRoles(any(User.class))).thenReturn(List.of("editor", "admin"));
+
+            service.resolveUser(oidcUser(Map.of(
+                    "preferred_username", "boss",
+                    "email", "boss@example.com",
+                    "realm_access", Map.of("roles", List.of("admin")))));
+
+            verify(userManager).grantRole(eq("admin"), any(User.class));
+        }
+    }
+
+    @Test
+    void nonAdminUserDoesNotGetAdminRole() throws Exception {
+        try (MockedStatic factory = bootstrappedRoller()) {
+            when(userManager.getUserByOpenIdUrl(SUBJECT)).thenReturn(null);
+            when(userManager.getRoles(any(User.class))).thenReturn(List.of("editor"));
+
+            service.resolveUser(oidcUser(Map.of(
+                    "preferred_username", "plain",
+                    "email", "plain@example.com",
+                    "roles", List.of("editor"))));
+
+            verify(userManager, never()).grantRole(eq("admin"), any(User.class));
+        }
+    }
+
+    /**
+     * Regression: addUser() grants "admin" to the very first user, but that row
+     * is not yet flushed, so grantRole()'s database-backed duplicate check can
+     * not see it. Without a flush in between the role is inserted twice.
+     */
+    @Test
+    void pendingRolesAreFlushedBeforeGrantingAdmin() throws Exception {
+        try (MockedStatic factory = bootstrappedRoller()) {
+            when(userManager.getUserByOpenIdUrl(SUBJECT)).thenReturn(null);
+            when(userManager.getRoles(any(User.class))).thenReturn(List.of("editor", "admin"));
+
+            service.resolveUser(oidcUser(Map.of(
+                    "preferred_username", "first",
+                    "email", "first@example.com",
+                    "roles", List.of("admin"))));
+
+            InOrder inOrder = inOrder(userManager, roller);
+            inOrder.verify(userManager).addUser(any(User.class));
+            inOrder.verify(roller).flush();
+            inOrder.verify(userManager).grantRole(eq("admin"), any(User.class));
+        }
+    }
+
+    @Test
+    void disabledUserIsRejected() throws Exception {
+        try (MockedStatic factory = bootstrappedRoller()) {
+            when(userManager.getUserByOpenIdUrl(SUBJECT)).thenReturn(existingUser);
+            when(existingUser.getEnabled()).thenReturn(Boolean.FALSE);
+
+            OidcUser user = oidcUser(Map.of());
+            assertThrows(OAuth2AuthenticationException.class, () -> service.resolveUser(user));
+        }
+    }
+
+    @Test
+    void missingEmailIsRejected() throws Exception {
+        try (MockedStatic factory = bootstrappedRoller()) {
+            when(userManager.getUserByOpenIdUrl(SUBJECT)).thenReturn(null);
+
+            OidcUser user = oidcUser(Map.of("preferred_username", "noemail"));
+            assertThrows(OAuth2AuthenticationException.class, () -> service.resolveUser(user));
+        }
+    }
+
+    @Test
+    void notBootstrappedIsRejected() {
+        try (MockedStatic factory = mockStatic(WebloggerFactory.class)) {
+            factory.when(WebloggerFactory::isBootstrapped).thenReturn(false);
+
+            OidcUser user = oidcUser(Map.of());
+            assertThrows(OAuth2AuthenticationException.class, () -> service.resolveUser(user));
+        }
+    }
+
+    /**
+     * The migration path: a site with database users turns on OIDC, and the
+     * provider asserts a verified email matching the existing account.
+     */
+    @Test
+    void existingUsernameIsLinkedWhenEmailIsVerified() throws Exception {
+        try (MockedStatic factory = bootstrappedRoller()) {
+            when(userManager.getUserByOpenIdUrl(SUBJECT)).thenReturn(null);
+            when(userManager.getUserByUserName("matt")).thenReturn(existingUser);
+            when(existingUser.getEmailAddress()).thenReturn("matt@example.com");
+            when(existingUser.getEnabled()).thenReturn(Boolean.TRUE);
+            when(userManager.getRoles(existingUser)).thenReturn(List.of("editor"));
+
+            OidcUser result = service.resolveUser(oidcUser(Map.of(
+                    "preferred_username", "matt",
+                    "email", "matt@example.com",
+                    "email_verified", Boolean.TRUE)));
+
+            verify(existingUser).setOpenIdUrl(SUBJECT);
+            verify(userManager).saveUser(existingUser);
+            verify(userManager, never()).addUser(any(User.class));
+            assertEquals(Set.of("editor"), authorityNames(result));
+        }
+    }
+
+    @Test
+    void existingUsernameIsNotLinkedWhenEmailIsUnverified() throws Exception {
+        try (MockedStatic factory = bootstrappedRoller()) {
+            when(userManager.getUserByOpenIdUrl(SUBJECT)).thenReturn(null);
+            when(userManager.getUserByUserName("matt")).thenReturn(existingUser);
+            when(existingUser.getEmailAddress()).thenReturn("matt@example.com");
+
+            OidcUser user = oidcUser(Map.of(
+                    "preferred_username", "matt",
+                    "email", "matt@example.com",
+                    "email_verified", Boolean.FALSE));
+
+            assertThrows(OAuth2AuthenticationException.class, () -> service.resolveUser(user));
+            verify(userManager, never()).saveUser(any(User.class));
+            verify(userManager, never()).addUser(any(User.class));
+        }
+    }
+
+    @Test
+    void existingUsernameIsNotLinkedWhenEmailDiffers() throws Exception {
+        try (MockedStatic factory = bootstrappedRoller()) {
+            when(userManager.getUserByOpenIdUrl(SUBJECT)).thenReturn(null);
+            when(userManager.getUserByUserName("matt")).thenReturn(existingUser);
+            when(existingUser.getEmailAddress()).thenReturn("someone-else@example.com");
+
+            OidcUser user = oidcUser(Map.of(
+                    "preferred_username", "matt",
+                    "email", "matt@example.com",
+                    "email_verified", Boolean.TRUE));
+
+            assertThrows(OAuth2AuthenticationException.class, () -> service.resolveUser(user));
+            verify(userManager, never()).saveUser(any(User.class));
+        }
+    }
+
+    @Test
+    void autoProvisionDisabledRejectsNewUser() throws Exception {
+        try (MockedStatic factory = bootstrappedRoller();
+             MockedStatic config = autoProvision(false)) {
+            when(userManager.getUserByOpenIdUrl(SUBJECT)).thenReturn(null);
+
+            OidcUser user = oidcUser(Map.of(
+                    "preferred_username", "newcomer",
+                    "email", "newcomer@example.com"));
+
+            assertThrows(OAuth2AuthenticationException.class, () -> service.resolveUser(user));
+            verify(userManager, never()).addUser(any(User.class));
+        }
+    }
+
+    /**
+     * Regression: provisioning must not depend on the runtime "allow new
+     * users" toggle, which is off by default and governs form registration.
+     * The identity provider decides who may sign in, so on a stock install
+     * every provider user after the first must still get an account.
+     */
+    @Test
+    void formRegistrationPolicyDoesNotBlockProvisioning() throws Exception {
+        try (MockedStatic factory = bootstrappedRoller();
+             MockedStatic config = registrationEnabled(false)) {
+            when(userManager.getUserByOpenIdUrl(SUBJECT)).thenReturn(null);
+            when(userManager.getUserCount()).thenReturn(5L);
+            when(userManager.getRoles(any(User.class))).thenReturn(List.of("editor"));
+
+            service.resolveUser(oidcUser(Map.of(
+                    "preferred_username", "newcomer",
+                    "email", "newcomer@example.com")));
+
+            verify(userManager).addUser(any(User.class));
+        }
+    }
+
+    private MockedStatic bootstrappedRoller() {
+        MockedStatic factory = mockStatic(WebloggerFactory.class);
+        factory.when(WebloggerFactory::isBootstrapped).thenReturn(true);
+        factory.when(WebloggerFactory::getWeblogger).thenReturn(roller);
+        when(roller.getUserManager()).thenReturn(userManager);
+        return factory;
+    }
+
+    private MockedStatic registrationEnabled(boolean enabled) {
+        MockedStatic config = mockStatic(WebloggerRuntimeConfig.class);
+        config.when(() -> WebloggerRuntimeConfig.getBooleanProperty("users.registration.enabled"))
+                .thenReturn(enabled);
+        return config;
+    }
+
+    private MockedStatic autoProvision(boolean enabled) {
+        MockedStatic config = mockStatic(WebloggerConfig.class);
+        config.when(() -> WebloggerConfig.getBooleanProperty("users.oidc.autoProvision.enabled"))
+                .thenReturn(enabled);
+        return config;
+    }
+
+    private Set authorityNames(OidcUser user) {
+        return user.getAuthorities().stream()
+                .map(GrantedAuthority::getAuthority)
+                .collect(Collectors.toSet());
+    }
+
+    private OidcUser oidcUser(Map extraClaims) {
         Map claims = new HashMap<>();
-        claims.put("sub", "abc");
-        claims.put("iss", "https://provider.example.com/");
+        claims.put("sub", "user123");
+        claims.put("iss", ISSUER);
         claims.put("aud", List.of("client-id"));
         claims.put("iat", Instant.now());
         claims.put("exp", Instant.now().plusSeconds(3600));
+        claims.putAll(extraClaims);
 
         OidcIdToken idToken = new OidcIdToken("token-value", Instant.now(),
                 Instant.now().plusSeconds(3600), claims);
-        OidcUser oidcUser = new DefaultOidcUser(List.of(), idToken);
-
-        String result = RollerOidcUserService.toOidcSubject(oidcUser);
-
-        assertEquals("https://provider.example.com/#abc", result);
+        return new DefaultOidcUser(List.of(), idToken);
     }
 }

From f37a3b71c31efe7ab393e1e3ff0b271fa139c75a Mon Sep 17 00:00:00 2001
From: Matt Raible 
Date: Wed, 12 Aug 2026 23:00:12 -0600
Subject: [PATCH 4/7] Enforce the configured authentication method server side
 and harden OIDC provisioning

Hiding login forms and provider buttons is not an authentication control, so both sides of the filter chain now check authentication.method: the registration repository serves no providers unless the method is oidc or db-oidc (closing /oauth2/authorization/* under db, ldap, and cma), the OIDC user service rejects the flow outright, and the user details service refuses password lookups in pure oidc mode.

users.firstUserAdmin would hand the admin role to whichever provider user reached a fresh install first, so auto-provisioned accounts no longer keep the bootstrap grant unless an admin role claim is asserted or the new users.oidc.firstUserAdmin property is enabled; the revocation is logged with the bootstrap alternatives.

Provider discovery is now cached per provider with a bounded retry backoff, so one unreachable identity provider no longer blocks the others or turns every login page render into a discovery attempt. A confidential client without a client-secret is rejected at configuration time with a pointer to client-authentication-method=none for PKCE public clients. OAuth2 failures redirect with error=oidc so the login page stops diagnosing every provider failure as a wrong password. All raised by Copilot review.
---
 .dockerignore                                 |   1 +
 .../RollerClientRegistrationRepository.java   | 141 +++++++++++-------
 .../core/security/RollerOidcUserService.java  |  21 +++
 .../security/RollerUserDetailsService.java    |   9 ++
 .../weblogger/ui/struts2/core/Login.java      |   7 +-
 .../roller/weblogger/config/roller.properties |   7 +
 app/src/main/webapp/WEB-INF/security.xml      |   8 +
 ...ollerClientRegistrationRepositoryTest.java |  45 ++++++
 .../security/RollerOidcUserServiceTest.java   |  97 ++++++++++++
 9 files changed, 283 insertions(+), 53 deletions(-)

diff --git a/.dockerignore b/.dockerignore
index 5a559df50d..788be24434 100644
--- a/.dockerignore
+++ b/.dockerignore
@@ -1,4 +1,5 @@
 .git
 docker/postgresql-data
+docker/postgresql-16-data
 docker/roller-data
 it-selenium
diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/core/security/RollerClientRegistrationRepository.java b/app/src/main/java/org/apache/roller/weblogger/ui/core/security/RollerClientRegistrationRepository.java
index 5e3e96b2e9..bae2f2c017 100644
--- a/app/src/main/java/org/apache/roller/weblogger/ui/core/security/RollerClientRegistrationRepository.java
+++ b/app/src/main/java/org/apache/roller/weblogger/ui/core/security/RollerClientRegistrationRepository.java
@@ -22,20 +22,30 @@
 import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+import org.apache.roller.weblogger.config.AuthMethod;
 import org.apache.roller.weblogger.config.WebloggerConfig;
 import org.springframework.security.oauth2.client.registration.ClientRegistration;
 import org.springframework.security.oauth2.client.registration.ClientRegistrationRepository;
 import org.springframework.security.oauth2.client.registration.ClientRegistrations;
 import org.springframework.security.oauth2.core.AuthorizationGrantType;
+import org.springframework.security.oauth2.core.ClientAuthenticationMethod;
 
 /**
  * Builds OAuth2/OIDC client registrations from Roller properties.
  *
+ * 
No registrations are served unless {@code authentication.method} is
+ * {@code oidc} or {@code db-oidc}, so configuring providers under another
+ * method does not open the {@code /oauth2/authorization/*} endpoints.
+ *
  * 
OIDC discovery is deferred until first access so the identity provider
- * does not need to be reachable during application startup.
+ * does not need to be reachable during application startup. Each provider is
+ * resolved and cached independently: one unreachable provider does not block
+ * the others, and a failed provider is retried with a bounded backoff instead
+ * of on every request.
  *
  * 
Properties follow the pattern:
  * 
@@ -44,14 +54,18 @@
  * oidc.{registrationId}.issuer-uri=...
  * oidc.{registrationId}.client-name=...  (optional, defaults to registrationId)
  * oidc.{registrationId}.scope=openid,profile,email  (optional)
+ * oidc.{registrationId}.client-authentication-method=none  (optional, for a
+ *     public client using PKCE; without it a client-secret is required)
  * 

  */
 public class RollerClientRegistrationRepository implements ClientRegistrationRepository, Iterable {
 
     private static final Log log = LogFactory.getLog(RollerClientRegistrationRepository.class);
     private static final String PREFIX = "oidc.";
+    private static final long RETRY_BACKOFF_MS = 60_000;
 
-    private volatile Map registrations;
+    private final Map resolved = new ConcurrentHashMap<>();
+    private final Map failedAt = new ConcurrentHashMap<>();
 
     @Override
     public ClientRegistration findByRegistrationId(String registrationId) {
@@ -63,30 +77,52 @@ public Iterator iterator() {
         return getRegistrations().values().iterator();
     }
 
+    /** Whether the configured authentication method allows OIDC login at all. */
+    static boolean oidcEnabled() {
+        AuthMethod method = WebloggerConfig.getAuthMethod();
+        return method == AuthMethod.OIDC || method == AuthMethod.DB_OIDC;
+    }
+
     /**
-     * Discovery runs on first use and the result is cached, but only once every
-     * configured provider resolved. A provider that was unreachable is retried
-     * on the next call rather than being cached as permanently broken.
+     * The successfully resolved registrations, in configuration order. Providers
+     * that have not resolved yet are attempted, unless they failed within the
+     * retry backoff window.
      */
     public Map getRegistrations() {
-        Map cached = registrations;
-        if (cached != null) {
-            return cached;
+        if (!oidcEnabled()) {
+            return Collections.emptyMap();
         }
-        synchronized (this) {
-            if (registrations != null) {
-                return registrations;
+
+        Map configured = configuredProviderIds();
+        long now = System.currentTimeMillis();
+
+        for (Map.Entry entry : configured.entrySet()) {
+            String id = entry.getKey();
+            if (resolved.containsKey(id)) {
+                continue;
+            }
+            Long lastFailure = failedAt.get(id);
+            if (lastFailure != null && now - lastFailure < RETRY_BACKOFF_MS) {
+                continue;
             }
-            Map built = buildRegistrations();
-            if (built.size() < configuredProviderIds().size()) {
-                return Collections.unmodifiableMap(built);
+            ClientRegistration registration = buildRegistration(id, entry.getValue());
+            if (registration != null) {
+                resolved.put(id, registration);
+                failedAt.remove(id);
+            } else {
+                failedAt.put(id, now);
             }
-            registrations = Collections.unmodifiableMap(built);
-            if (!registrations.isEmpty()) {
-                log.info("Configured OIDC providers: " + registrations.keySet());
+        }
+
+        // return in configuration order, only what resolved
+        Map result = new LinkedHashMap<>();
+        for (String id : configured.keySet()) {
+            ClientRegistration registration = resolved.get(id);
+            if (registration != null) {
+                result.put(id, registration);
             }
-            return registrations;
         }
+        return Collections.unmodifiableMap(result);
     }
 
     /** Registration ids that have an {@code oidc..client-id} property set. */
@@ -106,42 +142,47 @@ static Map configuredProviderIds() {
         return registrationIds;
     }
 
-    private Map buildRegistrations() {
-        Map registrationIds = configuredProviderIds();
-
-        Map result = new LinkedHashMap<>();
-        for (Map.Entry entry : registrationIds.entrySet()) {
-            String id = entry.getKey();
-            String clientId = entry.getValue();
-            String clientSecret = WebloggerConfig.getProperty(PREFIX + id + ".client-secret");
-            String issuerUri = WebloggerConfig.getProperty(PREFIX + id + ".issuer-uri");
-            String clientName = WebloggerConfig.getProperty(PREFIX + id + ".client-name", id);
-            String scopeStr = WebloggerConfig.getProperty(PREFIX + id + ".scope", "openid,profile,email");
-
-            if (clientId == null || clientId.isBlank() || issuerUri == null || issuerUri.isBlank()) {
-                log.warn("Skipping OIDC registration '" + id + "': client-id and issuer-uri are required");
-                continue;
-            }
+    private ClientRegistration buildRegistration(String id, String clientId) {
+        String clientSecret = WebloggerConfig.getProperty(PREFIX + id + ".client-secret");
+        String issuerUri = WebloggerConfig.getProperty(PREFIX + id + ".issuer-uri");
+        String clientName = WebloggerConfig.getProperty(PREFIX + id + ".client-name", id);
+        String scopeStr = WebloggerConfig.getProperty(PREFIX + id + ".scope", "openid,profile,email");
+        String clientAuthMethod = WebloggerConfig.getProperty(PREFIX + id + ".client-authentication-method");
 
-            try {
-                ClientRegistration.Builder builder = ClientRegistrations.fromIssuerLocation(issuerUri)
-                        .registrationId(id)
-                        .clientId(clientId)
-                        .clientName(clientName)
-                        .authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE)
-                        .scope(scopeStr.split(","));
+        if (clientId == null || clientId.isBlank() || issuerUri == null || issuerUri.isBlank()) {
+            log.warn("Skipping OIDC registration '" + id + "': client-id and issuer-uri are required");
+            return null;
+        }
 
-                if (clientSecret != null && !clientSecret.isBlank()) {
-                    builder.clientSecret(clientSecret);
-                }
+        boolean publicClient = "none".equalsIgnoreCase(clientAuthMethod);
+        if (!publicClient && (clientSecret == null || clientSecret.isBlank())) {
+            log.error("Skipping OIDC registration '" + id + "': no client-secret configured. Set oidc."
+                    + id + ".client-secret, or oidc." + id
+                    + ".client-authentication-method=none for a public client using PKCE.");
+            return null;
+        }
 
-                result.put(id, builder.build());
-                log.info("Registered OIDC provider: " + id + " (issuer: " + issuerUri + ")");
-            } catch (Exception e) {
-                log.error("Failed to configure OIDC provider '" + id + "' (issuer: " + issuerUri + ")", e);
+        try {
+            ClientRegistration.Builder builder = ClientRegistrations.fromIssuerLocation(issuerUri)
+                    .registrationId(id)
+                    .clientId(clientId)
+                    .clientName(clientName)
+                    .authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE)
+                    .scope(scopeStr.split(","));
+
+            if (publicClient) {
+                builder.clientAuthenticationMethod(ClientAuthenticationMethod.NONE);
+            } else {
+                builder.clientSecret(clientSecret);
             }
-        }
 
-        return result;
+            ClientRegistration registration = builder.build();
+            log.info("Registered OIDC provider: " + id + " (issuer: " + issuerUri + ")");
+            return registration;
+        } catch (Exception e) {
+            log.error("Failed to configure OIDC provider '" + id + "' (issuer: " + issuerUri
+                    + "), will retry in " + (RETRY_BACKOFF_MS / 1000) + "s", e);
+            return null;
+        }
     }
 }
diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/core/security/RollerOidcUserService.java b/app/src/main/java/org/apache/roller/weblogger/ui/core/security/RollerOidcUserService.java
index 8ba9dd76a8..f3cacadbec 100644
--- a/app/src/main/java/org/apache/roller/weblogger/ui/core/security/RollerOidcUserService.java
+++ b/app/src/main/java/org/apache/roller/weblogger/ui/core/security/RollerOidcUserService.java
@@ -61,6 +61,12 @@ public class RollerOidcUserService implements OAuth2UserService> getOidcProviders() {
     @Override
     public String execute() {
 
-        // set action error message if there was login error
-        if(getError() != null) {
-            addError("error.password.mismatch");
+        // set action error message if there was login error; OAuth2/OIDC
+        // failures redirect here with error=oidc, form login with error=true
+        if (getError() != null) {
+            addError("oidc".equals(getError()) ? "error.oidc.login" : "error.password.mismatch");
         }
 
         return SUCCESS;
diff --git a/app/src/main/resources/org/apache/roller/weblogger/config/roller.properties b/app/src/main/resources/org/apache/roller/weblogger/config/roller.properties
index 5246ccd742..96feacffe8 100644
--- a/app/src/main/resources/org/apache/roller/weblogger/config/roller.properties
+++ b/app/src/main/resources/org/apache/roller/weblogger/config/roller.properties
@@ -360,6 +360,13 @@ users.passwords.externalAuthValue=
 # linked, or that carry a verified email matching an existing account.
 users.oidc.autoProvision.enabled=true
 
+# users.firstUserAdmin makes the first registered account an administrator.
+# For auto-provisioned OIDC accounts that would mean whichever provider user
+# reaches a fresh install first, so the grant is off unless enabled here.
+# Alternatives: assert an "admin" role claim at the provider, or create the
+# admin account before enabling OIDC.
+users.oidc.firstUserAdmin=false
+
 # Password security settings
 passwds.encryption.enabled=true
 passwds.encryption.algorithm=bcrypt
diff --git a/app/src/main/webapp/WEB-INF/security.xml b/app/src/main/webapp/WEB-INF/security.xml
index e0620db5b5..60655bef0e 100644
--- a/app/src/main/webapp/WEB-INF/security.xml
+++ b/app/src/main/webapp/WEB-INF/security.xml
@@ -53,6 +53,7 @@
         
 
         
@@ -121,6 +122,13 @@
     
 
+    
+    
+        
+    
+
     
     
@@ -119,8 +96,11 @@
     
 
+    
     
+                class="org.springframework.security.web.authentication.SavedRequestAwareAuthenticationSuccessHandler">
+        
+    
 
     
diff --git a/app/src/test/java/org/apache/roller/weblogger/ui/core/security/RollerOAuth2SuccessHandlerTest.java b/app/src/test/java/org/apache/roller/weblogger/ui/core/security/RollerOAuth2SuccessHandlerTest.java
deleted file mode 100644
index 9322cdabd2..0000000000
--- a/app/src/test/java/org/apache/roller/weblogger/ui/core/security/RollerOAuth2SuccessHandlerTest.java
+++ /dev/null
@@ -1,67 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements.  The ASF licenses this file to You
- * under the Apache License, Version 2.0 (the "License"); you may not
- * use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.  For additional information regarding
- * copyright in this work, please see the NOTICE file in the top level
- * directory of this distribution.
- */
-package org.apache.roller.weblogger.ui.core.security;
-
-import jakarta.servlet.http.HttpServletRequest;
-import jakarta.servlet.http.HttpServletResponse;
-
-import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Test;
-import org.mockito.Mock;
-import org.mockito.MockitoAnnotations;
-import org.springframework.security.core.Authentication;
-
-import static org.mockito.Mockito.*;
-
-class RollerOAuth2SuccessHandlerTest {
-
-    @Mock
-    private HttpServletRequest request;
-
-    @Mock
-    private HttpServletResponse response;
-
-    @Mock
-    private Authentication authentication;
-
-    private RollerOAuth2SuccessHandler handler;
-
-    @BeforeEach
-    void setUp() {
-        MockitoAnnotations.openMocks(this);
-        handler = new RollerOAuth2SuccessHandler();
-    }
-
-    @Test
-    void redirectsToMenuUnderContextPath() throws Exception {
-        when(request.getContextPath()).thenReturn("/roller");
-
-        handler.onAuthenticationSuccess(request, response, authentication);
-
-        verify(response).sendRedirect("/roller/roller-ui/menu.rol");
-    }
-
-    @Test
-    void redirectsToMenuAtRootContext() throws Exception {
-        when(request.getContextPath()).thenReturn("");
-
-        handler.onAuthenticationSuccess(request, response, authentication);
-
-        verify(response).sendRedirect("/roller-ui/menu.rol");
-    }
-}
diff --git a/app/src/test/java/org/apache/roller/weblogger/ui/core/security/RollerOidcUserServiceTest.java b/app/src/test/java/org/apache/roller/weblogger/ui/core/security/RollerOidcUserServiceTest.java
index 5ab4509716..2bbbde7dc3 100644
--- a/app/src/test/java/org/apache/roller/weblogger/ui/core/security/RollerOidcUserServiceTest.java
+++ b/app/src/test/java/org/apache/roller/weblogger/ui/core/security/RollerOidcUserServiceTest.java
@@ -283,7 +283,7 @@ void notBootstrappedIsRejected() {
     void existingUsernameIsLinkedWhenEmailIsVerified() throws Exception {
         try (MockedStatic factory = bootstrappedRoller()) {
             when(userManager.getUserByOpenIdUrl(SUBJECT)).thenReturn(null);
-            when(userManager.getUserByUserName("matt")).thenReturn(existingUser);
+            when(userManager.getUserByUserName("matt", null)).thenReturn(existingUser);
             when(existingUser.getEmailAddress()).thenReturn("matt@example.com");
             when(existingUser.getEnabled()).thenReturn(Boolean.TRUE);
             when(userManager.getRoles(existingUser)).thenReturn(List.of("editor"));
@@ -304,7 +304,8 @@ void existingUsernameIsLinkedWhenEmailIsVerified() throws Exception {
     void existingUsernameIsNotLinkedWhenEmailIsUnverified() throws Exception {
         try (MockedStatic factory = bootstrappedRoller()) {
             when(userManager.getUserByOpenIdUrl(SUBJECT)).thenReturn(null);
-            when(userManager.getUserByUserName("matt")).thenReturn(existingUser);
+            when(userManager.getUserByUserName("matt", null)).thenReturn(existingUser);
+            when(existingUser.getEnabled()).thenReturn(Boolean.TRUE);
             when(existingUser.getEmailAddress()).thenReturn("matt@example.com");
 
             OidcUser user = oidcUser(Map.of(
@@ -322,7 +323,8 @@ void existingUsernameIsNotLinkedWhenEmailIsUnverified() throws Exception {
     void existingUsernameIsNotLinkedWhenEmailDiffers() throws Exception {
         try (MockedStatic factory = bootstrappedRoller()) {
             when(userManager.getUserByOpenIdUrl(SUBJECT)).thenReturn(null);
-            when(userManager.getUserByUserName("matt")).thenReturn(existingUser);
+            when(userManager.getUserByUserName("matt", null)).thenReturn(existingUser);
+            when(existingUser.getEnabled()).thenReturn(Boolean.TRUE);
             when(existingUser.getEmailAddress()).thenReturn("someone-else@example.com");
 
             OidcUser user = oidcUser(Map.of(
@@ -335,6 +337,32 @@ void existingUsernameIsNotLinkedWhenEmailDiffers() throws Exception {
         }
     }
 
+    /**
+     * Regression: a disabled or pending-activation account with the same
+     * username must refuse the login, not fall through to provisioning and die
+     * on the username unique constraint.
+     */
+    @Test
+    void linkRefusedWhenSameNameAccountIsDisabled() throws Exception {
+        try (MockedStatic factory = bootstrappedRoller()) {
+            when(userManager.getUserByOpenIdUrl(SUBJECT)).thenReturn(null);
+            when(userManager.getUserByUserName("matt", null)).thenReturn(existingUser);
+            when(existingUser.getEnabled()).thenReturn(Boolean.FALSE);
+
+            OidcUser user = oidcUser(Map.of(
+                    "preferred_username", "matt",
+                    "email", "matt@example.com",
+                    "email_verified", Boolean.TRUE));
+
+            OAuth2AuthenticationException ex =
+                    assertThrows(OAuth2AuthenticationException.class, () -> service.resolveUser(user));
+
+            assertEquals("user_disabled", ex.getError().getErrorCode());
+            verify(userManager, never()).saveUser(any(User.class));
+            verify(userManager, never()).addUser(any(User.class));
+        }
+    }
+
     @Test
     void autoProvisionDisabledRejectsNewUser() throws Exception {
         try (MockedStatic factory = bootstrappedRoller();