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.

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

SecurityConfig marked as Runtime, but used in Static Init #40318

Closed
radcortez opened this issue Apr 26, 2024 · 5 comments · Fixed by #40327
Closed

SecurityConfig marked as Runtime, but used in Static Init #40318

radcortez opened this issue Apr 26, 2024 · 5 comments · Fixed by #40327
Labels
Milestone

Comments

@radcortez
Copy link
Member

radcortez commented Apr 26, 2024

Describe the bug

The SecurityConfig mapping is marked as RUNTIME:

@ConfigMapping(prefix = "quarkus.security")
@ConfigRoot(phase = ConfigPhase.RUN_TIME)
public interface SecurityConfig {

But its configuration is used during STATIC_INIT:

public EagerSecurityFilter() {
var interceptorStorageHandle = Arc.container().instance(EagerSecurityInterceptorStorage.class);
this.interceptorStorage = interceptorStorageHandle.isAvailable() ? interceptorStorageHandle.get() : null;
Event<Object> event = Arc.container().beanManager().getEvent();
this.eventHelper = new SecurityEventHelper<>(event.select(AuthorizationSuccessEvent.class),
event.select(AuthorizationFailureEvent.class), AUTHORIZATION_SUCCESS, AUTHORIZATION_FAILURE,
Arc.container().beanManager(),
ConfigProvider.getConfig().getOptionalValue("quarkus.security.events.enabled", Boolean.class).orElse(false));
}

Due to a configuration bug, runtime defaults were leaking to static init. With that fixed, some tests started to fail because the default is false, and tests expected events to be available. Regardless, the EagerSecurityFilter configuration cannot be changed after the build because @Provider is initialized during static init.

This was discovered with #40225, and the mapping was changed to BUILD_AND_RUN_TIME_FIXED to fix the tests, but we must decide what the correct behavior should be.

@radcortez radcortez added the kind/bug Something isn't working label Apr 26, 2024
Copy link

quarkus-bot bot commented Apr 26, 2024

/cc @sberyozkin (security)

@radcortez
Copy link
Member Author

/cc @michalvavrik

@michalvavrik
Copy link
Member

Thank you for cc me.

I'd suggest we can initialize SecurityEventHelper lazily (on the first request) inside RESTEasy Classic. Implementation in Quarkus REST is better as it's initialized on application startup. If my memory serves well, in other cases it's only used at runtime, but we need to check when fixing this issue.

Regarding security impact of this bug - if quarkus.security.events.enabled runtime value wasn't reflected, it would mean that security events were not produced in case the property were only set at runtime. In my eyes, it's a bug, but not a vulnerability.

@sberyozkin
Copy link
Member

I recall that for the csrf work, I had to convert @Provider to ServerHandler, though that was for Quarkus REST.
Lazy initialization for the Resteasy Classic case sounds reasonable

@michalvavrik
Copy link
Member

JaxRsPermissionChecker is using it as well (it actually receives it as method parameter in one of cases from this filter). so I'll just put it there. Thanks for a feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants