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 99d02651..a4631d94 100644 --- a/src/main/java/stirling/software/SPDF/config/security/CustomLogoutSuccessHandler.java +++ b/src/main/java/stirling/software/SPDF/config/security/CustomLogoutSuccessHandler.java @@ -1,32 +1,27 @@ package stirling.software.SPDF.config.security; +import com.coveo.saml.SamlClient; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; import java.io.IOException; import java.security.cert.X509Certificate; import java.security.interfaces.RSAPrivateKey; import java.util.ArrayList; import java.util.List; - +import lombok.AllArgsConstructor; +import lombok.extern.slf4j.Slf4j; import org.springframework.core.io.Resource; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.core.Authentication; import org.springframework.security.oauth2.client.authentication.OAuth2AuthenticationToken; import org.springframework.security.saml2.provider.service.authentication.Saml2Authentication; import org.springframework.security.web.authentication.logout.SimpleUrlLogoutSuccessHandler; - -import com.coveo.saml.SamlClient; - -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; -import lombok.AllArgsConstructor; -import lombok.extern.slf4j.Slf4j; import stirling.software.SPDF.SPDFApplication; import stirling.software.SPDF.config.security.saml2.CertificateUtils; import stirling.software.SPDF.config.security.saml2.CustomSaml2AuthenticatedPrincipal; import stirling.software.SPDF.model.ApplicationProperties; import stirling.software.SPDF.model.ApplicationProperties.Security.OAUTH2; import stirling.software.SPDF.model.ApplicationProperties.Security.SAML2; -import stirling.software.SPDF.model.exception.UnsupportedProviderException; -import stirling.software.SPDF.model.provider.Provider; import stirling.software.SPDF.utils.UrlUtils; @Slf4j @@ -36,23 +31,20 @@ public class CustomLogoutSuccessHandler extends SimpleUrlLogoutSuccessHandler { private final ApplicationProperties applicationProperties; @Override - public void onLogoutSuccess( - HttpServletRequest request, HttpServletResponse response, Authentication authentication) + public void onLogoutSuccess(HttpServletRequest request, HttpServletResponse response, Authentication authentication) throws IOException { - if (!response.isCommitted()) { // Handle user logout due to disabled account if (request.getParameter("userIsDisabled") != null) { - response.sendRedirect( - request.getContextPath() + "/login?erroroauth=userIsDisabled"); - return; + response.sendRedirect(request.getContextPath() + "/login?erroroauth=userIsDisabled"); } + // Handle OAuth2 authentication error if (request.getParameter("oauth2AuthenticationErrorWeb") != null) { response.sendRedirect( request.getContextPath() + "/login?erroroauth=userAlreadyExistsWeb"); - return; } + if (authentication != null) { // Handle SAML2 logout redirection if (authentication instanceof Saml2Authentication) { @@ -66,10 +58,11 @@ public class CustomLogoutSuccessHandler extends SimpleUrlLogoutSuccessHandler { else if (authentication instanceof UsernamePasswordAuthenticationToken) { getRedirectStrategy().sendRedirect(request, response, "/login?logout=true"); } + // Handle unknown authentication types else { log.error( - "authentication class unknown: {}", + "Authentication class unknown: {}", authentication.getClass().getSimpleName()); getRedirectStrategy().sendRedirect(request, response, "/login?logout=true"); } @@ -145,28 +138,16 @@ public class CustomLogoutSuccessHandler extends SimpleUrlLogoutSuccessHandler { HttpServletRequest request, HttpServletResponse response, Authentication authentication) throws IOException { String param = "logout=true"; - String registrationId = null; - String issuer = null; - String clientId = null; + String registrationId; + String errorMessage; OAUTH2 oauth = applicationProperties.getSecurity().getOauth2(); if (authentication instanceof OAuth2AuthenticationToken oauthToken) { registrationId = oauthToken.getAuthorizedClientRegistrationId(); - - try { - // Get OAuth2 provider details from configuration - Provider provider = oauth.getClient().get(registrationId); - } catch (UnsupportedProviderException e) { - log.error(e.getMessage()); - } } else { registrationId = oauth.getProvider() != null ? oauth.getProvider() : ""; } - issuer = oauth.getIssuer(); - clientId = oauth.getClientId(); - String errorMessage = ""; - // Handle different error scenarios during logout if (request.getParameter("oauth2AuthenticationErrorWeb") != null) { param = "erroroauth=oauth2AuthenticationErrorWeb"; @@ -189,12 +170,11 @@ public class CustomLogoutSuccessHandler extends SimpleUrlLogoutSuccessHandler { // Redirect based on OAuth2 provider switch (registrationId.toLowerCase()) { case "keycloak" -> { - // Add Keycloak specific logout URL if needed String logoutUrl = - issuer + oauth.getIssuer() + "/protocol/openid-connect/logout" + "?client_id=" - + clientId + + oauth.getClientId() + "&post_logout_redirect_uri=" + response.encodeRedirectURL(redirectUrl); log.info("Redirecting to Keycloak logout URL: {}", logoutUrl); @@ -206,18 +186,13 @@ public class CustomLogoutSuccessHandler extends SimpleUrlLogoutSuccessHandler { redirectUrl); response.sendRedirect(redirectUrl); } - case "google" -> { - // Add Google specific logout URL if needed - // String googleLogoutUrl = + case "google" -> // String googleLogoutUrl = // "https://accounts.google.com/Logout?continue=https://appengine.google.com/_ah/logout?continue=" // + response.encodeRedirectURL(redirectUrl); - log.info("Google does not have a specific logout URL"); // log.info("Redirecting to Google logout URL: " + googleLogoutUrl); // response.sendRedirect(googleLogoutUrl); - } + log.info("Google does not have a specific logout URL"); default -> { - // String defaultRedirectUrl = request.getContextPath() + "/login?" + - // param; log.info("Redirecting to default logout URL: {}", redirectUrl); response.sendRedirect(redirectUrl); } 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 7fbc4436..63c291fd 100644 --- a/src/main/java/stirling/software/SPDF/config/security/SecurityConfiguration.java +++ b/src/main/java/stirling/software/SPDF/config/security/SecurityConfiguration.java @@ -226,7 +226,7 @@ public class SecurityConfiguration { .permitAll()); } // Handle OAUTH2 Logins - if (applicationProperties.getSecurity().isOauth2Activ()) { + if (applicationProperties.getSecurity().isOauth2Active()) { http.oauth2Login( oauth2 -> oauth2.loginPage("/oauth2") @@ -257,7 +257,7 @@ public class SecurityConfiguration { .permitAll()); } // Handle SAML - if (applicationProperties.getSecurity().isSaml2Activ()) { + if (applicationProperties.getSecurity().isSaml2Active()) { // && runningEE // Configure the authentication provider OpenSaml4AuthenticationProvider authenticationProvider = diff --git a/src/main/java/stirling/software/SPDF/config/security/oauth2/CustomOAuth2AuthenticationFailureHandler.java b/src/main/java/stirling/software/SPDF/config/security/oauth2/CustomOAuth2AuthenticationFailureHandler.java index 88905f76..f1c0efee 100644 --- a/src/main/java/stirling/software/SPDF/config/security/oauth2/CustomOAuth2AuthenticationFailureHandler.java +++ b/src/main/java/stirling/software/SPDF/config/security/oauth2/CustomOAuth2AuthenticationFailureHandler.java @@ -52,7 +52,6 @@ public class CustomOAuth2AuthenticationFailureHandler log.error("OAuth2 Authentication error: " + errorCode); log.error("OAuth2AuthenticationException", exception); getRedirectStrategy().sendRedirect(request, response, "/login?erroroauth=" + errorCode); - return; } log.error("Unhandled authentication exception", exception); super.onAuthenticationFailure(request, response, exception); diff --git a/src/main/java/stirling/software/SPDF/config/security/oauth2/CustomOAuth2AuthenticationSuccessHandler.java b/src/main/java/stirling/software/SPDF/config/security/oauth2/CustomOAuth2AuthenticationSuccessHandler.java index 67f5e810..f0fac7f1 100644 --- a/src/main/java/stirling/software/SPDF/config/security/oauth2/CustomOAuth2AuthenticationSuccessHandler.java +++ b/src/main/java/stirling/software/SPDF/config/security/oauth2/CustomOAuth2AuthenticationSuccessHandler.java @@ -25,13 +25,12 @@ import stirling.software.SPDF.utils.RequestUriUtils; public class CustomOAuth2AuthenticationSuccessHandler extends SavedRequestAwareAuthenticationSuccessHandler { - private LoginAttemptService loginAttemptService; - - private ApplicationProperties applicationProperties; - private UserService userService; + private final LoginAttemptService loginAttemptService; + private final ApplicationProperties applicationProperties; + private final UserService userService; public CustomOAuth2AuthenticationSuccessHandler( - final LoginAttemptService loginAttemptService, + LoginAttemptService loginAttemptService, ApplicationProperties applicationProperties, UserService userService) { this.applicationProperties = applicationProperties; @@ -75,23 +74,22 @@ public class CustomOAuth2AuthenticationSuccessHandler throw new LockedException( "Your account has been locked due to too many failed login attempts."); } + if (userService.isUserDisabled(username)) { getRedirectStrategy() .sendRedirect(request, response, "/logout?userIsDisabled=true"); - return; } if (userService.usernameExistsIgnoreCase(username) && userService.hasPassword(username) && !userService.isAuthenticationTypeByUsername(username, AuthenticationType.SSO) && oAuth.getAutoCreateUser()) { response.sendRedirect(contextPath + "/logout?oauth2AuthenticationErrorWeb=true"); - return; } + try { if (oAuth.getBlockRegistration() && !userService.usernameExistsIgnoreCase(username)) { response.sendRedirect(contextPath + "/logout?oauth2_admin_blocked_user=true"); - return; } if (principal instanceof OAuth2User) { userService.processSSOPostLogin(username, oAuth.getAutoCreateUser()); 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 4c3bd126..ac44a88b 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,9 @@ 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.provider.GitHubProvider; +import stirling.software.SPDF.model.provider.GoogleProvider; +import stirling.software.SPDF.model.provider.KeycloakProvider; import stirling.software.SPDF.model.provider.Provider; @Slf4j @@ -59,17 +62,57 @@ public class OAuth2Configuration { log.error("At least one OAuth2 provider must be configured"); System.exit(1); } + return new InMemoryClientRegistrationRepository(registrations); } - private Optional googleClientRegistration() { - if (isOauthOrClientEmpty()) { + private Optional keycloakClientRegistration() { + OAUTH2 oauth2 = applicationProperties.getSecurity().getOauth2(); + + if (isOauthOrClientEmpty(oauth2)) { return Optional.empty(); } - Provider google = applicationProperties.getSecurity().getOauth2().getClient().getGoogle(); + Client client = oauth2.getClient(); + KeycloakProvider keycloakClient = client.getKeycloak(); + Provider keycloak = + new KeycloakProvider( + keycloakClient.getIssuer(), + keycloakClient.getClientId(), + keycloakClient.getClientSecret(), + keycloakClient.getScopes(), + keycloakClient.getUseAsUsername()); - return validateSettings(google) + return validateProvider(keycloak) + ? Optional.of( + ClientRegistrations.fromIssuerLocation(keycloak.getIssuer()) + .registrationId(keycloak.getName()) + .clientId(keycloak.getClientId()) + .clientSecret(keycloak.getClientSecret()) + .scope(keycloak.getScopes()) + .userNameAttributeName(keycloak.getUseAsUsername()) + .clientName(keycloak.getClientName()) + .build()) + : Optional.empty(); + } + + private Optional googleClientRegistration() { + OAUTH2 oauth2 = applicationProperties.getSecurity().getOauth2(); + + if (isOauthOrClientEmpty(oauth2)) { + return Optional.empty(); + } + + Client client = oauth2.getClient(); + GoogleProvider googleClient = client.getGoogle(); + Provider google = + new GoogleProvider( + googleClient.getClientId(), + googleClient.getClientSecret(), + googleClient.getScopes(), + googleClient.getUseAsUsername()); + + return validateProvider(google) ? Optional.of( ClientRegistration.withRegistrationId(google.getName()) .clientId(google.getClientId()) @@ -86,35 +129,23 @@ public class OAuth2Configuration { : Optional.empty(); } - private Optional keycloakClientRegistration() { - if (isOauthOrClientEmpty()) { - return Optional.empty(); - } - - Provider keycloak = - applicationProperties.getSecurity().getOauth2().getClient().getKeycloak(); - - return validateSettings(keycloak) - ? Optional.of( - ClientRegistrations.fromIssuerLocation(keycloak.getIssuer()) - .registrationId(keycloak.getName()) - .clientId(keycloak.getClientId()) - .clientSecret(keycloak.getClientSecret()) - .scope(keycloak.getScopes()) - .userNameAttributeName(keycloak.getUseAsUsername()) - .clientName(keycloak.getClientName()) - .build()) - : Optional.empty(); - } - private Optional githubClientRegistration() { - if (isOauthOrClientEmpty()) { + OAUTH2 oauth2 = applicationProperties.getSecurity().getOauth2(); + + if (isOauthOrClientEmpty(oauth2)) { return Optional.empty(); } - Provider github = applicationProperties.getSecurity().getOauth2().getClient().getGithub(); + Client client = oauth2.getClient(); + GitHubProvider githubClient = client.getGithub(); + Provider github = + new GitHubProvider( + githubClient.getClientId(), + githubClient.getClientSecret(), + githubClient.getScopes(), + githubClient.getUseAsUsername()); - return validateSettings(github) + return validateProvider(github) ? Optional.of( ClientRegistration.withRegistrationId(github.getName()) .clientId(github.getClientId()) @@ -134,42 +165,41 @@ public class OAuth2Configuration { private Optional oidcClientRegistration() { OAUTH2 oauth = applicationProperties.getSecurity().getOauth2(); - if (oauth == null - || oauth.getIssuer() == null - || oauth.getIssuer().isEmpty() - || oauth.getClientId() == null - || oauth.getClientId().isEmpty() - || oauth.getClientSecret() == null - || oauth.getClientSecret().isEmpty() - || oauth.getScopes() == null - || oauth.getScopes().isEmpty() - || oauth.getUseAsUsername() == null - || oauth.getUseAsUsername().isEmpty()) { + if (isOauthOrClientEmpty(oauth)) { return Optional.empty(); } + if (isStringEmpty(oauth.getIssuer()) + || isStringEmpty(oauth.getClientId()) + || isStringEmpty(oauth.getClientSecret()) + || isCollectionEmpty(oauth.getScopes()) + || isStringEmpty(oauth.getUseAsUsername())) { + return Optional.empty(); + } + + String name = oauth.getProvider(); + String firstChar = String.valueOf(name.charAt(0)); + String clientName = name.replaceFirst(firstChar, firstChar.toUpperCase()); + return Optional.of( ClientRegistrations.fromIssuerLocation(oauth.getIssuer()) - .registrationId("oidc") + .registrationId(name) .clientId(oauth.getClientId()) .clientSecret(oauth.getClientSecret()) .scope(oauth.getScopes()) .userNameAttributeName(oauth.getUseAsUsername()) - .clientName("OIDC") - .redirectUri(REDIRECT_URI_PATH + "oidc") + .clientName(clientName) + .redirectUri(REDIRECT_URI_PATH + name) .authorizationGrantType(AUTHORIZATION_CODE) .build()); } - private boolean isOauthOrClientEmpty() { - OAUTH2 oauth = applicationProperties.getSecurity().getOauth2(); - + private boolean isOauthOrClientEmpty(OAUTH2 oauth) { if (oauth == null || !oauth.getEnabled()) { return false; } Client client = oauth.getClient(); - return client == null; } @@ -202,11 +232,9 @@ public class OAuth2Configuration { (String) oauth2Auth.getAttributes().get(useAsUsername)); if (userOpt.isPresent()) { User user = userOpt.get(); - if (user != null) { - mappedAuthorities.add( - new SimpleGrantedAuthority( - userService.findRole(user).getAuthority())); - } + mappedAuthorities.add( + new SimpleGrantedAuthority( + userService.findRole(user).getAuthority())); } } }); 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 36d6c61c..8caa4808 100644 --- a/src/main/java/stirling/software/SPDF/controller/web/AccountWebController.java +++ b/src/main/java/stirling/software/SPDF/controller/web/AccountWebController.java @@ -1,6 +1,6 @@ package stirling.software.SPDF.controller.web; -import static stirling.software.SPDF.utils.validation.Validator.validateSettings; +import static stirling.software.SPDF.utils.validation.Validator.validateProvider; import java.time.Instant; import java.time.temporal.ChronoUnit; @@ -40,12 +40,11 @@ import stirling.software.SPDF.repository.UserRepository; public class AccountWebController { public static final String OAUTH_2_AUTHORIZATION = "/oauth2/authorization/"; + private final ApplicationProperties applicationProperties; - private final SessionPersistentRegistry sessionPersistentRegistry; - - private final UserRepository // Assuming you have a repository for user operations - userRepository; + // Assuming you have a repository for user operations + private final UserRepository userRepository; public AccountWebController( ApplicationProperties applicationProperties, @@ -70,7 +69,10 @@ public class AccountWebController { if (oauth != null) { if (oauth.getEnabled()) { if (oauth.isSettingsValid()) { - providerList.put(OAUTH_2_AUTHORIZATION + "oidc", oauth.getProvider()); + String firstChar = String.valueOf(oauth.getProvider().charAt(0)); + String clientName = + oauth.getProvider().replaceFirst(firstChar, firstChar.toUpperCase()); + providerList.put(OAUTH_2_AUTHORIZATION + "oidc", clientName); } Client client = oauth.getClient(); @@ -78,21 +80,21 @@ public class AccountWebController { if (client != null) { GoogleProvider google = client.getGoogle(); - if (validateSettings(google)) { + if (validateProvider(google)) { providerList.put( OAUTH_2_AUTHORIZATION + google.getName(), google.getClientName()); } GitHubProvider github = client.getGithub(); - if (validateSettings(github)) { + if (validateProvider(github)) { providerList.put( OAUTH_2_AUTHORIZATION + github.getName(), github.getClientName()); } KeycloakProvider keycloak = client.getKeycloak(); - if (validateSettings(keycloak)) { + if (validateProvider(keycloak)) { providerList.put( OAUTH_2_AUTHORIZATION + keycloak.getName(), keycloak.getClientName()); @@ -103,7 +105,7 @@ public class AccountWebController { SAML2 saml2 = securityProps.getSaml2(); - if (securityProps.isSaml2Activ() + if (securityProps.isSaml2Active() && applicationProperties.getSystem().getEnableAlphaFunctionality()) { providerList.put("/saml2/authenticate/" + saml2.getRegistrationId(), "SAML 2"); } @@ -321,40 +323,28 @@ public class AccountWebController { if (authentication != null && authentication.isAuthenticated()) { Object principal = authentication.getPrincipal(); String username = null; - if (principal instanceof UserDetails) { - // Cast the principal object to UserDetails - UserDetails userDetails = (UserDetails) principal; + if (principal instanceof UserDetails userDetails) { // Retrieve username and other attributes username = userDetails.getUsername(); // Add oAuth2 Login attributes to the model model.addAttribute("oAuth2Login", false); } - if (principal instanceof OAuth2User) { - // Cast the principal object to OAuth2User - OAuth2User userDetails = (OAuth2User) principal; + if (principal instanceof OAuth2User userDetails) { // Retrieve username and other attributes - username = - userDetails.getAttribute( - applicationProperties.getSecurity().getOauth2().getUseAsUsername()); + username = userDetails.getName(); // Add oAuth2 Login attributes to the model model.addAttribute("oAuth2Login", true); } - if (principal instanceof CustomSaml2AuthenticatedPrincipal) { - // Cast the principal object to OAuth2User - CustomSaml2AuthenticatedPrincipal userDetails = - (CustomSaml2AuthenticatedPrincipal) principal; + if (principal instanceof CustomSaml2AuthenticatedPrincipal userDetails) { // Retrieve username and other attributes username = userDetails.getName(); // Add oAuth2 Login attributes to the model model.addAttribute("oAuth2Login", true); } if (username != null) { - // Fetch user details from the database - Optional user = - userRepository - .findByUsernameIgnoreCaseWithSettings( // Assuming findByUsername - // method exists - username); + // Fetch user details from the database, assuming findByUsername method exists + Optional user = userRepository.findByUsernameIgnoreCaseWithSettings(username); + if (!user.isPresent()) { return "redirect:/error"; } @@ -368,31 +358,20 @@ public class AccountWebController { log.error("exception", e); return "redirect:/error"; } + String messageType = request.getParameter("messageType"); if (messageType != null) { switch (messageType) { - case "notAuthenticated": - messageType = "notAuthenticatedMessage"; - break; - case "userNotFound": - messageType = "userNotFoundMessage"; - break; - case "incorrectPassword": - messageType = "incorrectPasswordMessage"; - break; - case "usernameExists": - messageType = "usernameExistsMessage"; - break; - case "invalidUsername": - messageType = "invalidUsernameMessage"; - break; - default: - break; + case "notAuthenticated" -> messageType = "notAuthenticatedMessage"; + case "userNotFound" -> messageType = "userNotFoundMessage"; + case "incorrectPassword" -> messageType = "incorrectPasswordMessage"; + case "usernameExists" -> messageType = "usernameExistsMessage"; + case "invalidUsername" -> messageType = "invalidUsernameMessage"; } - model.addAttribute("messageType", messageType); } // Add attributes to the model model.addAttribute("username", username); + model.addAttribute("messageType", messageType); model.addAttribute("role", user.get().getRolesAsString()); model.addAttribute("settings", settingsJson); model.addAttribute("changeCredsFlag", user.get().isFirstLogin()); diff --git a/src/main/java/stirling/software/SPDF/model/ApplicationProperties.java b/src/main/java/stirling/software/SPDF/model/ApplicationProperties.java index 56bb3518..341ab5ae 100644 --- a/src/main/java/stirling/software/SPDF/model/ApplicationProperties.java +++ b/src/main/java/stirling/software/SPDF/model/ApplicationProperties.java @@ -1,5 +1,7 @@ package stirling.software.SPDF.model; +import static stirling.software.SPDF.utils.validation.Validator.*; + import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; @@ -49,11 +51,11 @@ public class ApplicationProperties { public PropertySource dynamicYamlPropertySource(ConfigurableEnvironment environment) throws IOException { String configPath = InstallationPathConfig.getSettingsPath(); - log.debug("Attempting to load settings from: " + configPath); + log.debug("Attempting to load settings from: {}", configPath); File file = new File(configPath); if (!file.exists()) { - log.error("Warning: Settings file does not exist at: " + configPath); + log.error("Warning: Settings file does not exist at: {}", configPath); } Resource resource = new FileSystemResource(configPath); @@ -66,7 +68,7 @@ public class ApplicationProperties { new YamlPropertySourceFactory().createPropertySource(null, encodedResource); environment.getPropertySources().addFirst(propertySource); - log.debug("Loaded properties: " + propertySource.getSource()); + log.debug("Loaded properties: {}", propertySource.getSource()); return propertySource; } @@ -135,13 +137,13 @@ public class ApplicationProperties { || loginMethod.equalsIgnoreCase(LoginMethods.ALL.toString())); } - public boolean isOauth2Activ() { + public boolean isOauth2Active() { return (oauth2 != null && oauth2.getEnabled() && !loginMethod.equalsIgnoreCase(LoginMethods.NORMAL.toString())); } - public boolean isSaml2Activ() { + public boolean isSaml2Active() { return (saml2 != null && saml2.getEnabled() && !loginMethod.equalsIgnoreCase(LoginMethods.NORMAL.toString())); @@ -240,11 +242,11 @@ public class ApplicationProperties { } public boolean isSettingsValid() { - return isValid(this.getIssuer(), "issuer") - && isValid(this.getClientId(), "clientId") - && isValid(this.getClientSecret(), "clientSecret") - && isValid(this.getScopes(), "scopes") - && isValid(this.getUseAsUsername(), "useAsUsername"); + return isStringEmpty(this.getIssuer()) + && isStringEmpty(this.getClientId()) + && isStringEmpty(this.getClientSecret()) + && isCollectionEmpty(this.getScopes()) + && isStringEmpty(this.getUseAsUsername()); } @Data @@ -262,7 +264,8 @@ public class ApplicationProperties { throw new UnsupportedProviderException( "Logout from the provider " + registrationId - + " is not supported. Report it at https://github.com/Stirling-Tools/Stirling-PDF/issues"); + + " is not supported. " + + "Report it at https://github.com/Stirling-Tools/Stirling-PDF/issues"); }; } } diff --git a/src/main/java/stirling/software/SPDF/model/provider/GitHubProvider.java b/src/main/java/stirling/software/SPDF/model/provider/GitHubProvider.java index 8bdf273b..5b2aa65c 100644 --- a/src/main/java/stirling/software/SPDF/model/provider/GitHubProvider.java +++ b/src/main/java/stirling/software/SPDF/model/provider/GitHubProvider.java @@ -14,14 +14,15 @@ public class GitHubProvider extends Provider { private static final String TOKEN_URI = "https://github.com/login/oauth/access_token"; private static final String USER_INFO_URI = "https://api.github.com/user"; - public GitHubProvider(String clientId, String clientSecret, String useAsUsername) { + public GitHubProvider( + String clientId, String clientSecret, Collection scopes, String useAsUsername) { super( null, NAME, CLIENT_NAME, clientId, clientSecret, - new ArrayList<>(), + scopes, useAsUsername != null ? useAsUsername : "login", AUTHORIZATION_URI, TOKEN_URI, @@ -70,7 +71,7 @@ public class GitHubProvider extends Provider { return "GitHub [clientId=" + getClientId() + ", clientSecret=" - + (getClientSecret() != null && !getClientSecret().isEmpty() ? "MASKED" : "NULL") + + (getClientSecret() != null && !getClientSecret().isEmpty() ? "*****" : "NULL") + ", scopes=" + getScopes() + ", useAsUsername=" diff --git a/src/main/java/stirling/software/SPDF/model/provider/GoogleProvider.java b/src/main/java/stirling/software/SPDF/model/provider/GoogleProvider.java index bdf4e99a..26557965 100644 --- a/src/main/java/stirling/software/SPDF/model/provider/GoogleProvider.java +++ b/src/main/java/stirling/software/SPDF/model/provider/GoogleProvider.java @@ -15,14 +15,15 @@ public class GoogleProvider extends Provider { private static final String USER_INFO_URI = "https://www.googleapis.com/oauth2/v3/userinfo?alt=json"; - public GoogleProvider(String clientId, String clientSecret, String useAsUsername) { + public GoogleProvider( + String clientId, String clientSecret, Collection scopes, String useAsUsername) { super( null, NAME, CLIENT_NAME, clientId, clientSecret, - new ArrayList<>(), + scopes, useAsUsername, AUTHORIZATION_URI, TOKEN_URI, @@ -69,7 +70,7 @@ public class GoogleProvider extends Provider { return "Google [clientId=" + getClientId() + ", clientSecret=" - + (getClientSecret() != null && !getClientSecret().isEmpty() ? "MASKED" : "NULL") + + (getClientSecret() != null && !getClientSecret().isEmpty() ? "*****" : "NULL") + ", scopes=" + getScopes() + ", useAsUsername=" diff --git a/src/main/java/stirling/software/SPDF/model/provider/KeycloakProvider.java b/src/main/java/stirling/software/SPDF/model/provider/KeycloakProvider.java index 59710c00..6649c2fa 100644 --- a/src/main/java/stirling/software/SPDF/model/provider/KeycloakProvider.java +++ b/src/main/java/stirling/software/SPDF/model/provider/KeycloakProvider.java @@ -12,14 +12,18 @@ public class KeycloakProvider extends Provider { private static final String CLIENT_NAME = "Keycloak"; public KeycloakProvider( - String issuer, String clientId, String clientSecret, String useAsUsername) { + String issuer, + String clientId, + String clientSecret, + Collection scopes, + String useAsUsername) { super( issuer, NAME, CLIENT_NAME, clientId, clientSecret, - new ArrayList<>(), + scopes, useAsUsername, null, null, @@ -38,7 +42,7 @@ public class KeycloakProvider extends Provider { @Override public Collection getScopes() { - var scopes = super.getScopes(); + Collection scopes = super.getScopes(); if (scopes == null || scopes.isEmpty()) { scopes = new ArrayList<>(); @@ -56,7 +60,7 @@ public class KeycloakProvider extends Provider { + ", clientId=" + getClientId() + ", clientSecret=" - + (getClientSecret() != null && !getClientSecret().isBlank() ? "MASKED" : "NULL") + + (getClientSecret() != null && !getClientSecret().isBlank() ? "*****" : "NULL") + ", scopes=" + getScopes() + ", useAsUsername=" 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 d1a85894..c4606ce2 100644 --- a/src/main/java/stirling/software/SPDF/model/provider/Provider.java +++ b/src/main/java/stirling/software/SPDF/model/provider/Provider.java @@ -1,5 +1,8 @@ package stirling.software.SPDF.model.provider; +import static stirling.software.SPDF.utils.validation.Validator.isStringEmpty; + +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.stream.Collectors; @@ -9,7 +12,7 @@ import lombok.NoArgsConstructor; @Data @NoArgsConstructor -public abstract class Provider { +public class Provider { private String issuer; private String name; @@ -38,8 +41,8 @@ public abstract class Provider { this.clientName = clientName; this.clientId = clientId; this.clientSecret = clientSecret; - this.scopes = scopes; - this.useAsUsername = !useAsUsername.isBlank() ? useAsUsername : "email"; + this.scopes = scopes == null ? new ArrayList<>() : scopes; + this.useAsUsername = isStringEmpty(useAsUsername) ? "email" : useAsUsername; this.authorizationUri = authorizationUri; this.tokenUri = tokenUri; this.userInfoUri = userInfoUri; @@ -51,4 +54,23 @@ public abstract class Provider { Arrays.stream(scopes.split(",")).map(String::trim).collect(Collectors.toList()); } } + + @Override + public String toString() { + return "Provider [issuer=" + + getIssuer() + + ", name=" + + getName() + + ", clientName=" + + getClientName() + + ", clientId=" + + getClientId() + + ", clientSecret=" + + (getClientSecret() != null && !getClientSecret().isEmpty() ? "*****" : "NULL") + + ", scopes=" + + getScopes() + + ", useAsUsername=" + + getUseAsUsername() + + "]"; + } } diff --git a/src/main/java/stirling/software/SPDF/utils/validation/Validator.java b/src/main/java/stirling/software/SPDF/utils/validation/Validator.java index 598c0a03..0a184235 100644 --- a/src/main/java/stirling/software/SPDF/utils/validation/Validator.java +++ b/src/main/java/stirling/software/SPDF/utils/validation/Validator.java @@ -6,7 +6,7 @@ import stirling.software.SPDF.model.provider.Provider; public class Validator { - public static boolean validateSettings(Provider provider) { + public static boolean validateProvider(Provider provider) { if (provider == null) { return false; } @@ -23,14 +23,18 @@ public class Validator { return false; } - return !isStringEmpty(provider.getUseAsUsername()); + if (isStringEmpty(provider.getUseAsUsername())) { + return false; + } + + return true; } - private static boolean isStringEmpty(String input) { + public static boolean isStringEmpty(String input) { return input == null || input.isBlank(); } - private static boolean isCollectionEmpty(Collection input) { + public static boolean isCollectionEmpty(Collection input) { return input == null || input.isEmpty(); } } diff --git a/src/test/java/stirling/software/SPDF/utils/validation/ValidatorTest.java b/src/test/java/stirling/software/SPDF/utils/validation/ValidatorTest.java index ba9a5106..b9860432 100644 --- a/src/test/java/stirling/software/SPDF/utils/validation/ValidatorTest.java +++ b/src/test/java/stirling/software/SPDF/utils/validation/ValidatorTest.java @@ -30,20 +30,22 @@ class ValidatorTest { when(provider.getScopes()).thenReturn(List.of("read:user")); when(provider.getUseAsUsername()).thenReturn("email"); - assertTrue(Validator.validateSettings(provider)); + assertTrue(Validator.validateProvider(provider)); } @ParameterizedTest @MethodSource("providerParams") void testUnsuccessfulValidation(Provider provider) { - assertFalse(Validator.validateSettings(provider)); + assertFalse(Validator.validateProvider(provider)); } public static Stream providerParams() { - var generic = new GitHubProvider(null, "clientSecret", " "); - var google = new GoogleProvider(null, "clientSecret", "email"); - var github = new GitHubProvider("clientId", "", "email"); - var keycloak = new KeycloakProvider("issuer", "clientId", "clientSecret", " "); + Provider generic = null; + var google = new GoogleProvider(null, "clientSecret", List.of("scope"), "email"); + var github = new GitHubProvider("clientId", "", List.of("scope"), "login"); + var keycloak = new KeycloakProvider("issuer", "clientId", "clientSecret", List.of("scope"), "email"); + + keycloak.setUseAsUsername(null); return Stream.of( Arguments.of(generic),