Skip to content

Commit d9d1df3

Browse files
hsmith-greshamhsmith-gresham
authored
Merge pull request #31 from gresham-computing/DWN-39926_inputValidation
Dwn 39926 input validation
2 parents 8ad0c43 + 46b0312 commit d9d1df3

File tree

4 files changed

+103
-7
lines changed

4 files changed

+103
-7
lines changed

‎openid-connect-server/src/main/java/org/mitre/oauth2/web/ScopeAPI.java‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,11 @@
2121
package org.mitre.oauth2.web;
2222

2323
import java.util.Set;
24+
import java.util.regex.Pattern;
2425

2526
import org.mitre.oauth2.model.SystemScope;
2627
import org.mitre.oauth2.service.SystemScopeService;
28+
import org.mitre.openid.connect.exception.ScopeException;
2729
import org.mitre.openid.connect.view.HttpCodeView;
2830
import org.mitre.openid.connect.view.JsonEntityView;
2931
import org.mitre.openid.connect.view.JsonErrorView;
@@ -33,6 +35,7 @@
3335
import org.springframework.beans.factory.annotation.Autowired;
3436
import org.springframework.http.HttpStatus;
3537
import org.springframework.http.MediaType;
38+
import org.springframework.security.access.method.P;
3639
import org.springframework.security.access.prepost.PreAuthorize;
3740
import org.springframework.stereotype.Controller;
3841
import org.springframework.ui.ModelMap;
@@ -54,6 +57,8 @@ public class ScopeAPI {
5457

5558
public static final String URL = RootController.API_URL + "/scopes";
5659

60+
private static final String characterMatcher = "[a-zA-Z]+";
61+
private static final Pattern pattern = Pattern.compile(characterMatcher);
5762
@Autowired
5863
private SystemScopeService scopeService;
5964

@@ -101,7 +106,14 @@ public String updateScope(@PathVariable("id") Long id, @RequestBody String json,
101106
SystemScope existing = scopeService.getById(id);
102107

103108
SystemScope scope = gson.fromJson(json, SystemScope.class);
104-
109+
try {
110+
validateScope(scope);
111+
} catch (ScopeException e) {
112+
logger.error("updateScope failed due to ScopeException", e);
113+
m.put(HttpCodeView.CODE, HttpStatus.BAD_REQUEST);
114+
m.put(JsonErrorView.ERROR_MESSAGE, "Could not update scope. The server encountered a scope exception. Contact a system administrator for assistance.");
115+
return JsonErrorView.VIEWNAME;
116+
}
105117
if (existing != null && scope != null) {
106118

107119
if (existing.getId().equals(scope.getId())) {
@@ -138,6 +150,14 @@ public String createScope(@RequestBody String json, ModelMap m) {
138150
SystemScope scope = gson.fromJson(json, SystemScope.class);
139151

140152
SystemScope alreadyExists = scopeService.getByValue(scope.getValue());
153+
try {
154+
validateScope(scope);
155+
} catch (ScopeException e) {
156+
logger.error("createScope failed due to ScopeException", e);
157+
m.put(HttpCodeView.CODE, HttpStatus.BAD_REQUEST);
158+
m.put(JsonErrorView.ERROR_MESSAGE, "Could not create scope. The server encountered a scope exception. Contact a system administrator for assistance.");
159+
return JsonErrorView.VIEWNAME;
160+
}
141161
if (alreadyExists != null) {
142162
//Error, cannot save a scope with the same value as an existing one
143163
logger.error("Error: attempting to save a scope with a value that already exists: " + scope.getValue());
@@ -163,6 +183,12 @@ public String createScope(@RequestBody String json, ModelMap m) {
163183
}
164184
}
165185

186+
private void validateScope(SystemScope scope) throws ScopeException {
187+
if (!pattern.matcher(scope.getValue()).matches()) {
188+
throw new ScopeException(scope.getValue());
189+
}
190+
}
191+
166192
@PreAuthorize("hasRole('ROLE_ADMIN')")
167193
@RequestMapping(value = "/{id}", method = RequestMethod.DELETE)
168194
public String deleteScope(@PathVariable("id") Long id, ModelMap m) {

‎openid-connect-server/src/main/java/org/mitre/openid/connect/exception/ScopeException.java‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
/**
2+
* <copyright>
3+
* <p>
4+
* Copyright (c) 2010-2023 Gresham Technologies plc. All rights reserved.
5+
*
6+
* </copyright>
7+
*/
8+
package org.mitre.openid.connect.exception;
9+
10+
/**
11+
* @author hwsmith
12+
*/
13+
public class ScopeException extends Exception {
14+
15+
private final String invalidScope;
16+
17+
public ScopeException(String invalidScope) {
18+
this.invalidScope = invalidScope;
19+
}
20+
21+
public String getMessage() {
22+
return "The scope " + invalidScope + " is invalid as it contains non-alphabet characters";
23+
}
24+
25+
}

‎openid-connect-server/src/main/java/org/mitre/openid/connect/web/ClientAPI.java‎

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@
2121
import java.sql.SQLIntegrityConstraintViolationException;
2222
import java.text.ParseException;
2323
import java.util.Collection;
24+
import java.util.Set;
25+
import java.util.regex.Pattern;
2426

2527
import javax.persistence.PersistenceException;
2628

@@ -33,9 +35,8 @@
3335
import org.mitre.oauth2.model.PKCEAlgorithm;
3436
import org.mitre.oauth2.service.ClientDetailsEntityService;
3537
import org.mitre.oauth2.web.AuthenticationUtilities;
38+
import org.mitre.openid.connect.exception.ScopeException;
3639
import org.mitre.openid.connect.exception.ValidationException;
37-
import org.mitre.openid.connect.model.CachedImage;
38-
import org.mitre.openid.connect.service.ClientLogoLoadingService;
3940
import org.mitre.openid.connect.view.ClientEntityViewForAdmins;
4041
import org.mitre.openid.connect.view.ClientEntityViewForUsers;
4142
import org.mitre.openid.connect.view.HttpCodeView;
@@ -45,10 +46,8 @@
4546
import org.slf4j.LoggerFactory;
4647
import org.springframework.beans.factory.annotation.Autowired;
4748
import org.springframework.beans.factory.annotation.Qualifier;
48-
import org.springframework.http.HttpHeaders;
4949
import org.springframework.http.HttpStatus;
5050
import org.springframework.http.MediaType;
51-
import org.springframework.http.ResponseEntity;
5251
import org.springframework.security.access.prepost.PreAuthorize;
5352
import org.springframework.security.core.Authentication;
5453
import org.springframework.security.oauth2.common.util.OAuth2Utils;
@@ -130,6 +129,9 @@ public class ClientAPI {
130129

131130
public static final String URL = RootController.API_URL + "/clients";
132131

132+
private static final String characterMatcher = "[a-zA-Z]+";
133+
private static final Pattern pattern = Pattern.compile(characterMatcher);
134+
133135
@Autowired
134136
private ClientDetailsEntityService clientService;
135137

@@ -256,6 +258,12 @@ public String apiAddClient(@RequestBody String jsonString, Model m, Authenticati
256258
json = parser.parse(jsonString).getAsJsonObject();
257259
client = gson.fromJson(json, ClientDetailsEntity.class);
258260
client = validateSoftwareStatement(client);
261+
validateScopes(client.getScope());
262+
} catch (ScopeException e) {
263+
logger.error("apiAddClient failed due to ScopeException", e);
264+
m.addAttribute(HttpCodeView.CODE, HttpStatus.BAD_REQUEST);
265+
m.addAttribute(JsonErrorView.ERROR_MESSAGE, "Could not save new client. The server encountered a scope exception. Contact a system administrator for assistance.");
266+
return JsonErrorView.VIEWNAME;
259267
} catch (JsonSyntaxException e) {
260268
logger.error("apiAddClient failed due to JsonSyntaxException", e);
261269
m.addAttribute(HttpCodeView.CODE, HttpStatus.BAD_REQUEST);
@@ -369,6 +377,12 @@ public String apiUpdateClient(@PathVariable("id") Long id, @RequestBody String j
369377
json = parser.parse(jsonString).getAsJsonObject();
370378
client = gson.fromJson(json, ClientDetailsEntity.class);
371379
client = validateSoftwareStatement(client);
380+
validateScopes(client.getScope());
381+
} catch (ScopeException e) {
382+
logger.error("apiUpdateClient failed due to ScopeException", e);
383+
m.addAttribute(HttpCodeView.CODE, HttpStatus.BAD_REQUEST);
384+
m.addAttribute(JsonErrorView.ERROR_MESSAGE, "Could not update client. The server encountered a scope exception. Contact a system administrator for assistance.");
385+
return JsonErrorView.VIEWNAME;
372386
} catch (JsonSyntaxException e) {
373387
logger.error("apiUpdateClient failed due to JsonSyntaxException", e);
374388
m.addAttribute(HttpCodeView.CODE, HttpStatus.BAD_REQUEST);
@@ -460,6 +474,14 @@ public String apiUpdateClient(@PathVariable("id") Long id, @RequestBody String j
460474
}
461475
}
462476

477+
private void validateScopes(Set<String> scopes) throws ScopeException {
478+
for (String s : scopes) {
479+
if (!pattern.matcher(s).matches()) {
480+
throw new ScopeException(s);
481+
}
482+
}
483+
}
484+
463485
/**
464486
* Delete a client
465487
* @param id

‎openid-connect-server/src/main/java/org/mitre/openid/connect/web/WhitelistAPI.java‎

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,10 @@
2222

2323
import java.security.Principal;
2424
import java.util.Collection;
25+
import java.util.Set;
26+
import java.util.regex.Pattern;
2527

28+
import org.mitre.openid.connect.exception.ScopeException;
2629
import org.mitre.openid.connect.model.WhitelistedSite;
2730
import org.mitre.openid.connect.service.WhitelistedSiteService;
2831
import org.mitre.openid.connect.view.HttpCodeView;
@@ -56,6 +59,8 @@
5659
public class WhitelistAPI {
5760

5861
public static final String URL = RootController.API_URL + "/whitelist";
62+
private static final String characterMatcher = "[a-zA-Z]+";
63+
private static final Pattern pattern = Pattern.compile(characterMatcher);
5964

6065
@Autowired
6166
private WhitelistedSiteService whitelistService;
@@ -100,7 +105,12 @@ public String addNewWhitelistedSite(@RequestBody String jsonString, ModelMap m,
100105
try {
101106
json = parser.parse(jsonString).getAsJsonObject();
102107
whitelist = gson.fromJson(json, WhitelistedSite.class);
103-
108+
validateWhitelistScopes(whitelist.getAllowedScopes());
109+
} catch (ScopeException e) {
110+
logger.error("addNewWhitelistedSite failed due to ScopeException", e);
111+
m.addAttribute(HttpCodeView.CODE, HttpStatus.BAD_REQUEST);
112+
m.addAttribute(JsonErrorView.ERROR_MESSAGE, "Could not save new whitelisted site. The server encountered a scopes exception. Contact a system administrator for assistance.");
113+
return JsonErrorView.VIEWNAME;
104114
} catch (JsonParseException e) {
105115
logger.error("addNewWhitelistedSite failed due to JsonParseException", e);
106116
m.addAttribute(HttpCodeView.CODE, HttpStatus.BAD_REQUEST);
@@ -137,7 +147,12 @@ public String updateWhitelistedSite(@PathVariable("id") Long id, @RequestBody St
137147
try {
138148
json = parser.parse(jsonString).getAsJsonObject();
139149
whitelist = gson.fromJson(json, WhitelistedSite.class);
140-
150+
validateWhitelistScopes(whitelist.getAllowedScopes());
151+
} catch (ScopeException e) {
152+
logger.error("updateWhitelistedSite failed due to ScopeException", e);
153+
m.put(HttpCodeView.CODE, HttpStatus.BAD_REQUEST);
154+
m.put(JsonErrorView.ERROR_MESSAGE, "Could not update whitelisted site. The server encountered a scope exception. Contact a system administrator for assistance.");
155+
return JsonErrorView.VIEWNAME;
141156
} catch (JsonParseException e) {
142157
logger.error("updateWhitelistedSite failed due to JsonParseException", e);
143158
m.put(HttpCodeView.CODE, HttpStatus.BAD_REQUEST);
@@ -167,6 +182,14 @@ public String updateWhitelistedSite(@PathVariable("id") Long id, @RequestBody St
167182
}
168183
}
169184

185+
private void validateWhitelistScopes(Set<String> scopes) throws ScopeException {
186+
for (String s : scopes) {
187+
if (!pattern.matcher(s).matches()) {
188+
throw new ScopeException(s);
189+
}
190+
}
191+
}
192+
170193
/**
171194
* Delete a whitelisted site
172195
*

0 commit comments

Comments
 (0)