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 f5d1bbb3..86c893b0 100644 --- a/src/main/java/stirling/software/SPDF/config/security/CustomLogoutSuccessHandler.java +++ b/src/main/java/stirling/software/SPDF/config/security/CustomLogoutSuccessHandler.java @@ -197,14 +197,14 @@ public class CustomLogoutSuccessHandler extends SimpleUrlLogoutSuccessHandler { + clientId + "&post_logout_redirect_uri=" + response.encodeRedirectURL(redirect_url); - log.info("Redirecting to Keycloak logout URL: " + logoutUrl); + 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); + log.info("Redirecting to GitHub logout URL: {}", redirect_url); response.sendRedirect(redirect_url); } case "google" -> { 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 7a181803..e1c2a5b0 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 @@ -64,19 +64,13 @@ public class OAuth2Configuration { } private Optional googleClientRegistration() { - OAUTH2 oauth = applicationProperties.getSecurity().getOauth2(); - - if (oauth == null || !oauth.getEnabled()) { + if (isOauthOrClientEmpty()) { return Optional.empty(); } - Client client = oauth.getClient(); + GoogleProvider google = + applicationProperties.getSecurity().getOauth2().getClient().getGoogle(); - if (client == null) { - return Optional.empty(); - } - - GoogleProvider google = client.getGoogle(); return google != null && google.isSettingsValid() ? Optional.of( ClientRegistration.withRegistrationId(google.getName()) @@ -95,15 +89,13 @@ public class OAuth2Configuration { } private Optional keycloakClientRegistration() { - OAUTH2 oauth = applicationProperties.getSecurity().getOauth2(); - if (oauth == null || !oauth.getEnabled()) { + if (isOauthOrClientEmpty()) { return Optional.empty(); } - Client client = oauth.getClient(); - if (client == null) { - return Optional.empty(); - } - KeycloakProvider keycloak = client.getKeycloak(); + + KeycloakProvider keycloak = + applicationProperties.getSecurity().getOauth2().getClient().getKeycloak(); + return keycloak != null && keycloak.isSettingsValid() ? Optional.of( ClientRegistrations.fromIssuerLocation(keycloak.getIssuer()) @@ -181,7 +173,7 @@ public class OAuth2Configuration { Client client = oauth.getClient(); - return client != null; + return client == null; } /* 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 ff00eca5..40892d0c 100644 --- a/src/main/java/stirling/software/SPDF/model/provider/GithubProvider.java +++ b/src/main/java/stirling/software/SPDF/model/provider/GithubProvider.java @@ -5,7 +5,6 @@ import java.util.Collection; import lombok.NoArgsConstructor; -// @Setter @NoArgsConstructor public class GithubProvider extends Provider { @@ -22,7 +21,7 @@ public class GithubProvider extends Provider { public GithubProvider( String clientId, String clientSecret, Collection scopes, String useAsUsername) { - super(null, NAME, CLIENT_NAME, clientId, clientSecret, scopes, useAsUsername); + super(null, NAME, CLIENT_NAME, clientId, clientSecret, scopes, useAsUsername); this.clientId = clientId; this.clientSecret = clientSecret; this.scopes = scopes; 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 d1ecb539..78d231ce 100644 --- a/src/main/java/stirling/software/SPDF/model/provider/GoogleProvider.java +++ b/src/main/java/stirling/software/SPDF/model/provider/GoogleProvider.java @@ -5,7 +5,6 @@ import java.util.Collection; import lombok.NoArgsConstructor; -// @Setter @NoArgsConstructor public class GoogleProvider extends Provider { 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 77a50bdc..d96e0a8c 100644 --- a/src/main/java/stirling/software/SPDF/model/provider/KeycloakProvider.java +++ b/src/main/java/stirling/software/SPDF/model/provider/KeycloakProvider.java @@ -5,7 +5,6 @@ import java.util.Collection; import lombok.NoArgsConstructor; -// @Setter @NoArgsConstructor public class KeycloakProvider extends Provider { 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 e2a638c0..31d3ee94 100644 --- a/src/main/java/stirling/software/SPDF/model/provider/Provider.java +++ b/src/main/java/stirling/software/SPDF/model/provider/Provider.java @@ -4,10 +4,10 @@ import java.util.Arrays; import java.util.Collection; import java.util.stream.Collectors; -import lombok.Getter; +import lombok.Data; import lombok.NoArgsConstructor; -@Getter +@Data @NoArgsConstructor public abstract class Provider { @@ -37,6 +37,7 @@ public abstract class Provider { } // 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") @@ -53,32 +54,8 @@ public abstract class Provider { return value != null && !value.isEmpty(); } - public void setIssuer(String issuer) { - this.issuer = issuer; - } - - public void setName(String name) { - this.name = name; - } - - public void setClientName(String clientName) { - this.clientName = clientName; - } - - public void setClientId(String clientId) { - this.clientId = clientId; - } - - public void setClientSecret(String clientSecret) { - this.clientSecret = clientSecret; - } - public void setScopes(String scopes) { this.scopes = Arrays.stream(scopes.split(",")).map(String::trim).collect(Collectors.toList()); } - - public void setUseAsUsername(String useAsUsername) { - this.useAsUsername = useAsUsername; - } } diff --git a/src/main/java/stirling/software/SPDF/utils/validation/CollectionValidator.java b/src/main/java/stirling/software/SPDF/utils/validation/CollectionValidator.java new file mode 100644 index 00000000..97c0cf47 --- /dev/null +++ b/src/main/java/stirling/software/SPDF/utils/validation/CollectionValidator.java @@ -0,0 +1,11 @@ +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 new file mode 100644 index 00000000..bf45abe4 --- /dev/null +++ b/src/main/java/stirling/software/SPDF/utils/validation/StringValidator.java @@ -0,0 +1,9 @@ +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 new file mode 100644 index 00000000..d03d78d5 --- /dev/null +++ b/src/main/java/stirling/software/SPDF/utils/validation/Validator.java @@ -0,0 +1,6 @@ +package stirling.software.SPDF.utils.validation; + +public interface Validator { + + boolean validate(T input, String path); +}