spring-projects / spring-security

Spring Security
http://spring.io/projects/spring-security
Apache License 2.0
8.81k stars 5.9k forks source link

CookieServerCsrfTokenRepository does not add cookie #5766

Open sagacity opened 6 years ago

sagacity commented 6 years ago

Summary

I have modified the https://github.com/rwinch/spring-security-sample boot-webflux branch to add CSRF using the CookieServerCsrfTokenRepository.

Actual Behavior

If I do a GET to localhost:8080 I do not see a CSRF cookie being set.

Expected Behavior

A cookie is set so that on subsequent requests I can extract the CSRF token from there and pass it along using a CSRF header.

Configuration

I have modified the boot-webflux WebSecurityConfig like so:

@EnableWebFluxSecurity
public class WebSecurityConfig {
    @Bean
    public SecurityWebFilterChain securityWebFilterChain(ServerHttpSecurity http) {
        return http
                .authorizeExchange()
                .anyExchange().permitAll()
                .and()
                .csrf().csrfTokenRepository(new CookieServerCsrfTokenRepository())
                .and()
                .build();
    }

Version

I am using Spring Boot 2.1.0.RC2 which uses Spring Security 5.1.0.RC1.

Sample

See https://github.com/RoyJacobs/spring-security-cookie-repro

rwinch commented 6 years ago

Thanks for the feedback. Generally speaking the reactive repositories won't save the token unless something subscribed to the result. This is because if nothing subscribed, there is no way that the token could be known. Has anything subscribed to the CsrfToken? One option is to use something like this:

@ControllerAdvice
public class SecurityControllerAdvice {
    @ModelAttribute
    Mono<CsrfToken> csrfToken(ServerWebExchange exchange) {
        Mono<CsrfToken> csrfToken = exchange.getAttribute(CsrfToken.class.getName());
        return csrfToken.doOnSuccess(token -> exchange.getAttributes()
                .put(CsrfRequestDataValueProcessor.DEFAULT_CSRF_ATTR_NAME, token));
    }
}

This also exposes the token to be automatically available for anything using Spring Security's CsrfRequestDataValueProcessor which allows frameworks like Thymeleaf to automatically provide the CSRF token.

sagacity commented 6 years ago

@rwinch I can confirm that the Mono<CsrfToken> returned by generateToken method in the repository is never subscribed to (and therefore createCsrfToken is never called). I was under the assumption that configuring the tokenrepository would basically add the CSRF cookie to any request automatically.

I don't mind adding the controller advice but perhaps it would be good to clarify this in the documentation? In general there's no real documentation about how to obtain the CSRF token, which is why I assumed it would be added to any response.

Edit: On second thought, isn't the whole idea of the CookieServerCsrfTokenRepository that it would add the CSRF token to the response, without requiring a controller advice or additional setup? The Javadoc even says: "A ServerCsrfTokenRepository that persists the CSRF token in a cookie named "XSRF-TOKEN" and reads from the header "X-XSRF-TOKEN" following the conventions of AngularJS".

rwinch commented 6 years ago

Perhaps this does make sense to change the behavior of the cookie based implementation since a user can technically read the cookie directly. The session based implementation makes no sense to write it unless something subscribes because you cannot actually submit the value unless something is trying to use it.

I'm going to need to think about this change a bit.

sagacity commented 6 years ago

What could perhaps work if one could configure a CSRF URL which you can GET and which returns an empty response, apart from whatever is needed by CSRF. In this case this would be a cookie, in other TokenRepositories it could be a response header, etc

rwinch commented 6 years ago

I'm not sure I understand the suggestion. User's can already provide a URL that subscribes to the CSRF token which would write it out, so this would be a nothing to change.

Something else you could do is to create a ServerCsrfTokenRepository that delegates to another implementation and subscribes on creation of the token. This would ensure that the value is saved eagerly.

sagacity commented 6 years ago

Ah, I was unaware that this URL could already be configured. I didn't see anything in CsrfSpec that would allow me to set this.

Anyway, for now I will create the customized TokenRepository, that should definitely work. That's a satisfactory solution for now, but I'm still finding myself a bit confused that just enabling the CookieServerCsrfTokenRepository doesn't (without any additional configuration) simply add the token to a response cookie.

rwinch commented 6 years ago

I think of the custom repository as more of a work around.

In regards to the URL, I'm just speaking of something like the Controller Advice I provided. you could do something similar with a specific URL that resolves the token for you. This is not something Spring Security needs to do for you.

sagacity commented 6 years ago

Sorry, that's what I meant as well: The workaround is fine for now. If in the future the behaviour is changed to make this functionality a bit more intuitive (well...at least to the likes of me :D) then that would certainly be welcomed.

rwinch commented 6 years ago

Ok. Thanks for the response. I'm going to reopen the issue since we agree it would be nice for some sort of support for this. I still don't know exactly how we will go about it yet.

edeandrea commented 5 years ago

For a RESTful service I implemented this as a ResponseBodyResultHandler.

public class ServerCsrfTokenSubscribingResponseModifier extends ResponseBodyResultHandler {
    public ServerCsrfTokenSubscribingResponseModifier(List<HttpMessageWriter<?>> writers, RequestedContentTypeResolver resolver, ReactiveAdapterRegistry registry) {
        super(writers, resolver, registry);
        setOrder(99);
    }

    @Override
    public Mono<Void> handleResult(ServerWebExchange exchange, HandlerResult result) {
        return Optional.ofNullable(exchange.getAttribute(CsrfToken.class.getName()))
            .filter(Mono.class::isInstance)
            .map(Mono.class::cast)
            .orElseGet(Mono::empty)
            .then(super.handleResult(exchange, result));
    }
}

I agree though it would be much better for this to work out of the box.

plewand commented 5 years ago

A solution in Kotlin that let the backend of my Webflux application work with Angular frontend, maybe someone find it useful:

import com.crl.crlproxy.common.Constants
import mu.KotlinLogging
import org.springframework.http.ResponseCookie
import org.springframework.security.web.server.csrf.CsrfToken
import org.springframework.stereotype.Component
import org.springframework.web.server.ServerWebExchange
import org.springframework.web.server.WebFilter
import org.springframework.web.server.WebFilterChain
import reactor.core.publisher.Mono
import java.time.Duration

private val logger = KotlinLogging.logger {}

@Component
class CsrfHelperFilter : WebFilter {

    override fun filter(serverWebExchange: ServerWebExchange,
                        webFilterChain: WebFilterChain): Mono<Void> {
        val key = CsrfToken::class.java.name
        val csrfToken: Mono<CsrfToken> = serverWebExchange.getAttribute(key) ?: Mono.empty()
        return csrfToken.doOnSuccess { token ->
            val cookie = ResponseCookie.from(Constants.CSRF_COOKIE_NAME, token.token)
                    .maxAge(Duration.ofHours(1))
                    .httpOnly(false)
                    .path("/")
                    .build()
            logger.debug { "Cookie: $cookie" }
            serverWebExchange.response.cookies.add(Constants.CSRF_COOKIE_NAME, cookie)
        }.then(webFilterChain.filter(serverWebExchange))
    }
}

Additionally you need to register CookieServerCsrfTokenRepository with the corresponding cookie name. Important is CsrfToken class package.

cbornet commented 5 years ago

The non-reactive version of the csrf filter generates the token even if the request doesn't match requireCsrfProtectionMatcher. Shouldn't it be done the same way on the reactive part ?

ccpatriq commented 5 years ago

@rwinch I am a new spring user. How would I trigger a subscription when using functional end points. I believe @Controlleradvice will not work with Webflux. Or how can I retrieve the csrf token using router functions? Will the spring security team provide a new tutorial to spring security + angular 7+?

dasAnderl commented 5 years ago

this is really very unintuitive. i was also wondering why the cookie is not set. now i ended up here and i still dont see a clear intuitive solution in this conversation.

dillius commented 5 years ago

I am in the same boat. I have been trying for more than a day now to find a solution that works to includes CSRF information for ajax requests from a non-templated front end, and using functional routing.

There doesn't seem to be a good example or pattern, though I'm about to give @plewand 's CsrfHelperFilter a try.

dillius commented 5 years ago

@plewand 's solution did finally result in my having access to a CSRF token in a cookie for my frontend, though I'm still working out having Spring Security actually accept it on a subsequent POST request.

sagacity commented 5 years ago

The workaround works fine, even if you don't explicitly set the cookie yourself. Just subscribing is enough, i.e. (Kotlin, sorry):

csrfToken.doOnSuccess { }.then(webFilterChain.filter(serverWebExchange))
cbornet commented 5 years ago

Why not implementing the same behavior in Webflux as in Webmvc ?

dillius commented 5 years ago

I tend to agree that this should remain open as a bug.

This step seems like it should be unnecessary; correct configuration of the CookieServerCsrfTokenRepository should result in the cookie being utilized in the same manner as it would for WebMVC.

sdoxsee commented 4 years ago

Seems odd to require a workaround to add the CsrfToken for CookieServerCsrfTokenRepository. I'm a reactive rookie but this is my what I currently have working for my functional routes (since @ControllerAdvise won't work for unless you're using the annotated programming model)

  @Bean
  WebFilter addCsrfToken() {
    return (exchange, next) -> exchange
        .<Mono<CsrfToken>>getAttribute(CsrfToken.class.getName())
        .doOnSuccess(token -> {}) // do nothing, just subscribe :/
        .then(next.filter(exchange));
  }

@rwinch could we re-open this and think about a more intuitive experience? See upvotes on https://github.com/spring-projects/spring-security/issues/5766#issuecomment-482805601 above (currently 10!) Thanks!

Toerktumlare commented 4 years ago

I spent about 3 hours debugging the source code trying to understand why no cookie was set until i stumbled upon this issue. This seems still be the case, there should be some form of documentation about this in the official documentation.

This should be fixed in general, as it is counter intuitive.

devstartshop commented 4 years ago
import com.crl.crlproxy.common.Constants
import mu.KotlinLogging
import org.springframework.http.ResponseCookie
import org.springframework.security.web.server.csrf.CsrfToken
import org.springframework.stereotype.Component
import org.springframework.web.server.ServerWebExchange
import org.springframework.web.server.WebFilter
import org.springframework.web.server.WebFilterChain
import reactor.core.publisher.Mono
import java.time.Duration

private val logger = KotlinLogging.logger {}

@Component
class CsrfHelperFilter : WebFilter {

    override fun filter(serverWebExchange: ServerWebExchange,
                        webFilterChain: WebFilterChain): Mono<Void> {
        val key = CsrfToken::class.java.name
        val csrfToken: Mono<CsrfToken> = serverWebExchange.getAttribute(key) ?: Mono.empty()
        return csrfToken.doOnSuccess { token ->
            val cookie = ResponseCookie.from(Constants.CSRF_COOKIE_NAME, token.token)
                    .maxAge(Duration.ofHours(1))
                    .httpOnly(false)
                    .path("/")
                    .build()
            logger.debug { "Cookie: $cookie" }
            serverWebExchange.response.cookies.add(Constants.CSRF_COOKIE_NAME, cookie)
        }.then(webFilterChain.filter(serverWebExchange))
    }
}

Here is the Java Version just for reference:

import java.time.Duration;

import org.springframework.http.ResponseCookie;
import org.springframework.security.web.server.csrf.CsrfToken;
import org.springframework.stereotype.Component;
import org.springframework.web.server.ServerWebExchange;
import org.springframework.web.server.WebFilter;
import org.springframework.web.server.WebFilterChain;

import lombok.extern.slf4j.Slf4j;
import reactor.core.publisher.Mono;

@Component
@Slf4j
public class CsrfHelperFilter implements WebFilter {

    @Override
    public Mono<Void> filter(ServerWebExchange exchange, WebFilterChain chain) {
        String key = CsrfToken.class.getName();
        Mono<CsrfToken> csrfToken = null != exchange.getAttribute(key) ? exchange.getAttribute(key) : Mono.empty();
        return csrfToken.doOnSuccess(token -> {
            ResponseCookie cookie = ResponseCookie.from("XSRF-TOKEN", token.getToken()).maxAge(Duration.ofHours(1))
                    .httpOnly(false).path("/").build();
            log.debug("Cookie: {}", cookie);
            exchange.getResponse().getCookies().add("XSRF-TOKEN", cookie);
        }).then(chain.filter(exchange));
    }

}
tt4g commented 4 years ago

This is the big WebFlux and MVC difference. I wasted a lot of time debugging the reason why CSRF tokens were not found in the cookie.

spring-projects-issues commented 3 years ago

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

jzheaux commented 3 years ago

Pardon our dust here as we do some issue cleanup. Feedback was already provided earlier, and I don't think the ticket has been fully addressed, yet, so let's keep the issue open.

maradanasai commented 2 years ago

Any update on this?

rwinch commented 2 years ago

There are no updates on this.

To answer everyone's question, the reason that it is never persisted if it isn't subscribed to is that we attempt to avoid work that will not ever be valuable. If an application never subscribes to the CsrfToken, then there is no way for the application to submit a valid CsrfToken because the correct value could not have been read. If that is the case, there is no point in storing a CsrfToken and utilizing more resources than necessary (a secure random id is generated for the token value, for Session persistence the Session is bigger, for Cookie persistence the requests are larger, etc) because we know a valid CSRF token cannot be submitted to be compared against any stored value.

So my question is this: Is there a functional reason that you want the CsrfToken to be persisted before being subscribed to? My fear is that if we do this eagerly, there are going to be just as many people upset that we are eagerly doing work that does not yet provide value.

zg2pro commented 2 years ago

Hi, I've been trying devstartshop's workaround but it doesn't work properly. It creates an XSRF-TOKEN but the value never changes. The value of an XSRF-TOKEN should change for each request (or an option should at least allow that). Every query should provide a new value in response headers and the most recent will be used to call the next endpoint. If an xsrf token is session-scoped I really don't see the added value with a user-session token.

ch4mpy commented 1 year ago

I'm not sure I understand why the request shouldn't be larger when I configure a cookie repo of anything (or the session bigger when I choose a session repo): when I configure a repo, it is to store something and, to me, it seems accepted that this something is going to use some space where it is stored.

In other words, why configuring a cookie repo if nothing is stored in cookies? When can it be of any use? What is the value of requiring an explicit subscription to the CSRF cookie, when I don't have to subscribe to the session cookie for instance?

My usage of the the CSRF cookie repository is for JS applications (Angular, React, Vue, ...). How am I supposed to "not eagerly" provide the CSRF token to such applications?