Merge pull request 'Remove dummy user_id from OAuth state sessions' (#55) from fix/oauth-state-session into main

Reviewed-on: https://git.ts.mattnite.net/mattnite/forgejo-tickets/pulls/55
This commit is contained in:
Matthew Knight 2026-02-18 00:35:25 +00:00
commit 24f2522830
1 changed files with 36 additions and 18 deletions

View File

@ -53,12 +53,16 @@ func (h *OAuthHandler) Login(c *gin.Context) {
} }
state := generateState() state := generateState()
session, _ := h.deps.SessionStore.Get(c.Request, "oauth_state") isSecure := strings.HasPrefix(h.deps.Config.BaseURL, "https")
session.Values["state"] = state http.SetCookie(c.Writer, &http.Cookie{
session.Values["user_id"] = "00000000-0000-0000-0000-000000000000" // placeholder for save Name: "oauth_state",
if err := session.Save(c.Request, c.Writer); err != nil { Value: state,
log.Error().Err(err).Msg("save oauth state error") Path: "/",
} MaxAge: 600, // 10 minutes
HttpOnly: true,
Secure: isSecure,
SameSite: http.SameSiteLaxMode,
})
url := provider.Config.AuthCodeURL(state, oauth2.AccessTypeOffline) url := provider.Config.AuthCodeURL(state, oauth2.AccessTypeOffline)
c.Redirect(http.StatusTemporaryRedirect, url) c.Redirect(http.StatusTemporaryRedirect, url)
@ -73,12 +77,17 @@ func (h *OAuthHandler) Callback(c *gin.Context) {
} }
// Verify state // Verify state
session, _ := h.deps.SessionStore.Get(c.Request, "oauth_state") stateCookie, err := c.Request.Cookie("oauth_state")
expectedState, _ := session.Values["state"].(string) if err != nil || c.Query("state") != stateCookie.Value {
if c.Query("state") != expectedState {
c.String(http.StatusBadRequest, "Invalid state parameter") c.String(http.StatusBadRequest, "Invalid state parameter")
return return
} }
// Clear the state cookie
http.SetCookie(c.Writer, &http.Cookie{
Name: "oauth_state",
Path: "/",
MaxAge: -1,
})
code := c.Query("code") code := c.Query("code")
token, err := provider.Config.Exchange(c.Request.Context(), code) token, err := provider.Config.Exchange(c.Request.Context(), code)
@ -134,12 +143,16 @@ func (h *OAuthHandler) appleLogin(c *gin.Context) {
} }
state := generateState() state := generateState()
session, _ := h.deps.SessionStore.Get(c.Request, "oauth_state") isSecure := strings.HasPrefix(h.deps.Config.BaseURL, "https")
session.Values["state"] = state http.SetCookie(c.Writer, &http.Cookie{
session.Values["user_id"] = "00000000-0000-0000-0000-000000000000" Name: "oauth_state",
if err := session.Save(c.Request, c.Writer); err != nil { Value: state,
log.Error().Err(err).Msg("save oauth state error") Path: "/",
} MaxAge: 600, // 10 minutes
HttpOnly: true,
Secure: isSecure,
SameSite: http.SameSiteNoneMode, // Apple uses form_post cross-origin
})
url := appleProvider.Config.AuthCodeURL(state, oauth2.AccessTypeOffline, auth.AppleAuthCodeOption()) url := appleProvider.Config.AuthCodeURL(state, oauth2.AccessTypeOffline, auth.AppleAuthCodeOption())
c.Redirect(http.StatusTemporaryRedirect, url) c.Redirect(http.StatusTemporaryRedirect, url)
@ -161,12 +174,17 @@ func (h *OAuthHandler) AppleCallback(c *gin.Context) {
code := c.PostForm("code") code := c.PostForm("code")
state := c.PostForm("state") state := c.PostForm("state")
session, _ := h.deps.SessionStore.Get(c.Request, "oauth_state") stateCookie, err := c.Request.Cookie("oauth_state")
expectedState, _ := session.Values["state"].(string) if err != nil || state != stateCookie.Value {
if state != expectedState {
c.String(http.StatusBadRequest, "Invalid state parameter") c.String(http.StatusBadRequest, "Invalid state parameter")
return return
} }
// Clear the state cookie
http.SetCookie(c.Writer, &http.Cookie{
Name: "oauth_state",
Path: "/",
MaxAge: -1,
})
token, err := appleProvider.ExchangeCode(c.Request.Context(), code) token, err := appleProvider.ExchangeCode(c.Request.Context(), code)
if err != nil { if err != nil {