diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/SecurityRequirementDiff.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/SecurityRequirementDiff.java index 76b51a5d0..d382d50ac 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/compare/SecurityRequirementDiff.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/SecurityRequirementDiff.java @@ -38,6 +38,21 @@ private LinkedHashMap> contains( SecurityScheme leftSecurityScheme = leftComponents.getSecuritySchemes().get(schemeRef); LinkedHashMap> found = new LinkedHashMap<>(); + // Match by component key first: when the other requirement still references the same + // securityScheme key, it is the same scheme even if the apiKey parameter name changed, + // and the name change is diffed as a scheme change. Matching by name first mispairs + // schemes when a requirement contains several apiKey schemes. + for (Map.Entry> entry : right.entrySet()) { + SecurityScheme rightSecurityScheme = rightComponents.getSecuritySchemes().get(entry.getKey()); + if (schemeRef.equals(entry.getKey()) + && leftSecurityScheme.getType() == rightSecurityScheme.getType()) { + found.put(entry.getKey(), entry.getValue()); + return found; + } + } + + // Fall back to matching by parameter name so a renamed component key is not reported + // as a removed and re-added scheme while the parameter name clients send is unchanged. for (Map.Entry> entry : right.entrySet()) { SecurityScheme rightSecurityScheme = rightComponents.getSecuritySchemes().get(entry.getKey()); if (leftSecurityScheme.getType() == rightSecurityScheme.getType()) { diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/SecuritySchemeDiff.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/SecuritySchemeDiff.java index f68d638d3..29b52b452 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/compare/SecuritySchemeDiff.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/SecuritySchemeDiff.java @@ -82,6 +82,12 @@ protected DeferredChanged computeDiff( case APIKEY: changedSecurityScheme.setChangedIn( !Objects.equals(leftSecurityScheme.getIn(), rightSecurityScheme.getIn())); + // The parameter name is an apiKey-only property; when the scheme is no longer an + // apiKey on the other side the type change is reported instead. + if (rightSecurityScheme.getType() == SecurityScheme.Type.APIKEY) { + changedSecurityScheme.setChangedName( + !Objects.equals(leftSecurityScheme.getName(), rightSecurityScheme.getName())); + } break; case OAUTH2: @@ -121,6 +127,7 @@ private ChangedSecurityScheme getCopyWithoutScopes( .setChangedType(original.isChangedType()) .setChangedIn(original.isChangedIn()) .setChangedScheme(original.isChangedScheme()) + .setChangedName(original.isChangedName()) .setChangedBearerFormat(original.isChangedBearerFormat()) .setDescription(original.getDescription()) .setOAuthFlows(original.getOAuthFlows()) diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java index 6fbc1c992..f27d81c04 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/BackwardIncompatibleProp.java @@ -43,6 +43,7 @@ public enum BackwardIncompatibleProp { SECURITY_REQUIREMENT_SCHEMES_INCREASED( "incompatible.security.requirement.schemes.increased", true), SECURITY_REQUIREMENTS_DECREASED("incompatible.security.requirements.decreased", true), + SECURITY_SCHEME_API_KEY_NAME_CHANGED("incompatible.security.scheme.apikey.name.changed", true), SECURITY_SCHEME_BEARER_FORMAT_CHANGED("incompatible.security.scheme.bearer.format.changed", true), SECURITY_SCHEME_OAUTH2_AUTH_URL_CHANGED( "incompatible.security.scheme.oauth2.auth.url.changed", true), diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedSecurityScheme.java b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedSecurityScheme.java index 1adb56b6f..c583ca583 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedSecurityScheme.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedSecurityScheme.java @@ -14,6 +14,7 @@ public class ChangedSecurityScheme implements ComposedChanged { private boolean changedType; private boolean changedIn; private boolean changedScheme; + private boolean changedName; private boolean changedBearerFormat; private boolean changedOpenIdConnectUrl; private ChangedSecuritySchemeScopes changedScopes; @@ -38,12 +39,18 @@ public DiffResult isCoreChanged() { if (!changedType && !changedIn && !changedScheme + && !changedName && !changedBearerFormat && !changedOpenIdConnectUrl && (changedScopes == null || changedScopes.isUnchanged())) { return DiffResult.NO_CHANGES; } + if (changedName) { + if (SECURITY_SCHEME_API_KEY_NAME_CHANGED.enabled(context)) { + return DiffResult.INCOMPATIBLE; + } + } if (changedBearerFormat) { if (SECURITY_SCHEME_BEARER_FORMAT_CHANGED.enabled(context)) { return DiffResult.INCOMPATIBLE; @@ -95,6 +102,10 @@ public boolean isChangedScheme() { return this.changedScheme; } + public boolean isChangedName() { + return this.changedName; + } + public boolean isChangedBearerFormat() { return this.changedBearerFormat; } @@ -144,6 +155,11 @@ public ChangedSecurityScheme setChangedScheme(final boolean changedScheme) { return this; } + public ChangedSecurityScheme setChangedName(final boolean changedName) { + this.changedName = changedName; + return this; + } + public ChangedSecurityScheme setChangedBearerFormat(final boolean changedBearerFormat) { this.changedBearerFormat = changedBearerFormat; return this; @@ -182,6 +198,7 @@ public boolean equals(Object o) { return changedType == that.changedType && changedIn == that.changedIn && changedScheme == that.changedScheme + && changedName == that.changedName && changedBearerFormat == that.changedBearerFormat && changedOpenIdConnectUrl == that.changedOpenIdConnectUrl && Objects.equals(oldSecurityScheme, that.oldSecurityScheme) @@ -200,6 +217,7 @@ public int hashCode() { changedType, changedIn, changedScheme, + changedName, changedBearerFormat, changedOpenIdConnectUrl, changedScopes, @@ -220,6 +238,8 @@ public java.lang.String toString() { + this.isChangedIn() + ", changedScheme=" + this.isChangedScheme() + + ", changedName=" + + this.isChangedName() + ", changedBearerFormat=" + this.isChangedBearerFormat() + ", changedOpenIdConnectUrl=" diff --git a/core/src/main/java/org/openapitools/openapidiff/core/output/ConsoleRender.java b/core/src/main/java/org/openapitools/openapidiff/core/output/ConsoleRender.java index 2b8607232..b2f0a343b 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/output/ConsoleRender.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/output/ConsoleRender.java @@ -6,12 +6,15 @@ 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.SecurityRequirement; +import io.swagger.v3.oas.models.security.SecurityScheme; import java.io.IOException; import java.io.OutputStreamWriter; import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.Optional; +import java.util.stream.Collectors; import org.apache.commons.lang3.StringUtils; import org.openapitools.openapidiff.core.exception.RendererException; import org.openapitools.openapidiff.core.model.*; @@ -104,7 +107,189 @@ private void ol_changed( safelyAppend(outputStreamWriter, System.lineSeparator()); safelyAppend(outputStreamWriter, ul_response(operation.getApiResponses())); } + if (operation.resultSecurityRequirements().isDifferent()) { + safelyAppend(outputStreamWriter, StringUtils.repeat(' ', 2)); + safelyAppend(outputStreamWriter, "Security Requirements:"); + safelyAppend(outputStreamWriter, System.lineSeparator()); + safelyAppend( + outputStreamWriter, ul_securityRequirements(operation.getSecurityRequirements())); + } + } + } + + private String ul_securityRequirements(ChangedSecurityRequirements changedSecurityRequirements) { + StringBuilder sb = new StringBuilder(); + if (changedSecurityRequirements.getIncreased() != null) { + for (SecurityRequirement requirement : changedSecurityRequirements.getIncreased()) { + sb.append(itemSecurityRequirement("New security requirement", requirement)); + } + } + if (changedSecurityRequirements.getMissing() != null) { + for (SecurityRequirement requirement : changedSecurityRequirements.getMissing()) { + sb.append(itemSecurityRequirement("Deleted security requirement", requirement)); + } + } + for (ChangedSecurityRequirement requirement : changedSecurityRequirements.getChanged()) { + sb.append(itemChangedSecurityRequirement(requirement)); + } + return sb.toString(); + } + + private String itemSecurityRequirement(String title, SecurityRequirement securityRequirement) { + return String.format( + "%s- %s: %s%n", + StringUtils.repeat(' ', 4), title, securityRequirementWithScopes(securityRequirement)); + } + + private String itemChangedSecurityRequirement( + ChangedSecurityRequirement changedSecurityRequirement) { + StringBuilder sb = new StringBuilder(); + SecurityRequirement nameSource = changedSecurityRequirement.getNewSecurityRequirement(); + if (nameSource == null || nameSource.isEmpty()) { + nameSource = changedSecurityRequirement.getOldSecurityRequirement(); + } + sb.append( + String.format( + "%s- Changed security requirement: %s%n", + StringUtils.repeat(' ', 4), securityRequirementName(nameSource))); + if (changedSecurityRequirement.getIncreased() != null) { + sb.append( + itemSecuritySchemeChange( + "New security scheme", + securityRequirementWithScopes(changedSecurityRequirement.getIncreased()))); } + if (changedSecurityRequirement.getMissing() != null) { + sb.append( + itemSecuritySchemeChange( + "Deleted security scheme", + securityRequirementWithScopes(changedSecurityRequirement.getMissing()))); + } + for (ChangedSecurityScheme scheme : changedSecurityRequirement.getChanged()) { + sb.append(itemChangedSecurityScheme(scheme)); + } + return sb.toString(); + } + + private String itemChangedSecurityScheme(ChangedSecurityScheme changedSecurityScheme) { + SecurityScheme oldScheme = changedSecurityScheme.getOldSecurityScheme(); + SecurityScheme newScheme = changedSecurityScheme.getNewSecurityScheme(); + StringBuilder sb = new StringBuilder(); + if (changedSecurityScheme.isChangedType()) { + sb.append( + itemSecuritySchemeChange( + "Changed security scheme type", + String.format("%s -> %s", oldScheme.getType(), newScheme.getType()))); + } + if (changedSecurityScheme.isChangedIn()) { + sb.append( + itemSecuritySchemeChange( + "Changed security scheme location", + String.format("%s -> %s", oldScheme.getIn(), newScheme.getIn()))); + } + if (changedSecurityScheme.isChangedName()) { + sb.append( + itemSecuritySchemeChange( + "Changed security scheme name", + String.format("%s -> %s", oldScheme.getName(), newScheme.getName()))); + } + if (changedSecurityScheme.isChangedScheme()) { + sb.append( + itemSecuritySchemeChange( + "Changed security scheme", + String.format("%s -> %s", oldScheme.getScheme(), newScheme.getScheme()))); + } + if (changedSecurityScheme.isChangedBearerFormat()) { + sb.append( + itemSecuritySchemeChange( + "Changed bearer format", + String.format("%s -> %s", oldScheme.getBearerFormat(), newScheme.getBearerFormat()))); + } + if (changedSecurityScheme.isChangedOpenIdConnectUrl()) { + sb.append( + itemSecuritySchemeChange( + "Changed openIdConnectUrl", + String.format( + "%s -> %s", oldScheme.getOpenIdConnectUrl(), newScheme.getOpenIdConnectUrl()))); + } + if (changedSecurityScheme.getChangedScopes() != null) { + ChangedSecuritySchemeScopes changedScopes = changedSecurityScheme.getChangedScopes(); + if (!changedScopes.getIncreased().isEmpty()) { + sb.append( + itemSecuritySchemeChange( + "Added scope value(s)", String.join(", ", changedScopes.getIncreased()))); + } + if (!changedScopes.getMissing().isEmpty()) { + sb.append( + itemSecuritySchemeChange( + "Removed scope value(s)", String.join(", ", changedScopes.getMissing()))); + } + } + if (changedSecurityScheme.getOAuthFlows() != null) { + ChangedOAuthFlows changedOAuthFlows = changedSecurityScheme.getOAuthFlows(); + sb.append(itemChangedOAuthFlow("implicit", changedOAuthFlows.getImplicitOAuthFlow())); + sb.append(itemChangedOAuthFlow("password", changedOAuthFlows.getPasswordOAuthFlow())); + sb.append( + itemChangedOAuthFlow( + "clientCredentials", changedOAuthFlows.getClientCredentialOAuthFlow())); + sb.append( + itemChangedOAuthFlow( + "authorizationCode", changedOAuthFlows.getAuthorizationCodeOAuthFlow())); + } + return sb.toString(); + } + + private String itemChangedOAuthFlow(String flowName, ChangedOAuthFlow changedOAuthFlow) { + if (changedOAuthFlow == null) { + return ""; + } + StringBuilder sb = new StringBuilder(); + if (changedOAuthFlow.isAuthorizationUrl()) { + sb.append( + itemSecuritySchemeChange( + String.format("Changed %s flow authorizationUrl", flowName), + String.format( + "%s -> %s", + changedOAuthFlow.getOldOAuthFlow().getAuthorizationUrl(), + changedOAuthFlow.getNewOAuthFlow().getAuthorizationUrl()))); + } + if (changedOAuthFlow.isTokenUrl()) { + sb.append( + itemSecuritySchemeChange( + String.format("Changed %s flow tokenUrl", flowName), + String.format( + "%s -> %s", + changedOAuthFlow.getOldOAuthFlow().getTokenUrl(), + changedOAuthFlow.getNewOAuthFlow().getTokenUrl()))); + } + if (changedOAuthFlow.isRefreshUrl()) { + sb.append( + itemSecuritySchemeChange( + String.format("Changed %s flow refreshUrl", flowName), + String.format( + "%s -> %s", + changedOAuthFlow.getOldOAuthFlow().getRefreshUrl(), + changedOAuthFlow.getNewOAuthFlow().getRefreshUrl()))); + } + return sb.toString(); + } + + private String itemSecuritySchemeChange(String title, String detail) { + return String.format( + "%s* %s: %s%n", StringUtils.repeat(' ', 6), title, StringUtils.defaultString(detail)); + } + + private String securityRequirementWithScopes(SecurityRequirement securityRequirement) { + if (securityRequirement == null) { + return ""; + } + return securityRequirement.entrySet().stream() + .map( + entry -> String.format("%s: [%s]", entry.getKey(), String.join(", ", entry.getValue()))) + .collect(Collectors.joining(", ")); + } + + private String securityRequirementName(SecurityRequirement securityRequirement) { + return securityRequirement == null ? "" : String.join(", ", securityRequirement.keySet()); } private String ul_response(ChangedApiResponse changedApiResponse) { diff --git a/core/src/main/java/org/openapitools/openapidiff/core/output/HtmlRender.java b/core/src/main/java/org/openapitools/openapidiff/core/output/HtmlRender.java index 441d1c9dd..936b387f6 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/output/HtmlRender.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/output/HtmlRender.java @@ -26,7 +26,9 @@ 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 j2html.rendering.FlatHtml; import j2html.tags.ContainerTag; import j2html.tags.specialized.DivTag; @@ -36,15 +38,20 @@ import j2html.tags.specialized.UlTag; import java.io.IOException; import java.io.OutputStreamWriter; +import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.Optional; +import java.util.stream.Collectors; +import org.apache.commons.lang3.StringUtils; import org.openapitools.openapidiff.core.exception.RendererException; import org.openapitools.openapidiff.core.model.ChangedApiResponse; import org.openapitools.openapidiff.core.model.ChangedContent; import org.openapitools.openapidiff.core.model.ChangedMediaType; import org.openapitools.openapidiff.core.model.ChangedMetadata; +import org.openapitools.openapidiff.core.model.ChangedOAuthFlow; +import org.openapitools.openapidiff.core.model.ChangedOAuthFlows; import org.openapitools.openapidiff.core.model.ChangedOpenApi; import org.openapitools.openapidiff.core.model.ChangedOperation; import org.openapitools.openapidiff.core.model.ChangedParameter; @@ -53,6 +60,8 @@ import org.openapitools.openapidiff.core.model.ChangedSchema; import org.openapitools.openapidiff.core.model.ChangedSecurityRequirement; import org.openapitools.openapidiff.core.model.ChangedSecurityRequirements; +import org.openapitools.openapidiff.core.model.ChangedSecurityScheme; +import org.openapitools.openapidiff.core.model.ChangedSecuritySchemeScopes; import org.openapitools.openapidiff.core.model.DiffContext; import org.openapitools.openapidiff.core.model.DiffResult; import org.openapitools.openapidiff.core.model.Endpoint; @@ -270,14 +279,143 @@ private LiTag li_missingSecurityRequirement(SecurityRequirement securityRequirem private LiTag li_changedSecurityRequirement( ChangedSecurityRequirement changedSecurityRequirement) { - return li().withText(String.format("Changed security requirement : ")) - .with( - span( - (null == changedSecurityRequirement.getNewSecurityRequirement() - || null - == changedSecurityRequirement.getNewSecurityRequirement().toString()) - ? "" - : (changedSecurityRequirement.getNewSecurityRequirement().toString()))); + List schemeItems = new ArrayList<>(); + if (changedSecurityRequirement.getIncreased() != null) { + schemeItems.add( + li().withClass("add security scheme") + .withText("New security scheme : ") + .with(span(securityRequirementText(changedSecurityRequirement.getIncreased())))); + } + if (changedSecurityRequirement.getMissing() != null) { + schemeItems.add( + li().withClass("delete security scheme") + .withText("Deleted security scheme : ") + .with(span(securityRequirementText(changedSecurityRequirement.getMissing())))); + } + for (ChangedSecurityScheme scheme : changedSecurityRequirement.getChanged()) { + LiTag liScheme = li_changedSecurityScheme(scheme); + if (liScheme != null) { + schemeItems.add(liScheme); + } + } + LiTag li = + li().withText(String.format("Changed security requirement : ")) + .with( + span( + (null == changedSecurityRequirement.getNewSecurityRequirement() + || null + == changedSecurityRequirement + .getNewSecurityRequirement() + .toString()) + ? "" + : (changedSecurityRequirement.getNewSecurityRequirement().toString()))); + if (!schemeItems.isEmpty()) { + UlTag ul_schemes = ul().withClass("change security schemes"); + schemeItems.forEach(ul_schemes::with); + li.with(ul_schemes); + } + return li; + } + + private LiTag li_changedSecurityScheme(ChangedSecurityScheme changedSecurityScheme) { + String changes = securitySchemeChanges(changedSecurityScheme); + if (changes.isEmpty()) { + // suppress the item when there is no displayable scheme change (e.g. only the + // requirements around it changed) + return null; + } + return li().withClass("change security scheme") + .withText("Changed security scheme : ") + .with(span(changes)); + } + + private String securityRequirementText(SecurityRequirement securityRequirement) { + return securityRequirement.entrySet().stream() + .map( + entry -> String.format("%s: [%s]", entry.getKey(), String.join(", ", entry.getValue()))) + .collect(Collectors.joining(", ")); + } + + private String securitySchemeChanges(ChangedSecurityScheme changedSecurityScheme) { + SecurityScheme oldScheme = changedSecurityScheme.getOldSecurityScheme(); + SecurityScheme newScheme = changedSecurityScheme.getNewSecurityScheme(); + List changes = new ArrayList<>(); + if (changedSecurityScheme.isChangedType()) { + changes.add(String.format("type: %s -> %s", oldScheme.getType(), newScheme.getType())); + } + if (changedSecurityScheme.isChangedIn()) { + changes.add(String.format("in: %s -> %s", oldScheme.getIn(), newScheme.getIn())); + } + if (changedSecurityScheme.isChangedName()) { + changes.add(String.format("name: %s -> %s", oldScheme.getName(), newScheme.getName())); + } + if (changedSecurityScheme.isChangedScheme()) { + changes.add(String.format("scheme: %s -> %s", oldScheme.getScheme(), newScheme.getScheme())); + } + if (changedSecurityScheme.isChangedBearerFormat()) { + changes.add( + String.format( + "bearerFormat: %s -> %s", oldScheme.getBearerFormat(), newScheme.getBearerFormat())); + } + if (changedSecurityScheme.isChangedOpenIdConnectUrl()) { + changes.add( + String.format( + "openIdConnectUrl: %s -> %s", + oldScheme.getOpenIdConnectUrl(), newScheme.getOpenIdConnectUrl())); + } + if (changedSecurityScheme.getDescription() != null) { + changes.add( + String.format( + "description: %s -> %s", + StringUtils.defaultString(changedSecurityScheme.getDescription().getLeft()), + StringUtils.defaultString(changedSecurityScheme.getDescription().getRight()))); + } + if (changedSecurityScheme.getChangedScopes() != null) { + ChangedSecuritySchemeScopes changedScopes = changedSecurityScheme.getChangedScopes(); + if (!changedScopes.getIncreased().isEmpty()) { + changes.add("added scopes: " + String.join(", ", changedScopes.getIncreased())); + } + if (!changedScopes.getMissing().isEmpty()) { + changes.add("removed scopes: " + String.join(", ", changedScopes.getMissing())); + } + } + if (changedSecurityScheme.getOAuthFlows() != null) { + ChangedOAuthFlows changedOAuthFlows = changedSecurityScheme.getOAuthFlows(); + addOAuthFlowChanges("implicit", changedOAuthFlows.getImplicitOAuthFlow(), changes); + addOAuthFlowChanges("password", changedOAuthFlows.getPasswordOAuthFlow(), changes); + addOAuthFlowChanges( + "clientCredentials", changedOAuthFlows.getClientCredentialOAuthFlow(), changes); + addOAuthFlowChanges( + "authorizationCode", changedOAuthFlows.getAuthorizationCodeOAuthFlow(), changes); + } + return String.join("; ", changes); + } + + private void addOAuthFlowChanges( + String flowName, ChangedOAuthFlow changedOAuthFlow, List changes) { + if (changedOAuthFlow == null) { + return; + } + OAuthFlow oldFlow = changedOAuthFlow.getOldOAuthFlow(); + OAuthFlow newFlow = changedOAuthFlow.getNewOAuthFlow(); + if (changedOAuthFlow.isAuthorizationUrl()) { + changes.add( + String.format( + "%s flow authorizationUrl: %s -> %s", + flowName, oldFlow.getAuthorizationUrl(), newFlow.getAuthorizationUrl())); + } + if (changedOAuthFlow.isTokenUrl()) { + changes.add( + String.format( + "%s flow tokenUrl: %s -> %s", + flowName, oldFlow.getTokenUrl(), newFlow.getTokenUrl())); + } + if (changedOAuthFlow.isRefreshUrl()) { + changes.add( + String.format( + "%s flow refreshUrl: %s -> %s", + flowName, oldFlow.getRefreshUrl(), newFlow.getRefreshUrl())); + } } private UlTag ul_response(ChangedApiResponse changedApiResponse) { 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 531fd3809..8b5a52ec1 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,194 @@ 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: %s\n", LI, 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( + "%sChanged security requirement: %s\n", LI, code(securityRequirementName(nameSource)))); + if (changedSecurityRequirement.getIncreased() != null) { + sb.append( + itemSecurityScheme("New security scheme", changedSecurityRequirement.getIncreased())); + } + if (changedSecurityRequirement.getMissing() != null) { + sb.append( + itemSecurityScheme("Deleted security scheme", changedSecurityRequirement.getMissing())); + } + changedSecurityRequirement.getChanged().forEach(scheme -> sb.append(securityScheme(scheme))); + return sb.toString(); + } + + // Nested list items are indented two spaces per level so they render as a list; the + // four-space PRE_LI indent used elsewhere would render them as an indented code block. + protected String itemSecurityScheme(String title, SecurityRequirement securityRequirement) { + return format( + "%s%s%s: %s\n", + liIndent(1), LI, title, code(securityRequirementWithScopes(securityRequirement))); + } + + 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%sChanged security scheme type: `%s` -> `%s`\n", + liIndent(1), LI, oldScheme.getType(), newScheme.getType())); + } + if (changedSecurityScheme.isChangedIn()) { + sb.append( + format( + "%s%sChanged security scheme location: `%s` -> `%s`\n", + liIndent(1), LI, oldScheme.getIn(), newScheme.getIn())); + } + if (changedSecurityScheme.isChangedName()) { + sb.append( + format( + "%s%sChanged security scheme name: `%s` -> `%s`\n", + liIndent(1), LI, oldScheme.getName(), newScheme.getName())); + } + if (changedSecurityScheme.isChangedScheme()) { + sb.append( + format( + "%s%sChanged security scheme: `%s` -> `%s`\n", + liIndent(1), LI, oldScheme.getScheme(), newScheme.getScheme())); + } + if (changedSecurityScheme.isChangedBearerFormat()) { + sb.append( + format( + "%s%sChanged bearer format: `%s` -> `%s`\n", + liIndent(1), LI, oldScheme.getBearerFormat(), newScheme.getBearerFormat())); + } + if (changedSecurityScheme.isChangedOpenIdConnectUrl()) { + sb.append( + format( + "%s%sChanged openIdConnectUrl: %s -> %s\n", + liIndent(1), + LI, + url(oldScheme.getOpenIdConnectUrl()), + url(newScheme.getOpenIdConnectUrl()))); + } + sb.append(scopeChanges(changedSecurityScheme.getChangedScopes())); + sb.append(metadata(liIndent(1), "description", changedSecurityScheme.getDescription())); + if (changedSecurityScheme.getOAuthFlows() != null) { + sb.append(oAuthFlows(changedSecurityScheme.getOAuthFlows())); + } + return sb.toString(); + } + + private String scopeChanges(ChangedSecuritySchemeScopes changedScopes) { + if (changedScopes == null) { + return ""; + } + StringBuilder sb = new StringBuilder(); + if (!changedScopes.getIncreased().isEmpty()) { + sb.append( + format( + "%s%sAdded scope value(s): %s\n", + liIndent(1), LI, code(String.join(", ", changedScopes.getIncreased())))); + } + if (!changedScopes.getMissing().isEmpty()) { + sb.append( + format( + "%s%sRemoved scope value(s): %s\n", + liIndent(1), LI, code(String.join(", ", changedScopes.getMissing())))); + } + 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%sauthorizationUrl: %s -> %s\n", + liIndent(2), + LI, + url(oldFlow.getAuthorizationUrl()), + url(newFlow.getAuthorizationUrl()))); + } + if (changedOAuthFlow.isTokenUrl()) { + sb.append( + format( + "%s%stokenUrl: %s -> %s\n", + liIndent(2), LI, url(oldFlow.getTokenUrl()), url(newFlow.getTokenUrl()))); + } + if (changedOAuthFlow.isRefreshUrl()) { + sb.append( + format( + "%s%srefreshUrl: %s -> %s\n", + liIndent(2), LI, url(oldFlow.getRefreshUrl()), url(newFlow.getRefreshUrl()))); + } + if (sb.length() == 0) { + return ""; + } + return format("%s%sChanged %s flow:\n", liIndent(1), LI, 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())); @@ -596,6 +785,12 @@ protected String indent(int deepness) { return sb.toString(); } + // Two spaces per level, the CommonMark nested-list indent (indent() uses four spaces per + // level, which markdown renders as an indented code block). + protected String liIndent(int deepness) { + return StringUtils.repeat(' ', 2 * deepness); + } + protected Schema resolve(Schema schema) { return refPointer.resolveRef( diff.getNewSpecOpenApi().getComponents(), schema, schema.get$ref()); diff --git a/core/src/test/java/org/openapitools/openapidiff/core/SecurityDiffTest.java b/core/src/test/java/org/openapitools/openapidiff/core/SecurityDiffTest.java index 8c830ad56..b572caa10 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/SecurityDiffTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/SecurityDiffTest.java @@ -1,6 +1,8 @@ package org.openapitools.openapidiff.core; +import static java.util.Collections.emptyList; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.tuple; import static org.junit.jupiter.api.Assertions.assertThrows; import io.swagger.v3.oas.models.security.SecurityRequirement; @@ -101,4 +103,146 @@ public void testMissingSecurityDefinition() { IllegalArgumentException.class, () -> OpenApiCompare.fromLocations(OPENAPI_DOC5, OPENAPI_DOC5)); } + + /** Case 1: the component key is renamed but the apiKey parameter name is unchanged. */ + @Test + public void testApiKeyKeyRenamedNameSame() { + ChangedOpenApi changedOpenApi = + OpenApiCompare.fromLocations( + "issue-802-api-key-key-renamed-1.yaml", "issue-802-api-key-key-renamed-2.yaml"); + + assertThat(changedOpenApi.isUnchanged()).isTrue(); + assertThat(changedOpenApi.getChangedOperations()).isEmpty(); + } + + /** + * Case 2 (#802): same component key, the apiKey parameter name changed. The name change is + * reported on the scheme inside the matched requirement. + */ + @Test + public void testApiKeyNameChanged() { + ChangedOpenApi changedOpenApi = + OpenApiCompare.fromLocations( + "issue-802-api-key-name-changed-1.yaml", "issue-802-api-key-name-changed-2.yaml"); + + assertThat(changedOpenApi.getChangedOperations()).hasSize(1); + assertThat(changedOpenApi.isIncompatible()).isTrue(); + + ChangedSecurityRequirements changedSecurityRequirements = + changedOpenApi.getChangedOperations().get(0).getSecurityRequirements(); + assertThat(changedSecurityRequirements.getIncreased()).isNull(); + assertThat(changedSecurityRequirements.getMissing()).isNull(); + assertThat(changedSecurityRequirements.getChanged()).hasSize(1); + + ChangedSecurityRequirement changedSecurityRequirement = + changedSecurityRequirements.getChanged().get(0); + assertThat(changedSecurityRequirement.getIncreased()).isNull(); + assertThat(changedSecurityRequirement.getMissing()).isNull(); + assertThat(changedSecurityRequirement.getChanged()).hasSize(1); + + ChangedSecurityScheme changedSecurityScheme = changedSecurityRequirement.getChanged().get(0); + assertThat(changedSecurityScheme.isChangedName()).isTrue(); + assertThat(changedSecurityScheme.getOldSecurityScheme().getName()).isEqualTo("Authorization"); + assertThat(changedSecurityScheme.getNewSecurityScheme().getName()).isEqualTo("pat"); + } + + /** + * Case 3: both the component key and the parameter name changed. The scheme is reported as + * removed and added, not as a rename. + */ + @Test + public void testApiKeyKeyAndNameChanged() { + ChangedOpenApi changedOpenApi = + OpenApiCompare.fromLocations( + "issue-802-api-key-key-and-name-changed-1.yaml", + "issue-802-api-key-key-and-name-changed-2.yaml"); + + assertThat(changedOpenApi.getChangedOperations()).hasSize(1); + assertThat(changedOpenApi.isIncompatible()).isTrue(); + + ChangedSecurityRequirements changedSecurityRequirements = + changedOpenApi.getChangedOperations().get(0).getSecurityRequirements(); + assertThat(changedSecurityRequirements.getChanged()).hasSize(1); + + ChangedSecurityRequirement changedSecurityRequirement = + changedSecurityRequirements.getChanged().get(0); + assertThat(changedSecurityRequirement.getMissing()) + .hasSize(1) + .containsKey("PAT") + .containsValue(emptyList()); + assertThat(changedSecurityRequirement.getIncreased()) + .hasSize(1) + .containsKey("PAT2") + .containsValue(emptyList()); + // no rename is guessed + assertThat(changedSecurityRequirement.getChanged()).isEmpty(); + } + + /** Case 4: a new alternative security requirement is added, which is compatible. */ + @Test + public void testAlternativeSecurityRequirementAdded() { + ChangedOpenApi changedOpenApi = + OpenApiCompare.fromLocations( + "issue-802-security-requirement-added-1.yaml", + "issue-802-security-requirement-added-2.yaml"); + + assertThat(changedOpenApi.getChangedOperations()).hasSize(1); + assertThat(changedOpenApi.isCompatible()).isTrue(); + assertThat(changedOpenApi.isIncompatible()).isFalse(); + + ChangedSecurityRequirements changedSecurityRequirements = + changedOpenApi.getChangedOperations().get(0).getSecurityRequirements(); + assertThat(changedSecurityRequirements.getIncreased()) + .hasSize(1) + .first() + .satisfies(requirement -> assertThat(requirement).containsKey("petstore_auth")); + assertThat(changedSecurityRequirements.getMissing()).isNull(); + } + + /** + * Several apiKey schemes in one requirement are paired by component key first, so a name swap + * between two schemes is reported on both schemes instead of pairing each scheme with the other + * one by name. + */ + @Test + public void testSeveralApiKeysMatchedByComponentKeyFirst() { + ChangedOpenApi changedOpenApi = + OpenApiCompare.fromLocations( + "issue-802-api-key-two-schemes-names-swapped-1.yaml", + "issue-802-api-key-two-schemes-names-swapped-2.yaml"); + + assertThat(changedOpenApi.getChangedOperations()).hasSize(1); + assertThat(changedOpenApi.isIncompatible()).isTrue(); + + ChangedSecurityRequirement changedSecurityRequirement = + changedOpenApi.getChangedOperations().get(0).getSecurityRequirements().getChanged().get(0); + assertThat(changedSecurityRequirement.getChanged()).hasSize(2); + assertThat(changedSecurityRequirement.getChanged()) + .extracting( + scheme -> scheme.getOldSecurityScheme().getName(), + scheme -> scheme.getNewSecurityScheme().getName()) + .containsExactlyInAnyOrder( + tuple("Authorization", "X-Key"), tuple("X-Key", "Authorization")); + } + + @Test + public void testApiKeySchemeNameChangeWithinMatchedRequirement() { + ChangedOpenApi changedOpenApi = + OpenApiCompare.fromLocations( + "bc_security_scheme_base.yaml", "bc_security_scheme_api_key_name_changed.yaml"); + + assertThat(changedOpenApi.getChangedOperations()).hasSize(1); + ChangedSecurityRequirements changedSecurityRequirements = + changedOpenApi.getChangedOperations().get(0).getSecurityRequirements(); + assertThat(changedSecurityRequirements.getChanged()).hasSize(1); + + ChangedSecurityRequirement changedSecurityRequirement = + changedSecurityRequirements.getChanged().get(0); + assertThat(changedSecurityRequirement.getChanged()).hasSize(1); + + ChangedSecurityScheme changedSecurityScheme = changedSecurityRequirement.getChanged().get(0); + assertThat(changedSecurityScheme.isChangedName()).isTrue(); + assertThat(changedSecurityScheme.getOldSecurityScheme().getName()).isEqualTo("api_key"); + assertThat(changedSecurityScheme.getNewSecurityScheme().getName()).isEqualTo("api_key_renamed"); + } } diff --git a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/SecuritySchemeBCTest.java b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/SecuritySchemeBCTest.java index d89d824d5..94617e2aa 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/backcompat/SecuritySchemeBCTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/backcompat/SecuritySchemeBCTest.java @@ -52,4 +52,10 @@ public void scopesIncreased() { BackwardIncompatibleProp prop = SECURITY_SCHEME_SCOPES_INCREASED; assertSpecIncompatible(BASE, "bc_security_scheme_scopes_increased.yaml", prop); } + + @Test + public void apiKeyNameChanged() { + BackwardIncompatibleProp prop = SECURITY_SCHEME_API_KEY_NAME_CHANGED; + assertSpecIncompatible(BASE, "bc_security_scheme_api_key_name_changed.yaml", prop); + } } diff --git a/core/src/test/java/org/openapitools/openapidiff/core/output/ConsoleRenderTest.java b/core/src/test/java/org/openapitools/openapidiff/core/output/ConsoleRenderTest.java index a193e4e51..80e6becd5 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/output/ConsoleRenderTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/output/ConsoleRenderTest.java @@ -88,4 +88,38 @@ public void issue870_renderDoesNotFailWhenSchemaIsNullButExampleChanged() { render.render(diff, outputStreamWriter); assertThat(outputStream.toString()).isNotBlank(); } + + @Test + public void issue802_renderChangedApiKeySchemeNameInConsole() { + ConsoleRender render = new ConsoleRender(); + ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); + OutputStreamWriter outputStreamWriter = new OutputStreamWriter(outputStream); + ChangedOpenApi diff = + OpenApiCompare.fromLocations( + "issue-802-api-key-name-changed-1.yaml", "issue-802-api-key-name-changed-2.yaml"); + render.render(diff, outputStreamWriter); + assertThat(outputStream.toString()) + .contains("Security Requirements:") + .containsSubsequence( + "- Changed security requirement: PAT", + "* Changed security scheme name: Authorization -> pat"); + } + + @Test + public void issue802_renderRemovedAndAddedApiKeySchemeInConsole() { + ConsoleRender render = new ConsoleRender(); + ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); + OutputStreamWriter outputStreamWriter = new OutputStreamWriter(outputStream); + ChangedOpenApi diff = + OpenApiCompare.fromLocations( + "issue-802-api-key-key-and-name-changed-1.yaml", + "issue-802-api-key-key-and-name-changed-2.yaml"); + render.render(diff, outputStreamWriter); + assertThat(outputStream.toString()) + .contains("Security Requirements:") + .containsSubsequence( + "- Changed security requirement: PAT2", + "* New security scheme: PAT2: []", + "* Deleted security scheme: PAT: []"); + } } diff --git a/core/src/test/java/org/openapitools/openapidiff/core/output/HtmlRenderTest.java b/core/src/test/java/org/openapitools/openapidiff/core/output/HtmlRenderTest.java index 2d770e00c..550422c8b 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/output/HtmlRenderTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/output/HtmlRenderTest.java @@ -43,4 +43,40 @@ public void issue865_renderWithShowAllChangesDoesNotFailWhenSchemaIsNullButExamp render.render(diff, outputStreamWriter); assertThat(outputStream.toString()).isNotBlank(); } + + @Test + public void issue802_renderChangedApiKeySchemeNameInHtml() { + HtmlRender render = new HtmlRender(true); + ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); + OutputStreamWriter outputStreamWriter = new OutputStreamWriter(outputStream); + ChangedOpenApi diff = + OpenApiCompare.fromLocations( + "issue-802-api-key-name-changed-1.yaml", "issue-802-api-key-name-changed-2.yaml"); + render.render(diff, outputStreamWriter); + String output = outputStream.toString(); + assertThat(output).contains("Security Requirements"); + assertThat(output) + .contains("Changed security requirement : ") + .contains("name: Authorization -> pat"); + } + + @Test + public void issue802_renderRemovedAndAddedApiKeySchemeInHtml() { + HtmlRender render = new HtmlRender(true); + ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); + OutputStreamWriter outputStreamWriter = new OutputStreamWriter(outputStream); + ChangedOpenApi diff = + OpenApiCompare.fromLocations( + "issue-802-api-key-key-and-name-changed-1.yaml", + "issue-802-api-key-key-and-name-changed-2.yaml"); + render.render(diff, outputStreamWriter); + String output = outputStream.toString(); + assertThat(output).contains("Security Requirements"); + assertThat(output) + .contains("Changed security requirement : ") + .contains("New security scheme : ") + .contains(">PAT2: []") + .contains("Deleted security scheme : ") + .contains(">PAT: []"); + } } 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 597882105..2d36d4670 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,13 @@ package org.openapitools.openapidiff.core.output; import static org.assertj.core.api.Assertions.assertThat; +import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.SECURITY_SCHEME_API_KEY_NAME_CHANGED; 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 +54,85 @@ public void issue870_renderDoesNotFailWhenSchemaIsNullButExampleChanged() { render.render(diff, outputStreamWriter); assertThat(outputStream.toString()).isNotBlank(); } + + @Test + public void issue802_renderChangedApiKeySchemeNameInMarkdown() { + MarkdownRender render = new MarkdownRender(); + ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); + OutputStreamWriter outputStreamWriter = new OutputStreamWriter(outputStream); + ChangedOpenApi diff = + OpenApiCompare.fromLocations( + "issue-802-api-key-name-changed-1.yaml", "issue-802-api-key-name-changed-2.yaml"); + render.render(diff, outputStreamWriter); + String output = outputStream.toString(); + assertThat(output).contains("##### `GET` /info"); + assertThat(output).contains("Security Requirements:"); + assertThat(output).contains("* Changed security requirement: `PAT`"); + // two-space list indent: a four-space indent would render as a code block + assertThat(output).contains(" * Changed security scheme name: `Authorization` -> `pat`"); + assertThat(output).doesNotContain("\n * Changed security scheme name"); + // changing the apiKey parameter name is wire-breaking unless configured otherwise + assertThat(diff.isIncompatible()).isTrue(); + assertThat(output).contains("API changes broke backward compatibility"); + } + + @Test + public void issue802_renderRemovedAndAddedApiKeySchemeInMarkdown() { + MarkdownRender render = new MarkdownRender(); + ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); + OutputStreamWriter outputStreamWriter = new OutputStreamWriter(outputStream); + ChangedOpenApi diff = + OpenApiCompare.fromLocations( + "issue-802-api-key-key-and-name-changed-1.yaml", + "issue-802-api-key-key-and-name-changed-2.yaml"); + render.render(diff, outputStreamWriter); + String output = outputStream.toString(); + assertThat(output) + .contains("* Changed security requirement: `PAT2`") + .contains(" * New security scheme: `PAT2: []`") + .contains(" * Deleted security scheme: `PAT: []`"); + // no rename is guessed + assertThat(output).doesNotContain("Changed security scheme name"); + } + + @Test + public void issue802_renderAddedAlternativeSecurityRequirementInMarkdown() { + MarkdownRender render = new MarkdownRender(); + ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); + OutputStreamWriter outputStreamWriter = new OutputStreamWriter(outputStream); + ChangedOpenApi diff = + OpenApiCompare.fromLocations( + "issue-802-security-requirement-added-1.yaml", + "issue-802-security-requirement-added-2.yaml"); + render.render(diff, outputStreamWriter); + String output = outputStream.toString(); + assertThat(output).contains("* New security requirement: `petstore_auth: [read]`"); + assertThat(output).contains("API changes are backward compatible"); + } + + @Test + public void issue802_apiKeyNameChangeIsCompatibleWhenConfigDisabled() { + OpenApiDiffOptions options = + OpenApiDiffOptions.builder() + .configProperty(SECURITY_SCHEME_API_KEY_NAME_CHANGED.getPropertyName(), "false") + .build(); + ChangedOpenApi diff = + OpenApiCompare.fromLocations( + "issue-802-api-key-name-changed-1.yaml", + "issue-802-api-key-name-changed-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: `PAT`"); + assertThat(output).contains(" * Changed security scheme name: `Authorization` -> `pat`"); + assertThat(output).contains("API changes are backward compatible"); + } } diff --git a/core/src/test/resources/bc_security_scheme_api_key_name_changed.yaml b/core/src/test/resources/bc_security_scheme_api_key_name_changed.yaml new file mode 100644 index 000000000..7277e3ad2 --- /dev/null +++ b/core/src/test/resources/bc_security_scheme_api_key_name_changed.yaml @@ -0,0 +1,42 @@ +openapi: 3.0.0 +info: + description: myDesc + title: myTitle + version: 1.0.0 +paths: + /widgets: + get: + operationId: listWidgets + responses: + '200': + description: successful operation + content: + application/json: + schema: + type: string + security: + - basic-scheme-1: [] + - apikey-scheme-1: [] + - openidconnect-scheme-1: [] + - oauth2-scheme-1: + - "scope1" + - "scope2" +components: + securitySchemes: + apikey-scheme-1: + type: apiKey + name: api_key_renamed + in: header + basic-scheme-1: + type: http + openidconnect-scheme-1: + type: openIdConnect + openIdConnectUrl: https://example.com/api/openidconnect + oauth2-scheme-1: + type: oauth2 + flows: + implicit: + authorizationUrl: https://example.com/api/oauth/dialog + scopes: + scope1: scope1 description + scope2: scope2 description diff --git a/core/src/test/resources/issue-802-api-key-key-and-name-changed-1.yaml b/core/src/test/resources/issue-802-api-key-key-and-name-changed-1.yaml new file mode 100644 index 000000000..57e853932 --- /dev/null +++ b/core/src/test/resources/issue-802-api-key-key-and-name-changed-1.yaml @@ -0,0 +1,19 @@ +openapi: 3.0.0 +info: + title: Issue 802 API + version: V0 +paths: + /info: + get: + operationId: getInfo + responses: + '200': + description: OK + security: + - PAT: [] +components: + securitySchemes: + PAT: + type: apiKey + name: Authorization + in: header diff --git a/core/src/test/resources/issue-802-api-key-key-and-name-changed-2.yaml b/core/src/test/resources/issue-802-api-key-key-and-name-changed-2.yaml new file mode 100644 index 000000000..548096f73 --- /dev/null +++ b/core/src/test/resources/issue-802-api-key-key-and-name-changed-2.yaml @@ -0,0 +1,19 @@ +openapi: 3.0.0 +info: + title: Issue 802 API + version: V1 +paths: + /info: + get: + operationId: getInfo + responses: + '200': + description: OK + security: + - PAT2: [] +components: + securitySchemes: + PAT2: + type: apiKey + name: pat + in: header diff --git a/core/src/test/resources/issue-802-api-key-key-renamed-1.yaml b/core/src/test/resources/issue-802-api-key-key-renamed-1.yaml new file mode 100644 index 000000000..57e853932 --- /dev/null +++ b/core/src/test/resources/issue-802-api-key-key-renamed-1.yaml @@ -0,0 +1,19 @@ +openapi: 3.0.0 +info: + title: Issue 802 API + version: V0 +paths: + /info: + get: + operationId: getInfo + responses: + '200': + description: OK + security: + - PAT: [] +components: + securitySchemes: + PAT: + type: apiKey + name: Authorization + in: header diff --git a/core/src/test/resources/issue-802-api-key-key-renamed-2.yaml b/core/src/test/resources/issue-802-api-key-key-renamed-2.yaml new file mode 100644 index 000000000..6551e9454 --- /dev/null +++ b/core/src/test/resources/issue-802-api-key-key-renamed-2.yaml @@ -0,0 +1,19 @@ +openapi: 3.0.0 +info: + title: Issue 802 API + version: V1 +paths: + /info: + get: + operationId: getInfo + responses: + '200': + description: OK + security: + - PAT2: [] +components: + securitySchemes: + PAT2: + type: apiKey + name: Authorization + in: header diff --git a/core/src/test/resources/issue-802-api-key-name-changed-1.yaml b/core/src/test/resources/issue-802-api-key-name-changed-1.yaml new file mode 100644 index 000000000..57e853932 --- /dev/null +++ b/core/src/test/resources/issue-802-api-key-name-changed-1.yaml @@ -0,0 +1,19 @@ +openapi: 3.0.0 +info: + title: Issue 802 API + version: V0 +paths: + /info: + get: + operationId: getInfo + responses: + '200': + description: OK + security: + - PAT: [] +components: + securitySchemes: + PAT: + type: apiKey + name: Authorization + in: header diff --git a/core/src/test/resources/issue-802-api-key-name-changed-2.yaml b/core/src/test/resources/issue-802-api-key-name-changed-2.yaml new file mode 100644 index 000000000..8aa56dd8f --- /dev/null +++ b/core/src/test/resources/issue-802-api-key-name-changed-2.yaml @@ -0,0 +1,19 @@ +openapi: 3.0.0 +info: + title: Issue 802 API + version: V1 +paths: + /info: + get: + operationId: getInfo + responses: + '200': + description: OK + security: + - PAT: [] +components: + securitySchemes: + PAT: + type: apiKey + name: pat + in: header diff --git a/core/src/test/resources/issue-802-api-key-two-schemes-names-swapped-1.yaml b/core/src/test/resources/issue-802-api-key-two-schemes-names-swapped-1.yaml new file mode 100644 index 000000000..5fbb7f568 --- /dev/null +++ b/core/src/test/resources/issue-802-api-key-two-schemes-names-swapped-1.yaml @@ -0,0 +1,24 @@ +openapi: 3.0.0 +info: + title: Issue 802 API + version: V0 +paths: + /info: + get: + operationId: getInfo + responses: + '200': + description: OK + security: + - PAT: [] + KEY: [] +components: + securitySchemes: + PAT: + type: apiKey + name: Authorization + in: header + KEY: + type: apiKey + name: X-Key + in: header diff --git a/core/src/test/resources/issue-802-api-key-two-schemes-names-swapped-2.yaml b/core/src/test/resources/issue-802-api-key-two-schemes-names-swapped-2.yaml new file mode 100644 index 000000000..fb378c8e7 --- /dev/null +++ b/core/src/test/resources/issue-802-api-key-two-schemes-names-swapped-2.yaml @@ -0,0 +1,24 @@ +openapi: 3.0.0 +info: + title: Issue 802 API + version: V1 +paths: + /info: + get: + operationId: getInfo + responses: + '200': + description: OK + security: + - PAT: [] + KEY: [] +components: + securitySchemes: + PAT: + type: apiKey + name: X-Key + in: header + KEY: + type: apiKey + name: Authorization + in: header diff --git a/core/src/test/resources/issue-802-security-requirement-added-1.yaml b/core/src/test/resources/issue-802-security-requirement-added-1.yaml new file mode 100644 index 000000000..57e853932 --- /dev/null +++ b/core/src/test/resources/issue-802-security-requirement-added-1.yaml @@ -0,0 +1,19 @@ +openapi: 3.0.0 +info: + title: Issue 802 API + version: V0 +paths: + /info: + get: + operationId: getInfo + responses: + '200': + description: OK + security: + - PAT: [] +components: + securitySchemes: + PAT: + type: apiKey + name: Authorization + in: header diff --git a/core/src/test/resources/issue-802-security-requirement-added-2.yaml b/core/src/test/resources/issue-802-security-requirement-added-2.yaml new file mode 100644 index 000000000..becbaf39a --- /dev/null +++ b/core/src/test/resources/issue-802-security-requirement-added-2.yaml @@ -0,0 +1,28 @@ +openapi: 3.0.0 +info: + title: Issue 802 API + version: V1 +paths: + /info: + get: + operationId: getInfo + responses: + '200': + description: OK + security: + - PAT: [] + - petstore_auth: + - read +components: + securitySchemes: + PAT: + type: apiKey + name: Authorization + in: header + petstore_auth: + type: oauth2 + flows: + implicit: + authorizationUrl: https://example.com/api/oauth/dialog + scopes: + read: Read access to the info