From 7793be694940488f02b23348547a79a95b763c4c Mon Sep 17 00:00:00 2001 From: DarioGii Date: Sat, 25 Jan 2025 20:05:09 +0000 Subject: [PATCH] wip --- .../security/CustomLogoutSuccessHandler.java | 22 ++-- .../security/oauth2/OAuth2Configuration.java | 24 ++-- .../security/saml2/SAML2Configuration.java | 16 +-- .../controller/web/AccountWebController.java | 116 ++++++++---------- .../SPDF/model/ApplicationProperties.java | 4 +- ...ithubProvider.java => GitHubProvider.java} | 52 +++++--- .../SPDF/model/provider/GoogleProvider.java | 45 ++++--- .../SPDF/model/provider/KeycloakProvider.java | 49 ++++---- .../SPDF/model/provider/Provider.java | 35 +++--- .../utils/validation/CollectionValidator.java | 11 -- .../utils/validation/StringValidator.java | 9 -- .../SPDF/utils/validation/Validator.java | 34 ++++- .../SPDF/utils/validation/ValidatorTest.java | 56 +++++++++ 13 files changed, 269 insertions(+), 204 deletions(-) rename src/main/java/stirling/software/SPDF/model/provider/{GithubProvider.java => GitHubProvider.java} (54%) delete mode 100644 src/main/java/stirling/software/SPDF/utils/validation/CollectionValidator.java delete mode 100644 src/main/java/stirling/software/SPDF/utils/validation/StringValidator.java create mode 100644 src/test/java/stirling/software/SPDF/utils/validation/ValidatorTest.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 86c893b0..99d02651 100644 --- a/src/main/java/stirling/software/SPDF/config/security/CustomLogoutSuccessHandler.java +++ b/src/main/java/stirling/software/SPDF/config/security/CustomLogoutSuccessHandler.java @@ -184,7 +184,7 @@ public class CustomLogoutSuccessHandler extends SimpleUrlLogoutSuccessHandler { param = "error=badcredentials"; } - String redirect_url = UrlUtils.getOrigin(request) + "/login?" + param; + String redirectUrl = UrlUtils.getOrigin(request) + "/login?" + param; // Redirect based on OAuth2 provider switch (registrationId.toLowerCase()) { @@ -196,30 +196,30 @@ public class CustomLogoutSuccessHandler extends SimpleUrlLogoutSuccessHandler { + "?client_id=" + clientId + "&post_logout_redirect_uri=" - + response.encodeRedirectURL(redirect_url); + + response.encodeRedirectURL(redirectUrl); log.info("Redirecting to Keycloak logout URL: {}", logoutUrl); response.sendRedirect(logoutUrl); } case "github" -> { - // Add GitHub specific logout URL if needed - // todo: why does the redirect go to github? shouldn't it come to Stirling PDF? - String githubLogoutUrl = "https://github.com/logout"; - log.info("Redirecting to GitHub logout URL: {}", redirect_url); - response.sendRedirect(redirect_url); + log.info( + "No redirect URL for GitHub. Redirecting to default logout URL: {}", + redirectUrl); + response.sendRedirect(redirectUrl); } case "google" -> { // Add Google specific logout URL if needed // String googleLogoutUrl = // "https://accounts.google.com/Logout?continue=https://appengine.google.com/_ah/logout?continue=" - // + response.encodeRedirectURL(redirect_url); + // + response.encodeRedirectURL(redirectUrl); log.info("Google does not have a specific logout URL"); // log.info("Redirecting to Google logout URL: " + googleLogoutUrl); // response.sendRedirect(googleLogoutUrl); } default -> { - String defaultRedirectUrl = request.getContextPath() + "/login?" + param; - log.info("Redirecting to default logout URL: {}", defaultRedirectUrl); - response.sendRedirect(defaultRedirectUrl); + // 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/oauth2/OAuth2Configuration.java b/src/main/java/stirling/software/SPDF/config/security/oauth2/OAuth2Configuration.java index e1c2a5b0..4c3bd126 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 @@ -1,6 +1,7 @@ package stirling.software.SPDF.config.security.oauth2; import static org.springframework.security.oauth2.core.AuthorizationGrantType.AUTHORIZATION_CODE; +import static stirling.software.SPDF.utils.validation.Validator.*; import java.util.ArrayList; import java.util.HashSet; @@ -27,9 +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.provider.GithubProvider; -import stirling.software.SPDF.model.provider.GoogleProvider; -import stirling.software.SPDF.model.provider.KeycloakProvider; +import stirling.software.SPDF.model.provider.Provider; @Slf4j @Configuration @@ -68,10 +67,9 @@ public class OAuth2Configuration { return Optional.empty(); } - GoogleProvider google = - applicationProperties.getSecurity().getOauth2().getClient().getGoogle(); + Provider google = applicationProperties.getSecurity().getOauth2().getClient().getGoogle(); - return google != null && google.isSettingsValid() + return validateSettings(google) ? Optional.of( ClientRegistration.withRegistrationId(google.getName()) .clientId(google.getClientId()) @@ -79,7 +77,7 @@ public class OAuth2Configuration { .scope(google.getScopes()) .authorizationUri(google.getAuthorizationUri()) .tokenUri(google.getTokenUri()) - .userInfoUri(google.getUserinfoUri()) + .userInfoUri(google.getUserInfoUri()) .userNameAttributeName(google.getUseAsUsername()) .clientName(google.getClientName()) .redirectUri(REDIRECT_URI_PATH + google.getName()) @@ -93,10 +91,10 @@ public class OAuth2Configuration { return Optional.empty(); } - KeycloakProvider keycloak = + Provider keycloak = applicationProperties.getSecurity().getOauth2().getClient().getKeycloak(); - return keycloak != null && keycloak.isSettingsValid() + return validateSettings(keycloak) ? Optional.of( ClientRegistrations.fromIssuerLocation(keycloak.getIssuer()) .registrationId(keycloak.getName()) @@ -114,10 +112,9 @@ public class OAuth2Configuration { return Optional.empty(); } - GithubProvider github = - applicationProperties.getSecurity().getOauth2().getClient().getGithub(); + Provider github = applicationProperties.getSecurity().getOauth2().getClient().getGithub(); - return github != null && github.isSettingsValid() + return validateSettings(github) ? Optional.of( ClientRegistration.withRegistrationId(github.getName()) .clientId(github.getClientId()) @@ -125,7 +122,7 @@ public class OAuth2Configuration { .scope(github.getScopes()) .authorizationUri(github.getAuthorizationUri()) .tokenUri(github.getTokenUri()) - .userInfoUri(github.getUserinfoUri()) + .userInfoUri(github.getUserInfoUri()) .userNameAttributeName(github.getUseAsUsername()) .clientName(github.getClientName()) .redirectUri(REDIRECT_URI_PATH + github.getName()) @@ -180,6 +177,7 @@ public class OAuth2Configuration { This following function is to grant Authorities to the OAUTH2 user from the values stored in the database. This is required for the internal; 'hasRole()' function to give out the correct role. */ + @Bean @ConditionalOnProperty( value = "security.oauth2.enabled", diff --git a/src/main/java/stirling/software/SPDF/config/security/saml2/SAML2Configuration.java b/src/main/java/stirling/software/SPDF/config/security/saml2/SAML2Configuration.java index 0e4b83d1..c58bd974 100644 --- a/src/main/java/stirling/software/SPDF/config/security/saml2/SAML2Configuration.java +++ b/src/main/java/stirling/software/SPDF/config/security/saml2/SAML2Configuration.java @@ -24,24 +24,17 @@ import stirling.software.SPDF.model.ApplicationProperties.Security.SAML2; @Configuration @Slf4j -@ConditionalOnProperty( - value = "security.saml2.enabled", - havingValue = "true", - matchIfMissing = false) +@ConditionalOnProperty(value = "security.saml2.enabled", havingValue = "true") public class SAML2Configuration { private final ApplicationProperties applicationProperties; public SAML2Configuration(ApplicationProperties applicationProperties) { - this.applicationProperties = applicationProperties; } @Bean - @ConditionalOnProperty( - name = "security.saml2.enabled", - havingValue = "true", - matchIfMissing = false) + @ConditionalOnProperty(name = "security.saml2.enabled", havingValue = "true") public RelyingPartyRegistrationRepository relyingPartyRegistrations() throws Exception { SAML2 samlConf = applicationProperties.getSecurity().getSaml2(); X509Certificate idpCert = CertificateUtils.readCertificate(samlConf.getidpCert()); @@ -71,10 +64,7 @@ public class SAML2Configuration { } @Bean - @ConditionalOnProperty( - name = "security.saml2.enabled", - havingValue = "true", - matchIfMissing = false) + @ConditionalOnProperty(name = "security.saml2.enabled", havingValue = "true") public OpenSaml4AuthenticationRequestResolver authenticationRequestResolver( RelyingPartyRegistrationRepository relyingPartyRegistrationRepository) { OpenSaml4AuthenticationRequestResolver resolver = 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 c8d84f09..36d6c61c 100644 --- a/src/main/java/stirling/software/SPDF/controller/web/AccountWebController.java +++ b/src/main/java/stirling/software/SPDF/controller/web/AccountWebController.java @@ -1,5 +1,7 @@ package stirling.software.SPDF.controller.web; +import static stirling.software.SPDF.utils.validation.Validator.validateSettings; + import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.*; @@ -27,7 +29,7 @@ import stirling.software.SPDF.model.ApplicationProperties.Security; import stirling.software.SPDF.model.ApplicationProperties.Security.OAUTH2; import stirling.software.SPDF.model.ApplicationProperties.Security.OAUTH2.Client; import stirling.software.SPDF.model.ApplicationProperties.Security.SAML2; -import stirling.software.SPDF.model.provider.GithubProvider; +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.repository.UserRepository; @@ -60,28 +62,37 @@ public class AccountWebController { if (authentication != null && authentication.isAuthenticated()) { return "redirect:/"; } + Map providerList = new HashMap<>(); Security securityProps = applicationProperties.getSecurity(); OAUTH2 oauth = securityProps.getOauth2(); + if (oauth != null) { if (oauth.getEnabled()) { if (oauth.isSettingsValid()) { providerList.put(OAUTH_2_AUTHORIZATION + "oidc", oauth.getProvider()); } + Client client = oauth.getClient(); + if (client != null) { GoogleProvider google = client.getGoogle(); - if (google.isSettingsValid()) { + + if (validateSettings(google)) { providerList.put( OAUTH_2_AUTHORIZATION + google.getName(), google.getClientName()); } - GithubProvider github = client.getGithub(); - if (github.isSettingsValid()) { + + GitHubProvider github = client.getGithub(); + + if (validateSettings(github)) { providerList.put( OAUTH_2_AUTHORIZATION + github.getName(), github.getClientName()); } + KeycloakProvider keycloak = client.getKeycloak(); - if (keycloak.isSettingsValid()) { + + if (validateSettings(keycloak)) { providerList.put( OAUTH_2_AUTHORIZATION + keycloak.getName(), keycloak.getClientName()); @@ -89,101 +100,74 @@ public class AccountWebController { } } } + SAML2 saml2 = securityProps.getSaml2(); + if (securityProps.isSaml2Activ() && applicationProperties.getSystem().getEnableAlphaFunctionality()) { providerList.put("/saml2/authenticate/" + saml2.getRegistrationId(), "SAML 2"); } + // Remove any null keys/values from the providerList providerList .entrySet() .removeIf(entry -> entry.getKey() == null || entry.getValue() == null); model.addAttribute("providerlist", providerList); model.addAttribute("loginMethod", securityProps.getLoginMethod()); + boolean altLogin = !providerList.isEmpty() ? securityProps.isAltLogin() : false; + model.addAttribute("altLogin", altLogin); model.addAttribute("currentPage", "login"); String error = request.getParameter("error"); + if (error != null) { switch (error) { - case "badcredentials": - error = "login.invalid"; - break; - case "locked": - error = "login.locked"; - break; - case "oauth2AuthenticationError": - error = "userAlreadyExistsOAuthMessage"; - break; - default: - break; + case "badcredentials" -> error = "login.invalid"; + case "locked" -> error = "login.locked"; + case "oauth2AuthenticationError" -> error = "userAlreadyExistsOAuthMessage"; } + model.addAttribute("error", error); } String erroroauth = request.getParameter("erroroauth"); if (erroroauth != null) { switch (erroroauth) { - case "oauth2AutoCreateDisabled": - erroroauth = "login.oauth2AutoCreateDisabled"; - break; - case "invalidUsername": - erroroauth = "login.invalid"; - break; - case "userAlreadyExistsWeb": - erroroauth = "userAlreadyExistsWebMessage"; - break; - case "oauth2AuthenticationErrorWeb": - erroroauth = "login.oauth2InvalidUserType"; - break; - case "invalid_token_response": - erroroauth = "login.oauth2InvalidTokenResponse"; - break; - case "authorization_request_not_found": - erroroauth = "login.oauth2RequestNotFound"; - break; - case "access_denied": - erroroauth = "login.oauth2AccessDenied"; - break; - case "invalid_user_info_response": - erroroauth = "login.oauth2InvalidUserInfoResponse"; - break; - case "invalid_request": - erroroauth = "login.oauth2invalidRequest"; - break; - case "invalid_id_token": - erroroauth = "login.oauth2InvalidIdToken"; - break; - case "oauth2_admin_blocked_user": - erroroauth = "login.oauth2AdminBlockedUser"; - break; - case "userIsDisabled": - erroroauth = "login.userIsDisabled"; - break; - case "invalid_destination": - erroroauth = "login.invalid_destination"; - break; - case "relying_party_registration_not_found": - erroroauth = "login.relyingPartyRegistrationNotFound"; - break; + case "oauth2AutoCreateDisabled" -> erroroauth = "login.oauth2AutoCreateDisabled"; + case "invalidUsername" -> erroroauth = "login.invalid"; + case "userAlreadyExistsWeb" -> erroroauth = "userAlreadyExistsWebMessage"; + case "oauth2AuthenticationErrorWeb" -> erroroauth = "login.oauth2InvalidUserType"; + case "invalid_token_response" -> erroroauth = "login.oauth2InvalidTokenResponse"; + case "authorization_request_not_found" -> + erroroauth = "login.oauth2RequestNotFound"; + case "access_denied" -> erroroauth = "login.oauth2AccessDenied"; + case "invalid_user_info_response" -> + erroroauth = "login.oauth2InvalidUserInfoResponse"; + case "invalid_request" -> erroroauth = "login.oauth2invalidRequest"; + case "invalid_id_token" -> erroroauth = "login.oauth2InvalidIdToken"; + case "oauth2_admin_blocked_user" -> erroroauth = "login.oauth2AdminBlockedUser"; + case "userIsDisabled" -> erroroauth = "login.userIsDisabled"; + case "invalid_destination" -> erroroauth = "login.invalid_destination"; + case "relying_party_registration_not_found" -> + erroroauth = "login.relyingPartyRegistrationNotFound"; // Valid InResponseTo was not available from the validation context, unable to // evaluate - case "invalid_in_response_to": - erroroauth = "login.invalid_in_response_to"; - break; - case "not_authentication_provider_found": - erroroauth = "login.not_authentication_provider_found"; - break; - default: - break; + case "invalid_in_response_to" -> erroroauth = "login.invalid_in_response_to"; + case "not_authentication_provider_found" -> + erroroauth = "login.not_authentication_provider_found"; } + model.addAttribute("erroroauth", erroroauth); } + if (request.getParameter("messageType") != null) { model.addAttribute("messageType", "changedCredsMessage"); } + if (request.getParameter("logout") != null) { model.addAttribute("logoutMessage", "You have been logged out."); } + return "login"; } diff --git a/src/main/java/stirling/software/SPDF/model/ApplicationProperties.java b/src/main/java/stirling/software/SPDF/model/ApplicationProperties.java index 5b5d6370..1aa12895 100644 --- a/src/main/java/stirling/software/SPDF/model/ApplicationProperties.java +++ b/src/main/java/stirling/software/SPDF/model/ApplicationProperties.java @@ -34,7 +34,7 @@ import lombok.extern.slf4j.Slf4j; import stirling.software.SPDF.config.InstallationPathConfig; import stirling.software.SPDF.config.YamlPropertySourceFactory; import stirling.software.SPDF.model.exception.UnsupportedProviderException; -import stirling.software.SPDF.model.provider.GithubProvider; +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; @@ -253,7 +253,7 @@ public class ApplicationProperties { @Data public static class Client { private GoogleProvider google = new GoogleProvider(); - private GithubProvider github = new GithubProvider(); + private GitHubProvider github = new GitHubProvider(); private KeycloakProvider keycloak = new KeycloakProvider(); public Provider get(String registrationId) throws UnsupportedProviderException { diff --git a/src/main/java/stirling/software/SPDF/model/provider/GithubProvider.java b/src/main/java/stirling/software/SPDF/model/provider/GitHubProvider.java similarity index 54% rename from src/main/java/stirling/software/SPDF/model/provider/GithubProvider.java rename to src/main/java/stirling/software/SPDF/model/provider/GitHubProvider.java index 40892d0c..8bdf273b 100644 --- a/src/main/java/stirling/software/SPDF/model/provider/GithubProvider.java +++ b/src/main/java/stirling/software/SPDF/model/provider/GitHubProvider.java @@ -6,7 +6,7 @@ import java.util.Collection; import lombok.NoArgsConstructor; @NoArgsConstructor -public class GithubProvider extends Provider { +public class GitHubProvider extends Provider { private static final String NAME = "github"; private static final String CLIENT_NAME = "GitHub"; @@ -14,51 +14,67 @@ 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"; - private String clientId; - private String clientSecret; - private Collection scopes = new ArrayList<>(); - private String useAsUsername = "login"; - - public GithubProvider( - String clientId, String clientSecret, Collection scopes, String useAsUsername) { - super(null, NAME, CLIENT_NAME, clientId, clientSecret, scopes, useAsUsername); - this.clientId = clientId; - this.clientSecret = clientSecret; - this.scopes = scopes; - this.useAsUsername = useAsUsername; + public GitHubProvider(String clientId, String clientSecret, String useAsUsername) { + super( + null, + NAME, + CLIENT_NAME, + clientId, + clientSecret, + new ArrayList<>(), + useAsUsername != null ? useAsUsername : "login", + AUTHORIZATION_URI, + TOKEN_URI, + USER_INFO_URI); } + @Override public String getAuthorizationUri() { return AUTHORIZATION_URI; } + @Override public String getTokenUri() { return TOKEN_URI; } - public String getUserinfoUri() { + @Override + public String getUserInfoUri() { return USER_INFO_URI; } + @Override + public String getName() { + return NAME; + } + + @Override + public String getClientName() { + return CLIENT_NAME; + } + @Override public Collection getScopes() { + Collection scopes = super.getScopes(); + if (scopes == null || scopes.isEmpty()) { scopes = new ArrayList<>(); scopes.add("read:user"); } + return scopes; } @Override public String toString() { return "GitHub [clientId=" - + clientId + + getClientId() + ", clientSecret=" - + (clientSecret != null && !clientSecret.isEmpty() ? "MASKED" : "NULL") + + (getClientSecret() != null && !getClientSecret().isEmpty() ? "MASKED" : "NULL") + ", scopes=" - + scopes + + getScopes() + ", useAsUsername=" - + useAsUsername + + getUseAsUsername() + "]"; } } 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 78d231ce..bdf4e99a 100644 --- a/src/main/java/stirling/software/SPDF/model/provider/GoogleProvider.java +++ b/src/main/java/stirling/software/SPDF/model/provider/GoogleProvider.java @@ -15,18 +15,18 @@ public class GoogleProvider extends Provider { private static final String USER_INFO_URI = "https://www.googleapis.com/oauth2/v3/userinfo?alt=json"; - private String clientId; - private String clientSecret; - private Collection scopes = new ArrayList<>(); - private String useAsUsername = "email"; - - public GoogleProvider( - String clientId, String clientSecret, Collection scopes, String useAsUsername) { - super(null, NAME, CLIENT_NAME, clientId, clientSecret, scopes, useAsUsername); - this.clientId = clientId; - this.clientSecret = clientSecret; - this.scopes = scopes; - this.useAsUsername = useAsUsername; + public GoogleProvider(String clientId, String clientSecret, String useAsUsername) { + super( + null, + NAME, + CLIENT_NAME, + clientId, + clientSecret, + new ArrayList<>(), + useAsUsername, + AUTHORIZATION_URI, + TOKEN_URI, + USER_INFO_URI); } public String getAuthorizationUri() { @@ -41,26 +41,39 @@ public class GoogleProvider extends Provider { return USER_INFO_URI; } + @Override + public String getName() { + return NAME; + } + + @Override + public String getClientName() { + return CLIENT_NAME; + } + @Override public Collection getScopes() { + Collection scopes = super.getScopes(); + if (scopes == null || scopes.isEmpty()) { scopes = new ArrayList<>(); scopes.add("https://www.googleapis.com/auth/userinfo.email"); scopes.add("https://www.googleapis.com/auth/userinfo.profile"); } + return scopes; } @Override public String toString() { return "Google [clientId=" - + clientId + + getClientId() + ", clientSecret=" - + (clientSecret != null && !clientSecret.isEmpty() ? "MASKED" : "NULL") + + (getClientSecret() != null && !getClientSecret().isEmpty() ? "MASKED" : "NULL") + ", scopes=" - + scopes + + getScopes() + ", useAsUsername=" - + useAsUsername + + getUseAsUsername() + "]"; } } 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 d96e0a8c..59710c00 100644 --- a/src/main/java/stirling/software/SPDF/model/provider/KeycloakProvider.java +++ b/src/main/java/stirling/software/SPDF/model/provider/KeycloakProvider.java @@ -11,24 +11,29 @@ public class KeycloakProvider extends Provider { private static final String NAME = "keycloak"; private static final String CLIENT_NAME = "Keycloak"; - private String issuer; - private String clientId; - private String clientSecret; - private Collection scopes; - private String useAsUsername = "email"; - public KeycloakProvider( - String issuer, - String clientId, - String clientSecret, - Collection scopes, - String useAsUsername) { - super(issuer, NAME, CLIENT_NAME, clientId, clientSecret, scopes, useAsUsername); - this.useAsUsername = useAsUsername; - this.issuer = issuer; - this.clientId = clientId; - this.clientSecret = clientSecret; - this.scopes = scopes; + String issuer, String clientId, String clientSecret, String useAsUsername) { + super( + issuer, + NAME, + CLIENT_NAME, + clientId, + clientSecret, + new ArrayList<>(), + useAsUsername, + null, + null, + null); + } + + @Override + public String getName() { + return NAME; + } + + @Override + public String getClientName() { + return CLIENT_NAME; } @Override @@ -47,15 +52,15 @@ public class KeycloakProvider extends Provider { @Override public String toString() { return "Keycloak [issuer=" - + issuer + + getIssuer() + ", clientId=" - + clientId + + getClientId() + ", clientSecret=" - + (clientSecret != null && !clientSecret.isBlank() ? "MASKED" : "NULL") + + (getClientSecret() != null && !getClientSecret().isBlank() ? "MASKED" : "NULL") + ", scopes=" - + scopes + + getScopes() + ", useAsUsername=" - + useAsUsername + + getUseAsUsername() + "]"; } } 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 31d3ee94..d1a85894 100644 --- a/src/main/java/stirling/software/SPDF/model/provider/Provider.java +++ b/src/main/java/stirling/software/SPDF/model/provider/Provider.java @@ -18,6 +18,9 @@ public abstract class Provider { private String clientSecret; private Collection scopes; private String useAsUsername; + private String authorizationUri; + private String tokenUri; + private String userInfoUri; public Provider( String issuer, @@ -26,7 +29,10 @@ public abstract class Provider { String clientId, String clientSecret, Collection scopes, - String useAsUsername) { + String useAsUsername, + String authorizationUri, + String tokenUri, + String userInfoUri) { this.issuer = issuer; this.name = name; this.clientName = clientName; @@ -34,28 +40,15 @@ public abstract class Provider { this.clientSecret = clientSecret; this.scopes = scopes; this.useAsUsername = !useAsUsername.isBlank() ? useAsUsername : "email"; - } - - // todo: why are we passing name here if it's not used? - // todo: use util class/method - public boolean isSettingsValid() { - return isValid(this.getIssuer(), "issuer") - && isValid(this.getClientId(), "clientId") - && isValid(this.getClientSecret(), "clientSecret") - && isValid(this.getScopes(), "scopes") - && isValid(this.getUseAsUsername(), "useAsUsername"); - } - - private boolean isValid(String value, String name) { - return value != null && !value.isBlank(); - } - - private boolean isValid(Collection value, String name) { - return value != null && !value.isEmpty(); + this.authorizationUri = authorizationUri; + this.tokenUri = tokenUri; + this.userInfoUri = userInfoUri; } public void setScopes(String scopes) { - this.scopes = - Arrays.stream(scopes.split(",")).map(String::trim).collect(Collectors.toList()); + if (scopes != null && !scopes.isBlank()) { + this.scopes = + Arrays.stream(scopes.split(",")).map(String::trim).collect(Collectors.toList()); + } } } diff --git a/src/main/java/stirling/software/SPDF/utils/validation/CollectionValidator.java b/src/main/java/stirling/software/SPDF/utils/validation/CollectionValidator.java deleted file mode 100644 index 97c0cf47..00000000 --- a/src/main/java/stirling/software/SPDF/utils/validation/CollectionValidator.java +++ /dev/null @@ -1,11 +0,0 @@ -package stirling.software.SPDF.utils.validation; - -import java.util.Collection; - -public class CollectionValidator implements Validator> { - - @Override - public boolean validate(Collection input, String path) { - return input != null && !input.isEmpty(); - } -} diff --git a/src/main/java/stirling/software/SPDF/utils/validation/StringValidator.java b/src/main/java/stirling/software/SPDF/utils/validation/StringValidator.java deleted file mode 100644 index bf45abe4..00000000 --- a/src/main/java/stirling/software/SPDF/utils/validation/StringValidator.java +++ /dev/null @@ -1,9 +0,0 @@ -package stirling.software.SPDF.utils.validation; - -public class StringValidator implements Validator { - - @Override - public boolean validate(String input, String path) { - return input != null && !input.isBlank(); - } -} 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 d03d78d5..598c0a03 100644 --- a/src/main/java/stirling/software/SPDF/utils/validation/Validator.java +++ b/src/main/java/stirling/software/SPDF/utils/validation/Validator.java @@ -1,6 +1,36 @@ package stirling.software.SPDF.utils.validation; -public interface Validator { +import java.util.Collection; - boolean validate(T input, String path); +import stirling.software.SPDF.model.provider.Provider; + +public class Validator { + + public static boolean validateSettings(Provider provider) { + if (provider == null) { + return false; + } + + if (isStringEmpty(provider.getClientId())) { + return false; + } + + if (isStringEmpty(provider.getClientSecret())) { + return false; + } + + if (isCollectionEmpty(provider.getScopes())) { + return false; + } + + return !isStringEmpty(provider.getUseAsUsername()); + } + + private static boolean isStringEmpty(String input) { + return input == null || input.isBlank(); + } + + private 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 new file mode 100644 index 00000000..ba9a5106 --- /dev/null +++ b/src/test/java/stirling/software/SPDF/utils/validation/ValidatorTest.java @@ -0,0 +1,56 @@ +package stirling.software.SPDF.utils.validation; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.mockito.junit.jupiter.MockitoExtension; +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; + +import java.util.List; +import java.util.stream.Stream; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class ValidatorTest { + + @Test + void testSuccessfulValidation() { + var provider = mock(GitHubProvider.class); + + when(provider.getClientId()).thenReturn("clientId"); + when(provider.getClientSecret()).thenReturn("clientSecret"); + when(provider.getScopes()).thenReturn(List.of("read:user")); + when(provider.getUseAsUsername()).thenReturn("email"); + + assertTrue(Validator.validateSettings(provider)); + } + + @ParameterizedTest + @MethodSource("providerParams") + void testUnsuccessfulValidation(Provider provider) { + assertFalse(Validator.validateSettings(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", " "); + + return Stream.of( + Arguments.of(generic), + Arguments.of(google), + Arguments.of(github), + Arguments.of(keycloak) + ); + } + +} \ No newline at end of file