diff --git a/pkg/services/login/loginservice/loginservice.go b/pkg/services/login/loginservice/loginservice.go index d869a489ffd..8610a85a76d 100644 --- a/pkg/services/login/loginservice/loginservice.go +++ b/pkg/services/login/loginservice/loginservice.go @@ -38,6 +38,11 @@ func (ls *Implementation) CreateUser(cmd models.CreateUserCommand) (*models.User // 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 user, err := ls.AuthInfoService.LookupAndUpdate(ctx, &models.GetUserByAuthInfoQuery{ @@ -50,13 +55,13 @@ func (ls *Implementation) UpsertUser(ctx context.Context, cmd *models.UpsertUser return err } 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, err := ls.QuotaService.QuotaReached(cmd.ReqContext, "user") if err != nil { - cmd.ReqContext.Logger.Warn("Error getting user quota.", "error", err) + logger.Warn("Error getting user quota.", "error", err) return login.ErrGettingUserQuota } if limitReached { diff --git a/pkg/services/login/loginservice/loginservice_test.go b/pkg/services/login/loginservice/loginservice_test.go index ff86322a958..d5ddbe2e61b 100644 --- a/pkg/services/login/loginservice/loginservice_test.go +++ b/pkg/services/login/loginservice/loginservice_test.go @@ -6,14 +6,17 @@ import ( "errors" "testing" + "github.com/grafana/grafana/pkg/services/login" + "github.com/go-kit/log" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/grafana/grafana/pkg/infra/log/level" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/login/logintest" "github.com/grafana/grafana/pkg/services/quota" "github.com/grafana/grafana/pkg/services/sqlstore/mockstore" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) func Test_syncOrgRoles_doesNotBreakWhenTryingToRemoveLastOrgAdmin(t *testing.T) { @@ -112,6 +115,28 @@ func Test_teamSync(t *testing.T) { }) } +func TestUpsertUser_crashOnLog_issue62538(t *testing.T) { + authInfoMock := &logintest.AuthInfoServiceFake{} + authInfoMock.ExpectedError = models.ErrUserNotFound + loginsvc := Implementation{ + QuotaService: "a.QuotaService{}, + 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() models.User { user := models.User{ Id: 1,