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

bug: Reactive Security context is not available in subscriptions #1442

Open
sipe90 opened this issue Feb 27, 2023 · 13 comments
Open

bug: Reactive Security context is not available in subscriptions #1442

sipe90 opened this issue Feb 27, 2023 · 13 comments
Labels
bug Something isn't working

Comments

@sipe90
Copy link

sipe90 commented Feb 27, 2023

I have a Spring Boot WebFlux application with Spring Security OAuth2 resource server authentication.
When I try to access the Security Context within a subscription, null is returned instead of the context.

Spring Boot version is 3.0.2 and DGS Framework version is 6.0.0

Expected behavior

Security context (fetched with ReactiveSecurityContextHolder.getContext()) should not be null when invoked within a subscription method during an authenticated request.

Actual behavior

ReactiveSecurityContextHolder.getContext() returns null when invoked inside a subscription method.

Steps to reproduce

The security configuration looks like this:

@Configuration
@EnableWebFluxSecurity
class WebFluxSecurityConfiguration {

    @Bean
    fun springSecurityFilterChain(http: ServerHttpSecurity): SecurityWebFilterChain {
        val roles = arrayOf("role1", "role2")

        return http {
            authorizeExchange {
                authorize("/actuator/health", permitAll)
                authorize(anyExchange, hasAnyRole(*roles))
            }
            oauth2ResourceServer {
                val tokenConverter = ServerBearerTokenAuthenticationConverter()
                tokenConverter.setAllowUriQueryParameter(true) // This allows passing authentication token in a query parameter in websocket requests. eg. ws://localhost:8080/subscriptions?access_token=<token>
                bearerTokenConverter = tokenConverter
                jwt {
                    jwtAuthenticationConverter = CustomAuthenticationConverter()
                }
            }
        }
    }
}

And the subscription like this:

@DgsComponent
internal class EventsDataFetcher(private val eventService: EventService) {

    private val logger = KotlinLogging.logger {}

    @DgsSubscription(field = DgsConstants.SUBSCRIPTION.Events)
    fun events(): Flux<Event> =
        ReactiveSecurityContextHolder.getContext()
            .doOnSuccess { logger.info { it } }
            .thenMany(eventService.streamEvents())

When subscribing to the events, null is logged.

With queries, the request context can be accessed:

@DgsComponent
class UserDataFetcher(private val userService: UserService) {

    @DgsQuery
    fun currentUser(): Mono<User> {
        return ReactiveSecurityContextHolder.getContext()
            .map { it.authentication as CustomAuthentication }
            .map { auth ->
                User(
                    id = auth.principal.id,
                    firstName = auth.principal.firstName,
                    lastName = auth.principal.lastName,
                    roles = auth.authorities
                        .filterIsInstance<RoleAuthority>()
                        .map(RoleAuthority::role)
                )
            }
    }
}

I also tried creating a WebSocketHandler where I also successfully managed to access the security context.

@sipe90 sipe90 added the bug Something isn't working label Feb 27, 2023
@srinivasankavitha
Copy link
Contributor

srinivasankavitha commented Feb 27, 2023 via email

@sipe90
Copy link
Author

sipe90 commented Feb 27, 2023

That PR doesn't seem to affect the reactive websocket implementation, which is DgsReactiveWebsocketHandler from graphql-dgs-spring-webflux-autoconfigure.

@sipe90
Copy link
Author

sipe90 commented Feb 28, 2023

I've managed to come up with a workaround using a custom context:

@Component
class DgsSecurityContextBuilder : DgsReactiveCustomContextBuilderWithRequest<CustomAuthentication> {
    override fun build(
        extensions: Map<String, Any>?,
        headers: HttpHeaders?,
        serverRequest: ServerRequest?
    ): Mono<CustomAuthentication> {
        return ReactiveSecurityContextHolder.getContext().map { it.authentication as CustomAuthentication }
    }
}
@DgsSubscription(field = DgsConstants.SUBSCRIPTION.Events)
fun events(dfe: DataFetchingEnvironment): Flux<Event> {
	val auth = DgsContext.getCustomContext<CustomAuthentication>(dfe)

	if (!auth.authorities.contains(ActionAuthority(Action.STREAM_EVENTS))) {
		return Flux.error(AccessDeniedException("Forbidden"))
	}

	return eventService.streamEvents()
}

I would still prefer to be able to access the authentication from Spring's context since I use Reactive Method Security with custom AuthorizationInterceptors and annotations.

This workaround should suffice for now, but I hope this issue gets fixed soon.

@Mechwv
Copy link

Mechwv commented Feb 28, 2023

@sipe90 Thx for your solution, this workaround works fine for subscriptions (just tested it out myself)

@srinivasankavitha
Copy link
Contributor

srinivasankavitha commented Feb 28, 2023 via email

@StringKe
Copy link

StringKe commented May 5, 2023

@sipe90 Implementing DgsReactiveCustomContextBuilderWithRequest in webflux websocket without any data.

Have you encountered this problem?

@lthoulon-locala
Copy link

This looks like it's still an issue, right ? It seems linked to my question here: #1644

@lthoulon-locala
Copy link

@sipe90 Implementing DgsReactiveCustomContextBuilderWithRequest in webflux websocket without any data.

Have you encountered this problem?

I got the same issue here. Workaround doesn't work for me. It doesn't seem logical that it would fix the problem either as it is trying to get the authentication from the context that is not filled. 🤷

@lthoulon-locala
Copy link

Actually the workaround works when the authorization header is provided on the handshake request.

@srinivasankavitha
Copy link
Contributor

@lthoulon-locala - does setting the auth header in the handshake request unblock you?

@lthoulon-locala
Copy link

lthoulon-locala commented Sep 25, 2023

@srinivasankavitha
Here is was I was able to do with the auth header in the handshake request:

@Component
class DgsSecurityContextBuilder : DgsReactiveCustomContextBuilderWithRequest<Authentication> {
    override fun build(
        extensions: Map<String, Any>?,
        headers: HttpHeaders?,
        serverRequest: ServerRequest?
    ): Mono<Authentication> = ReactiveSecurityContextHolder.getContext().map { it.authentication }
}

@DgsComponent
class AggregateSubscriptions(val handler: AggregateSubscriptionsHandler) {
    @DgsSubscription
    fun aggregateNotifications(
        @InputArgument id: UUID,
        @InputArgument notificationTypes: List<NotificationType>?,
        dfe: DataFetchingEnvironment
    ) = let {
        val auth = DgsContext.getCustomContext<Authentication>(dfe)
        handler.aggregateNotifications(id, notificationTypes, auth)
    }
}

@Component
class AggregateSubscriptionsHandler {
    // I put the 2 annotations that I tested but of course one should only use one at the time.
    @PreAuthorize("hasAuthority('$AGGREGATE_READ')") // Rincewind: THAT DOESN'T WORK
    @PreAuthorize("@myPermissionEvaluator.hasAuthority(#auth, '$AGGREGATE_READ')")
    fun aggregateNotifications(id: UUID, notificationTypes: List<NotificationType>?, auth: Authentication) =
        Flux.interval(Duration.ofMillis(1000))
            .map {
                Notification.Builder()
                    .withAggregateId(id.toString())
                    .withNotificationType(NotificationType.AdvertiserAdded)
                    .withPayload(listOf(KeyValue.Builder().withKey("t").withValue(it.toString()).build()))
                    .build()
            }
}

So yes, this unblocks me although it's not ideal because i'll have different code in my queries and subscriptions. Also it means I can't use GraphiQL to test my subscriptions as it sends the authz parameters as part of the connection_init message. Hopefully Postman's GraphQL client uses headers still.

Any chance we could get a fix soon on this matter ?
Thanks

@lthoulon-locala
Copy link

Referencing this feature request: #450

@srinivasankavitha
Copy link
Contributor

Thanks for the context @lthoulon-locala. As mentioned in the linked issue, we won't be able to prioritize this anytime soon, since Webflux stack is not internally used as much. We do accept contributions, in case you are interested or need this sooner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants