Login: Fix panic when UpsertUser is called without ReqContext (grafan…

…a#62539)
InfluxCommunity · Jan 31, 2023 · b1151dd · b1151dd
1 parent cbc10f9
commit b1151dd
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 2 deletions.
diff --git a/pkg/services/login/loginservice/loginservice.go b/pkg/services/login/loginservice/loginservice.go
@@ -44,6 +44,11 @@ type Implementation struct {
 
 // UpsertUser updates an existing user, or if it doesn't exist, inserts a new one.
 func (ls *Implementation) UpsertUser(ctx context.Context, cmd *login.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, &login.GetUserByAuthInfoQuery{
@@ -57,7 +62,7 @@ func (ls *Implementation) UpsertUser(ctx context.Context, cmd *login.UpsertUserC
 		}
 
 		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
 		}
 
@@ -67,7 +72,7 @@ func (ls *Implementation) UpsertUser(ctx context.Context, cmd *login.UpsertUserC
 		for _, srv := range []string{user.QuotaTargetSrv, org.QuotaTargetSrv} {
 			limitReached, errLimit := ls.QuotaService.CheckQuotaReached(ctx, quota.TargetSrv(srv), nil)
 			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
@@ -137,6 +137,28 @@ func Test_teamSync(t *testing.T) {
 	})
 }
 
+func TestUpsertUser_crashOnLog_issue62538(t *testing.T) {
+	authInfoMock := &logintest.AuthInfoServiceFake{}
+	authInfoMock.ExpectedError = user.ErrUserNotFound
+	loginsvc := Implementation{
+		QuotaService:    quotatest.New(false, nil),
+		AuthInfoService: authInfoMock,
+	}
+
+	email := "test_user@example.org"
+	upsertCmd := &login.UpsertUserCommand{
+		ExternalUser:     &login.ExternalUserInfo{Email: email},
+		UserLookupParams: login.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,