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 d400f346..56088f1e 100644 --- a/src/main/java/stirling/software/SPDF/config/security/CustomLogoutSuccessHandler.java +++ b/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); - response.sendRedirect(githubLogoutUrl); + log.info("Redirecting to GitHub logout URL: " + redirect_url); + response.sendRedirect(redirect_url); break; case "google": // Add Google specific logout URL if needed 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 15c403d2..82af61bc 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,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 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()) - .tokenUri(google.getTokenuri()) - .userInfoUri(google.getUserinfouri()) + .authorizationUri(google.getAuthorizationUri()) + .tokenUri(google.getTokenUri()) + .userInfoUri(google.getUserinfoUri()) .userNameAttributeName(google.getUseAsUsername()) .clientName(google.getClientName()) - .redirectUri("{baseUrl}/login/oauth2/code/" + google.getName()) - .authorizationGrantType( - org.springframework.security.oauth2.core - .AuthorizationGrantType.AUTHORIZATION_CODE) + .redirectUri(REDIRECT_URI_PATH + google.getName()) + .authorizationGrantType(AUTHORIZATION_CODE) .build()) : Optional.empty(); } @@ -112,36 +119,36 @@ public class OAuth2Configuration { } private Optional githubClientRegistration() { - 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(); - } - GithubProvider github = client.getGithub(); + + GithubProvider github = applicationProperties + .getSecurity() + .getOauth2() + .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()) - .tokenUri(github.getTokenuri()) - .userInfoUri(github.getUserinfouri()) + .authorizationUri(github.getAuthorizationUri()) + .tokenUri(github.getTokenUri()) + .userInfoUri(github.getUserinfoUri()) .userNameAttributeName(github.getUseAsUsername()) .clientName(github.getClientName()) - .redirectUri("{baseUrl}/login/oauth2/code/" + github.getName()) - .authorizationGrantType( - org.springframework.security.oauth2.core - .AuthorizationGrantType.AUTHORIZATION_CODE) + .redirectUri(REDIRECT_URI_PATH + github.getName()) + .authorizationGrantType(AUTHORIZATION_CODE) .build()) : Optional.empty(); } private Optional 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") - .authorizationGrantType( - org.springframework.security.oauth2.core.AuthorizationGrantType - .AUTHORIZATION_CODE) + .redirectUri(REDIRECT_URI_PATH + "oidc") + .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. diff --git a/src/main/java/stirling/software/SPDF/model/ApplicationProperties.java b/src/main/java/stirling/software/SPDF/model/ApplicationProperties.java index a10c945a..480b0f53 100644 --- a/src/main/java/stirling/software/SPDF/model/ApplicationProperties.java +++ b/src/main/java/stirling/software/SPDF/model/ApplicationProperties.java @@ -229,7 +229,7 @@ public class ApplicationProperties { List 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()) { - case "google": - return getGoogle(); - case "github": - return getGithub(); - case "keycloak": - return getKeycloak(); - default: - throw new UnsupportedProviderException( - "Logout from the provider is not supported? Report it at https://github.com/Stirling-Tools/Stirling-PDF/issues"); - } + return switch (registrationId.toLowerCase()) { + case "google" -> getGoogle(); + case "github" -> getGithub(); + case "keycloak" -> getKeycloak(); + default -> throw new UnsupportedProviderException( + "Logout from the provider 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.java b/src/main/java/stirling/software/SPDF/model/Provider.java index cdb94ee0..955da612 100644 --- a/src/main/java/stirling/software/SPDF/model/Provider.java +++ b/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 scopes; + private String useAsUsername; - public String getName() { - return name; + public Provider( + String issuer, + String name, + String clientName, + String clientId, + String clientSecret, + Collection 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() { - return clientName; + // todo: why are we passing name here if it's not used? + 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) { - if (value != null && !value.trim().isEmpty()) { - return true; - } - return false; + private boolean isValid(String value, String name) { + return value != null && !value.isBlank(); } - protected boolean isValid(Collection value, String name) { - if (value != null && !value.isEmpty()) { - return true; - } - return false; + private boolean isValid(Collection value, String name) { + return value != null && !value.isEmpty(); } - @Override - public Collection getScopes() { - throw new UnsupportedOperationException("Unimplemented method 'getScope'"); + protected void setIssuer(String issuer) { + this.issuer = issuer; } - @Override - public void setScopes(String scopes) { - throw new UnsupportedOperationException("Unimplemented method 'setScope'"); + protected void setName(String name) { + this.name = name; } - @Override - public String getUseAsUsername() { - throw new UnsupportedOperationException("Unimplemented method 'getUseAsUsername'"); + protected void setClientName(String clientName) { + this.clientName = clientName; } - @Override - public void setUseAsUsername(String useAsUsername) { - throw new UnsupportedOperationException("Unimplemented method 'setUseAsUsername'"); + protected void setClientId(String clientId) { + this.clientId = clientId; } - @Override - public String getIssuer() { - throw new UnsupportedOperationException("Unimplemented method 'getIssuer'"); + protected void setClientSecret(String clientSecret) { + this.clientSecret = clientSecret; } - @Override - public void setIssuer(String issuer) { - throw new UnsupportedOperationException("Unimplemented method 'setIssuer'"); + protected void setScopes(String scopes) { + this.scopes = Arrays.stream(scopes.split(",")).map(String::trim).collect(Collectors.toList()); } - @Override - public String getClientSecret() { - throw new UnsupportedOperationException("Unimplemented method 'getClientSecret'"); + protected void setUseAsUsername(String useAsUsername) { + this.useAsUsername = useAsUsername; } - @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'"); - } } diff --git a/src/main/java/stirling/software/SPDF/model/ProviderInterface.java b/src/main/java/stirling/software/SPDF/model/ProviderInterface.java deleted file mode 100644 index faab1928..00000000 --- a/src/main/java/stirling/software/SPDF/model/ProviderInterface.java +++ /dev/null @@ -1,26 +0,0 @@ -package stirling.software.SPDF.model; - -import java.util.Collection; - -public interface ProviderInterface { - - Collection 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); -} 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 afe7fcb7..2bb4fe32 100644 --- a/src/main/java/stirling/software/SPDF/model/provider/GithubProvider.java +++ b/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 tokenUri = "https://github.com/login/oauth/access_token"; - private static final String userInfoUri = "https://api.github.com/user"; + private static final String NAME = "github"; + private static final String CLIENT_NAME = "GitHub"; + 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 scopes = new ArrayList<>(); private String useAsUsername = "login"; - public String getAuthorizationuri() { - return authorizationUri; - } - - 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) { + public GithubProvider(String clientId, String clientSecret, Collection scopes, String useAsUsername) { + super(null, NAME, CLIENT_NAME, clientId, clientSecret, scopes, useAsUsername); 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"); - } } 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 e43e1327..4839f80a 100644 --- a/src/main/java/stirling/software/SPDF/model/provider/GoogleProvider.java +++ b/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 tokenUri = "https://www.googleapis.com/oauth2/v4/token"; - private static final String userInfoUri = + private static final String NAME = "google"; + private static final String CLIENT_NAME = "Google"; + 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 scopes = new ArrayList<>(); private String useAsUsername = "email"; - public String getAuthorizationuri() { - return authorizationUri; - } - - 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) { + public GoogleProvider(String clientId, String clientSecret, Collection scopes, String useAsUsername) { + super(null, NAME, CLIENT_NAME, clientId, clientSecret, scopes, useAsUsername); 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"); - } } 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 d715b103..ad674916 100644 --- a/src/main/java/stirling/software/SPDF/model/provider/KeycloakProvider.java +++ b/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 scopes = new ArrayList<>(); + private Collection scopes; private String useAsUsername = "email"; - @Override - public String getIssuer() { - return this.issuer; - } - - @Override - public void setIssuer(String issuer) { + 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; - } - - @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 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"); - } }