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

Provide more control for setting the secure flag on the session cookie [SPR-16980] #21518

Closed
spring-projects-issues opened this issue Jun 28, 2018 · 6 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Jun 28, 2018

Mahan Hashemizadeh opened SPR-16980 and commented

CookieWebSessionIdResolver sets the secure flag if it detects that the scheme is https.

Could this be adjusted so that we can set this value ourselves, in the same way that we can set the cookie name?
Or maybe even via a property like it is in the servlet world (server.servlet.session.cookie.secure)?

Currently my workaround is to extend CookieWebSessionIdResolver and override setSessionId:

public class MySecureCookieWebSessionIdResolver extends CookieWebSessionIdResolver {

    @Override
    public void setSessionId(final ServerWebExchange exchange, final String id) {
        String name = getCookieName();
        String path = exchange.getRequest().getPath().contextPath().value() + "/";
        exchange.getResponse().getCookies().set(name, ResponseCookie.from(name, id).path(path).maxAge(getCookieMaxAge()).httpOnly(true).secure(true).build());
    }
}

And then create a configuration bean:

@Bean
public WebSessionIdResolver webSessionIdResolver() {
    return new MySecureCookieWebSessionIdResolver();
}

Issue Links:

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

I think it would have to be something like Function<ServerWebExchange,Boolean> since the default value is computed rather than fixed.

@spring-projects-issues
Copy link
Collaborator Author

Mahan Hashemizadeh commented

Not sure if the comment is directed at me or someone else but I can share my own idea (based on the 5.0 codebase) and you can see what you think (code additions are marked with the comment // New code):

 

public class PotentialCookieWebSessionIdResolver implements WebSessionIdResolver {

    private String cookieName = "SESSION";

    private Duration cookieMaxAge = Duration.ofSeconds(-1);

    // New code
    private SecureStrategy secureStrategy = SecureStrategy.DEFAULT;

    // New code
    public SecureStrategy getSecureStrategy() {
        return secureStrategy;
    }

    // New code
    public void setSecureStrategy(SecureStrategy secureStrategy) {
        this.secureStrategy = secureStrategy;
    }

    /**
     * Set the name of the cookie to use for the session id.
     * <p>By default set to "SESSION".
     * @param cookieName the cookie name
     */
    public void setCookieName(String cookieName) {
        Assert.hasText(cookieName, "'cookieName' must not be empty");
        this.cookieName = cookieName;
    }

    /**
     * Return the configured cookie name.
     */
    public String getCookieName() {
        return this.cookieName;
    }

    /**
     * Set the value for the "Max-Age" attribute of the cookie that holds the
     * session id. For the range of values see {@link ResponseCookie#getMaxAge()}.
     * <p>By default set to -1.
     * @param maxAge the maxAge duration value
     */
    public void setCookieMaxAge(Duration maxAge) {
        this.cookieMaxAge = maxAge;
    }

    /**
     * Return the configured "Max-Age" attribute value for the session cookie.
     */
    public Duration getCookieMaxAge() {
        return this.cookieMaxAge;
    }


    @Override
    public List<String> resolveSessionIds(ServerWebExchange exchange) {
        MultiValueMap<String, HttpCookie> cookieMap = exchange.getRequest().getCookies();
        List<HttpCookie> cookies = cookieMap.get(getCookieName());
        if (cookies == null) {
            return Collections.emptyList();
        }
        return cookies.stream().map(HttpCookie::getValue).collect(Collectors.toList());
    }

    @Override
    public void setSessionId(ServerWebExchange exchange, String id) {
        Assert.notNull(id, "'id' is required");
        setSessionCookie(exchange, id, getCookieMaxAge());
    }

    @Override
    public void expireSession(ServerWebExchange exchange) {
        setSessionCookie(exchange, "", Duration.ofSeconds(0));
    }

    private void setSessionCookie(ServerWebExchange exchange, String id, Duration maxAge) {
        String name = getCookieName();

        // New code
        boolean secure = getSecureFlag(exchange);

        String path = exchange.getRequest().getPath().contextPath().value() + "/";
        exchange.getResponse().getCookies().set(name,
                                                ResponseCookie.from(name, id).path(path)
                                                              .maxAge(maxAge).httpOnly(true).secure(secure).build());
    }

    // New code
    private boolean getSecureFlag(final ServerWebExchange exchange) {
        if (secureStrategy == SecureStrategy.DEFAULT) {
            return "https".equalsIgnoreCase(exchange.getRequest().getURI().getScheme());
        }

        return (secureStrategy == SecureStrategy.SECURE) ? true : false;
    }

    // New code
    public enum SecureStrategy {
        DEFAULT, OFF, SECURE;
    }
}

 
And then one can register a configuration bean like so:

@Bean
public WebSessionIdResolver webSessionIdResolver() {
    PotentialCookieWebSessionIdResolver webSessionIdResolver = new PotentialCookieWebSessionIdResolver();
    webSessionIdResolver.setSecureStrategy(PotentialCookieWebSessionIdResolver.SecureStrategy.SECURE);

    return webSessionIdResolver;
}

 

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

This isn't anything that can't be done with Boolean which can differentiate between true, false, and not set (i.e. default). That said I'd rather go with Function here since that provides the full capability and more accurately reflects that it is an actual "strategy". Providing a fixed value is trivial:

CookieWebSessionIdResolver resolver = new CookieWebSessionIdResolver();
resolver.setSecureStrategy(exchange -> true);

@spring-projects-issues
Copy link
Collaborator Author

Mahan Hashemizadeh commented

That's fine with me.

I'm looking forward to having this functionality available in 5.1.

Thanks for the quick response.

 

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Jun 29, 2018

Rossen Stoyanchev commented

Given a recent addition as part of #20964, I've made it slightly more general, so you can set anything on the cookie.

@spring-projects-issues
Copy link
Collaborator Author

Mahan Hashemizadeh commented

That's a great bit of code. Flexible, fluid and intuitive.

Nice work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants