From 9325917ce224fb57ea85437f0bb2b60ce51db752 Mon Sep 17 00:00:00 2001 From: Harry Smith Date: Wed, 18 Jan 2023 13:46:40 +0000 Subject: [PATCH] DWN-39926 : validate create and update scope directly --- .../java/org/mitre/oauth2/web/ScopeAPI.java | 27 ++++++++++++++++++- ...opesException.java => ScopeException.java} | 4 +-- .../openid/connect/web/WhitelistAPI.java | 18 ++++++------- 3 files changed, 37 insertions(+), 12 deletions(-) rename openid-connect-server/src/main/java/org/mitre/openid/connect/exception/{WhitelistScopesException.java => ScopeException.java} (77%) diff --git a/openid-connect-server/src/main/java/org/mitre/oauth2/web/ScopeAPI.java b/openid-connect-server/src/main/java/org/mitre/oauth2/web/ScopeAPI.java index 0b0dd70f1..07b8acd59 100644 --- a/openid-connect-server/src/main/java/org/mitre/oauth2/web/ScopeAPI.java +++ b/openid-connect-server/src/main/java/org/mitre/oauth2/web/ScopeAPI.java @@ -24,6 +24,7 @@ import java.util.Set; import org.mitre.oauth2.model.SystemScope; import org.mitre.oauth2.service.SystemScopeService; +import org.mitre.openid.connect.exception.ScopeException; import org.mitre.openid.connect.view.HttpCodeView; import org.mitre.openid.connect.view.JsonEntityView; import org.mitre.openid.connect.view.JsonErrorView; @@ -33,6 +34,7 @@ import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; +import org.springframework.security.access.method.P; import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.stereotype.Controller; import org.springframework.ui.ModelMap; @@ -54,6 +56,8 @@ public class ScopeAPI { public static final String URL = RootController.API_URL + "/scopes"; + private static final String characterMatcher = "[a-zA-Z]+"; + @Autowired private SystemScopeService scopeService; @@ -101,7 +105,14 @@ public class ScopeAPI { SystemScope existing = scopeService.getById(id); SystemScope scope = gson.fromJson(json, SystemScope.class); - + try { + validateScope(scope); + } catch (ScopeException e) { + logger.error("updateScope failed due to ScopeException. {}", e.getMessage()); + m.put(HttpCodeView.CODE, HttpStatus.BAD_REQUEST); + m.put(JsonErrorView.ERROR_MESSAGE, "Could not update scope. The server encountered a scope exception. Contact a system administrator for assistance."); + return JsonErrorView.VIEWNAME; + } if (existing != null && scope != null) { if (existing.getId().equals(scope.getId())) { @@ -138,6 +149,14 @@ public class ScopeAPI { SystemScope scope = gson.fromJson(json, SystemScope.class); SystemScope alreadyExists = scopeService.getByValue(scope.getValue()); + try { + validateScope(scope); + } catch (ScopeException e) { + logger.error("createScope failed due to ScopeException. {}", e.getMessage()); + m.put(HttpCodeView.CODE, HttpStatus.BAD_REQUEST); + m.put(JsonErrorView.ERROR_MESSAGE, "Could not create scope. The server encountered a scope exception. Contact a system administrator for assistance."); + return JsonErrorView.VIEWNAME; + } if (alreadyExists != null) { //Error, cannot save a scope with the same value as an existing one logger.error("Error: attempting to save a scope with a value that already exists: " + scope.getValue()); @@ -163,6 +182,12 @@ public class ScopeAPI { } } + private void validateScope(SystemScope scope) throws ScopeException { + if (!scope.getValue().matches(characterMatcher)) { + throw new ScopeException(scope.getValue()); + } + } + @PreAuthorize("hasRole('ROLE_ADMIN')") @RequestMapping(value = "/{id}", method = RequestMethod.DELETE) public String deleteScope(@PathVariable("id") Long id, ModelMap m) { diff --git a/openid-connect-server/src/main/java/org/mitre/openid/connect/exception/WhitelistScopesException.java b/openid-connect-server/src/main/java/org/mitre/openid/connect/exception/ScopeException.java similarity index 77% rename from openid-connect-server/src/main/java/org/mitre/openid/connect/exception/WhitelistScopesException.java rename to openid-connect-server/src/main/java/org/mitre/openid/connect/exception/ScopeException.java index 9b662faaf..0b0ae4f86 100644 --- a/openid-connect-server/src/main/java/org/mitre/openid/connect/exception/WhitelistScopesException.java +++ b/openid-connect-server/src/main/java/org/mitre/openid/connect/exception/ScopeException.java @@ -10,11 +10,11 @@ package org.mitre.openid.connect.exception; /** * @author hwsmith */ -public class WhitelistScopesException extends Exception { +public class ScopeException extends Exception { private final String invalidScope; - public WhitelistScopesException(String invalidScope) { + public ScopeException(String invalidScope) { this.invalidScope = invalidScope; } diff --git a/openid-connect-server/src/main/java/org/mitre/openid/connect/web/WhitelistAPI.java b/openid-connect-server/src/main/java/org/mitre/openid/connect/web/WhitelistAPI.java index 2aafbafc5..07425f173 100644 --- a/openid-connect-server/src/main/java/org/mitre/openid/connect/web/WhitelistAPI.java +++ b/openid-connect-server/src/main/java/org/mitre/openid/connect/web/WhitelistAPI.java @@ -24,7 +24,7 @@ import java.security.Principal; import java.util.Collection; import java.util.Set; -import org.mitre.openid.connect.exception.WhitelistScopesException; +import org.mitre.openid.connect.exception.ScopeException; import org.mitre.openid.connect.model.WhitelistedSite; import org.mitre.openid.connect.service.WhitelistedSiteService; import org.mitre.openid.connect.view.HttpCodeView; @@ -104,10 +104,10 @@ public class WhitelistAPI { json = parser.parse(jsonString).getAsJsonObject(); whitelist = gson.fromJson(json, WhitelistedSite.class); validateWhitelistScopes(whitelist.getAllowedScopes()); - } catch (WhitelistScopesException e) { - logger.error("addNewWhitelistedSite failed due to WhitelistScopesException. {}", e.getMessage()); + } catch (ScopeException e) { + logger.error("addNewWhitelistedSite failed due to ScopeException. {}", e.getMessage()); m.addAttribute(HttpCodeView.CODE, HttpStatus.BAD_REQUEST); - m.addAttribute(JsonErrorView.ERROR_MESSAGE, "Could not save new whitelisted site. The server encountered a whitelist scopes exception. Contact a system administrator for assistance."); + m.addAttribute(JsonErrorView.ERROR_MESSAGE, "Could not save new whitelisted site. The server encountered a scopes exception. Contact a system administrator for assistance."); return JsonErrorView.VIEWNAME; } catch (JsonParseException e) { logger.error("addNewWhitelistedSite failed due to JsonParseException", e); @@ -146,10 +146,10 @@ public class WhitelistAPI { json = parser.parse(jsonString).getAsJsonObject(); whitelist = gson.fromJson(json, WhitelistedSite.class); validateWhitelistScopes(whitelist.getAllowedScopes()); - } catch (WhitelistScopesException e) { - logger.error("updateWhitelistedSite failed due to WhitelistScopesException. {}", e.getMessage()); + } catch (ScopeException e) { + logger.error("updateWhitelistedSite failed due to ScopeException. {}", e.getMessage()); m.put(HttpCodeView.CODE, HttpStatus.BAD_REQUEST); - m.put(JsonErrorView.ERROR_MESSAGE, "Could not update whitelisted site. The server encountered a whitelist scopes exception. Contact a system administrator for assistance."); + m.put(JsonErrorView.ERROR_MESSAGE, "Could not update whitelisted site. The server encountered a scope exception. Contact a system administrator for assistance."); return JsonErrorView.VIEWNAME; } catch (JsonParseException e) { logger.error("updateWhitelistedSite failed due to JsonParseException", e); @@ -180,10 +180,10 @@ public class WhitelistAPI { } } - private void validateWhitelistScopes(Set scopes) throws WhitelistScopesException { + private void validateWhitelistScopes(Set scopes) throws ScopeException { for (String s : scopes) { if (!s.matches(characterMatcher)) { - throw new WhitelistScopesException(s); + throw new ScopeException(s); } } }