mirror of
https://github.com/go-gitea/gitea.git
synced 2026-05-23 05:42:33 +09:00
fix(auth): set User-Agent on avatar fetch and sync avatar on link-account register (#37564) (#37588)
## Summary Fixes [go-gitea/gitea#37564](https://github.com/go-gitea/gitea/issues/37564): when an OIDC provider returns a `picture` claim, Gitea is supposed to download that image as the user's avatar (if `[oauth2_client] UPDATE_AVATAR = true`). Two latent bugs prevented this from working consistently: 1. **Default Go User-Agent rejected by some image hosts.** `oauth2UpdateAvatarIfNeed` used `http.Get`, which sends `User-Agent: Go-http-client/1.1`. Hosts like `upload.wikimedia.org` reject that UA with `403`, and every error path silently returned, so the user was left with an identicon and **no log line** to diagnose the issue. 2. **Link-account *register* path skipped avatar sync.** First-time OIDC sign-ins where auto-registration is disabled (or required a username/password retype) go through `LinkAccountPostRegister`, which created the user but never called `oauth2SignInSync`. So the avatar / full name / SSH keys from the IdP were dropped on the floor for those users, even though the existing-account-link path (`oauth2LinkAccount`) and the auto-register path (`handleOAuth2SignIn`) both already did the sync. ## Changes - `routers/web/auth/oauth.go` — `oauth2UpdateAvatarIfNeed` now uses `http.NewRequest` + `http.DefaultClient.Do`, sets `User-Agent: Gitea <version>`, and logs every failure path at `Warn` (invalid URL, fetch error, non-200, body read error, oversize body, upload error). No silent failures. - `routers/web/auth/linkaccount.go` — `LinkAccountPostRegister` now calls `oauth2SignInSync` after a successful user creation, mirroring the auto-register and link-existing-account flows. - `tests/integration/oauth_avatar_test.go` — new `TestOAuth2AvatarFromPicture` integration test with five sub-cases: - `AutoRegister_FetchesAvatarFromPictureWithGiteaUA` — happy path, asserts `use_custom_avatar=true`, an avatar hash is set, exactly one HTTP request was made, and the request carried a `Gitea ` UA. The mock server enforces the UA prefix to mirror real-world hosts that reject Go's default UA. - `AutoRegister_NonOK_DoesNotUpdateAvatar` — server returns 403; user's avatar must remain unset. - `AutoRegister_EmptyPicture_NoFetch` — empty `picture` claim must not trigger any HTTP request. - `AutoRegister_UpdateAvatarFalse_NoFetch` — `UPDATE_AVATAR=false` must not trigger any HTTP request. - `LinkAccountRegister_FetchesAvatarFromPicture` — guards the `linkaccount.go` fix; without the new `oauth2SignInSync` call this assertion fails. ## Related - Upstream issue: go-gitea/gitea#37564 -------------------------------------------- AI Editor was used in this PR --------- Signed-off-by: silverwind <me@silverwind.io> Co-authored-by: silverwind <me@silverwind.io> Co-authored-by: Claude (Opus 4.7) <noreply@anthropic.com> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com> Co-authored-by: Nicolas <bircni@icloud.com>
This commit is contained in:
@@ -5,6 +5,7 @@ package log
|
||||
|
||||
import (
|
||||
"context"
|
||||
"net/url"
|
||||
"reflect"
|
||||
"runtime"
|
||||
"strings"
|
||||
@@ -226,6 +227,8 @@ func (l *LoggerImpl) Log(skip int, event *Event, format string, logArgs ...any)
|
||||
}
|
||||
} else if ls := asLogStringer(v); ls != nil {
|
||||
msgArgs[i] = logStringFormatter{v: ls}
|
||||
} else if str, ok := v.(string); ok {
|
||||
msgArgs[i] = protectSensitiveInfo(str)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -235,6 +238,24 @@ func (l *LoggerImpl) Log(skip int, event *Event, format string, logArgs ...any)
|
||||
l.SendLogEvent(event)
|
||||
}
|
||||
|
||||
func protectSensitiveInfo(s string) string {
|
||||
u, err := url.Parse(s)
|
||||
if err != nil || (u.Scheme != "http" && u.Scheme != "https") || u.Host == "" {
|
||||
return s
|
||||
}
|
||||
q := u.Query()
|
||||
for _, vals := range q {
|
||||
for i := range vals {
|
||||
vals[i] = "_"
|
||||
}
|
||||
}
|
||||
masked := &url.URL{Scheme: u.Scheme, Host: u.Host, Path: u.Path, RawQuery: q.Encode()}
|
||||
if u.User != nil {
|
||||
masked.User = url.User("_masked_")
|
||||
}
|
||||
return masked.String()
|
||||
}
|
||||
|
||||
func (l *LoggerImpl) GetLevel() Level {
|
||||
return Level(l.level.Load())
|
||||
}
|
||||
|
||||
@@ -177,3 +177,10 @@ func TestLoggerExpressionFilter(t *testing.T) {
|
||||
|
||||
assert.Equal(t, []string{"foo\n", "foo bar\n", "by filename\n"}, w1.FetchLogs())
|
||||
}
|
||||
|
||||
func TestProtectSensitiveInfo(t *testing.T) {
|
||||
assert.Empty(t, protectSensitiveInfo(""))
|
||||
assert.Equal(t, "mailto:user@example.com", protectSensitiveInfo("mailto:user@example.com"))
|
||||
assert.Equal(t, "https://example.com", protectSensitiveInfo("https://example.com"))
|
||||
assert.Equal(t, "https://_masked_@example.com/path?k=_", protectSensitiveInfo("https://u:p@example.com/path?k=v#hash"))
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user