Skip to content

Commit 490ca8c

Browse files
thc202thc202
committed
openapi: honour context exclusions
Do not import endpoints excluded by the provided context. Part of zaproxy/zaproxy#8021. Signed-off-by: thc202 <[email protected]>
1 parent 091d4f9 commit 490ca8c

File tree

18 files changed

+170
-55
lines changed

18 files changed

+170
-55
lines changed

‎addOns/openapi/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
66
## Unreleased
77
### Added
88
- Allow to import the OpenAPI definitions with a user (Issue 7739).
9+
- Honour context exclusions when importing (Issue 8021).
910

1011
### Fixed
1112
- Allow to select the contexts of the Automation Framework plan when configuring the job.

‎addOns/openapi/src/main/java/org/zaproxy/zap/extension/openapi/ExtensionOpenApi.java‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -423,15 +423,13 @@ private List<String> importOpenApiDefinition(
423423
public void run() {
424424
ProgressPane currentImportPane = null;
425425
try {
426-
List<RequestModel> requestModels = converter.getRequestModels();
427-
if (contextId != -1) {
428-
Context context = getModel().getSession().getContext(contextId);
429-
if (context != null) {
430-
converter.updateVariantChecks(
431-
context,
432-
variantChecksMap.computeIfAbsent(
433-
contextId, VariantOpenApiChecks::new));
434-
}
426+
Context context = getModel().getSession().getContext(contextId);
427+
List<RequestModel> requestModels = converter.getRequestModels(context);
428+
if (context != null) {
429+
converter.updateVariantChecks(
430+
context,
431+
variantChecksMap.computeIfAbsent(
432+
contextId, VariantOpenApiChecks::new));
435433
}
436434
if (requestor == null) {
437435
return;

‎addOns/openapi/src/main/java/org/zaproxy/zap/extension/openapi/converter/Converter.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,9 @@
2222
import java.util.List;
2323
import org.zaproxy.zap.extension.openapi.converter.swagger.SwaggerException;
2424
import org.zaproxy.zap.extension.openapi.network.RequestModel;
25+
import org.zaproxy.zap.model.Context;
2526

2627
public interface Converter {
2728

28-
List<RequestModel> getRequestModels() throws SwaggerException;
29+
List<RequestModel> getRequestModels(Context context) throws SwaggerException;
2930
}

‎addOns/openapi/src/main/java/org/zaproxy/zap/extension/openapi/converter/swagger/SwaggerConverter.java‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -198,14 +198,17 @@ public List<OperationModel> getOperationModels() throws SwaggerException {
198198
}
199199

200200
@Override
201-
public List<RequestModel> getRequestModels() throws SwaggerException {
202-
return convertToRequest(getOperationModels());
201+
public List<RequestModel> getRequestModels(Context context) throws SwaggerException {
202+
return convertToRequest(context, getOperationModels());
203203
}
204204

205-
private List<RequestModel> convertToRequest(List<OperationModel> operations) {
205+
private List<RequestModel> convertToRequest(Context context, List<OperationModel> operations) {
206206
List<RequestModel> requests = new LinkedList<>();
207207
for (OperationModel operation : operations) {
208-
requests.add(requestConverter.convert(operation, generators));
208+
var model = requestConverter.convert(operation, generators);
209+
if (context == null || !context.isExcluded(model.getUrl())) {
210+
requests.add(model);
211+
}
209212
}
210213
return requests;
211214
}
@@ -479,15 +482,22 @@ public void updateVariantChecks(
479482
if (PATH_PART_PATTERN.matcher(uri).find()) {
480483
String regex = uri.replaceAll(PATH_PART_PATTERN.pattern(), "[^/?]+");
481484
variantChecks.pathsWithParamsRegex.put(operation, Pattern.compile(regex));
482-
if (!context.isIncluded(uri.replaceAll("[{}]", ""))) {
485+
if (isContextIncludeNeeded(context, uri.replaceAll("[{}]", ""))) {
483486
context.addIncludeInContextRegex(regex);
484487
}
485488
} else {
486489
variantChecks.pathsWithNoParams.add(operation);
487-
if (!context.isIncluded(uri)) {
490+
if (isContextIncludeNeeded(context, uri)) {
488491
context.addIncludeInContextRegex(uri);
489492
}
490493
}
491494
}
492495
}
496+
497+
private static boolean isContextIncludeNeeded(Context context, String uri) {
498+
if (context.isExcluded(uri)) {
499+
return false;
500+
}
501+
return !context.isIncluded(uri);
502+
}
493503
}

‎addOns/openapi/src/main/java/org/zaproxy/zap/extension/openapi/spider/OpenApiSpider.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ public boolean parseResource(ParseContext ctx) {
5757
message.getRequestHeader().getURI().toString(),
5858
message.getResponseBody().toString(),
5959
valGenSupplier.get());
60-
requestor.run(ctx.getUser(), converter.getRequestModels());
60+
requestor.run(ctx.getUser(), converter.getRequestModels(ctx.getContext()));
6161
} catch (Exception e) {
6262
LOGGER.debug(e.getMessage(), e);
6363
return false;

‎addOns/openapi/src/main/javahelp/org/zaproxy/zap/extension/openapi/resources/help/contents/openapi.html‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ <H3>Context</H3>
3131
<li>the imported spec is saved to the session database</li>
3232
<li>data driven nodes are generated for endpoints with path parameters</li>
3333
</ul>
34+
<strong>Note:</strong> Endpoints excluded by the Context will not be imported.
3435

3536
<H3>User</H3>
3637
The dialogues also allow selecting a User, optionally, to do authenticated imports.

‎addOns/openapi/src/test/java/org/zaproxy/zap/extension/openapi/converter/swagger/SwaggerConverterUnitTest.java‎

Lines changed: 53 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@
3131
import static org.hamcrest.Matchers.not;
3232
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
3333
import static org.junit.jupiter.api.Assertions.assertThrows;
34+
import static org.mockito.ArgumentMatchers.anyString;
35+
import static org.mockito.BDDMockito.given;
36+
import static org.mockito.Mockito.mock;
3437

3538
import io.swagger.v3.oas.models.servers.Server;
3639
import io.swagger.v3.oas.models.servers.ServerVariable;
@@ -44,6 +47,7 @@
4447
import org.zaproxy.zap.extension.openapi.OpenApiExceptions.EmptyDefinitionException;
4548
import org.zaproxy.zap.extension.openapi.OpenApiExceptions.InvalidUrlException;
4649
import org.zaproxy.zap.extension.openapi.network.RequestModel;
50+
import org.zaproxy.zap.model.Context;
4751

4852
/** Unit test for {@link SwaggerConverter}. */
4953
class SwaggerConverterUnitTest extends AbstractOpenApiTest {
@@ -1117,7 +1121,7 @@ void shouldUsePathServers() throws Exception {
11171121
String definition = getHtml("openapi_path_servers.yaml");
11181122
SwaggerConverter converter = new SwaggerConverter(definition, null);
11191123
// When
1120-
List<RequestModel> requests = converter.getRequestModels();
1124+
List<RequestModel> requests = converter.getRequestModels(null);
11211125
// Then
11221126
assertThat(converter.getErrorMessages(), is(empty()));
11231127
assertThat(
@@ -1136,7 +1140,7 @@ void shouldUseTargetUrlForPathServers() throws Exception {
11361140
SwaggerConverter converter =
11371141
new SwaggerConverter("/v2/", "http://localhost/definition/", definition, null);
11381142
// When
1139-
List<RequestModel> requests = converter.getRequestModels();
1143+
List<RequestModel> requests = converter.getRequestModels(null);
11401144
// Then
11411145
assertThat(converter.getErrorMessages(), is(empty()));
11421146
assertThat(
@@ -1153,7 +1157,7 @@ void shouldUseOperationServers() throws Exception {
11531157
String definition = getHtml("openapi_operation_servers.yaml");
11541158
SwaggerConverter converter = new SwaggerConverter(definition, null);
11551159
// When
1156-
List<RequestModel> requests = converter.getRequestModels();
1160+
List<RequestModel> requests = converter.getRequestModels(null);
11571161
// Then
11581162
assertThat(converter.getErrorMessages(), is(empty()));
11591163
assertThat(
@@ -1177,7 +1181,7 @@ void shouldUseTargetUrlForOperationServers() throws Exception {
11771181
SwaggerConverter converter =
11781182
new SwaggerConverter("/v2/", "http://localhost/definition/", definition, null);
11791183
// When
1180-
List<RequestModel> requests = converter.getRequestModels();
1184+
List<RequestModel> requests = converter.getRequestModels(null);
11811185
// Then
11821186
assertThat(converter.getErrorMessages(), is(empty()));
11831187
assertThat(
@@ -1193,6 +1197,51 @@ void shouldUseTargetUrlForOperationServers() throws Exception {
11931197
"PATCH http://server8.localhost/v2/operations/with/servers"));
11941198
}
11951199

1200+
@Test
1201+
void shouldConvertAllEndpointsIfNoContextProvided() throws Exception {
1202+
// Given
1203+
String definition = getHtml("openapi_paths_misc.yaml");
1204+
SwaggerConverter converter = new SwaggerConverter(definition, null);
1205+
Context context = null;
1206+
// When
1207+
List<RequestModel> requests = converter.getRequestModels(context);
1208+
// Then
1209+
assertThat(converter.getErrorMessages(), is(empty()));
1210+
assertThat(
1211+
urlsOf(requests),
1212+
contains(
1213+
"http://server.localhost/path/a/",
1214+
"http://server.localhost/path/b/",
1215+
"http://server.localhost/path/b/1/",
1216+
"http://server.localhost/path/c/",
1217+
"http://server.localhost/path/c/1/",
1218+
"http://server.localhost/path/c/2/"));
1219+
}
1220+
1221+
@Test
1222+
void shouldNotConvertEndpointsExcludedFromContext() throws Exception {
1223+
// Given
1224+
String definition = getHtml("openapi_paths_misc.yaml");
1225+
SwaggerConverter converter = new SwaggerConverter(definition, null);
1226+
var context = mock(Context.class);
1227+
given(context.isExcluded(anyString()))
1228+
.willAnswer(
1229+
e -> {
1230+
var uri = e.getArgument(0).toString();
1231+
return uri.contains("/path/b/") || uri.contains("/path/c/1/");
1232+
});
1233+
// When
1234+
List<RequestModel> requests = converter.getRequestModels(context);
1235+
// Then
1236+
assertThat(converter.getErrorMessages(), is(empty()));
1237+
assertThat(
1238+
urlsOf(requests),
1239+
contains(
1240+
"http://server.localhost/path/a/",
1241+
"http://server.localhost/path/c/",
1242+
"http://server.localhost/path/c/2/"));
1243+
}
1244+
11961245
private static List<String> urlsOf(List<RequestModel> requests) {
11971246
return requests.stream().map(RequestModel::getUrl).collect(Collectors.toList());
11981247
}

‎addOns/openapi/src/test/java/org/zaproxy/zap/extension/openapi/v1/OpenApiUnitTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ public void handleMessage(HttpMessage message, int initiator) {
8484
}
8585
};
8686
requestor.addListener(listener);
87-
requestor.run(converter.getRequestModels());
87+
requestor.run(converter.getRequestModels(null));
8888

8989
checkPetStore2dot0Requests(accessedUrls, "localhost:" + nano.getListeningPort());
9090
}

‎addOns/openapi/src/test/java/org/zaproxy/zap/extension/openapi/v2/OpenApiPrimitivesInBodyUnitTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ public void handleMessage(HttpMessage message, int initiator) {
8383
}
8484
};
8585
requestor.addListener(listener);
86-
requestor.run(converter.getRequestModels());
86+
requestor.run(converter.getRequestModels(null));
8787

8888
checkRequests(accessedUrls, "localhost:" + nano.getListeningPort());
8989
}

‎addOns/openapi/src/test/java/org/zaproxy/zap/extension/openapi/v2/OpenApiUnitTest.java‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ public void handleMessage(HttpMessage message, int initiator) {
7676
}
7777
};
7878
requestor.addListener(listener);
79-
requestor.run(converter.getRequestModels());
79+
requestor.run(converter.getRequestModels(null));
8080

8181
checkPetStoreRequests(accessedUrls, "localhost:" + nano.getListeningPort());
8282
}
@@ -109,7 +109,7 @@ public void handleMessage(HttpMessage message, int initiator) {
109109
}
110110
};
111111
requestor.addListener(listener);
112-
requestor.run(converter.getRequestModels());
112+
requestor.run(converter.getRequestModels(null));
113113

114114
checkPetStoreRequests(accessedUrls, "localhost:" + nano.getListeningPort());
115115
}
@@ -145,7 +145,7 @@ public void handleMessage(HttpMessage message, int initiator) {
145145
}
146146
};
147147
requestor.addListener(listener);
148-
requestor.run(converter.getRequestModels());
148+
requestor.run(converter.getRequestModels(null));
149149

150150
checkPetStoreRequests(accessedUrls, altHost);
151151
}
@@ -183,7 +183,7 @@ public void handleMessage(HttpMessage message, int initiator) {
183183
};
184184
requestor.addListener(listener);
185185
// When
186-
requestor.run(converter.getRequestModels());
186+
requestor.run(converter.getRequestModels(null));
187187
// Then
188188
checkPetStoreRequests(accessedUrls, defaultHost);
189189
}
@@ -220,7 +220,7 @@ public void handleMessage(HttpMessage message, int initiator) {
220220
};
221221
requestor.addListener(listener);
222222
// When / Then
223-
assertThrows(SwaggerException.class, () -> requestor.run(converter.getRequestModels()));
223+
assertThrows(SwaggerException.class, () -> requestor.run(converter.getRequestModels(null)));
224224
}
225225

226226
@Test
@@ -256,7 +256,7 @@ public void handleMessage(HttpMessage message, int initiator) {
256256
};
257257
requestor.addListener(listener);
258258
// When
259-
requestor.run(converter.getRequestModels());
259+
requestor.run(converter.getRequestModels(null));
260260
// Then
261261
checkPetStoreRequests(accessedUrls, "localhost:" + nano.getListeningPort());
262262
}
@@ -280,7 +280,7 @@ void shouldFailToExplorePetStoreWithoutScheme() throws Exception {
280280
requestor.getResponseBody(defnMsg.getRequestHeader().getURI()),
281281
null);
282282
// When / Then
283-
assertThrows(SwaggerException.class, () -> converter.getRequestModels());
283+
assertThrows(SwaggerException.class, () -> converter.getRequestModels(null));
284284
}
285285

286286
@Test
@@ -312,7 +312,7 @@ public void handleMessage(HttpMessage message, int initiator) {
312312
};
313313
requestor.addListener(listener);
314314
// When
315-
requestor.run(converter.getRequestModels());
315+
requestor.run(converter.getRequestModels(null));
316316
// Then
317317
assertThat(converter.getErrorMessages(), is(empty()));
318318
assertEquals(
@@ -382,7 +382,7 @@ public void handleMessage(HttpMessage message, int initiator) {
382382
}
383383
};
384384
requestor.addListener(listener);
385-
requestor.run(converter.getRequestModels());
385+
requestor.run(converter.getRequestModels(null));
386386

387387
checkPetStoreRequestsValGen(accessedUrls, "localhost:" + nano.getListeningPort());
388388
}
@@ -416,7 +416,7 @@ public void handleMessage(HttpMessage message, int initiator) {
416416
}
417417
};
418418
requestor.addListener(listener);
419-
requestor.run(converter.getRequestModels());
419+
requestor.run(converter.getRequestModels(null));
420420

421421
assertEquals(accessedUrls.size(), 2);
422422

0 commit comments

Comments
 (0)