Skip to content

Conversation

@appleboy
Copy link
Owner

  • Add automatic support for httpOnly refresh token cookies, alongside access token cookies
  • Introduce the option to configure the refresh token cookie name via RefreshTokenCookieName
  • Modify Login, Logout, and Refresh handlers to set and clear both access and refresh token cookies
  • Enhance refresh token extraction: tokens can be retrieved from cookie, POST form, query parameter, or JSON body
  • Update documentation (English, Simplified Chinese, Traditional Chinese) to describe new refresh token cookie behavior and usage patterns
  • Add a helper for setting the refresh token cookie
  • Adjust example OAuth SSO server to set the refresh token cookie

- Add automatic support for httpOnly refresh token cookies, alongside access token cookies
- Introduce the option to configure the refresh token cookie name via RefreshTokenCookieName
- Modify Login, Logout, and Refresh handlers to set and clear both access and refresh token cookies
- Enhance refresh token extraction: tokens can be retrieved from cookie, POST form, query parameter, or JSON body
- Update documentation (English, Simplified Chinese, Traditional Chinese) to describe new refresh token cookie behavior and usage patterns
- Add a helper for setting the refresh token cookie
- Adjust example OAuth SSO server to set the refresh token cookie

Signed-off-by: appleboy <appleboy.tw@gmail.com>
Copilot AI review requested due to automatic review settings December 28, 2025 07:26
Comment on lines +1121 to +1129
c.SetCookie(
mw.RefreshTokenCookieName,
refreshToken,
maxage,
"/",
mw.CookieDomain,
mw.SecureCookie,
true, // Always httpOnly for security
)

Check warning

Code scanning / CodeQL

Cookie 'Secure' attribute is not set to true Medium

Cookie does not set Secure attribute to true.

Copilot Autofix

AI 15 days ago

In general, to fix this issue you must ensure that cookies carrying sensitive data are always created with Secure: true, so they are only transmitted over HTTPS. For code using Gin’s Context.SetCookie, this means passing true as the secure argument (the sixth argument in the call), rather than a configurable or possibly false value, at least for session or token cookies.

The best fix here, without changing existing behavior elsewhere, is to hardcode the secure parameter to true specifically for the refresh token cookie in SetRefreshTokenCookie, just as HttpOnly is already hardcoded to true. This keeps the existing configurability for other cookies (like the access token cookie in SetCookie) while ensuring that the refresh token, which is more sensitive and longer-lived, is always sent only over HTTPS. Concretely, in auth_jwt.go around line 1121, change the c.SetCookie invocation in SetRefreshTokenCookie so that the sixth parameter is true instead of mw.SecureCookie. No new imports, methods, or other definitions are needed.

Suggested changeset 1
auth_jwt.go

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/auth_jwt.go b/auth_jwt.go
--- a/auth_jwt.go
+++ b/auth_jwt.go
@@ -1124,7 +1124,7 @@
 			maxage,
 			"/",
 			mw.CookieDomain,
-			mw.SecureCookie,
+			true, // Always secure for refresh token
 			true, // Always httpOnly for security
 		)
 	}
EOF
@@ -1124,7 +1124,7 @@
maxage,
"/",
mw.CookieDomain,
mw.SecureCookie,
true, // Always secure for refresh token
true, // Always httpOnly for security
)
}
Copilot is powered by AI and may make mistakes. Always verify output.
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request adds comprehensive support for httpOnly refresh token cookies to enhance security in browser-based authentication flows. The changes enable automatic management of refresh tokens via secure cookies, reducing the burden on client applications while following OAuth 2.0 security best practices.

Key Changes:

  • Added configurable RefreshTokenCookieName field (default: "refresh_token") for customizing the refresh token cookie name
  • Enhanced refresh token extraction to support multiple sources: cookies (prioritized), POST form data, query parameters, and JSON body
  • Updated Login, Logout, and Refresh handlers to automatically set and clear both access and refresh token cookies when SendCookie is enabled

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
auth_jwt.go Core implementation adding RefreshTokenCookieName field, SetRefreshTokenCookie helper method, updated token extraction logic, and modifications to Login/Logout/Refresh handlers for dual cookie support
_example/oauth_sso/server.go Updated OAuth success handler to set refresh token cookie alongside access token cookie
README.md Added documentation for refresh token cookie support, extraction priorities, and updated configuration table with new RefreshTokenCookieName option
README.zh-CN.md Simplified Chinese translation of refresh token cookie documentation and configuration updates
README.zh-TW.md Traditional Chinese translation of refresh token cookie documentation and configuration updates

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 602 to 630
func (mw *GinJWTMiddleware) extractRefreshToken(c *gin.Context) string {
token := c.PostForm("refresh_token")
if token == "" {
token = c.Query("refresh_token")
// Try to get refresh token from cookie first (most common for browser-based apps)
token, _ := c.Cookie(mw.RefreshTokenCookieName)
if token != "" {
return token
}
if token == "" {
var reqBody struct {
RefreshToken string `json:"refresh_token"`
}
if err := c.ShouldBindJSON(&reqBody); err == nil {
token = reqBody.RefreshToken
}

// Try POST form
token = c.PostForm("refresh_token")
if token != "" {
return token
}

// Try query parameter
token = c.Query("refresh_token")
if token != "" {
return token
}

// Try JSON body
var reqBody struct {
RefreshToken string `json:"refresh_token"`
}
if err := c.ShouldBindJSON(&reqBody); err == nil {
token = reqBody.RefreshToken
}

return token
}
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The extractRefreshToken function has a critical bug in its token extraction logic. When c.PostForm is called on line 610, it consumes the request body for form-encoded data. Subsequently, when c.ShouldBindJSON is called on line 625, it attempts to read the same body for JSON parsing, but the body has already been consumed and cannot be read again. This means the JSON body extraction path will never work if PostForm is called first.

To fix this, the function should check the Content-Type header and only attempt the appropriate parsing method based on the content type, or use a different approach that doesn't consume the body prematurely. Consider checking Content-Type: "application/x-www-form-urlencoded" for forms vs "application/json" for JSON, and only call the relevant binding method.

Copilot uses AI. Check for mistakes.
Comment on lines +1254 to +1255
# Method 4: Send refresh token as query parameter
http -v POST localhost:8000/refresh?refresh_token=your_refresh_token_here
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This example recommends sending the refresh_token as a URL query parameter, which causes long‑lived bearer tokens to appear in server/proxy access logs, browser history, and Referer headers. An attacker who gains access to these logs or other observation points can steal and reuse the refresh token to mint new access tokens. To reduce this leakage risk, avoid using query strings for refresh tokens and prefer httpOnly cookies or request bodies (form/JSON) instead.

Suggested change
# Method 4: Send refresh token as query parameter
http -v POST localhost:8000/refresh?refresh_token=your_refresh_token_here
# Method 4: Using curl with JSON body
curl -v -X POST localhost:8000/refresh -H "Content-Type: application/json" -d '{"refresh_token":"your_refresh_token_here"}'

Copilot uses AI. Check for mistakes.
Comment on lines +1255 to +1256
# 方法 4:作为查询参数发送刷新令牌
http -v POST localhost:8000/refresh?refresh_token=your_refresh_token_here
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

此示例建议通过 URL 查询参数传递 refresh_token,这会使长期有效的令牌出现在服务器/代理访问日志、浏览器历史记录和 Referer 头中。攻击者一旦获取这些日志或请求信息,就可以窃取并重放刷新令牌来换取新的访问令牌。为降低泄露风险,应避免在查询字符串中传递刷新令牌,优先使用 httpOnly Cookie 或请求体(表单/JSON)承载刷新令牌。

Copilot uses AI. Check for mistakes.
Comment on lines +1254 to +1256

# 方法 4:作為查詢參數發送刷新 Token
http -v POST localhost:8000/refresh?refresh_token=your_refresh_token_here
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

此範例建議透過 URL 查詢參數傳送 refresh_token,會讓長效權杖出現在伺服器/代理存取日誌、瀏覽器歷史與 Referer 標頭中。攻擊者只要取得這些日誌或相關請求資訊,就能竊取並重放刷新權杖以換取新的存取權杖。為降低外洩風險,請避免在查詢字串中傳遞刷新權杖,優先改用 httpOnly Cookie 或請求本體(表單/JSON)承載刷新權杖。

Suggested change
# 方法 4:作為查詢參數發送刷新 Token
http -v POST localhost:8000/refresh?refresh_token=your_refresh_token_here

Copilot uses AI. Check for mistakes.
- Refactor code formatting for better readability in OAuth SSO server error handling
- Add a comprehensive test suite for JWT refresh token cookie behavior, including setting, extraction, precedence, rotation, clearing, custom naming, and revocation scenarios

Signed-off-by: appleboy <appleboy.tw@gmail.com>
- Use http.NewRequestWithContext and a background context instead of http.NewRequest when creating requests in tests
- Replace usage of HeaderMap for accessing Set-Cookie headers with Result().Header.Values for improved compatibility with newer Go versions

Signed-off-by: appleboy <appleboy.tw@gmail.com>
@appleboy appleboy merged commit df528da into master Dec 28, 2025
7 checks passed
@appleboy appleboy deleted the cookie branch December 28, 2025 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants