Skip to content

Commit

Permalink
Bump smallrye-open-api from 3.2.0 to 3.3.0, fix OpenAPI security issues
Browse files Browse the repository at this point in the history 
- Handle method-level `@RolesAllowed` that override class-level
`@RolesAllowed` values, fixes quarkusio#30997
- Render `BaseStream<T, S>` as array of `T` in OpenAPI document,
fixes quarkusio#30248 (via smallrye-open-api 3.3.0)
- Do not place scopes in OpenAPI security requirements unless the
security scheme is OAuth2 or OIDC, fixes quarkusio#27373
- Include only OIDC discovery URL in OpenAPI when auto-security is
active, fixes quarkusio#21126

Signed-off-by: Michael Edgar <[email protected]>
  • Loading branch information
@MikeEdgar
MikeEdgar committed Mar 8, 2023
1 parent 2771bdc commit 5ec7d2c
Show file tree
Hide file tree
Showing 9 changed files with 248 additions and 213 deletions.
4 changes: 2 additions & 2 deletions bom/application/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
<smallrye-config.version>3.1.3</smallrye-config.version>
<smallrye-health.version>4.0.1</smallrye-health.version>
<smallrye-metrics.version>4.0.0</smallrye-metrics.version>
<smallrye-open-api.version>3.2.0</smallrye-open-api.version>
<smallrye-open-api.version>3.3.0</smallrye-open-api.version>
<smallrye-graphql.version>2.1.1</smallrye-graphql.version>
<smallrye-opentracing.version>3.0.3</smallrye-opentracing.version>
<smallrye-fault-tolerance.version>6.2.1</smallrye-fault-tolerance.version>
Expand Down Expand Up @@ -1847,7 +1847,7 @@
<artifactId>quarkus-vertx-http-dev-ui-resources</artifactId>
<version>${project.version}</version>
</dependency>

<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-reactive-routes</artifactId>
Expand Down
98 changes: 29 additions & 69 deletions ...oyment/src/main/java/io/quarkus/smallrye/openapi/deployment/SmallRyeOpenApiProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.HashSet;
Expand Down Expand Up @@ -320,42 +321,22 @@ OpenApiFilteredIndexViewBuildItem smallryeOpenApiIndex(CombinedIndexBuildItem co

@BuildStep
void addAutoFilters(BuildProducer<AddToOpenAPIDefinitionBuildItem> addToOpenAPIDefinitionProducer,
List<SecurityInformationBuildItem> securityInformationBuildItems,
OpenApiFilteredIndexViewBuildItem apiFilteredIndexViewBuildItem,
SmallRyeOpenApiConfig config) {

List<AnnotationInstance> rolesAllowedAnnotations = new ArrayList<>();
for (DotName rolesAllowed : SecurityConstants.ROLES_ALLOWED) {
rolesAllowedAnnotations.addAll(apiFilteredIndexViewBuildItem.getIndex().getAnnotations(rolesAllowed));
}

Map<String, List<String>> methodReferences = new HashMap<>();
DotName securityRequirement = DotName.createSimple(SecurityRequirement.class.getName());
for (AnnotationInstance ai : rolesAllowedAnnotations) {
if (ai.target().kind().equals(AnnotationTarget.Kind.METHOD)) {
MethodInfo method = ai.target().asMethod();
if (isValidOpenAPIMethodForAutoAdd(method, securityRequirement)) {
String ref = JandexUtil.createUniqueMethodReference(method.declaringClass(), method);
methodReferences.put(ref, List.of(ai.value().asStringArray()));
}
}
if (ai.target().kind().equals(AnnotationTarget.Kind.CLASS)) {
ClassInfo classInfo = ai.target().asClass();
List<MethodInfo> methods = classInfo.methods();
for (MethodInfo method : methods) {
if (isValidOpenAPIMethodForAutoAdd(method, securityRequirement)) {
String ref = JandexUtil.createUniqueMethodReference(classInfo, method);
methodReferences.put(ref, List.of(ai.value().asStringArray()));
}
}

}
}

// Add a security scheme from config
if (config.securityScheme.isPresent()) {
addToOpenAPIDefinitionProducer
.produce(new AddToOpenAPIDefinitionBuildItem(
new SecurityConfigFilter(config)));
} else {
OASFilter autoSecurityFilter = getAutoSecurityFilter(securityInformationBuildItems, config);

if (autoSecurityFilter != null) {
addToOpenAPIDefinitionProducer
.produce(new AddToOpenAPIDefinitionBuildItem(autoSecurityFilter));
}
}

// Add Auto roles allowed
Expand Down Expand Up @@ -407,34 +388,19 @@ private OASFilter getAutoSecurityFilter(List<SecurityInformationBuildItem> secur
config.securitySchemeDescription,
config.basicSecuritySchemeValue);
case oidc:
Optional<SecurityInformationBuildItem.OpenIDConnectInformation> maybeInfo = securityInformationBuildItem
.getOpenIDConnectInformation();

if (maybeInfo.isPresent()) {
SecurityInformationBuildItem.OpenIDConnectInformation info = maybeInfo.get();

AutoUrl authorizationUrl = new AutoUrl(
config.oidcOpenIdConnectUrl.orElse(null),
info.getUrlConfigKey(),
"/protocol/openid-connect/auth");

AutoUrl refreshUrl = new AutoUrl(
config.oidcOpenIdConnectUrl.orElse(null),
info.getUrlConfigKey(),
"/protocol/openid-connect/token");

AutoUrl tokenUrl = new AutoUrl(
config.oidcOpenIdConnectUrl.orElse(null),
info.getUrlConfigKey(),
"/protocol/openid-connect/token/introspect");

return new OpenIDConnectSecurityFilter(
config.securitySchemeName,
config.securitySchemeDescription,
authorizationUrl, refreshUrl, tokenUrl);

}
break;
return securityInformationBuildItem.getOpenIDConnectInformation()
.map(info -> {
AutoUrl openIdConnectUrl = new AutoUrl(
config.oidcOpenIdConnectUrl.orElse(null),
info.getUrlConfigKey(),
"/.well-known/openid-configuration");

return new OpenIDConnectSecurityFilter(
config.securitySchemeName,
config.securitySchemeDescription,
openIdConnectUrl);
})
.orElse(null);
default:
break;
}
Expand Down Expand Up @@ -523,7 +489,7 @@ private Map<String, List<String>> getRolesAllowedMethodReferences(
for (MethodInfo method : methods) {
if (isValidOpenAPIMethodForAutoAdd(method, securityRequirement)) {
String ref = JandexUtil.createUniqueMethodReference(classInfo, method);
methodReferences.put(ref, List.of(ai.value().asStringArray()));
methodReferences.putIfAbsent(ref, List.of(ai.value().asStringArray()));
}
}
}
Expand Down Expand Up @@ -758,8 +724,7 @@ public void build(BuildProducer<FeatureBuildItem> feature,
nativeImageResources.produce(new NativeImageResourceBuildItem(name));
}

OpenApiDocument finalStoredOpenApiDocument = storeDocument(out, smallRyeOpenApiConfig, staticModel, annotationModel,
openAPIBuildItems);
OpenApiDocument finalStoredOpenApiDocument = storeDocument(out, smallRyeOpenApiConfig, finalDocument.get());
openApiDocumentProducer.produce(new OpenApiDocumentBuildItem(finalStoredOpenApiDocument));
}

Expand Down Expand Up @@ -1041,23 +1006,19 @@ private OpenApiDocument loadDocument(OpenAPI staticModel, OpenAPI annotationMode

private OpenApiDocument storeDocument(OutputTargetBuildItem out,
SmallRyeOpenApiConfig smallRyeOpenApiConfig,
OpenAPI staticModel,
OpenAPI annotationModel,
List<AddToOpenAPIDefinitionBuildItem> openAPIBuildItems) throws IOException {
return storeDocument(out, smallRyeOpenApiConfig, staticModel, annotationModel, openAPIBuildItems, true);
OpenAPI loadedModel) throws IOException {
return storeDocument(out, smallRyeOpenApiConfig, loadedModel, true);
}

private OpenApiDocument storeDocument(OutputTargetBuildItem out,
SmallRyeOpenApiConfig smallRyeOpenApiConfig,
OpenAPI staticModel,
OpenAPI annotationModel,
List<AddToOpenAPIDefinitionBuildItem> openAPIBuildItems,
OpenAPI loadedModel,
boolean includeRuntimeFilters) throws IOException {

Config config = ConfigProvider.getConfig();
OpenApiConfig openApiConfig = new OpenApiConfigImpl(config);

OpenApiDocument document = prepareOpenApiDocument(staticModel, annotationModel, openAPIBuildItems);
OpenApiDocument document = prepareOpenApiDocument(loadedModel, null, Collections.emptyList());

if (includeRuntimeFilters) {
document.filter(filter(openApiConfig)); // This usually happens at runtime, so when storing we want to filter here too.
Expand All @@ -1074,7 +1035,7 @@ private OpenApiDocument storeDocument(OutputTargetBuildItem out,
} catch (RuntimeException re) {
if (includeRuntimeFilters) {
// This is a Runtime filter, so it might not work at build time. In that case we ignore the filter.
return storeDocument(out, smallRyeOpenApiConfig, staticModel, annotationModel, openAPIBuildItems, false);
return storeDocument(out, smallRyeOpenApiConfig, loadedModel, false);
} else {
throw re;
}
Expand All @@ -1083,7 +1044,6 @@ private OpenApiDocument storeDocument(OutputTargetBuildItem out,
boolean shouldStore = smallRyeOpenApiConfig.storeSchemaDirectory.isPresent();
if (shouldStore) {
for (Format format : Format.values()) {
String name = OpenApiConstants.BASE_NAME + format;
byte[] schemaDocument = OpenApiSerializer.serialize(document.get(), format).getBytes(StandardCharsets.UTF_8);
storeGeneratedSchema(smallRyeOpenApiConfig, out, schemaDocument, format);
}
Expand Down
139 changes: 71 additions & 68 deletions ...t/src/main/java/io/quarkus/smallrye/openapi/deployment/filter/AutoRolesAllowedFilter.java
View file
Original file line number Diff line number Diff line change
@@ -1,22 +1,27 @@
package io.quarkus.smallrye.openapi.deployment.filter;

import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Stream;

import org.eclipse.microprofile.openapi.OASFactory;
import org.eclipse.microprofile.openapi.OASFilter;
import org.eclipse.microprofile.openapi.models.Components;
import org.eclipse.microprofile.openapi.models.OpenAPI;
import org.eclipse.microprofile.openapi.models.Operation;
import org.eclipse.microprofile.openapi.models.PathItem;
import org.eclipse.microprofile.openapi.models.Paths;
import org.eclipse.microprofile.openapi.models.responses.APIResponse;
import org.eclipse.microprofile.openapi.models.responses.APIResponses;
import org.eclipse.microprofile.openapi.models.security.SecurityRequirement;
import org.eclipse.microprofile.openapi.models.security.SecurityScheme;

import io.smallrye.openapi.api.models.OperationImpl;
import io.smallrye.openapi.api.models.responses.APIResponseImpl;
import io.smallrye.openapi.api.models.security.SecurityRequirementImpl;

/**
* Automatically add security requirement to RolesAllowed methods
Expand Down Expand Up @@ -49,6 +54,10 @@ public boolean hasRolesAllowedMethodReferences() {
return this.rolesAllowedMethodReferences != null && !this.rolesAllowedMethodReferences.isEmpty();
}

public boolean hasRolesAllowedMethodReference(String methodRef) {
return this.rolesAllowedMethodReferences != null && this.rolesAllowedMethodReferences.containsKey(methodRef);
}

public List<String> getAuthenticatedMethodReferences() {
return authenticatedMethodReferences;
}
Expand All @@ -61,6 +70,10 @@ public boolean hasAuthenticatedMethodReferences() {
return this.authenticatedMethodReferences != null && !this.authenticatedMethodReferences.isEmpty();
}

public boolean hasAuthenticatedMethodReference(String methodRef) {
return this.authenticatedMethodReferences != null && this.authenticatedMethodReferences.contains(methodRef);
}

public String getDefaultSecuritySchemeName() {
return defaultSecuritySchemeName;
}
Expand All @@ -71,81 +84,71 @@ public void setDefaultSecuritySchemeName(String defaultSecuritySchemeName) {

@Override
public void filterOpenAPI(OpenAPI openAPI) {
if (!hasRolesAllowedMethodReferences() && !hasAuthenticatedMethodReferences()) {
return;
}

if (hasRolesAllowedMethodReferences() || hasAuthenticatedMethodReferences()) {
String securitySchemeName = getSecuritySchemeName(openAPI);
Paths paths = openAPI.getPaths();
if (paths != null) {
Map<String, PathItem> pathItems = paths.getPathItems();
if (pathItems != null && !pathItems.isEmpty()) {
Set<Map.Entry<String, PathItem>> pathItemsEntries = pathItems.entrySet();
for (Map.Entry<String, PathItem> pathItem : pathItemsEntries) {
Map<PathItem.HttpMethod, Operation> operations = pathItem.getValue().getOperations();
if (operations != null && !operations.isEmpty()) {

for (Operation operation : operations.values()) {

OperationImpl operationImpl = (OperationImpl) operation;

if (hasRolesAllowedMethodReferences()
&& rolesAllowedMethodReferences.keySet().contains(operationImpl.getMethodRef())) {
SecurityRequirement securityRequirement = new SecurityRequirementImpl();
List<String> roles = rolesAllowedMethodReferences.get(operationImpl.getMethodRef());
securityRequirement = securityRequirement.addScheme(securitySchemeName, roles);
operation = operation.addSecurityRequirement(securityRequirement);
APIResponses responses = operation.getResponses();
for (APIResponseImpl response : getSecurityResponses()) {
if (!responses.hasAPIResponse(response.getResponseCode())) {
responses.addAPIResponse(response.getResponseCode(), response);
}
}
operation = operation.responses(responses);
} else if (hasAuthenticatedMethodReferences()
&& authenticatedMethodReferences.contains(operationImpl.getMethodRef())) {
SecurityRequirement securityRequirement = new SecurityRequirementImpl();
securityRequirement = securityRequirement.addScheme(securitySchemeName);
operation = operation.addSecurityRequirement(securityRequirement);
APIResponses responses = operation.getResponses();
for (APIResponseImpl response : getSecurityResponses()) {
if (!responses.hasAPIResponse(response.getResponseCode())) {
responses.addAPIResponse(response.getResponseCode(), response);
}
}
operation = operation.responses(responses);
}
}
}
var securityScheme = getSecurityScheme(openAPI);
String schemeName = securityScheme.map(Map.Entry::getKey).orElse(defaultSecuritySchemeName);
boolean scopesValidForScheme = securityScheme.map(Map.Entry::getValue)
.map(SecurityScheme::getType)
.map(Set.of(SecurityScheme.Type.OAUTH2, SecurityScheme.Type.OPENIDCONNECT)::contains)
.orElse(false);
Map<String, APIResponse> defaultSecurityErrors = getSecurityResponses();

Optional.ofNullable(openAPI.getPaths())
.map(Paths::getPathItems)
.map(Map::entrySet)
.map(Collection::stream)
.orElseGet(Stream::empty)
.map(Map.Entry::getValue)
.map(PathItem::getOperations)
.filter(Objects::nonNull)
.map(Map::values)
.flatMap(Collection::stream)
.forEach(operation -> {
String methodRef = OperationImpl.getMethodRef(operation);

if (hasRolesAllowedMethodReference(methodRef)) {
List<String> scopes = rolesAllowedMethodReferences.get(methodRef);
addSecurityRequirement(operation, schemeName, scopesValidForScheme ? scopes : Collections.emptyList());
addDefaultSecurityResponses(operation, defaultSecurityErrors);
} else if (hasAuthenticatedMethodReference(methodRef)) {
addSecurityRequirement(operation, schemeName, Collections.emptyList());
addDefaultSecurityResponses(operation, defaultSecurityErrors);
}
}
}
}
});
}

private String getSecuritySchemeName(OpenAPI openAPI) {

private Optional<Map.Entry<String, SecurityScheme>> getSecurityScheme(OpenAPI openAPI) {
// Might be set in annotations
if (openAPI.getComponents() != null && openAPI.getComponents().getSecuritySchemes() != null
&& !openAPI.getComponents().getSecuritySchemes().isEmpty()) {
Map<String, SecurityScheme> securitySchemes = openAPI.getComponents().getSecuritySchemes();
return securitySchemes.keySet().iterator().next();
}
return defaultSecuritySchemeName;
return Optional.ofNullable(openAPI.getComponents())
.map(Components::getSecuritySchemes)
.map(Map::entrySet)
.map(Collection::stream)
.orElseGet(Stream::empty)
.findFirst();
}

private List<APIResponseImpl> getSecurityResponses() {
List<APIResponseImpl> responses = new ArrayList<>();
private void addSecurityRequirement(Operation operation, String schemeName, List<String> scopes) {
SecurityRequirement securityRequirement = OASFactory.createSecurityRequirement();
securityRequirement = securityRequirement.addScheme(schemeName, scopes);
operation.addSecurityRequirement(securityRequirement);
}

APIResponseImpl notAuthorized = new APIResponseImpl();
notAuthorized.setDescription("Not Authorized");
notAuthorized.setResponseCode("401");
responses.add(notAuthorized);
private void addDefaultSecurityResponses(Operation operation, Map<String, APIResponse> defaultSecurityErrors) {
APIResponses responses = operation.getResponses();

APIResponseImpl forbidden = new APIResponseImpl();
forbidden.setDescription("Not Allowed");
forbidden.setResponseCode("403");
responses.add(forbidden);
defaultSecurityErrors.entrySet()
.stream()
.filter(e -> !responses.hasAPIResponse(e.getKey()))
.forEach(e -> responses.addAPIResponse(e.getKey(), e.getValue()));
}

return responses;
private Map<String, APIResponse> getSecurityResponses() {
return Map.of(
"401", OASFactory.createAPIResponse().description("Not Authorized"),
"403", OASFactory.createAPIResponse().description("Not Allowed"));
}

}
Loading

0 comments on commit 5ec7d2c

Please sign in to comment.