ch4mpy / spring-addons

Ease spring OAuth2 resource-servers configuration and testing
Apache License 2.0
530 stars 87 forks source link

CSRF httpOnly configuration #112

Closed giovannicandido closed 1 year ago

giovannicandido commented 1 year ago

Enabling csrf for angular application with the config:

com.c4-soft.springaddons.security.client.csrf=cookie_accessible_from_js com.c4-soft.springaddons.security.csrf=cookie_accessible_from_js The cookie is sill only accessible by http:

XSRF-TOKEN=805f84f7-8c2a-4cb2-9c66-97072324090e; Path=/api; HttpOnly

Code sample The code in ServletConfigurationSupport::

case COOKIE_HTTP_ONLY:
                    configurer.csrfTokenRepository(new CookieCsrfTokenRepository())
                            .csrfTokenRequestHandler(delegate::handle);
                    http.addFilterAfter(new CsrfCookieFilter(), BasicAuthenticationFilter.class);
                    break;
                case COOKIE_ACCESSIBLE_FROM_JS:
                    // Adapted from
                    // https://docs.spring.io/spring-security/reference/5.8/migration/servlet/exploits.html#_i_am_using_angularjs_or_another_javascript_framework
                    configurer.csrfTokenRepository(new CookieCsrfTokenRepository())
                            .csrfTokenRequestHandler(delegate::handle);
                    http.addFilterAfter(new CsrfCookieFilter(), BasicAuthenticationFilter.class);
                    break;

Maybe should be like the test class AddonsWebmvcTestConfig

case COOKIE_HTTP_ONLY:
                http.csrf().csrfTokenRepository(new CookieCsrfTokenRepository());
                break;
            case COOKIE_ACCESSIBLE_FROM_JS:
                http.csrf().csrfTokenRepository(CookieCsrfTokenRepository.withHttpOnlyFalse())
                        .csrfTokenRequestHandler(new XorCsrfTokenRequestAttributeHandler()::handle);
                break;

Expected behavior

The token is not set to httpOnly

ch4mpy commented 1 year ago

Yes it should, and what you suggest is the way to go. It is a regression introduced with 6.1.6.

@giovannicandido, off course, I can fix this and publish a new version within an hour, but I think you should get the credits. Please let me know if you are OK to send a PR.

giovannicandido commented 1 year ago

@ch4mpy Yes I can send a PR.

ch4mpy commented 1 year ago

@giovannicandido fix released in 6.1.9. Thanks again for spotting and fixing that.