Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix OIDC logout unnecessary redirect #4773

Merged
merged 3 commits into from
Mar 4, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
@@ -13,6 +13,7 @@ Apollo 2.2.0
* [[Multi-Database Support] Without Reliance on boolean integer compare](https://github.com/apolloconfig/apollo/pull/4757)
* [[Multi-Database Support] package postgre h2 dependency](https://github.com/apolloconfig/apollo/pull/4757)
* [[Multi-Database Support] Optimize table case](https://github.com/apolloconfig/apollo/pull/4768)
* [Fix OIDC logout unnecessary redirect](https://github.com/apolloconfig/apollo/pull/4773)

------------------
All issues and pull requests are [here](https://github.com/apolloconfig/apollo/milestone/13?closed=1)
Original file line number Diff line number Diff line change
@@ -390,6 +390,7 @@ protected void configure(HttpSecurity http) throws Exception {
this.clientRegistrationRepository)));
http.oauth2Client();
http.logout(configure -> {
configure.logoutUrl("/user/logout");
OidcClientInitiatedLogoutSuccessHandler logoutSuccessHandler = new OidcClientInitiatedLogoutSuccessHandler(
this.clientRegistrationRepository);
logoutSuccessHandler.setPostLogoutRedirectUri("{baseUrl}");
Original file line number Diff line number Diff line change
@@ -18,6 +18,8 @@

import com.ctrip.framework.apollo.portal.entity.bo.UserInfo;
import com.ctrip.framework.apollo.portal.spi.configuration.OidcExtendProperties;
import java.util.Map;
import java.util.Map.Entry;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import org.slf4j.Logger;
@@ -36,6 +38,12 @@ public class OidcAuthenticationSuccessEventListener implements
private static final Logger log = LoggerFactory
.getLogger(OidcAuthenticationSuccessEventListener.class);

private static final Logger oidcLog = LoggerFactory.getLogger(
OidcAuthenticationSuccessEventListener.class.getName() + ".oidc");

private static final Logger jwtLog = LoggerFactory.getLogger(
OidcAuthenticationSuccessEventListener.class.getName() + ".jwt");

private final OidcLocalUserService oidcLocalUserService;

private final OidcExtendProperties oidcExtendProperties;
@@ -63,18 +71,36 @@ public void onApplicationEvent(AuthenticationSuccessEvent event) {
}

private void oidcUserLogin(OidcUser oidcUser) {
String subject = oidcUser.getSubject();
String userDisplayName = OidcUserInfoUtil.getOidcUserDisplayName(oidcUser,
this.oidcExtendProperties);
String email = oidcUser.getEmail();

this.logOidc(oidcUser, subject, userDisplayName, email);

UserInfo newUserInfo = new UserInfo();
newUserInfo.setUserId(oidcUser.getSubject());
newUserInfo.setName(
OidcUserInfoUtil.getOidcUserDisplayName(oidcUser, this.oidcExtendProperties));
newUserInfo.setEmail(oidcUser.getEmail());
if (this.contains(oidcUser.getSubject())) {
newUserInfo.setUserId(subject);
newUserInfo.setName(userDisplayName);
newUserInfo.setEmail(email);
if (this.contains(subject)) {
this.oidcLocalUserService.updateUserInfo(newUserInfo);
return;
}
this.oidcLocalUserService.createLocalUser(newUserInfo);
}

private void logOidc(OidcUser oidcUser, String subject, String userDisplayName,
String email) {
oidcLog.debug("oidc authentication success, sub=[{}] userDisplayName=[{}] email=[{}]", subject,
userDisplayName, email);
if (oidcLog.isTraceEnabled()) {
Map<String, Object> claims = oidcUser.getClaims();
for (Entry<String, Object> entry : claims.entrySet()) {
oidcLog.trace("oidc authentication claims [{}={}]", entry.getKey(), entry.getValue());
}
}
}

private boolean contains(String userId) {
if (this.userIdCache.containsKey(userId)) {
return true;
@@ -88,12 +114,29 @@ private boolean contains(String userId) {
}

private void jwtLogin(Jwt jwt) {
if (this.contains(jwt.getSubject())) {
String subject = jwt.getSubject();
String userDisplayName = OidcUserInfoUtil.getJwtUserDisplayName(jwt,
this.oidcExtendProperties);

this.logJwt(jwt, subject, userDisplayName);

if (this.contains(subject)) {
return;
}
UserInfo newUserInfo = new UserInfo();
newUserInfo.setUserId(jwt.getSubject());
newUserInfo.setName(OidcUserInfoUtil.getJwtUserDisplayName(jwt, this.oidcExtendProperties));
newUserInfo.setUserId(subject);
newUserInfo.setName(userDisplayName);
this.oidcLocalUserService.createLocalUser(newUserInfo);
}

private void logJwt(Jwt jwt, String subject, String userDisplayName) {
jwtLog.debug("jwt authentication success, sub=[{}] userDisplayName=[{}]", subject,
userDisplayName);
if (jwtLog.isTraceEnabled()) {
Map<String, Object> claims = jwt.getClaims();
for (Entry<String, Object> entry : claims.entrySet()) {
jwtLog.trace("jwt authentication claims [{}={}]", entry.getKey(), entry.getValue());
}
}
}
}
Original file line number Diff line number Diff line change
@@ -28,10 +28,6 @@ public class OidcLogoutHandler implements LogoutHandler {

@Override
public void logout(HttpServletRequest request, HttpServletResponse response) {
try {
response.sendRedirect(request.getContextPath() + "/logout");
} catch (IOException e) {
throw new RuntimeException(e);
}
// do nothing here
}
}