Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OpenAPI: Always run OpenIDConnectSecurityFilter at runtime #38231

Merged
merged 1 commit into from
Feb 5, 2024
Merged

OpenAPI: Always run OpenIDConnectSecurityFilter at runtime #38231

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
310 changes: 151 additions & 159 deletions ...oyment/src/main/java/io/quarkus/smallrye/openapi/deployment/SmallRyeOpenApiProcessor.java
View file
Open in desktop

Large diffs are not rendered by default.

31 changes: 31 additions & 0 deletions ...ent/src/test/java/io/quarkus/smallrye/openapi/test/jaxrs/OIDCSecurityAutoAddTestTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package io.quarkus.smallrye.openapi.test.jaxrs;

import java.util.List;

import org.jboss.shrinkwrap.api.asset.StringAsset;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.builder.Version;
import io.quarkus.maven.dependency.Dependency;
import io.quarkus.test.QuarkusUnitTest;

class OIDCSecurityAutoAddTestTest extends OIDCSecurityTestBase {

@RegisterExtension
static QuarkusUnitTest runner = new QuarkusUnitTest()
.withApplicationRoot((jar) -> jar
.addClasses(OpenApiResource.class, ResourceBean.class)
.addAsResource(
new StringAsset(""
+ "quarkus.smallrye-openapi.security-scheme-name=OIDCCompanyAuthentication\n"
+ "quarkus.smallrye-openapi.security-scheme-description=OIDC Authentication\n"
+ "quarkus.smallrye-openapi.oidc-open-id-connect-url=BUILD-TIME-OVERRIDDEN\n"
+ "quarkus.http.auth.permission.\"oidc\".policy=authenticated\n"
+ "quarkus.http.auth.permission.\"oidc\".paths=/resource/*\n"
+ "quarkus.oidc.auth-server-url=BUILD-TIME-OVERRIDDEN\n"
+ "quarkus.devservices.enabled=false"),
"application.properties"))
.setForcedDependencies(List.of(
Dependency.of("io.quarkus", "quarkus-oidc", Version.getVersion())))
.overrideRuntimeConfigKey("quarkus.oidc.auth-server-url", "http://localhost:8081/auth/realms/OpenAPIOIDC");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is great PR, but isn't it bit strange that you can override quarkus.smallrye-openapi.oidc-open-id-connect-url=BUILD-TIME-OVERRIDDEN with quarkus.oidc.auth-server-url? If users want to override it, why are setting it at the first place? If they set the quarkus.smallrye-openapi.oidc-open-id-connect-url maybe they want to generate OpenAPI document for different environment from local computer?

I can find arguments against my suggestion, but at least you should document this behavior.
Users can always go wild and do something like quarkus.smallrye-openapi.oidc-open-id-connect-url=${dev-env-url:${quarkus.oidc.auth-server-url}}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but isn't it bit strange that you can override quarkus.smallrye-openapi.oidc-open-id-connect-url=BUILD-TIME-OVERRIDDEN with quarkus.oidc.auth-server-url

@michalvavrik

Right, this is an existing relationship/functionality. This change enables the override to work even when the application does not use @RolesAllowed or @Authenticated annotations.

Almost all of the quarkus.smallrye-openapi. configurations are build-time. Eventually, those that modify the final OpenAPI document like quarkus.smallrye-openapi.oidc-open-id-connect-url should probably be applied at runtime, but that is bigger change. My hope here is to remove a small roadblock that requires users to provide an OASFilter to set this URL at runtime when they are not using the annotations.

If users want to override it, why are setting it at the first place? If they set the quarkus.smallrye-openapi.oidc-open-id-connect-url maybe they want to generate OpenAPI document for different environment from local computer?

I agree that is a likely usage. I think ultimately once quarkus.smallrye-openapi.oidc-open-id-connect-url is a runtime config, it would take precedence over quarkus.oidc.auth-server-url.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copy that, thanks

}
24 changes: 24 additions & 0 deletions ...deployment/src/test/java/io/quarkus/smallrye/openapi/test/jaxrs/OIDCSecurityTestBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package io.quarkus.smallrye.openapi.test.jaxrs;

import static org.hamcrest.Matchers.allOf;
import static org.hamcrest.Matchers.hasEntry;

import org.junit.jupiter.api.Test;

import io.restassured.RestAssured;

abstract class OIDCSecurityTestBase {

@Test
void testOIDCAuthentication() {
RestAssured.given().header("Accept", "application/json")
.when().get("/q/openapi")
.then().body("components.securitySchemes.OIDCCompanyAuthentication",
allOf(
hasEntry("type", "openIdConnect"),
hasEntry("description", "OIDC Authentication"),
hasEntry("openIdConnectUrl",
"http://localhost:8081/auth/realms/OpenAPIOIDC/.well-known/openid-configuration")));
}

}
19 changes: 2 additions & 17 deletions .../src/test/java/io/quarkus/smallrye/openapi/test/jaxrs/OIDCSecurityWithConfigTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
package io.quarkus.smallrye.openapi.test.jaxrs;

import org.hamcrest.Matchers;
import org.jboss.shrinkwrap.api.asset.StringAsset;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.test.QuarkusUnitTest;
import io.restassured.RestAssured;

public class OIDCSecurityWithConfigTestCase {
class OIDCSecurityWithConfigTestCase extends OIDCSecurityTestBase {

@RegisterExtension
static QuarkusUnitTest runner = new QuarkusUnitTest()
.withApplicationRoot((jar) -> jar
Expand All @@ -18,18 +16,5 @@ public class OIDCSecurityWithConfigTestCase {
+ "quarkus.smallrye-openapi.security-scheme-name=OIDCCompanyAuthentication\n"
+ "quarkus.smallrye-openapi.security-scheme-description=OIDC Authentication\n"
+ "quarkus.smallrye-openapi.oidc-open-id-connect-url=http://localhost:8081/auth/realms/OpenAPIOIDC/.well-known/openid-configuration"),

"application.properties"));

@Test
public void testOIDCAuthentication() {
RestAssured.given().header("Accept", "application/json")
.when().get("/q/openapi")
.then().body("components.securitySchemes.OIDCCompanyAuthentication", Matchers.hasEntry("type", "openIdConnect"))
.and()
.body("components.securitySchemes.OIDCCompanyAuthentication",
Matchers.hasEntry("description", "OIDC Authentication"))
.and().body("components.securitySchemes.OIDCCompanyAuthentication", Matchers.hasEntry("openIdConnectUrl",
"http://localhost:8081/auth/realms/OpenAPIOIDC/.well-known/openid-configuration"));
}
}
4 changes: 2 additions & 2 deletions ...ons/smallrye-openapi/runtime/src/main/java/io/quarkus/smallrye/openapi/OpenApiFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@
*/
int priority() default 1;

static enum RunStage {
enum RunStage {
BUILD,
RUN,
BOTH
}
}
}
5 changes: 1 addition & 4 deletions ...ime/src/main/java/io/quarkus/smallrye/openapi/runtime/filter/AutoBasicSecurityFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import java.util.Map;

import org.eclipse.microprofile.openapi.OASFactory;
import org.eclipse.microprofile.openapi.models.security.SecurityScheme;

/**
Expand Down Expand Up @@ -31,10 +30,8 @@ public void setBasicSecuritySchemeValue(String basicSecuritySchemeValue) {
}

@Override
protected SecurityScheme getSecurityScheme() {
SecurityScheme securityScheme = OASFactory.createSecurityScheme();
protected void updateSecurityScheme(SecurityScheme securityScheme) {
securityScheme.setType(SecurityScheme.Type.HTTP);
securityScheme.setScheme(basicSecuritySchemeValue);
return securityScheme;
}
}
5 changes: 1 addition & 4 deletions ...c/main/java/io/quarkus/smallrye/openapi/runtime/filter/AutoBearerTokenSecurityFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import java.util.Map;

import org.eclipse.microprofile.openapi.OASFactory;
import org.eclipse.microprofile.openapi.models.security.SecurityScheme;

/**
Expand Down Expand Up @@ -42,11 +41,9 @@ public void setBearerFormat(String bearerFormat) {
}

@Override
protected SecurityScheme getSecurityScheme() {
SecurityScheme securityScheme = OASFactory.createSecurityScheme();
protected void updateSecurityScheme(SecurityScheme securityScheme) {
securityScheme.setType(SecurityScheme.Type.HTTP);
securityScheme.setScheme(securitySchemeValue);
securityScheme.setBearerFormat(bearerFormat);
return securityScheme;
}
}
29 changes: 20 additions & 9 deletions .../runtime/src/main/java/io/quarkus/smallrye/openapi/runtime/filter/AutoSecurityFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
package io.quarkus.smallrye.openapi.runtime.filter;

import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Optional;

import org.eclipse.microprofile.config.Config;
import org.eclipse.microprofile.config.ConfigProvider;
Expand Down Expand Up @@ -56,30 +57,40 @@ public void setSecuritySchemeExtensions(Map<String, String> securitySchemeExtens
this.securitySchemeExtensions = securitySchemeExtensions;
}

public boolean runtimeRequired() {
return false;
}

@Override
public void filterOpenAPI(OpenAPI openAPI) {
// Make sure components are created
if (openAPI.getComponents() == null) {
openAPI.setComponents(OASFactory.createComponents());
}

Map<String, SecurityScheme> securitySchemes = new HashMap<>();
Map<String, SecurityScheme> securitySchemes = new LinkedHashMap<>();

// Add any existing security
if (openAPI.getComponents().getSecuritySchemes() != null
&& !openAPI.getComponents().getSecuritySchemes().isEmpty()) {
securitySchemes.putAll(openAPI.getComponents().getSecuritySchemes());
Optional.ofNullable(openAPI.getComponents().getSecuritySchemes())
.ifPresent(securitySchemes::putAll);

SecurityScheme securityScheme = securitySchemes.computeIfAbsent(
securitySchemeName,
name -> OASFactory.createSecurityScheme());

updateSecurityScheme(securityScheme);

if (securitySchemeDescription != null) {
securityScheme.setDescription(securitySchemeDescription);
}

SecurityScheme securityScheme = getSecurityScheme();
securityScheme.setDescription(securitySchemeDescription);
securitySchemeExtensions.forEach(securityScheme::addExtension);

securitySchemes.put(securitySchemeName, securityScheme);
openAPI.getComponents().setSecuritySchemes(securitySchemes);
}

protected abstract SecurityScheme getSecurityScheme();
protected abstract void updateSecurityScheme(SecurityScheme securityScheme);

protected String getUrl(String configKey, String defaultValue, String shouldEndWith) {
Config c = ConfigProvider.getConfig();
Expand All @@ -92,4 +103,4 @@ protected String getUrl(String configKey, String defaultValue, String shouldEndW
return u;
}

}
}
10 changes: 6 additions & 4 deletions ...src/main/java/io/quarkus/smallrye/openapi/runtime/filter/OpenIDConnectSecurityFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import java.util.Map;

import org.eclipse.microprofile.openapi.OASFactory;
import org.eclipse.microprofile.openapi.models.security.SecurityScheme;

/**
Expand Down Expand Up @@ -32,11 +31,14 @@ public void setOpenIdConnectUrl(AutoUrl openIdConnectUrl) {
}

@Override
protected SecurityScheme getSecurityScheme() {
SecurityScheme securityScheme = OASFactory.createSecurityScheme();
public boolean runtimeRequired() {
return true;
}

@Override
protected void updateSecurityScheme(SecurityScheme securityScheme) {
securityScheme.setType(SecurityScheme.Type.OPENIDCONNECT);
securityScheme.setOpenIdConnectUrl(openIdConnectUrl.getFinalUrlValue());
return securityScheme;
}

}
Loading