Ensure authentication succeeds before customizing

Signed-off-by: Jonathan Howard <[email protected]>
stevespringett · Jan 31, 2025 · 398223f · 398223f
1 parent a0365fb
commit 398223f
Showing 1 changed file with 8 additions and 4 deletions.
diff --git a/alpine-server/src/main/java/alpine/server/auth/OidcAuthenticationService.java b/alpine-server/src/main/java/alpine/server/auth/OidcAuthenticationService.java
@@ -186,8 +186,9 @@ private OidcUser authenticateInternal(final OidcProfile profile) throws AlpineAu
                     LOGGER.debug("Assigning subject identifier " + profile.getSubject() + " to user " + user.getUsername());
                     user.setSubjectIdentifier(profile.getSubject());
                     user.setEmail(profile.getEmail());
+                    user = qm.updateOidcUser(user);
                     customizer.onAuthenticationSuccess(profile, idToken, accessToken);
-                    return qm.updateOidcUser(user);
+                    return user;
                 } else if (!user.getSubjectIdentifier().equals(profile.getSubject())) {
                     LOGGER.error("Refusing to authenticate user " + user.getUsername() + ": subject identifier has changed (" +
                             user.getSubjectIdentifier() + " to " + profile.getSubject() + ")");
@@ -199,15 +200,17 @@ private OidcUser authenticateInternal(final OidcProfile profile) throws AlpineAu
                     user = qm.updateOidcUser(user);
                 }
                 if (config.getPropertyAsBoolean(Config.AlpineKey.OIDC_TEAM_SYNCHRONIZATION)) {
+                    user = qm.synchronizeTeamMembership(user, profile.getGroups());
                     customizer.onAuthenticationSuccess(profile, idToken, accessToken);
-                    return qm.synchronizeTeamMembership(user, profile.getGroups());
+                    return user;
                 }
                 customizer.onAuthenticationSuccess(profile, idToken, accessToken);
                 return user;
             } else if (config.getPropertyAsBoolean(Config.AlpineKey.OIDC_USER_PROVISIONING)) {
                 LOGGER.debug("The user (" + profile.getUsername() + ") authenticated successfully but the account has not been provisioned");
+                user = autoProvision(qm, profile);
                 customizer.onAuthenticationSuccess(profile, idToken, accessToken);
-                return autoProvision(qm, profile);
+                return user;
             } else {
                 LOGGER.debug("The user (" + profile.getUsername() + ") is unmapped and user provisioning is not enabled");
                 throw new AlpineAuthenticationException(AlpineAuthenticationException.CauseType.UNMAPPED_ACCOUNT);
@@ -224,8 +227,9 @@ private OidcUser autoProvision(final AlpineQueryManager qm, final OidcProfile pr
 
         if (config.getPropertyAsBoolean(Config.AlpineKey.OIDC_TEAM_SYNCHRONIZATION)) {
             LOGGER.debug("Synchronizing teams for user " + user.getUsername());
+            user = qm.synchronizeTeamMembership(user, profile.getGroups());
             customizer.onAuthenticationSuccess(profile, idToken, accessToken);
-            return qm.synchronizeTeamMembership(user, profile.getGroups());
+            return user;
         }
 
         final List<String> defaultTeams = config.getPropertyAsList(Config.AlpineKey.OIDC_TEAMS_DEFAULT);