mirror of
https://github.com/go-gitea/gitea.git
synced 2026-05-18 19:11:06 +00:00
## 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>
187 lines
5.0 KiB
Go
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"))
|
|
}
|