mirror of
https://github.com/go-gitea/gitea.git
synced 2026-05-17 03:51:52 +00:00
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>
271 lines
7.1 KiB
Go
271 lines
7.1 KiB
Go
// Copyright 2023 The Gitea Authors. All rights reserved.
|
|
// SPDX-License-Identifier: MIT
|
|
|
|
package log
|
|
|
|
import (
|
|
"context"
|
|
"net/url"
|
|
"reflect"
|
|
"runtime"
|
|
"strings"
|
|
"sync"
|
|
"sync/atomic"
|
|
"time"
|
|
|
|
"code.gitea.io/gitea/modules/json"
|
|
"code.gitea.io/gitea/modules/util"
|
|
)
|
|
|
|
type LoggerImpl struct {
|
|
LevelLogger
|
|
|
|
ctx context.Context
|
|
ctxCancel context.CancelFunc
|
|
|
|
level atomic.Int32
|
|
stacktraceLevel atomic.Int32
|
|
|
|
eventWriterMu sync.RWMutex
|
|
eventWriters map[string]EventWriter
|
|
}
|
|
|
|
var (
|
|
_ BaseLogger = (*LoggerImpl)(nil)
|
|
_ LevelLogger = (*LoggerImpl)(nil)
|
|
)
|
|
|
|
// SendLogEvent sends a log event to all writers
|
|
func (l *LoggerImpl) SendLogEvent(event *Event) {
|
|
l.eventWriterMu.RLock()
|
|
defer l.eventWriterMu.RUnlock()
|
|
|
|
if len(l.eventWriters) == 0 {
|
|
FallbackErrorf("[no logger writer]: %s", event.MsgSimpleText)
|
|
return
|
|
}
|
|
|
|
// the writers have their own goroutines, the message arguments (with Stringer) shouldn't be used in other goroutines
|
|
// so the event message must be formatted here
|
|
msgFormat, msgArgs := event.msgFormat, event.msgArgs
|
|
event.msgFormat, event.msgArgs = "(already processed by formatters)", nil
|
|
|
|
for _, w := range l.eventWriters {
|
|
if event.Level < w.GetLevel() {
|
|
continue
|
|
}
|
|
formatted := &EventFormatted{
|
|
Origin: event,
|
|
Msg: w.Base().FormatMessage(w.Base().Mode, event, msgFormat, msgArgs...),
|
|
}
|
|
select {
|
|
case w.Base().Queue <- formatted:
|
|
default:
|
|
bs, _ := json.Marshal(event)
|
|
FallbackErrorf("log writer %q queue is full, event: %v", w.GetWriterName(), string(bs))
|
|
}
|
|
}
|
|
}
|
|
|
|
// syncLevelInternal syncs the level of the logger with the levels of the writers
|
|
func (l *LoggerImpl) syncLevelInternal() {
|
|
lowestLevel := NONE
|
|
for _, w := range l.eventWriters {
|
|
if w.GetLevel() < lowestLevel {
|
|
lowestLevel = w.GetLevel()
|
|
}
|
|
}
|
|
l.level.Store(int32(lowestLevel))
|
|
|
|
lowestLevel = NONE
|
|
for _, w := range l.eventWriters {
|
|
if w.Base().Mode.StacktraceLevel < lowestLevel {
|
|
lowestLevel = w.GetLevel()
|
|
}
|
|
}
|
|
l.stacktraceLevel.Store(int32(lowestLevel))
|
|
}
|
|
|
|
// removeWriterInternal removes a writer from the logger, and stops it if it's not shared
|
|
func (l *LoggerImpl) removeWriterInternal(w EventWriter) {
|
|
if !w.Base().shared {
|
|
eventWriterStopWait(w) // only stop non-shared writers, shared writers are managed by the manager
|
|
}
|
|
delete(l.eventWriters, w.GetWriterName())
|
|
}
|
|
|
|
// AddWriters adds writers to the logger, and starts them. Existing writers will be replaced by new ones.
|
|
func (l *LoggerImpl) AddWriters(writer ...EventWriter) {
|
|
l.eventWriterMu.Lock()
|
|
defer l.eventWriterMu.Unlock()
|
|
l.addWritersInternal(writer...)
|
|
}
|
|
|
|
func (l *LoggerImpl) addWritersInternal(writer ...EventWriter) {
|
|
for _, w := range writer {
|
|
if old, ok := l.eventWriters[w.GetWriterName()]; ok {
|
|
l.removeWriterInternal(old)
|
|
}
|
|
}
|
|
|
|
for _, w := range writer {
|
|
l.eventWriters[w.GetWriterName()] = w
|
|
eventWriterStartGo(l.ctx, w, false)
|
|
}
|
|
|
|
l.syncLevelInternal()
|
|
}
|
|
|
|
// RemoveWriter removes a writer from the logger, and the writer is closed and flushed if it is not shared
|
|
func (l *LoggerImpl) RemoveWriter(modeName string) error {
|
|
l.eventWriterMu.Lock()
|
|
defer l.eventWriterMu.Unlock()
|
|
|
|
w, ok := l.eventWriters[modeName]
|
|
if !ok {
|
|
return util.ErrNotExist
|
|
}
|
|
|
|
l.removeWriterInternal(w)
|
|
l.syncLevelInternal()
|
|
return nil
|
|
}
|
|
|
|
// ReplaceAllWriters replaces all writers from the logger, non-shared writers are closed and flushed
|
|
func (l *LoggerImpl) ReplaceAllWriters(writer ...EventWriter) {
|
|
l.eventWriterMu.Lock()
|
|
defer l.eventWriterMu.Unlock()
|
|
|
|
for _, w := range l.eventWriters {
|
|
l.removeWriterInternal(w)
|
|
}
|
|
l.eventWriters = map[string]EventWriter{}
|
|
l.addWritersInternal(writer...)
|
|
}
|
|
|
|
// DumpWriters dumps the writers as a JSON map, it's used for debugging and display purposes.
|
|
func (l *LoggerImpl) DumpWriters() map[string]any {
|
|
l.eventWriterMu.RLock()
|
|
defer l.eventWriterMu.RUnlock()
|
|
|
|
writers := make(map[string]any, len(l.eventWriters))
|
|
for k, w := range l.eventWriters {
|
|
bs, err := json.Marshal(w.Base().Mode)
|
|
if err != nil {
|
|
FallbackErrorf("marshal writer %q to dump failed: %v", k, err)
|
|
continue
|
|
}
|
|
m := map[string]any{}
|
|
_ = json.Unmarshal(bs, &m)
|
|
m["WriterType"] = w.GetWriterType()
|
|
writers[k] = m
|
|
}
|
|
return writers
|
|
}
|
|
|
|
// Close closes the logger, non-shared writers are closed and flushed
|
|
func (l *LoggerImpl) Close() {
|
|
l.ReplaceAllWriters()
|
|
l.ctxCancel()
|
|
}
|
|
|
|
// IsEnabled returns true if the logger is enabled: it has a working level and has writers
|
|
// Fatal is not considered as enabled, because it's a special case and the process just exits
|
|
func (l *LoggerImpl) IsEnabled() bool {
|
|
l.eventWriterMu.RLock()
|
|
defer l.eventWriterMu.RUnlock()
|
|
return l.level.Load() < int32(FATAL) && len(l.eventWriters) > 0
|
|
}
|
|
|
|
func asLogStringer(v any) LogStringer {
|
|
if s, ok := v.(LogStringer); ok {
|
|
return s
|
|
} else if a := reflect.ValueOf(v); a.Kind() == reflect.Struct {
|
|
// in case the receiver is a pointer, but the value is a struct
|
|
vp := reflect.New(a.Type())
|
|
vp.Elem().Set(a)
|
|
if s, ok := vp.Interface().(LogStringer); ok {
|
|
return s
|
|
}
|
|
}
|
|
return nil
|
|
}
|
|
|
|
// Log prepares the log event, if the level matches, the event will be sent to the writers
|
|
func (l *LoggerImpl) Log(skip int, event *Event, format string, logArgs ...any) {
|
|
if Level(l.level.Load()) > event.Level {
|
|
return
|
|
}
|
|
|
|
if event.Time.IsZero() {
|
|
event.Time = time.Now()
|
|
}
|
|
if event.Caller == "" {
|
|
pc, filename, line, ok := runtime.Caller(skip + 1)
|
|
if ok {
|
|
fn := runtime.FuncForPC(pc)
|
|
if fn != nil {
|
|
fnName := fn.Name()
|
|
event.Caller = strings.ReplaceAll(fnName, "[...]", "") + "()" // generic function names are "foo[...]"
|
|
}
|
|
}
|
|
event.Filename, event.Line = strings.TrimPrefix(filename, projectPackagePrefix), line
|
|
if l.stacktraceLevel.Load() <= int32(event.Level) {
|
|
event.Stacktrace = Stack(skip + 1)
|
|
}
|
|
}
|
|
|
|
// get a simple text message without color
|
|
msgArgs := make([]any, len(logArgs))
|
|
copy(msgArgs, logArgs)
|
|
|
|
// handle LogStringer values
|
|
for i, v := range msgArgs {
|
|
if cv, ok := v.(*ColoredValue); ok {
|
|
if ls := asLogStringer(cv.v); ls != nil {
|
|
cv.v = logStringFormatter{v: ls}
|
|
}
|
|
} else if ls := asLogStringer(v); ls != nil {
|
|
msgArgs[i] = logStringFormatter{v: ls}
|
|
} else if str, ok := v.(string); ok {
|
|
msgArgs[i] = protectSensitiveInfo(str)
|
|
}
|
|
}
|
|
|
|
event.MsgSimpleText = colorSprintf(false, format, msgArgs...)
|
|
event.msgFormat = format
|
|
event.msgArgs = msgArgs
|
|
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())
|
|
}
|
|
|
|
func NewLoggerWithWriters(ctx context.Context, name string, writer ...EventWriter) *LoggerImpl {
|
|
l := &LoggerImpl{}
|
|
l.ctx, l.ctxCancel = newProcessTypedContext(ctx, "Logger: "+name)
|
|
l.LevelLogger = BaseLoggerToGeneralLogger(l)
|
|
l.eventWriters = map[string]EventWriter{}
|
|
l.AddWriters(writer...)
|
|
return l
|
|
}
|