wip - refactoring & cleanup

src/main/java/stirling/software/SPDF/config/security/CustomLogoutSuccessHandler.java

@@ -208,9 +208,10 @@ public class CustomLogoutSuccessHandler extends SimpleUrlLogoutSuccessHandler {
                 break;
             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: " + githubLogoutUrl);
+                log.info("Redirecting to GitHub logout URL: " + redirect_url);
-                response.sendRedirect(githubLogoutUrl);
+                response.sendRedirect(redirect_url);
                 break;
             case "google":
                 // Add Google specific logout URL if needed

src/main/java/stirling/software/SPDF/config/security/oauth2/OAuth2Configuration.java

@@ -28,17 +28,22 @@ 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 static org.springframework.security.oauth2.core.AuthorizationGrantType.AUTHORIZATION_CODE;
 
-@Configuration
 @Slf4j
+@Configuration
 @ConditionalOnProperty(value = "security.oauth2.enabled", havingValue = "true")
 public class OAuth2Configuration {
 
+    public static final String REDIRECT_URI_PATH = "{baseUrl}/login/oauth2/code/";
+
     private final ApplicationProperties applicationProperties;
     @Lazy private final UserService userService;
 
     public OAuth2Configuration(
-            ApplicationProperties applicationProperties, @Lazy UserService userService) {
+            ApplicationProperties applicationProperties,
+            @Lazy UserService userService
+    ) {
         this.userService = userService;
         this.applicationProperties = applicationProperties;
     }
@@ -61,13 +66,17 @@ public class OAuth2Configuration {
 
     private Optional<ClientRegistration> googleClientRegistration() {
         OAUTH2 oauth = applicationProperties.getSecurity().getOauth2();
+
         if (oauth == null || !oauth.getEnabled()) {
             return Optional.empty();
         }
+
         Client client = oauth.getClient();
+
         if (client == null) {
             return Optional.empty();
         }
+
         GoogleProvider google = client.getGoogle();
         return google != null && google.isSettingsValid()
                 ? Optional.of(
@@ -75,15 +84,13 @@ public class OAuth2Configuration {
                                 .clientId(google.getClientId())
                                 .clientSecret(google.getClientSecret())
                                 .scope(google.getScopes())
-                                .authorizationUri(google.getAuthorizationuri())
+                                .authorizationUri(google.getAuthorizationUri())
-                                .tokenUri(google.getTokenuri())
+                                .tokenUri(google.getTokenUri())
-                                .userInfoUri(google.getUserinfouri())
+                                .userInfoUri(google.getUserinfoUri())
                                 .userNameAttributeName(google.getUseAsUsername())
                                 .clientName(google.getClientName())
-                                .redirectUri("{baseUrl}/login/oauth2/code/" + google.getName())
+                                .redirectUri(REDIRECT_URI_PATH + google.getName())
-                                .authorizationGrantType(
+                                .authorizationGrantType(AUTHORIZATION_CODE)
-                                        org.springframework.security.oauth2.core
-                                                .AuthorizationGrantType.AUTHORIZATION_CODE)
                                 .build())
                 : Optional.empty();
     }
@@ -112,36 +119,36 @@ public class OAuth2Configuration {
     }
 
     private Optional<ClientRegistration> githubClientRegistration() {
-        OAUTH2 oauth = applicationProperties.getSecurity().getOauth2();
+        if (isOauthOrClientEmpty()) {
-        if (oauth == null || !oauth.getEnabled()) {
             return Optional.empty();
         }
-        Client client = oauth.getClient();
+
-        if (client == null) {
+        GithubProvider github = applicationProperties
-            return Optional.empty();
+                .getSecurity()
-        }
+                .getOauth2()
-        GithubProvider github = client.getGithub();
+                .getClient()
+                .getGithub();
+
         return github != null && github.isSettingsValid()
                 ? Optional.of(
                         ClientRegistration.withRegistrationId(github.getName())
                                 .clientId(github.getClientId())
                                 .clientSecret(github.getClientSecret())
                                 .scope(github.getScopes())
-                                .authorizationUri(github.getAuthorizationuri())
+                                .authorizationUri(github.getAuthorizationUri())
-                                .tokenUri(github.getTokenuri())
+                                .tokenUri(github.getTokenUri())
-                                .userInfoUri(github.getUserinfouri())
+                                .userInfoUri(github.getUserinfoUri())
                                 .userNameAttributeName(github.getUseAsUsername())
                                 .clientName(github.getClientName())
-                                .redirectUri("{baseUrl}/login/oauth2/code/" + github.getName())
+                                .redirectUri(REDIRECT_URI_PATH + github.getName())
-                                .authorizationGrantType(
+                                .authorizationGrantType(AUTHORIZATION_CODE)
-                                        org.springframework.security.oauth2.core
-                                                .AuthorizationGrantType.AUTHORIZATION_CODE)
                                 .build())
                 : Optional.empty();
     }
 
     private Optional<ClientRegistration> oidcClientRegistration() {
         OAUTH2 oauth = applicationProperties.getSecurity().getOauth2();
+
         if (oauth == null
                 || oauth.getIssuer() == null
                 || oauth.getIssuer().isEmpty()
@@ -155,6 +162,7 @@ public class OAuth2Configuration {
                 || oauth.getUseAsUsername().isEmpty()) {
             return Optional.empty();
         }
+
         return Optional.of(
                 ClientRegistrations.fromIssuerLocation(oauth.getIssuer())
                         .registrationId("oidc")
@@ -163,13 +171,23 @@ public class OAuth2Configuration {
                         .scope(oauth.getScopes())
                         .userNameAttributeName(oauth.getUseAsUsername())
                         .clientName("OIDC")
-                        .redirectUri("{baseUrl}/login/oauth2/code/oidc")
+                        .redirectUri(REDIRECT_URI_PATH + "oidc")
-                        .authorizationGrantType(
+                        .authorizationGrantType(AUTHORIZATION_CODE)
-                                org.springframework.security.oauth2.core.AuthorizationGrantType
-                                        .AUTHORIZATION_CODE)
                         .build());
     }
 
+    private boolean isOauthOrClientEmpty() {
+        OAUTH2 oauth = applicationProperties.getSecurity().getOauth2();
+
+        if (oauth == null || !oauth.getEnabled()) {
+            return false;
+        }
+
+        Client client = oauth.getClient();
+
+        return client != null;
+    }
+
     /*
     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.

src/main/java/stirling/software/SPDF/model/ApplicationProperties.java

@@ -229,7 +229,7 @@ public class ApplicationProperties {
                 List<String> scopesList =
                         Arrays.stream(scopes.split(","))
                                 .map(String::trim)
-                                .collect(Collectors.toList());
+                                .toList();
                 this.scopes.addAll(scopesList);
             }
 
@@ -256,17 +256,13 @@ public class ApplicationProperties {
                 private KeycloakProvider keycloak = new KeycloakProvider();
 
                 public Provider get(String registrationId) throws UnsupportedProviderException {
-                    switch (registrationId.toLowerCase()) {
+                    return switch (registrationId.toLowerCase()) {
-                        case "google":
+                        case "google" -> getGoogle();
-                            return getGoogle();
+                        case "github" -> getGithub();
-                        case "github":
+                        case "keycloak" -> getKeycloak();
-                            return getGithub();
+                        default -> throw new UnsupportedProviderException(
-                        case "keycloak":
+                                "Logout from the provider is not supported? Report it at https://github.com/Stirling-Tools/Stirling-PDF/issues");
-                            return getKeycloak();
+                    };
-                        default:
-                            throw new UnsupportedProviderException(
-                                    "Logout from the provider is not supported? Report it at https://github.com/Stirling-Tools/Stirling-PDF/issues");
-                    }
                 }
             }
         }

src/main/java/stirling/software/SPDF/model/Provider.java

@@ -1,80 +1,84 @@
 package stirling.software.SPDF.model;
 
+import java.util.Arrays;
 import java.util.Collection;
+import java.util.stream.Collectors;
+import lombok.Getter;
+import lombok.NoArgsConstructor;
 
-public abstract class Provider implements ProviderInterface {
+@Getter
+@NoArgsConstructor
+public abstract class Provider {
+
+    private String issuer;
     private String name;
     private String clientName;
+    private String clientId;
+    private String clientSecret;
+    private Collection<String> scopes;
+    private String useAsUsername;
 
-    public String getName() {
+    public Provider(
-        return name;
+            String issuer,
+            String name,
+            String clientName,
+            String clientId,
+            String clientSecret,
+            Collection<String> scopes,
+            String useAsUsername
+    ) {
+        this.issuer = issuer;
+        this.name = name;
+        this.clientName = clientName;
+        this.clientId = clientId;
+        this.clientSecret = clientSecret;
+        this.scopes = scopes;
+        this.useAsUsername = !useAsUsername.isBlank() ? useAsUsername : "email";
     }
 
-    public String getClientName() {
+    //    todo: why are we passing name here if it's not used?
-        return clientName;
+    public boolean isSettingsValid() {
+        return isValid(this.getIssuer(), "issuer")
+                && isValid(this.getClientId(), "clientId")
+                && isValid(this.getClientSecret(), "clientSecret")
+                && isValid(this.getScopes(), "scopes")
+                && isValid(this.getUseAsUsername(), "useAsUsername");
     }
 
-    protected boolean isValid(String value, String name) {
+    private boolean isValid(String value, String name) {
-        if (value != null && !value.trim().isEmpty()) {
+        return value != null && !value.isBlank();
-            return true;
-        }
-        return false;
     }
 
-    protected boolean isValid(Collection<String> value, String name) {
+    private boolean isValid(Collection<String> value, String name) {
-        if (value != null && !value.isEmpty()) {
+        return value != null && !value.isEmpty();
-            return true;
-        }
-        return false;
     }
 
-    @Override
+    protected void setIssuer(String issuer) {
-    public Collection<String> getScopes() {
+        this.issuer = issuer;
-        throw new UnsupportedOperationException("Unimplemented method 'getScope'");
     }
 
-    @Override
+    protected void setName(String name) {
-    public void setScopes(String scopes) {
+        this.name = name;
-        throw new UnsupportedOperationException("Unimplemented method 'setScope'");
     }
 
-    @Override
+    protected void setClientName(String clientName) {
-    public String getUseAsUsername() {
+        this.clientName = clientName;
-        throw new UnsupportedOperationException("Unimplemented method 'getUseAsUsername'");
     }
 
-    @Override
+    protected void setClientId(String clientId) {
-    public void setUseAsUsername(String useAsUsername) {
+        this.clientId = clientId;
-        throw new UnsupportedOperationException("Unimplemented method 'setUseAsUsername'");
     }
 
-    @Override
+    protected void setClientSecret(String clientSecret) {
-    public String getIssuer() {
+        this.clientSecret = clientSecret;
-        throw new UnsupportedOperationException("Unimplemented method 'getIssuer'");
     }
 
-    @Override
+    protected void setScopes(String scopes) {
-    public void setIssuer(String issuer) {
+        this.scopes = Arrays.stream(scopes.split(",")).map(String::trim).collect(Collectors.toList());
-        throw new UnsupportedOperationException("Unimplemented method 'setIssuer'");
     }
 
-    @Override
+    protected void setUseAsUsername(String useAsUsername) {
-    public String getClientSecret() {
+        this.useAsUsername = useAsUsername;
-        throw new UnsupportedOperationException("Unimplemented method 'getClientSecret'");
     }
 
-    @Override
-    public void setClientSecret(String clientSecret) {
-        throw new UnsupportedOperationException("Unimplemented method 'setClientSecret'");
-    }
-
-    @Override
-    public String getClientId() {
-        throw new UnsupportedOperationException("Unimplemented method 'getClientId'");
-    }
-
-    @Override
-    public void setClientId(String clientId) {
-        throw new UnsupportedOperationException("Unimplemented method 'setClientId'");
-    }
 }

src/main/java/stirling/software/SPDF/model/ProviderInterface.java

@@ -1,26 +0,0 @@
-package stirling.software.SPDF.model;
-
-import java.util.Collection;
-
-public interface ProviderInterface {
-
-    Collection<String> getScopes();
-
-    void setScopes(String scopes);
-
-    String getUseAsUsername();
-
-    void setUseAsUsername(String useAsUsername);
-
-    String getIssuer();
-
-    void setIssuer(String issuer);
-
-    String getClientSecret();
-
-    void setClientSecret(String clientSecret);
-
-    String getClientId();
-
-    void setClientId(String clientId);
-}

src/main/java/stirling/software/SPDF/model/provider/GithubProvider.java

@@ -5,56 +5,41 @@ import java.util.Arrays;
 import java.util.Collection;
 import java.util.stream.Collectors;
 
+import lombok.NoArgsConstructor;
 import stirling.software.SPDF.model.Provider;
 
+@NoArgsConstructor
 public class GithubProvider extends Provider {
 
-    private static final String authorizationUri = "https://github.com/login/oauth/authorize";
+    private static final String NAME = "github";
-    private static final String tokenUri = "https://github.com/login/oauth/access_token";
+    private static final String CLIENT_NAME = "GitHub";
-    private static final String userInfoUri = "https://api.github.com/user";
+    private static final String AUTHORIZATION_URI = "https://github.com/login/oauth/authorize";
+    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<String> scopes = new ArrayList<>();
     private String useAsUsername = "login";
 
-    public String getAuthorizationuri() {
+    public GithubProvider(String clientId, String clientSecret, Collection<String> scopes, String useAsUsername) {
-        return authorizationUri;
+        super(null, NAME, CLIENT_NAME, clientId, clientSecret, scopes, useAsUsername);
-    }
-
-    public String getTokenuri() {
-        return tokenUri;
-    }
-
-    public String getUserinfouri() {
-        return userInfoUri;
-    }
-
-    @Override
-    public String getIssuer() {
-        return new String();
-    }
-
-    @Override
-    public void setIssuer(String issuer) {}
-
-    @Override
-    public String getClientId() {
-        return this.clientId;
-    }
-
-    @Override
-    public void setClientId(String clientId) {
         this.clientId = clientId;
-    }
-
-    @Override
-    public String getClientSecret() {
-        return this.clientSecret;
-    }
-
-    @Override
-    public void setClientSecret(String clientSecret) {
         this.clientSecret = clientSecret;
+        this.scopes = scopes;
+        this.useAsUsername = useAsUsername;
+    }
+
+    public String getAuthorizationUri() {
+        return AUTHORIZATION_URI;
+    }
+
+    public String getTokenUri() {
+        return TOKEN_URI;
+    }
+
+    public String getUserinfoUri() {
+        return USER_INFO_URI;
     }
 
     @Override
@@ -66,22 +51,6 @@ public class GithubProvider extends Provider {
         return scopes;
     }
 
-    @Override
-    public void setScopes(String scopes) {
-        this.scopes =
-                Arrays.stream(scopes.split(",")).map(String::trim).collect(Collectors.toList());
-    }
-
-    @Override
-    public String getUseAsUsername() {
-        return this.useAsUsername;
-    }
-
-    @Override
-    public void setUseAsUsername(String useAsUsername) {
-        this.useAsUsername = useAsUsername;
-    }
-
     @Override
     public String toString() {
         return "GitHub [clientId="
@@ -94,21 +63,4 @@ public class GithubProvider extends Provider {
                 + useAsUsername
                 + "]";
     }
-
-    @Override
-    public String getName() {
-        return "github";
-    }
-
-    @Override
-    public String getClientName() {
-        return "GitHub";
-    }
-
-    public boolean isSettingsValid() {
-        return super.isValid(this.getClientId(), "clientId")
-                && super.isValid(this.getClientSecret(), "clientSecret")
-                && super.isValid(this.getScopes(), "scopes")
-                && isValid(this.getUseAsUsername(), "useAsUsername");
-    }
 }

src/main/java/stirling/software/SPDF/model/provider/GoogleProvider.java

@@ -3,59 +3,46 @@ package stirling.software.SPDF.model.provider;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
 import java.util.stream.Collectors;
 
+import lombok.NoArgsConstructor;
 import stirling.software.SPDF.model.Provider;
 
+@NoArgsConstructor
 public class GoogleProvider extends Provider {
 
-    private static final String authorizationUri = "https://accounts.google.com/o/oauth2/v2/auth";
+    private static final String NAME = "google";
-    private static final String tokenUri = "https://www.googleapis.com/oauth2/v4/token";
+    private static final String CLIENT_NAME = "Google";
-    private static final String userInfoUri =
+    private static final String AUTHORIZATION_URI = "https://accounts.google.com/o/oauth2/v2/auth";
+    private static final String TOKEN_URI = "https://www.googleapis.com/oauth2/v4/token";
+    private static final String USER_INFO_URI =
             "https://www.googleapis.com/oauth2/v3/userinfo?alt=json";
+
     private String clientId;
     private String clientSecret;
     private Collection<String> scopes = new ArrayList<>();
     private String useAsUsername = "email";
 
-    public String getAuthorizationuri() {
+    public GoogleProvider(String clientId, String clientSecret, Collection<String> scopes, String useAsUsername) {
-        return authorizationUri;
+        super(null, NAME, CLIENT_NAME, clientId, clientSecret, scopes, useAsUsername);
-    }
-
-    public String getTokenuri() {
-        return tokenUri;
-    }
-
-    public String getUserinfouri() {
-        return userInfoUri;
-    }
-
-    @Override
-    public String getIssuer() {
-        return new String();
-    }
-
-    @Override
-    public void setIssuer(String issuer) {}
-
-    @Override
-    public String getClientId() {
-        return this.clientId;
-    }
-
-    @Override
-    public void setClientId(String clientId) {
         this.clientId = clientId;
-    }
-
-    @Override
-    public String getClientSecret() {
-        return this.clientSecret;
-    }
-
-    @Override
-    public void setClientSecret(String clientSecret) {
         this.clientSecret = clientSecret;
+        this.scopes = scopes;
+        this.useAsUsername = useAsUsername;
+    }
+
+    public String getAuthorizationUri() {
+        return AUTHORIZATION_URI;
+    }
+
+    public String getTokenUri() {
+        return TOKEN_URI;
+    }
+
+    public String getUserinfoUri() {
+        return USER_INFO_URI;
     }
 
     @Override
@@ -68,22 +55,6 @@ public class GoogleProvider extends Provider {
         return scopes;
     }
 
-    @Override
-    public void setScopes(String scopes) {
-        this.scopes =
-                Arrays.stream(scopes.split(",")).map(String::trim).collect(Collectors.toList());
-    }
-
-    @Override
-    public String getUseAsUsername() {
-        return this.useAsUsername;
-    }
-
-    @Override
-    public void setUseAsUsername(String useAsUsername) {
-        this.useAsUsername = useAsUsername;
-    }
-
     @Override
     public String toString() {
         return "Google [clientId="
@@ -97,20 +68,4 @@ public class GoogleProvider extends Provider {
                 + "]";
     }
 
-    @Override
-    public String getName() {
-        return "google";
-    }
-
-    @Override
-    public String getClientName() {
-        return "Google";
-    }
-
-    public boolean isSettingsValid() {
-        return super.isValid(this.getClientId(), "clientId")
-                && super.isValid(this.getClientSecret(), "clientSecret")
-                && super.isValid(this.getScopes(), "scopes")
-                && isValid(this.getUseAsUsername(), "useAsUsername");
-    }
 }

src/main/java/stirling/software/SPDF/model/provider/KeycloakProvider.java

@@ -5,72 +5,43 @@ import java.util.Arrays;
 import java.util.Collection;
 import java.util.stream.Collectors;
 
+import lombok.NoArgsConstructor;
 import stirling.software.SPDF.model.Provider;
 
+@NoArgsConstructor
 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<String> scopes = new ArrayList<>();
+    private Collection<String> scopes;
     private String useAsUsername = "email";
 
-    @Override
+    public KeycloakProvider(String issuer, String clientId, String clientSecret, Collection<String> scopes, String useAsUsername) {
-    public String getIssuer() {
+        super(issuer, NAME, CLIENT_NAME, clientId, clientSecret, scopes, useAsUsername);
-        return this.issuer;
+        this.useAsUsername = useAsUsername;
-    }
-
-    @Override
-    public void setIssuer(String issuer) {
         this.issuer = issuer;
-    }
-
-    @Override
-    public String getClientId() {
-        return this.clientId;
-    }
-
-    @Override
-    public void setClientId(String clientId) {
         this.clientId = clientId;
-    }
-
-    @Override
-    public String getClientSecret() {
-        return this.clientSecret;
-    }
-
-    @Override
-    public void setClientSecret(String clientSecret) {
         this.clientSecret = clientSecret;
+        this.scopes = scopes;
     }
 
     @Override
     public Collection<String> getScopes() {
+        var scopes = super.getScopes();
+
         if (scopes == null || scopes.isEmpty()) {
             scopes = new ArrayList<>();
             scopes.add("profile");
             scopes.add("email");
         }
+
         return scopes;
     }
 
-    @Override
-    public void setScopes(String scopes) {
-        this.scopes =
-                Arrays.stream(scopes.split(",")).map(String::trim).collect(Collectors.toList());
-    }
-
-    @Override
-    public String getUseAsUsername() {
-        return this.useAsUsername;
-    }
-
-    @Override
-    public void setUseAsUsername(String useAsUsername) {
-        this.useAsUsername = useAsUsername;
-    }
-
     @Override
     public String toString() {
         return "Keycloak [issuer="
@@ -78,7 +49,7 @@ public class KeycloakProvider extends Provider {
                 + ", clientId="
                 + clientId
                 + ", clientSecret="
-                + (clientSecret != null && !clientSecret.isEmpty() ? "MASKED" : "NULL")
+                + (clientSecret != null && !clientSecret.isBlank() ? "MASKED" : "NULL")
                 + ", scopes="
                 + scopes
                 + ", useAsUsername="
@@ -86,21 +57,4 @@ public class KeycloakProvider extends Provider {
                 + "]";
     }
 
-    @Override
-    public String getName() {
-        return "keycloak";
-    }
-
-    @Override
-    public String getClientName() {
-        return "Keycloak";
-    }
-
-    public boolean isSettingsValid() {
-        return isValid(this.getIssuer(), "issuer")
-                && isValid(this.getClientId(), "clientId")
-                && isValid(this.getClientSecret(), "clientSecret")
-                && isValid(this.getScopes(), "scopes")
-                && isValid(this.getUseAsUsername(), "useAsUsername");
-    }
 }