diff --git a/core/src/main/java/org/openapitools/openapidiff/core/output/MarkdownRender.java b/core/src/main/java/org/openapitools/openapidiff/core/output/MarkdownRender.java index 531fd380..5b62d86b 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/output/MarkdownRender.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/output/MarkdownRender.java @@ -11,12 +11,16 @@ import io.swagger.v3.oas.models.media.Schema; import io.swagger.v3.oas.models.parameters.Parameter; import io.swagger.v3.oas.models.responses.ApiResponse; +import io.swagger.v3.oas.models.security.OAuthFlow; +import io.swagger.v3.oas.models.security.SecurityRequirement; +import io.swagger.v3.oas.models.security.SecurityScheme; import java.io.IOException; import java.io.OutputStreamWriter; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.stream.Collectors; import org.apache.commons.lang3.StringUtils; import org.openapitools.openapidiff.core.exception.RendererException; import org.openapitools.openapidiff.core.model.*; @@ -152,9 +156,156 @@ protected void listEndpoints( safelyAppend(outputStreamWriter, titleH5("Return Type:")); safelyAppend(outputStreamWriter, responses(operation.getApiResponses())); } + if (operation.resultSecurityRequirements().isDifferent()) { + safelyAppend(outputStreamWriter, titleH5("Security Requirements:")); + safelyAppend( + outputStreamWriter, securityRequirements(operation.getSecurityRequirements())); + } }); } + protected String securityRequirements(ChangedSecurityRequirements changedSecurityRequirements) { + StringBuilder sb = new StringBuilder("\n"); + if (changedSecurityRequirements.getIncreased() != null) { + changedSecurityRequirements.getIncreased().stream() + .map(item -> itemSecurityRequirement("New security requirement", item)) + .forEach(sb::append); + } + if (changedSecurityRequirements.getMissing() != null) { + changedSecurityRequirements.getMissing().stream() + .map(item -> itemSecurityRequirement("Deleted security requirement", item)) + .forEach(sb::append); + } + changedSecurityRequirements.getChanged().stream() + .map(this::itemSecurityRequirement) + .forEach(sb::append); + return sb.toString(); + } + + protected String itemSecurityRequirement(String title, SecurityRequirement securityRequirement) { + return format("%s: %s\n\n", title, code(securityRequirementWithScopes(securityRequirement))); + } + + protected String itemSecurityRequirement(ChangedSecurityRequirement changedSecurityRequirement) { + StringBuilder sb = new StringBuilder(); + SecurityRequirement nameSource = changedSecurityRequirement.getNewSecurityRequirement(); + if (nameSource == null || nameSource.isEmpty()) { + nameSource = changedSecurityRequirement.getOldSecurityRequirement(); + } + sb.append( + format("Changed security requirement: %s\n\n", code(securityRequirementName(nameSource)))); + if (changedSecurityRequirement.getIncreased() != null) { + sb.append( + itemSecurityRequirement( + "New security scheme", changedSecurityRequirement.getIncreased())); + } + if (changedSecurityRequirement.getMissing() != null) { + sb.append( + itemSecurityRequirement( + "Deleted security scheme", changedSecurityRequirement.getMissing())); + } + changedSecurityRequirement.getChanged().forEach(scheme -> sb.append(securityScheme(scheme))); + return sb.append('\n').toString(); + } + + private String securityRequirementWithScopes(SecurityRequirement securityRequirement) { + return securityRequirement == null + ? "" + : securityRequirement.entrySet().stream() + .map(entry -> format("%s: [%s]", entry.getKey(), String.join(", ", entry.getValue()))) + .collect(Collectors.joining(", ")); + } + + private String securityRequirementName(SecurityRequirement securityRequirement) { + return securityRequirement == null ? "" : String.join(", ", securityRequirement.keySet()); + } + + protected String securityScheme(ChangedSecurityScheme changedSecurityScheme) { + StringBuilder sb = new StringBuilder(); + SecurityScheme oldScheme = changedSecurityScheme.getOldSecurityScheme(); + SecurityScheme newScheme = changedSecurityScheme.getNewSecurityScheme(); + if (changedSecurityScheme.isChangedType()) { + sb.append( + format( + "%s* Changed security scheme type: `%s` -> `%s`\n", + indent(1), oldScheme.getType(), newScheme.getType())); + } + if (changedSecurityScheme.isChangedIn()) { + sb.append( + format( + "%s* Changed security scheme location: `%s` -> `%s`\n", + indent(1), oldScheme.getIn(), newScheme.getIn())); + } + if (changedSecurityScheme.isChangedScheme()) { + sb.append( + format( + "%s* Changed security scheme: `%s` -> `%s`\n", + indent(1), oldScheme.getScheme(), newScheme.getScheme())); + } + if (changedSecurityScheme.isChangedBearerFormat()) { + sb.append( + format( + "%s* Changed bearer format: `%s` -> `%s`\n", + indent(1), oldScheme.getBearerFormat(), newScheme.getBearerFormat())); + } + if (changedSecurityScheme.isChangedOpenIdConnectUrl()) { + sb.append( + format( + "%s* Changed openIdConnectUrl: %s -> %s\n", + indent(1), + url(oldScheme.getOpenIdConnectUrl()), + url(newScheme.getOpenIdConnectUrl()))); + } + sb.append(listDiff(1, "scope", changedSecurityScheme.getChangedScopes())); + sb.append(metadata(indent(1), "description", changedSecurityScheme.getDescription())); + if (changedSecurityScheme.getOAuthFlows() != null) { + sb.append(oAuthFlows(changedSecurityScheme.getOAuthFlows())); + } + return sb.toString(); + } + + protected String oAuthFlows(ChangedOAuthFlows changedOAuthFlows) { + return oAuthFlow("implicit", changedOAuthFlows.getImplicitOAuthFlow()) + + oAuthFlow("password", changedOAuthFlows.getPasswordOAuthFlow()) + + oAuthFlow("clientCredentials", changedOAuthFlows.getClientCredentialOAuthFlow()) + + oAuthFlow("authorizationCode", changedOAuthFlows.getAuthorizationCodeOAuthFlow()); + } + + protected String oAuthFlow(String flowName, ChangedOAuthFlow changedOAuthFlow) { + if (changedOAuthFlow == null) { + return ""; + } + StringBuilder sb = new StringBuilder(); + OAuthFlow oldFlow = changedOAuthFlow.getOldOAuthFlow(); + OAuthFlow newFlow = changedOAuthFlow.getNewOAuthFlow(); + if (changedOAuthFlow.isAuthorizationUrl()) { + sb.append( + format( + "%s* authorizationUrl: %s -> %s\n", + indent(2), url(oldFlow.getAuthorizationUrl()), url(newFlow.getAuthorizationUrl()))); + } + if (changedOAuthFlow.isTokenUrl()) { + sb.append( + format( + "%s* tokenUrl: %s -> %s\n", + indent(2), url(oldFlow.getTokenUrl()), url(newFlow.getTokenUrl()))); + } + if (changedOAuthFlow.isRefreshUrl()) { + sb.append( + format( + "%s* refreshUrl: %s -> %s\n", + indent(2), url(oldFlow.getRefreshUrl()), url(newFlow.getRefreshUrl()))); + } + if (sb.length() == 0) { + return ""; + } + return format("%s* Changed %s flow:\n", indent(1), flowName) + sb; + } + + private String url(String url) { + return code(StringUtils.defaultString(url)); + } + protected String responses(ChangedApiResponse changedApiResponse) { StringBuilder sb = new StringBuilder("\n"); sb.append(listResponse("New response", changedApiResponse.getIncreased())); diff --git a/core/src/test/java/org/openapitools/openapidiff/core/output/MarkdownRenderTest.java b/core/src/test/java/org/openapitools/openapidiff/core/output/MarkdownRenderTest.java index 59788210..9a150bed 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/output/MarkdownRenderTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/output/MarkdownRenderTest.java @@ -1,11 +1,17 @@ package org.openapitools.openapidiff.core.output; import static org.assertj.core.api.Assertions.assertThat; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.SECURITY_REQUIREMENTS_DECREASED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.SECURITY_REQUIREMENT_SCHEMES_INCREASED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.SECURITY_SCHEME_OAUTH2_AUTH_URL_CHANGED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.SECURITY_SCHEME_OAUTH2_TOKEN_URL_CHANGED; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.SECURITY_SCHEME_SCOPES_INCREASED; import java.io.ByteArrayOutputStream; import java.io.OutputStreamWriter; import org.junit.jupiter.api.Test; import org.openapitools.openapidiff.core.OpenApiCompare; +import org.openapitools.openapidiff.core.compare.OpenApiDiffOptions; import org.openapitools.openapidiff.core.model.ChangedOpenApi; public class MarkdownRenderTest { @@ -52,4 +58,68 @@ public void issue870_renderDoesNotFailWhenSchemaIsNullButExampleChanged() { render.render(diff, outputStreamWriter); assertThat(outputStream.toString()).isNotBlank(); } + + @Test + public void issue893_renderChangedSecuritySchemeDetailsInMarkdown() { + MarkdownRender render = new MarkdownRender(); + ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); + OutputStreamWriter outputStreamWriter = new OutputStreamWriter(outputStream); + ChangedOpenApi diff = + OpenApiCompare.fromLocations("issue-893-oauth-1.yaml", "issue-893-oauth-2.yaml"); + render.render(diff, outputStreamWriter); + String output = outputStream.toString(); + assertThat(output).contains("##### `POST` /users"); + assertThat(output).contains("Security Requirements:"); + assertThat(output).contains("Changed security requirement: `OAuth2`"); + // a blank line separates the heading from the nested scheme bullets + assertThat(output).contains("Changed security requirement: `OAuth2`\n\n"); + assertThat(output).contains("Changed authorizationCode flow:"); + assertThat(output).contains("authorizationUrl: `` -> `some_value`"); + assertThat(output).contains("tokenUrl: `` -> `some_value`"); + // added and deleted security requirements are rendered with their scopes + assertThat(output).contains("##### `GET` /orders"); + assertThat(output).contains("New security requirement: `OpenIdConnect: [openid, profile]`"); + assertThat(output).contains("Deleted security requirement: `BasicAuth: []`"); + // schemes added to or removed from a changed requirement are rendered with their scopes + assertThat(output).contains("Changed security requirement: `OAuth2, NewApiKey`"); + assertThat(output).contains("New security scheme: `NewApiKey: []`"); + assertThat(output).contains("Deleted security scheme: `OldApiKey: []`"); + assertThat(output).contains("Added scope value:"); + assertThat(output).contains("* `write`"); + // the compatibility status follows the diff model, which marks OAuth2 flow URL + // changes and increased OAuth2 scopes as incompatible unless configured otherwise + assertThat(diff.isIncompatible()).isTrue(); + assertThat(output).contains("API changes broke backward compatibility"); + } + + @Test + public void issue893_securitySchemeUrlChangeIsCompatibleWhenConfigDisabled() { + OpenApiDiffOptions options = + OpenApiDiffOptions.builder() + .configProperty(SECURITY_SCHEME_OAUTH2_AUTH_URL_CHANGED.getPropertyName(), "false") + .configProperty(SECURITY_SCHEME_OAUTH2_TOKEN_URL_CHANGED.getPropertyName(), "false") + .configProperty(SECURITY_REQUIREMENTS_DECREASED.getPropertyName(), "false") + .configProperty(SECURITY_REQUIREMENT_SCHEMES_INCREASED.getPropertyName(), "false") + .configProperty(SECURITY_SCHEME_SCOPES_INCREASED.getPropertyName(), "false") + .build(); + ChangedOpenApi diff = + OpenApiCompare.fromLocations( + "issue-893-oauth-1.yaml", "issue-893-oauth-2.yaml", null, options); + assertThat(diff.isDifferent()).isTrue(); + assertThat(diff.isCompatible()).isTrue(); + + MarkdownRender render = new MarkdownRender(); + ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); + OutputStreamWriter outputStreamWriter = new OutputStreamWriter(outputStream); + render.render(diff, outputStreamWriter); + String output = outputStream.toString(); + assertThat(output).contains("Security Requirements:"); + assertThat(output).contains("Changed security requirement: `OAuth2`"); + assertThat(output).contains("authorizationUrl: `` -> `some_value`"); + assertThat(output).contains("New security requirement: `OpenIdConnect: [openid, profile]`"); + assertThat(output).contains("Deleted security requirement: `BasicAuth: []`"); + assertThat(output).contains("New security scheme: `NewApiKey: []`"); + assertThat(output).contains("Deleted security scheme: `OldApiKey: []`"); + assertThat(output).contains("API changes are backward compatible"); + } } diff --git a/core/src/test/resources/issue-893-oauth-1.yaml b/core/src/test/resources/issue-893-oauth-1.yaml new file mode 100644 index 00000000..b9f9c10b --- /dev/null +++ b/core/src/test/resources/issue-893-oauth-1.yaml @@ -0,0 +1,40 @@ +openapi: 3.1.0 +info: + title: Company API + version: V0 +paths: + /users: + post: + operationId: createUser + responses: + '201': + description: Created + security: + - OAuth2: [] + /orders: + get: + operationId: listOrders + responses: + '200': + description: OK + security: + - OAuth2: [read] + OldApiKey: [] + - BasicAuth: [] +components: + securitySchemes: + OAuth2: + type: oauth2 + flows: + authorizationCode: + authorizationUrl: '' + tokenUrl: '' + scopes: + read: Read access + OldApiKey: + type: apiKey + in: header + name: X-Old-Key + BasicAuth: + type: http + scheme: basic diff --git a/core/src/test/resources/issue-893-oauth-2.yaml b/core/src/test/resources/issue-893-oauth-2.yaml new file mode 100644 index 00000000..7d6a6494 --- /dev/null +++ b/core/src/test/resources/issue-893-oauth-2.yaml @@ -0,0 +1,41 @@ +openapi: 3.1.0 +info: + title: Company API + version: V0 +paths: + /users: + post: + operationId: createUser + responses: + '201': + description: Created + security: + - OAuth2: [] + /orders: + get: + operationId: listOrders + responses: + '200': + description: OK + security: + - OAuth2: [read, write] + NewApiKey: [] + - OpenIdConnect: [openid, profile] +components: + securitySchemes: + OAuth2: + type: oauth2 + flows: + authorizationCode: + authorizationUrl: some_value + tokenUrl: some_value + scopes: + read: Read access + write: Write access + NewApiKey: + type: apiKey + in: header + name: X-New-Key + OpenIdConnect: + type: openIdConnect + openIdConnectUrl: https://example.com/.well-known/openid-configuration