Files
gitea/modules/log/logger_test.go
pandareen ef801bb661 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>
2026-05-15 11:22:36 -07:00

187 lines
5.0 KiB
Go

// Copyright 2023 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package log
import (
"regexp"
"sync"
"testing"
"time"
"github.com/stretchr/testify/assert"
)
type dummyWriter struct {
*EventWriterBaseImpl
delay time.Duration
mu sync.Mutex
logs []string
}
func (d *dummyWriter) Write(p []byte) (n int, err error) {
if d.delay > 0 {
time.Sleep(d.delay)
}
d.mu.Lock()
defer d.mu.Unlock()
d.logs = append(d.logs, string(p))
return len(p), nil
}
func (d *dummyWriter) Close() error {
return nil
}
func (d *dummyWriter) FetchLogs() []string {
d.mu.Lock()
defer d.mu.Unlock()
logs := d.logs
d.logs = nil
return logs
}
func newDummyWriter(name string, level Level, delay time.Duration) *dummyWriter {
w := &dummyWriter{
EventWriterBaseImpl: NewEventWriterBase(name, "dummy", WriterMode{Level: level, Flags: FlagsFromBits(0)}),
}
w.delay = delay
w.Base().OutputWriteCloser = w
return w
}
func TestLogger(t *testing.T) {
logger := NewLoggerWithWriters(t.Context(), "test")
dump := logger.DumpWriters()
assert.Empty(t, dump)
assert.Equal(t, NONE, logger.GetLevel())
assert.False(t, logger.IsEnabled())
w1 := newDummyWriter("dummy-1", DEBUG, 0)
logger.AddWriters(w1)
assert.Equal(t, DEBUG, logger.GetLevel())
w2 := newDummyWriter("dummy-2", WARN, 200*time.Millisecond)
logger.AddWriters(w2)
assert.Equal(t, DEBUG, logger.GetLevel())
dump = logger.DumpWriters()
assert.Len(t, dump, 2)
logger.Trace("trace-level") // this level is not logged
logger.Debug("debug-level")
logger.Error("error-level")
// w2 is slow, so only w1 has logs
time.Sleep(100 * time.Millisecond)
assert.Equal(t, []string{"debug-level\n", "error-level\n"}, w1.FetchLogs())
assert.Empty(t, w2.FetchLogs())
logger.Close()
// after Close, all logs are flushed
assert.Empty(t, w1.FetchLogs())
assert.Equal(t, []string{"error-level\n"}, w2.FetchLogs())
}
func TestLoggerPause(t *testing.T) {
logger := NewLoggerWithWriters(t.Context(), "test")
w1 := newDummyWriter("dummy-1", DEBUG, 0)
logger.AddWriters(w1)
GetManager().PauseAll()
time.Sleep(50 * time.Millisecond)
logger.Info("info-level")
time.Sleep(100 * time.Millisecond)
assert.Empty(t, w1.FetchLogs())
GetManager().ResumeAll()
time.Sleep(100 * time.Millisecond)
assert.Equal(t, []string{"info-level\n"}, w1.FetchLogs())
logger.Close()
}
type testLogString struct {
Field string
}
func (t testLogString) LogString() string {
return "log-string"
}
type testLogStringPtrReceiver struct {
Field string
}
func (t *testLogStringPtrReceiver) LogString() string {
return "log-string-ptr-receiver"
}
func genericFunc[T any](logger Logger, v T) {
logger.Info("from genericFunc: %v", v)
}
func TestLoggerOutput(t *testing.T) {
t.Run("LogString", func(t *testing.T) {
logger := NewLoggerWithWriters(t.Context(), "test")
w1 := newDummyWriter("dummy-1", DEBUG, 0)
w1.Mode.Colorize = true
logger.AddWriters(w1)
logger.Info("%s %s %#v %v", testLogString{}, &testLogString{}, testLogString{Field: "detail"}, NewColoredValue(testLogString{}, FgRed))
logger.Info("%s %s %#v %v", testLogStringPtrReceiver{}, &testLogStringPtrReceiver{}, testLogStringPtrReceiver{Field: "detail"}, NewColoredValue(testLogStringPtrReceiver{}, FgRed))
logger.Close()
assert.Equal(t, []string{
"log-string log-string log.testLogString{Field:\"detail\"} \x1b[31mlog-string\x1b[0m\n",
"log-string-ptr-receiver log-string-ptr-receiver &log.testLogStringPtrReceiver{Field:\"detail\"} \x1b[31mlog-string-ptr-receiver\x1b[0m\n",
}, w1.FetchLogs())
})
t.Run("Caller", func(t *testing.T) {
logger := NewLoggerWithWriters(t.Context(), "test")
w1 := newDummyWriter("dummy-1", DEBUG, 0)
w1.EventWriterBaseImpl.Mode.Flags.flags = Lmedfile | Lshortfuncname
logger.AddWriters(w1)
anonymousFunc := func(logger Logger) {
logger.Info("from anonymousFunc")
}
genericFunc(logger, "123")
anonymousFunc(logger)
logger.Close()
logs := w1.FetchLogs()
assert.Len(t, logs, 2)
assert.Regexp(t, `modules/log/logger_test.go:\w+:`+regexp.QuoteMeta(`genericFunc() from genericFunc: 123`), logs[0])
assert.Regexp(t, `modules/log/logger_test.go:\w+:`+regexp.QuoteMeta(`TestLoggerOutput.2.1() from anonymousFunc`), logs[1])
})
}
func TestLoggerExpressionFilter(t *testing.T) {
logger := NewLoggerWithWriters(t.Context(), "test")
w1 := newDummyWriter("dummy-1", DEBUG, 0)
w1.Mode.Expression = "foo.*"
logger.AddWriters(w1)
logger.Info("foo")
logger.Info("bar")
logger.Info("foo bar")
logger.SendLogEvent(&Event{Level: INFO, Filename: "foo.go", MsgSimpleText: "by filename"})
logger.Close()
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"))
}