From acabb69e1f4c9704fb8d9a0228d91b6d817df43b Mon Sep 17 00:00:00 2001 From: DarioGii Date: Thu, 30 Jan 2025 18:42:32 +0000 Subject: [PATCH] wip - testing different name attributes for SSO --- .../security/CustomLogoutSuccessHandler.java | 4 ++- .../oauth2/CustomOAuth2UserService.java | 16 +++++---- .../security/oauth2/OAuth2Configuration.java | 34 ++++++++++--------- .../controller/web/AccountWebController.java | 2 +- .../SPDF/model/ApplicationProperties.java | 10 +++--- .../exception/NoProviderFoundException.java | 11 ++++++ .../SPDF/model/provider/Provider.java | 4 +-- src/main/resources/settings.yml.template | 2 +- src/main/resources/templates/login.html | 4 +-- 9 files changed, 51 insertions(+), 36 deletions(-) create mode 100644 src/main/java/stirling/software/SPDF/model/exception/NoProviderFoundException.java diff --git a/src/main/java/stirling/software/SPDF/config/security/CustomLogoutSuccessHandler.java b/src/main/java/stirling/software/SPDF/config/security/CustomLogoutSuccessHandler.java index 8f6460606..f2d02bec4 100644 --- a/src/main/java/stirling/software/SPDF/config/security/CustomLogoutSuccessHandler.java +++ b/src/main/java/stirling/software/SPDF/config/security/CustomLogoutSuccessHandler.java @@ -169,7 +169,9 @@ public class CustomLogoutSuccessHandler extends SimpleUrlLogoutSuccessHandler { } /** - * Handles different error scenarios during logout. Will return a String containing the error request parameter. + * Handles different error scenarios during logout. Will return a String containing + * the error request parameter. + * * @param request the user's HttpServletRequest request. * @return a String containing the error request parameter. */ 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 81fb2f8ae..0941ecdc4 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 @@ -24,11 +24,11 @@ public class CustomOAuth2UserService implements OAuth2UserService duser = userService.findByUsernameIgnoreCase(username); - if (duser.isPresent()) { + Optional internalUser = userService.findByUsernameIgnoreCase(username); + + if (internalUser.isPresent()) { if (loginAttemptService.isBlocked(username)) { throw new LockedException( "Your account has been locked due to too many failed login attempts."); diff --git a/src/main/java/stirling/software/SPDF/config/security/oauth2/OAuth2Configuration.java b/src/main/java/stirling/software/SPDF/config/security/oauth2/OAuth2Configuration.java index ac44a88b6..1cadc75bc 100644 --- a/src/main/java/stirling/software/SPDF/config/security/oauth2/OAuth2Configuration.java +++ b/src/main/java/stirling/software/SPDF/config/security/oauth2/OAuth2Configuration.java @@ -28,6 +28,7 @@ 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; +import stirling.software.SPDF.model.exception.NoProviderFoundException; import stirling.software.SPDF.model.provider.GitHubProvider; import stirling.software.SPDF.model.provider.GoogleProvider; import stirling.software.SPDF.model.provider.KeycloakProvider; @@ -51,7 +52,8 @@ public class OAuth2Configuration { @Bean @ConditionalOnProperty(value = "security.oauth2.enabled", havingValue = "true") - public ClientRegistrationRepository clientRegistrationRepository() { + public ClientRegistrationRepository clientRegistrationRepository() + throws NoProviderFoundException { List registrations = new ArrayList<>(); githubClientRegistration().ifPresent(registrations::add); oidcClientRegistration().ifPresent(registrations::add); @@ -59,8 +61,8 @@ public class OAuth2Configuration { keycloakClientRegistration().ifPresent(registrations::add); if (registrations.isEmpty()) { - log.error("At least one OAuth2 provider must be configured"); - System.exit(1); + log.error("No OAuth2 provider registered"); + throw new NoProviderFoundException("At least one OAuth2 provider must be configured."); } return new InMemoryClientRegistrationRepository(registrations); @@ -69,7 +71,7 @@ public class OAuth2Configuration { private Optional keycloakClientRegistration() { OAUTH2 oauth2 = applicationProperties.getSecurity().getOauth2(); - if (isOauthOrClientEmpty(oauth2)) { + if (isOAuth2Enabled(oauth2) || isClientInitialised(oauth2)) { return Optional.empty(); } @@ -97,13 +99,13 @@ public class OAuth2Configuration { } private Optional googleClientRegistration() { - OAUTH2 oauth2 = applicationProperties.getSecurity().getOauth2(); + OAUTH2 oAuth2 = applicationProperties.getSecurity().getOauth2(); - if (isOauthOrClientEmpty(oauth2)) { + if (isOAuth2Enabled(oAuth2) || isClientInitialised(oAuth2)) { return Optional.empty(); } - Client client = oauth2.getClient(); + Client client = oAuth2.getClient(); GoogleProvider googleClient = client.getGoogle(); Provider google = new GoogleProvider( @@ -130,13 +132,13 @@ public class OAuth2Configuration { } private Optional githubClientRegistration() { - OAUTH2 oauth2 = applicationProperties.getSecurity().getOauth2(); + OAUTH2 oAuth2 = applicationProperties.getSecurity().getOauth2(); - if (isOauthOrClientEmpty(oauth2)) { + if (isOAuth2Enabled(oAuth2)) { return Optional.empty(); } - Client client = oauth2.getClient(); + Client client = oAuth2.getClient(); GitHubProvider githubClient = client.getGithub(); Provider github = new GitHubProvider( @@ -165,7 +167,7 @@ public class OAuth2Configuration { private Optional oidcClientRegistration() { OAUTH2 oauth = applicationProperties.getSecurity().getOauth2(); - if (isOauthOrClientEmpty(oauth)) { + if (isOAuth2Enabled(oauth) || isClientInitialised(oauth)) { return Optional.empty(); } @@ -194,12 +196,12 @@ public class OAuth2Configuration { .build()); } - private boolean isOauthOrClientEmpty(OAUTH2 oauth) { - if (oauth == null || !oauth.getEnabled()) { - return false; - } + private boolean isOAuth2Enabled(OAUTH2 oAuth2) { + return oAuth2 == null || !oAuth2.getEnabled(); + } - Client client = oauth.getClient(); + private boolean isClientInitialised(OAUTH2 oauth2) { + Client client = oauth2.getClient(); return client == null; } 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 f59017161..4dfcf0ca6 100644 --- a/src/main/java/stirling/software/SPDF/controller/web/AccountWebController.java +++ b/src/main/java/stirling/software/SPDF/controller/web/AccountWebController.java @@ -114,7 +114,7 @@ public class AccountWebController { providerList .entrySet() .removeIf(entry -> entry.getKey() == null || entry.getValue() == null); - model.addAttribute("providerlist", providerList); + model.addAttribute("providerList", providerList); model.addAttribute("loginMethod", securityProps.getLoginMethod()); boolean altLogin = !providerList.isEmpty() ? securityProps.isAltLogin() : false; diff --git a/src/main/java/stirling/software/SPDF/model/ApplicationProperties.java b/src/main/java/stirling/software/SPDF/model/ApplicationProperties.java index 053eb3779..38f05b6e1 100644 --- a/src/main/java/stirling/software/SPDF/model/ApplicationProperties.java +++ b/src/main/java/stirling/software/SPDF/model/ApplicationProperties.java @@ -245,11 +245,11 @@ public class ApplicationProperties { } public boolean isSettingsValid() { - return isStringEmpty(this.getIssuer()) - && isStringEmpty(this.getClientId()) - && isStringEmpty(this.getClientSecret()) - && isCollectionEmpty(this.getScopes()) - && isStringEmpty(this.getUseAsUsername()); + return !isStringEmpty(this.getIssuer()) + && !isStringEmpty(this.getClientId()) + && !isStringEmpty(this.getClientSecret()) + && !isCollectionEmpty(this.getScopes()) + && !isStringEmpty(this.getUseAsUsername()); } @Data diff --git a/src/main/java/stirling/software/SPDF/model/exception/NoProviderFoundException.java b/src/main/java/stirling/software/SPDF/model/exception/NoProviderFoundException.java new file mode 100644 index 000000000..162070f38 --- /dev/null +++ b/src/main/java/stirling/software/SPDF/model/exception/NoProviderFoundException.java @@ -0,0 +1,11 @@ +package stirling.software.SPDF.model.exception; + +public class NoProviderFoundException extends Exception { + public NoProviderFoundException(String message) { + super(message); + } + + public NoProviderFoundException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/src/main/java/stirling/software/SPDF/model/provider/Provider.java b/src/main/java/stirling/software/SPDF/model/provider/Provider.java index c4606ce25..903ec6e92 100644 --- a/src/main/java/stirling/software/SPDF/model/provider/Provider.java +++ b/src/main/java/stirling/software/SPDF/model/provider/Provider.java @@ -57,9 +57,7 @@ public class Provider { @Override public String toString() { - return "Provider [issuer=" - + getIssuer() - + ", name=" + return "Provider [name=" + getName() + ", clientName=" + getClientName() diff --git a/src/main/resources/settings.yml.template b/src/main/resources/settings.yml.template index 5cfea15f1..6e7681c1d 100644 --- a/src/main/resources/settings.yml.template +++ b/src/main/resources/settings.yml.template @@ -32,7 +32,7 @@ security: google: clientId: '' # client ID for Google OAuth2 clientSecret: '' # client secret for Google OAuth2 - scopes: https://www.googleapis.com/auth/userinfo.email, https://www.googleapis.com/auth/userinfo.profile # scopes for Google OAuth2 + scopes: email, profile # scopes for Google OAuth2 useAsUsername: email # field to use as the username for Google OAuth2 github: clientId: '' # client ID for GitHub OAuth2 diff --git a/src/main/resources/templates/login.html b/src/main/resources/templates/login.html index 1d248154b..dfd44822d 100644 --- a/src/main/resources/templates/login.html +++ b/src/main/resources/templates/login.html @@ -42,7 +42,7 @@ const runningEE = /*[[${@runningEE}]]*/ false; const SSOAutoLogin = /*[[${@SSOAutoLogin}]]*/ false; const loginMethod = /*[[${loginMethod}]]*/ 'normal'; - const providerList = /*[[${providerlist}]]*/ {}; + const providerList = /*[[${providerList}]]*/ {}; const shouldAutoRedirect = !hasRedirectError && !hasLogout && !hasMessage && @@ -164,7 +164,7 @@