fix(auth): set User-Agent on avatar fetch and sync avatar on link-account register (#37564) (#37588) (#37726)
Backport #37588 by @pandareen ## 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. ## Test plan - [x] `go test -tags 'sqlite sqlite_unlock_notify' -run '^TestOAuth2AvatarFromPicture$' ./tests/integration/ -v` — 5/5 sub-tests pass. - [x] Manual: log in as a Keycloak user with `picture` claim pointing at `https://avatars.githubusercontent.com/u/9919?v=4` — Gitea avatar is replaced with the GitHub picture. - [x] Manual: same flow with `https://upload.wikimedia.org/...` — request now succeeds (or returns a clearly logged `Warn` line if rate-limited with `429`); previously it silently 403'd. - [x] Manual: `UPDATE_AVATAR=false` — user keeps the identicon, no outbound request in container logs. - [ ] Reviewer: please double-check that no other call sites of `oauth2UpdateAvatarIfNeed` rely on the old `http.Get` behaviour. ## Related - Upstream issue: go-gitea/gitea#37564 -------------------------------------------- AI Editor was used in this PR --------- Signed-off-by: silverwind <me@silverwind.io> Co-authored-by: pandareen <7270563+pandareen@users.noreply.github.com> 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:
@@ -263,6 +263,11 @@ func LinkAccountPostRegister(ctx *context.Context) {
|
||||
return
|
||||
}
|
||||
|
||||
oauth2SignInSync(ctx, linkAccountData.AuthSourceID, u, linkAccountData.GothUser)
|
||||
if ctx.Written() {
|
||||
return
|
||||
}
|
||||
|
||||
authSource, err := auth.GetSourceByID(ctx, linkAccountData.AuthSourceID)
|
||||
if err != nil {
|
||||
ctx.ServerError("GetSourceByID", err)
|
||||
|
||||
+37
-15
@@ -13,6 +13,7 @@ import (
|
||||
"net/url"
|
||||
"sort"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"code.gitea.io/gitea/models/auth"
|
||||
user_model "code.gitea.io/gitea/models/user"
|
||||
@@ -301,21 +302,42 @@ func showLinkingLogin(ctx *context.Context, authSourceID int64, gothUser goth.Us
|
||||
ctx.Redirect(setting.AppSubURL + "/user/link_account")
|
||||
}
|
||||
|
||||
func oauth2UpdateAvatarIfNeed(ctx *context.Context, url string, u *user_model.User) {
|
||||
if setting.OAuth2Client.UpdateAvatar && len(url) > 0 {
|
||||
resp, err := http.Get(url)
|
||||
if err == nil {
|
||||
defer func() {
|
||||
_ = resp.Body.Close()
|
||||
}()
|
||||
}
|
||||
// ignore any error
|
||||
if err == nil && resp.StatusCode == http.StatusOK {
|
||||
data, err := io.ReadAll(io.LimitReader(resp.Body, setting.Avatar.MaxFileSize+1))
|
||||
if err == nil && int64(len(data)) <= setting.Avatar.MaxFileSize {
|
||||
_ = user_service.UploadAvatar(ctx, u, data)
|
||||
}
|
||||
}
|
||||
var oauth2AvatarHTTPClient = &http.Client{Timeout: 30 * time.Second}
|
||||
|
||||
func oauth2UpdateAvatarIfNeed(ctx *context.Context, avatarURL string, u *user_model.User) {
|
||||
if !setting.OAuth2Client.UpdateAvatar || len(avatarURL) == 0 {
|
||||
return
|
||||
}
|
||||
req, err := http.NewRequestWithContext(ctx, http.MethodGet, avatarURL, nil)
|
||||
if err != nil {
|
||||
log.Warn("invalid avatar URL %q: %v", avatarURL, err)
|
||||
return
|
||||
}
|
||||
// Some hosts (e.g. Wikimedia) reject Go's default User-Agent.
|
||||
req.Header.Set("User-Agent", "Gitea "+setting.AppVer)
|
||||
|
||||
resp, err := oauth2AvatarHTTPClient.Do(req)
|
||||
if err != nil {
|
||||
log.Warn("fetch %q failed: %v", avatarURL, err)
|
||||
return
|
||||
}
|
||||
defer func() { _ = resp.Body.Close() }()
|
||||
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
log.Warn("fetch %q returned status %d", avatarURL, resp.StatusCode)
|
||||
return
|
||||
}
|
||||
data, err := io.ReadAll(io.LimitReader(resp.Body, setting.Avatar.MaxFileSize+1))
|
||||
if err != nil {
|
||||
log.Warn("read body from %q failed: %v", avatarURL, err)
|
||||
return
|
||||
}
|
||||
if int64(len(data)) > setting.Avatar.MaxFileSize {
|
||||
log.Warn("avatar from %q exceeds max size %d", avatarURL, setting.Avatar.MaxFileSize)
|
||||
return
|
||||
}
|
||||
if err := user_service.UploadAvatar(ctx, u, data); err != nil {
|
||||
log.Warn("UploadAvatar for user %q failed: %v", u.Name, err)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user