From b6f361b627f07256e9d5857b61be894758d9f5dc Mon Sep 17 00:00:00 2001 From: Harry Smith Date: Tue, 3 Jan 2023 15:01:42 +0000 Subject: [PATCH 1/7] DWN-39926 : validate whitelist scopes are alphabet characters --- .../exception/WhitelistScopesException.java | 25 +++++++++++++++++++ .../openid/connect/web/WhitelistAPI.java | 25 +++++++++++++++++-- 2 files changed, 48 insertions(+), 2 deletions(-) create mode 100644 openid-connect-server/src/main/java/org/mitre/openid/connect/exception/WhitelistScopesException.java 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/WhitelistScopesException.java new file mode 100644 index 0000000000..9b662faaf6 --- /dev/null +++ b/openid-connect-server/src/main/java/org/mitre/openid/connect/exception/WhitelistScopesException.java @@ -0,0 +1,25 @@ +/** + * + *

+ * Copyright (c) 2010-2023 Gresham Technologies plc. All rights reserved. + * + * + */ +package org.mitre.openid.connect.exception; + +/** + * @author hwsmith + */ +public class WhitelistScopesException extends Exception { + + private final String invalidScope; + + public WhitelistScopesException(String invalidScope) { + this.invalidScope = invalidScope; + } + + public String getMessage() { + return "The scope " + invalidScope + " is invalid as it contains non-alphabet characters"; + } + +} 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 969a5febcd..932edea2f4 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 @@ -22,7 +22,9 @@ import java.security.Principal; import java.util.Collection; +import java.util.Set; +import org.mitre.openid.connect.exception.WhitelistScopesException; import org.mitre.openid.connect.model.WhitelistedSite; import org.mitre.openid.connect.service.WhitelistedSiteService; import org.mitre.openid.connect.view.HttpCodeView; @@ -56,6 +58,7 @@ public class WhitelistAPI { public static final String URL = RootController.API_URL + "/whitelist"; + private static final String characterMatcher = "[a-zA-Z]+"; @Autowired private WhitelistedSiteService whitelistService; @@ -100,7 +103,12 @@ public String addNewWhitelistedSite(@RequestBody String jsonString, ModelMap m, try { json = parser.parse(jsonString).getAsJsonObject(); whitelist = gson.fromJson(json, WhitelistedSite.class); - + validateWhitelistScopes(whitelist.getAllowedScopes()); + } catch (WhitelistScopesException e) { + logger.error("addNewWhitelistedSite failed due to WhitelistException. {}", 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 exception. Contact a system administrator for assistance."); + return JsonErrorView.VIEWNAME; } catch (JsonParseException e) { logger.error("addNewWhitelistedSite failed due to JsonParseException", e); m.addAttribute(HttpCodeView.CODE, HttpStatus.BAD_REQUEST); @@ -137,7 +145,12 @@ public String updateWhitelistedSite(@PathVariable("id") Long id, @RequestBody St try { json = parser.parse(jsonString).getAsJsonObject(); whitelist = gson.fromJson(json, WhitelistedSite.class); - + validateWhitelistScopes(whitelist.getAllowedScopes()); + } catch (WhitelistScopesException e) { + logger.error("addNewWhitelistedSite failed due to WhitelistScopeException. {}", 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 scope exception. Contact a system administrator for assistance."); + return JsonErrorView.VIEWNAME; } catch (JsonParseException e) { logger.error("updateWhitelistedSite failed due to JsonParseException", e); m.put(HttpCodeView.CODE, HttpStatus.BAD_REQUEST); @@ -167,6 +180,14 @@ public String updateWhitelistedSite(@PathVariable("id") Long id, @RequestBody St } } + private void validateWhitelistScopes(Set scopes) throws WhitelistScopesException { + for (String s : scopes) { + if (!s.matches(characterMatcher)) { + throw new WhitelistScopesException(s); + } + } + } + /** * Delete a whitelisted site * From 0bea906f1e3f2a1228b13ebaf898d5e6d7735e7d Mon Sep 17 00:00:00 2001 From: Harry Smith Date: Wed, 4 Jan 2023 08:41:49 +0000 Subject: [PATCH 2/7] DWN-39926 : amend exception name in log messages --- .../java/org/mitre/openid/connect/web/WhitelistAPI.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 932edea2f4..6c046a4f93 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 @@ -105,9 +105,9 @@ public String addNewWhitelistedSite(@RequestBody String jsonString, ModelMap m, whitelist = gson.fromJson(json, WhitelistedSite.class); validateWhitelistScopes(whitelist.getAllowedScopes()); } catch (WhitelistScopesException e) { - logger.error("addNewWhitelistedSite failed due to WhitelistException. {}", e.getMessage()); + logger.error("addNewWhitelistedSite failed due to WhitelistScopesException. {}", 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 exception. Contact a system administrator for assistance."); + m.addAttribute(JsonErrorView.ERROR_MESSAGE, "Could not save new whitelisted site. The server encountered a whitelist scopes exception. Contact a system administrator for assistance."); return JsonErrorView.VIEWNAME; } catch (JsonParseException e) { logger.error("addNewWhitelistedSite failed due to JsonParseException", e); @@ -147,9 +147,9 @@ public String updateWhitelistedSite(@PathVariable("id") Long id, @RequestBody St whitelist = gson.fromJson(json, WhitelistedSite.class); validateWhitelistScopes(whitelist.getAllowedScopes()); } catch (WhitelistScopesException e) { - logger.error("addNewWhitelistedSite failed due to WhitelistScopeException. {}", e.getMessage()); + logger.error("addNewWhitelistedSite failed due to WhitelistScopesException. {}", 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 scope exception. Contact a system administrator for assistance."); + m.addAttribute(JsonErrorView.ERROR_MESSAGE, "Could not save new whitelisted site. The server encountered a whitelist scopes exception. Contact a system administrator for assistance."); return JsonErrorView.VIEWNAME; } catch (JsonParseException e) { logger.error("updateWhitelistedSite failed due to JsonParseException", e); From 32240a5c5c8077d9ba4d2cd8bd55f6bc758c961f Mon Sep 17 00:00:00 2001 From: Harry Smith Date: Wed, 4 Jan 2023 10:04:04 +0000 Subject: [PATCH 3/7] DWN-39926 : use put rather than addAttribute for updating whitelists --- .../java/org/mitre/openid/connect/web/WhitelistAPI.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 6c046a4f93..2aafbafc52 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 @@ -147,9 +147,9 @@ public String updateWhitelistedSite(@PathVariable("id") Long id, @RequestBody St whitelist = gson.fromJson(json, WhitelistedSite.class); validateWhitelistScopes(whitelist.getAllowedScopes()); } catch (WhitelistScopesException e) { - logger.error("addNewWhitelistedSite failed due to WhitelistScopesException. {}", 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."); + logger.error("updateWhitelistedSite failed due to WhitelistScopesException. {}", 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."); return JsonErrorView.VIEWNAME; } catch (JsonParseException e) { logger.error("updateWhitelistedSite failed due to JsonParseException", e); From 9325917ce224fb57ea85437f0bb2b60ce51db752 Mon Sep 17 00:00:00 2001 From: Harry Smith Date: Wed, 18 Jan 2023 13:46:40 +0000 Subject: [PATCH 4/7] 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 0b0dd70f1b..07b8acd59b 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 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.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 String updateScope(@PathVariable("id") Long id, @RequestBody String json, 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 String createScope(@RequestBody String json, ModelMap m) { 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 String createScope(@RequestBody String json, ModelMap m) { } } + 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 9b662faaf6..0b0ae4f866 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 @@ /** * @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 2aafbafc52..07425f1739 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.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 String addNewWhitelistedSite(@RequestBody String jsonString, ModelMap m, 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 String updateWhitelistedSite(@PathVariable("id") Long id, @RequestBody St 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 String updateWhitelistedSite(@PathVariable("id") Long id, @RequestBody St } } - 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); } } } From 42b6aa57bdb54a6a5668e5371809b4636c514bd9 Mon Sep 17 00:00:00 2001 From: Harry Smith Date: Mon, 23 Jan 2023 09:00:19 +0000 Subject: [PATCH 5/7] DWN-39926 : use patterns to avoid multiple compilation --- .../src/main/java/org/mitre/oauth2/web/ScopeAPI.java | 5 +++-- .../main/java/org/mitre/openid/connect/web/WhitelistAPI.java | 4 +++- 2 files changed, 6 insertions(+), 3 deletions(-) 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 07b8acd59b..28fc3eb396 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 @@ -21,6 +21,7 @@ package org.mitre.oauth2.web; import java.util.Set; +import java.util.regex.Pattern; import org.mitre.oauth2.model.SystemScope; import org.mitre.oauth2.service.SystemScopeService; @@ -57,7 +58,7 @@ public class ScopeAPI { public static final String URL = RootController.API_URL + "/scopes"; private static final String characterMatcher = "[a-zA-Z]+"; - + private static final Pattern pattern = Pattern.compile(characterMatcher); @Autowired private SystemScopeService scopeService; @@ -183,7 +184,7 @@ public String createScope(@RequestBody String json, ModelMap m) { } private void validateScope(SystemScope scope) throws ScopeException { - if (!scope.getValue().matches(characterMatcher)) { + if (!pattern.matcher(scope.getValue()).matches()) { throw new ScopeException(scope.getValue()); } } 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 07425f1739..35e7c77eb4 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 @@ -23,6 +23,7 @@ import java.security.Principal; import java.util.Collection; import java.util.Set; +import java.util.regex.Pattern; import org.mitre.openid.connect.exception.ScopeException; import org.mitre.openid.connect.model.WhitelistedSite; @@ -59,6 +60,7 @@ public class WhitelistAPI { public static final String URL = RootController.API_URL + "/whitelist"; private static final String characterMatcher = "[a-zA-Z]+"; + private static final Pattern pattern = Pattern.compile(characterMatcher); @Autowired private WhitelistedSiteService whitelistService; @@ -182,7 +184,7 @@ public String updateWhitelistedSite(@PathVariable("id") Long id, @RequestBody St private void validateWhitelistScopes(Set scopes) throws ScopeException { for (String s : scopes) { - if (!s.matches(characterMatcher)) { + if (!pattern.matcher(s).matches()) { throw new ScopeException(s); } } From 9119dddef1dfce8bfdb2d7760e1d05defac7a4fe Mon Sep 17 00:00:00 2001 From: Harry Smith Date: Thu, 26 Jan 2023 09:39:09 +0000 Subject: [PATCH 6/7] DWN-39926 : validate scopes on manage client page --- .../mitre/openid/connect/web/ClientAPI.java | 30 ++++++++++++++++--- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/openid-connect-server/src/main/java/org/mitre/openid/connect/web/ClientAPI.java b/openid-connect-server/src/main/java/org/mitre/openid/connect/web/ClientAPI.java index 6213f2c77f..6a064b622a 100644 --- a/openid-connect-server/src/main/java/org/mitre/openid/connect/web/ClientAPI.java +++ b/openid-connect-server/src/main/java/org/mitre/openid/connect/web/ClientAPI.java @@ -21,6 +21,8 @@ import java.sql.SQLIntegrityConstraintViolationException; import java.text.ParseException; import java.util.Collection; +import java.util.Set; +import java.util.regex.Pattern; import javax.persistence.PersistenceException; @@ -33,9 +35,8 @@ import org.mitre.oauth2.model.PKCEAlgorithm; import org.mitre.oauth2.service.ClientDetailsEntityService; import org.mitre.oauth2.web.AuthenticationUtilities; +import org.mitre.openid.connect.exception.ScopeException; import org.mitre.openid.connect.exception.ValidationException; -import org.mitre.openid.connect.model.CachedImage; -import org.mitre.openid.connect.service.ClientLogoLoadingService; import org.mitre.openid.connect.view.ClientEntityViewForAdmins; import org.mitre.openid.connect.view.ClientEntityViewForUsers; import org.mitre.openid.connect.view.HttpCodeView; @@ -45,10 +46,8 @@ import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; -import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; -import org.springframework.http.ResponseEntity; import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.security.core.Authentication; import org.springframework.security.oauth2.common.util.OAuth2Utils; @@ -130,6 +129,9 @@ public class ClientAPI { public static final String URL = RootController.API_URL + "/clients"; + private static final String characterMatcher = "[a-zA-Z]+"; + private static final Pattern pattern = Pattern.compile(characterMatcher); + @Autowired private ClientDetailsEntityService clientService; @@ -256,6 +258,12 @@ public String apiAddClient(@RequestBody String jsonString, Model m, Authenticati json = parser.parse(jsonString).getAsJsonObject(); client = gson.fromJson(json, ClientDetailsEntity.class); client = validateSoftwareStatement(client); + validateScopes(client.getScope()); + } catch (ScopeException e) { + logger.error("apiAddClient failed due to ScopeException", e); + m.addAttribute(HttpCodeView.CODE, HttpStatus.BAD_REQUEST); + m.addAttribute(JsonErrorView.ERROR_MESSAGE, "Could not save new client. The server encountered a scope exception. Contact a system administrator for assistance."); + return JsonErrorView.VIEWNAME; } catch (JsonSyntaxException e) { logger.error("apiAddClient failed due to JsonSyntaxException", e); m.addAttribute(HttpCodeView.CODE, HttpStatus.BAD_REQUEST); @@ -369,6 +377,12 @@ public String apiUpdateClient(@PathVariable("id") Long id, @RequestBody String j json = parser.parse(jsonString).getAsJsonObject(); client = gson.fromJson(json, ClientDetailsEntity.class); client = validateSoftwareStatement(client); + validateScopes(client.getScope()); + } catch (ScopeException e) { + logger.error("apiUpdateClient failed due to ScopeException", e); + m.addAttribute(HttpCodeView.CODE, HttpStatus.BAD_REQUEST); + m.addAttribute(JsonErrorView.ERROR_MESSAGE, "Could not update client. The server encountered a scope exception. Contact a system administrator for assistance."); + return JsonErrorView.VIEWNAME; } catch (JsonSyntaxException e) { logger.error("apiUpdateClient failed due to JsonSyntaxException", e); m.addAttribute(HttpCodeView.CODE, HttpStatus.BAD_REQUEST); @@ -460,6 +474,14 @@ public String apiUpdateClient(@PathVariable("id") Long id, @RequestBody String j } } + private void validateScopes(Set scopes) throws ScopeException { + for (String s : scopes) { + if (!pattern.matcher(s).matches()) { + throw new ScopeException(s); + } + } + } + /** * Delete a client * @param id From 46b0312715e77f353c656f194bd3980055f705dd Mon Sep 17 00:00:00 2001 From: Harry Smith Date: Thu, 26 Jan 2023 10:10:52 +0000 Subject: [PATCH 7/7] DWN-39926 : pass whole exception not just the message --- .../src/main/java/org/mitre/oauth2/web/ScopeAPI.java | 4 ++-- .../main/java/org/mitre/openid/connect/web/WhitelistAPI.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) 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 28fc3eb396..250a22c867 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 @@ -109,7 +109,7 @@ public String updateScope(@PathVariable("id") Long id, @RequestBody String json, try { validateScope(scope); } catch (ScopeException e) { - logger.error("updateScope failed due to ScopeException. {}", e.getMessage()); + logger.error("updateScope failed due to ScopeException", e); 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; @@ -153,7 +153,7 @@ public String createScope(@RequestBody String json, ModelMap m) { try { validateScope(scope); } catch (ScopeException e) { - logger.error("createScope failed due to ScopeException. {}", e.getMessage()); + logger.error("createScope failed due to ScopeException", e); 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; 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 35e7c77eb4..db74ba53b2 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 @@ -107,7 +107,7 @@ public String addNewWhitelistedSite(@RequestBody String jsonString, ModelMap m, whitelist = gson.fromJson(json, WhitelistedSite.class); validateWhitelistScopes(whitelist.getAllowedScopes()); } catch (ScopeException e) { - logger.error("addNewWhitelistedSite failed due to ScopeException. {}", e.getMessage()); + logger.error("addNewWhitelistedSite failed due to ScopeException", e); m.addAttribute(HttpCodeView.CODE, HttpStatus.BAD_REQUEST); 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; @@ -149,7 +149,7 @@ public String updateWhitelistedSite(@PathVariable("id") Long id, @RequestBody St whitelist = gson.fromJson(json, WhitelistedSite.class); validateWhitelistScopes(whitelist.getAllowedScopes()); } catch (ScopeException e) { - logger.error("updateWhitelistedSite failed due to ScopeException. {}", e.getMessage()); + logger.error("updateWhitelistedSite failed due to ScopeException", e); m.put(HttpCodeView.CODE, HttpStatus.BAD_REQUEST); 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;