Merge pull request #964 from Chengyumeng/fix/cookie

fix when destroy session can't remove cookie in subdomain

Former-commit-id: 5caf0fa5d92f53ba7b744dc1b3b7d47f688a96db

sessions/cookie.go

@@ -41,20 +41,22 @@ func AddCookie(ctx context.Context, cookie *http.Cookie, reclaim bool) {
 
 // RemoveCookie deletes a cookie by it's name/key
 // If "purge" is true then it removes the, temp, cookie from the request as well.
-func RemoveCookie(ctx context.Context, name string, purge bool) {
+func RemoveCookie(ctx context.Context, config Config) {
-	c, err := ctx.Request().Cookie(name)
+	cookie, err := ctx.Request().Cookie(config.Cookie)
 	if err != nil {
 		return
 	}
 
-	c.Expires = CookieExpireDelete
+	cookie.Expires = CookieExpireDelete
 	// MaxAge<0 means delete cookie now, equivalently 'Max-Age: 0'
-	c.MaxAge = -1
+	cookie.MaxAge = -1
-	c.Value = ""
+	cookie.Value = ""
-	c.Path = "/"
+	cookie.Path = "/"
-	AddCookie(ctx, c, purge)
+	cookie.Domain = formatCookieDomain(ctx, config.DisableSubdomainPersistence)
 
-	if purge {
+	AddCookie(ctx, cookie, config.AllowReclaim)
+
+	if config.AllowReclaim {
 		// delete request's cookie also, which is temporary available.
 		ctx.Request().Header.Set("Cookie", "")
 	}
@@ -89,3 +91,39 @@ func IsValidCookieDomain(domain string) bool {
 
 	return true
 }
+
+func formatCookieDomain(ctx context.Context, disableSubdomainPersistence bool) string {
+	if disableSubdomainPersistence {
+		return ""
+	}
+
+	requestDomain := ctx.Host()
+	if portIdx := strings.IndexByte(requestDomain, ':'); portIdx > 0 {
+		requestDomain = requestDomain[0:portIdx]
+	}
+
+	if !IsValidCookieDomain(requestDomain) {
+		return ""
+	}
+
+	// RFC2109, we allow level 1 subdomains, but no further
+	// if we have localhost.com , we want the localhost.cos.
+	// so if we have something like: mysubdomain.localhost.com we want the localhost here
+	// if we have mysubsubdomain.mysubdomain.localhost.com we want the .mysubdomain.localhost.com here
+	// slow things here, especially the 'replace' but this is a good and understable( I hope) way to get the be able to set cookies from subdomains & domain with 1-level limit
+	if dotIdx := strings.LastIndexByte(requestDomain, '.'); dotIdx > 0 {
+		// is mysubdomain.localhost.com || mysubsubdomain.mysubdomain.localhost.com
+		s := requestDomain[0:dotIdx] // set mysubdomain.localhost || mysubsubdomain.mysubdomain.localhost
+		if secondDotIdx := strings.LastIndexByte(s, '.'); secondDotIdx > 0 {
+			//is mysubdomain.localhost ||  mysubsubdomain.mysubdomain.localhost
+			s = s[secondDotIdx+1:] // set to localhost || mysubdomain.localhost
+		}
+		// replace the s with the requestDomain before the domain's siffux
+		subdomainSuff := strings.LastIndexByte(requestDomain, '.')
+		if subdomainSuff > len(s) { // if it is actual exists as subdomain suffix
+			requestDomain = strings.Replace(requestDomain, requestDomain[0:subdomainSuff], s, 1) // set to localhost.com || mysubdomain.localhost.com
+		}
+	}
+	// finally set the .localhost.com (for(1-level) || .mysubdomain.localhost.com (for 2-level subdomain allow)
+	return "." + requestDomain // . to allow persistence
+}

sessions/sessions.go

@@ -2,7 +2,6 @@ package sessions
 
 import (
 	"net/http"
-	"strings"
 	"time"
 
 	"github.com/kataras/iris/context"
@@ -44,37 +43,7 @@ func (s *Sessions) updateCookie(ctx context.Context, sid string, expires time.Du
 
 	cookie.Value = sid
 	cookie.Path = "/"
-	if !s.config.DisableSubdomainPersistence {
+	cookie.Domain = formatCookieDomain(ctx, s.config.DisableSubdomainPersistence)
-
-		requestDomain := ctx.Host()
-		if portIdx := strings.IndexByte(requestDomain, ':'); portIdx > 0 {
-			requestDomain = requestDomain[0:portIdx]
-		}
-		if IsValidCookieDomain(requestDomain) {
-
-			// RFC2109, we allow level 1 subdomains, but no further
-			// if we have localhost.com , we want the localhost.cos.
-			// so if we have something like: mysubdomain.localhost.com we want the localhost here
-			// if we have mysubsubdomain.mysubdomain.localhost.com we want the .mysubdomain.localhost.com here
-			// slow things here, especially the 'replace' but this is a good and understable( I hope) way to get the be able to set cookies from subdomains & domain with 1-level limit
-			if dotIdx := strings.LastIndexByte(requestDomain, '.'); dotIdx > 0 {
-				// is mysubdomain.localhost.com || mysubsubdomain.mysubdomain.localhost.com
-				s := requestDomain[0:dotIdx] // set mysubdomain.localhost || mysubsubdomain.mysubdomain.localhost
-				if secondDotIdx := strings.LastIndexByte(s, '.'); secondDotIdx > 0 {
-					//is mysubdomain.localhost ||  mysubsubdomain.mysubdomain.localhost
-					s = s[secondDotIdx+1:] // set to localhost || mysubdomain.localhost
-				}
-				// replace the s with the requestDomain before the domain's siffux
-				subdomainSuff := strings.LastIndexByte(requestDomain, '.')
-				if subdomainSuff > len(s) { // if it is actual exists as subdomain suffix
-					requestDomain = strings.Replace(requestDomain, requestDomain[0:subdomainSuff], s, 1) // set to localhost.com || mysubdomain.localhost.com
-				}
-			}
-			// finally set the .localhost.com (for(1-level) || .mysubdomain.localhost.com (for 2-level subdomain allow)
-			cookie.Domain = "." + requestDomain // . to allow persistence
-		}
-	}
-
 	cookie.HttpOnly = true
 	// MaxAge=0 means no 'Max-Age' attribute specified.
 	// MaxAge<0 means delete cookie now, equivalently 'Max-Age: 0'
@@ -147,7 +116,7 @@ func (s *Sessions) Destroy(ctx context.Context) {
 	if cookieValue == "" { // nothing to destroy
 		return
 	}
-	RemoveCookie(ctx, s.config.Cookie, s.config.AllowReclaim)
+	RemoveCookie(ctx, s.config)
 
 	s.provider.Destroy(cookieValue)
 }