From 7d9edfca6d7abced4b51616e29190cbc08d96332 Mon Sep 17 00:00:00 2001
From: Ludy87 <Ludy87@users.noreply.github.com>
Date: Thu, 6 Jun 2024 21:03:06 +0200
Subject: [PATCH] Enhance OAuth2 Client Registration with Dynamic Provider
 Details

---
 .../security/SecurityConfiguration.java       | 16 ++++++------
 .../CustomOAuth2LogoutSuccessHandler.java     |  2 +-
 .../oauth2/CustomOAuth2UserService.java       | 25 ++++++++++++++++---
 .../controller/web/AccountWebController.java  |  8 +++---
 .../SPDF/model/ApplicationProperties.java     | 18 +++++++++++--
 .../software/SPDF/model/Provider.java         |  5 ++++
 6 files changed, 56 insertions(+), 18 deletions(-)

diff --git a/src/main/java/stirling/software/SPDF/config/security/SecurityConfiguration.java b/src/main/java/stirling/software/SPDF/config/security/SecurityConfiguration.java
index ce6ce799..fad0f285 100644
--- a/src/main/java/stirling/software/SPDF/config/security/SecurityConfiguration.java
+++ b/src/main/java/stirling/software/SPDF/config/security/SecurityConfiguration.java
@@ -238,7 +238,7 @@ public class SecurityConfiguration {
         GoogleProvider google = client.getGoogle();
         return google != null && google.isSettingsValid()
                 ? Optional.of(
-                        ClientRegistration.withRegistrationId("google")
+                        ClientRegistration.withRegistrationId(google.getName())
                                 .clientId(google.getClientId())
                                 .clientSecret(google.getClientSecret())
                                 .scope(google.getScopes())
@@ -246,8 +246,8 @@ public class SecurityConfiguration {
                                 .tokenUri(google.getTokenuri())
                                 .userInfoUri(google.getUserinfouri())
                                 .userNameAttributeName(google.getUseAsUsername())
-                                .clientName("Google")
-                                .redirectUri("{baseUrl}/login/oauth2/code/google")
+                                .clientName(google.getClientName())
+                                .redirectUri("{baseUrl}/login/oauth2/code/" + google.getName())
                                 .authorizationGrantType(
                                         org.springframework.security.oauth2.core
                                                 .AuthorizationGrantType.AUTHORIZATION_CODE)
@@ -269,12 +269,12 @@ public class SecurityConfiguration {
         return keycloak != null && keycloak.isSettingsValid()
                 ? Optional.of(
                         ClientRegistrations.fromIssuerLocation(keycloak.getIssuer())
-                                .registrationId("keycloak")
+                                .registrationId(keycloak.getName())
                                 .clientId(keycloak.getClientId())
                                 .clientSecret(keycloak.getClientSecret())
                                 .scope(keycloak.getScopes())
                                 .userNameAttributeName(keycloak.getUseAsUsername())
-                                .clientName("Keycloak")
+                                .clientName(keycloak.getClientName())
                                 .build())
                 : Optional.empty();
     }
@@ -291,7 +291,7 @@ public class SecurityConfiguration {
         GithubProvider github = client.getGithub();
         return github != null && github.isSettingsValid()
                 ? Optional.of(
-                        ClientRegistration.withRegistrationId("github")
+                        ClientRegistration.withRegistrationId(github.getName())
                                 .clientId(github.getClientId())
                                 .clientSecret(github.getClientSecret())
                                 .scope(github.getScopes())
@@ -299,8 +299,8 @@ public class SecurityConfiguration {
                                 .tokenUri(github.getTokenuri())
                                 .userInfoUri(github.getUserinfouri())
                                 .userNameAttributeName(github.getUseAsUsername())
-                                .clientName("GitHub")
-                                .redirectUri("{baseUrl}/login/oauth2/code/github")
+                                .clientName(github.getClientName())
+                                .redirectUri("{baseUrl}/login/oauth2/code/" + github.getName())
                                 .authorizationGrantType(
                                         org.springframework.security.oauth2.core
                                                 .AuthorizationGrantType.AUTHORIZATION_CODE)
diff --git a/src/main/java/stirling/software/SPDF/config/security/oauth2/CustomOAuth2LogoutSuccessHandler.java b/src/main/java/stirling/software/SPDF/config/security/oauth2/CustomOAuth2LogoutSuccessHandler.java
index e209d0cd..9b12b279 100644
--- a/src/main/java/stirling/software/SPDF/config/security/oauth2/CustomOAuth2LogoutSuccessHandler.java
+++ b/src/main/java/stirling/software/SPDF/config/security/oauth2/CustomOAuth2LogoutSuccessHandler.java
@@ -81,7 +81,7 @@ public class CustomOAuth2LogoutSuccessHandler extends SimpleUrlLogoutSuccessHand
             logger.info("Session invalidated: " + sessionId);
         }
 
-        switch (registrationId) {
+        switch (registrationId.toLowerCase()) {
             case "keycloak":
                 // Add Keycloak specific logout URL if needed
                 String logoutUrl =
diff --git a/src/main/java/stirling/software/SPDF/config/security/oauth2/CustomOAuth2UserService.java b/src/main/java/stirling/software/SPDF/config/security/oauth2/CustomOAuth2UserService.java
index a5d65ff0..b9766480 100644
--- a/src/main/java/stirling/software/SPDF/config/security/oauth2/CustomOAuth2UserService.java
+++ b/src/main/java/stirling/software/SPDF/config/security/oauth2/CustomOAuth2UserService.java
@@ -16,6 +16,8 @@ import org.springframework.security.oauth2.core.oidc.user.OidcUser;
 import stirling.software.SPDF.config.security.LoginAttemptService;
 import stirling.software.SPDF.config.security.UserService;
 import stirling.software.SPDF.model.ApplicationProperties;
+import stirling.software.SPDF.model.ApplicationProperties.Security.OAUTH2;
+import stirling.software.SPDF.model.ApplicationProperties.Security.OAUTH2.Client;
 import stirling.software.SPDF.model.User;
 
 public class CustomOAuth2UserService implements OAuth2UserService<OidcUserRequest, OidcUser> {
@@ -41,11 +43,27 @@ public class CustomOAuth2UserService implements OAuth2UserService<OidcUserReques
 
     @Override
     public OidcUser loadUser(OidcUserRequest userRequest) throws OAuth2AuthenticationException {
-        String usernameAttribute =
-                applicationProperties.getSecurity().getOAUTH2().getUseAsUsername();
+        OAUTH2 oauth2 = applicationProperties.getSecurity().getOAUTH2();
+        String usernameAttribute = oauth2.getUseAsUsername();
+        if (usernameAttribute == null || usernameAttribute.trim().isEmpty()) {
+            Client client = oauth2.getClient();
+            if (client != null && client.getKeycloak() != null) {
+                usernameAttribute = client.getKeycloak().getUseAsUsername();
+            } else {
+                usernameAttribute = "email";
+            }
+        }
+
         try {
             OidcUser user = delegate.loadUser(userRequest);
             String username = user.getUserInfo().getClaimAsString(usernameAttribute);
+
+            // Check if the username claim is null or empty
+            if (username == null || username.trim().isEmpty()) {
+                throw new IllegalArgumentException(
+                        "Claim '" + usernameAttribute + "' cannot be null or empty");
+            }
+
             Optional<User> duser = userService.findByUsernameIgnoreCase(username);
             if (duser.isPresent()) {
                 if (loginAttemptService.isBlocked(username)) {
@@ -56,13 +74,14 @@ public class CustomOAuth2UserService implements OAuth2UserService<OidcUserReques
                     throw new IllegalArgumentException("Password must not be null");
                 }
             }
+
             // Return a new OidcUser with adjusted attributes
             return new DefaultOidcUser(
                     user.getAuthorities(),
                     userRequest.getIdToken(),
                     user.getUserInfo(),
                     usernameAttribute);
-        } catch (java.lang.IllegalArgumentException e) {
+        } catch (IllegalArgumentException e) {
             logger.error("Error loading OIDC user: {}", e.getMessage());
             throw new OAuth2AuthenticationException(new OAuth2Error(e.getMessage()), e);
         } catch (Exception e) {
diff --git a/src/main/java/stirling/software/SPDF/controller/web/AccountWebController.java b/src/main/java/stirling/software/SPDF/controller/web/AccountWebController.java
index 79a84ecb..9d67e395 100644
--- a/src/main/java/stirling/software/SPDF/controller/web/AccountWebController.java
+++ b/src/main/java/stirling/software/SPDF/controller/web/AccountWebController.java
@@ -52,23 +52,23 @@ public class AccountWebController {
         OAUTH2 oauth = applicationProperties.getSecurity().getOAUTH2();
         if (oauth != null) {
             if (oauth.isSettingsValid()) {
-                providerList.put("oidc", "OpenID Connect");
+                providerList.put("oidc", oauth.getProvider());
             }
             Client client = oauth.getClient();
             if (client != null) {
                 GoogleProvider google = client.getGoogle();
                 if (google.isSettingsValid()) {
-                    providerList.put("google", "Google");
+                    providerList.put(google.getName(), google.getClientName());
                 }
 
                 GithubProvider github = client.getGithub();
                 if (github.isSettingsValid()) {
-                    providerList.put("github", "Github");
+                    providerList.put(github.getName(), github.getClientName());
                 }
 
                 KeycloakProvider keycloak = client.getKeycloak();
                 if (keycloak.isSettingsValid()) {
-                    providerList.put("keycloak", "Keycloak");
+                    providerList.put(keycloak.getName(), keycloak.getClientName());
                 }
             }
         }
diff --git a/src/main/java/stirling/software/SPDF/model/ApplicationProperties.java b/src/main/java/stirling/software/SPDF/model/ApplicationProperties.java
index 8b78e130..3ba5ffb6 100644
--- a/src/main/java/stirling/software/SPDF/model/ApplicationProperties.java
+++ b/src/main/java/stirling/software/SPDF/model/ApplicationProperties.java
@@ -356,7 +356,7 @@ public class ApplicationProperties {
                 private KeycloakProvider keycloak = new KeycloakProvider();
 
                 public Provider get(String registrationId) throws Exception {
-                    switch (registrationId) {
+                    switch (registrationId.toLowerCase()) {
                         case "google":
                             return getGoogle();
                         case "github":
@@ -495,6 +495,11 @@ public class ApplicationProperties {
             return "google";
         }
 
+        @Override
+        public String getClientName() {
+            return "Google";
+        }
+
         public boolean isSettingsValid() {
             return super.isValid(this.getClientId(), "clientId")
                     && super.isValid(this.getClientSecret(), "clientSecret")
@@ -594,6 +599,11 @@ public class ApplicationProperties {
             return "github";
         }
 
+        @Override
+        public String getClientName() {
+            return "GitHub";
+        }
+
         public boolean isSettingsValid() {
             return super.isValid(this.getClientId(), "clientId")
                     && super.isValid(this.getClientSecret(), "clientSecret")
@@ -642,7 +652,6 @@ public class ApplicationProperties {
         @Override
         public Collection<String> getScopes() {
             if (scopes == null || scopes.isEmpty()) {
-                scopes.add("openid");
                 scopes.add("profile");
                 scopes.add("email");
             }
@@ -684,6 +693,11 @@ public class ApplicationProperties {
             return "keycloak";
         }
 
+        @Override
+        public String getClientName() {
+            return "Keycloak";
+        }
+
         public boolean isSettingsValid() {
             return isValid(this.getIssuer(), "issuer")
                     && isValid(this.getClientId(), "clientId")
diff --git a/src/main/java/stirling/software/SPDF/model/Provider.java b/src/main/java/stirling/software/SPDF/model/Provider.java
index a8dce446..c378975d 100644
--- a/src/main/java/stirling/software/SPDF/model/Provider.java
+++ b/src/main/java/stirling/software/SPDF/model/Provider.java
@@ -4,11 +4,16 @@ import java.util.Collection;
 
 public class Provider implements ProviderInterface {
     private String name;
+    private String clientName;
 
     public String getName() {
         return name;
     }
 
+    public String getClientName() {
+        return clientName;
+    }
+
     protected boolean isValid(String value, String name) {
         if (value != null && !value.trim().isEmpty()) {
             return true;