From 3e12fce4418cf9268f4ed40df974eda96e2b7c7c Mon Sep 17 00:00:00 2001 From: Emil Tullstedt Date: Tue, 31 Jan 2023 13:45:54 +0100 Subject: [PATCH] [v9.2.x] Login: Fix panic when UpsertUser is called without ReqContext (#62574) * Login: Fix panic when UpsertUser is called without ReqContext (#62539) (cherry picked from commit b1151dd118f2a1c305a6f757661e42e59cf145c1) (cherry picked from commit 679cc18648295053a6ab9b764b85cd68f773f81b) --- .../login/loginservice/loginservice.go | 9 +++++-- .../login/loginservice/loginservice_test.go | 27 +++++++++++++++++-- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/pkg/services/login/loginservice/loginservice.go b/pkg/services/login/loginservice/loginservice.go index 6744501f118..fb5bdfb6432 100644 --- a/pkg/services/login/loginservice/loginservice.go +++ b/pkg/services/login/loginservice/loginservice.go @@ -54,6 +54,11 @@ func (ls *Implementation) CreateUser(cmd user.CreateUserCommand) (*user.User, er // UpsertUser updates an existing user, or if it doesn't exist, inserts a new one. func (ls *Implementation) UpsertUser(ctx context.Context, cmd *models.UpsertUserCommand) error { + var logger log.Logger = logger + if cmd.ReqContext != nil && cmd.ReqContext.Logger != nil { + logger = cmd.ReqContext.Logger + } + extUser := cmd.ExternalUser usr, errAuthLookup := ls.AuthInfoService.LookupAndUpdate(ctx, &models.GetUserByAuthInfoQuery{ @@ -67,13 +72,13 @@ func (ls *Implementation) UpsertUser(ctx context.Context, cmd *models.UpsertUser } if !cmd.SignupAllowed { - cmd.ReqContext.Logger.Warn("Not allowing login, user not found in internal user database and allow signup = false", "authmode", extUser.AuthModule) + logger.Warn("Not allowing login, user not found in internal user database and allow signup = false", "authmode", extUser.AuthModule) return login.ErrSignupNotAllowed } limitReached, errLimit := ls.QuotaService.QuotaReached(cmd.ReqContext, "user") if errLimit != nil { - cmd.ReqContext.Logger.Warn("Error getting user quota.", "error", errLimit) + logger.Warn("Error getting user quota.", "error", errLimit) return login.ErrGettingUserQuota } if limitReached { diff --git a/pkg/services/login/loginservice/loginservice_test.go b/pkg/services/login/loginservice/loginservice_test.go index fd0fbfad2b1..248f94c54bc 100644 --- a/pkg/services/login/loginservice/loginservice_test.go +++ b/pkg/services/login/loginservice/loginservice_test.go @@ -8,6 +8,9 @@ import ( "github.com/go-kit/log" "github.com/go-kit/log/level" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/login" "github.com/grafana/grafana/pkg/services/login/logintest" @@ -16,8 +19,6 @@ import ( "github.com/grafana/grafana/pkg/services/sqlstore/mockstore" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/services/user/usertest" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) func Test_syncOrgRoles_doesNotBreakWhenTryingToRemoveLastOrgAdmin(t *testing.T) { @@ -118,6 +119,28 @@ func Test_teamSync(t *testing.T) { }) } +func TestUpsertUser_crashOnLog_issue62538(t *testing.T) { + authInfoMock := &logintest.AuthInfoServiceFake{} + authInfoMock.ExpectedError = user.ErrUserNotFound + loginsvc := Implementation{ + QuotaService: "aimpl.Service{}, + AuthInfoService: authInfoMock, + } + + email := "test_user@example.org" + upsertCmd := &models.UpsertUserCommand{ + ExternalUser: &models.ExternalUserInfo{Email: email}, + UserLookupParams: models.UserLookupParams{Email: &email}, + SignupAllowed: false, + } + + var err error + require.NotPanics(t, func() { + err = loginsvc.UpsertUser(context.Background(), upsertCmd) + }) + require.ErrorIs(t, err, login.ErrSignupNotAllowed) +} + func createSimpleUser() user.User { user := user.User{ ID: 1,