Merge pull request #433 from authorizerdev/fix/phone_number_verification

fix: phone_number_verified_at set during signup

server/handlers/verify_email.go

@@ -74,7 +74,13 @@ func VerifyEmailHandler() gin.HandlerFunc {
 			now := time.Now().Unix()
 			user.EmailVerifiedAt = &now
 			isSignUp = true
-			db.Provider.UpdateUser(c, user)
+			user, err = db.Provider.UpdateUser(c, user)
+			if err != nil {
+				log.Debug("Error updating user: ", err)
+				errorRes["error"] = err.Error()
+				utils.HandleRedirectORJsonResponse(c, http.StatusBadRequest, errorRes, generateRedirectURL(redirectURL, errorRes))
+				return
+			}
 		}
 		// delete from verification table
 		db.Provider.DeleteVerificationRequest(c, verificationRequest)

server/resolvers/login.go

@@ -78,7 +78,7 @@ func LoginResolver(ctx context.Context, params model.LoginInput) (*model.AuthRes
 	}
 	if err != nil {
 		log.Debug("Failed to get user: ", err)
-		return res, fmt.Errorf(`bad user credentials`)
+		return res, fmt.Errorf(`user not found`)
 	}
 	if user.RevokedTimestamp != nil {
 		log.Debug("User access is revoked")

server/resolvers/signup.go

@@ -73,7 +73,7 @@ func SignupResolver(ctx context.Context, params model.SignUpInput) (*model.AuthR
 	}
 	isEmailSignup := email != ""
 	isMobileSignup := phoneNumber != ""
-	if isBasicAuthDisabled {
+	if isBasicAuthDisabled && isEmailSignup {
 		log.Debug("Basic authentication is disabled")
 		return res, fmt.Errorf(`basic authentication is disabled for this instance`)
 	}
@@ -222,12 +222,12 @@ func SignupResolver(ctx context.Context, params model.SignUpInput) (*model.AuthR
 		log.Debug("Error getting email verification disabled: ", err)
 		isEmailVerificationDisabled = true
 	}
-	if isEmailVerificationDisabled {
+	if isEmailVerificationDisabled && isEmailSignup {
 		now := time.Now().Unix()
 		user.EmailVerifiedAt = &now
 	}
 	disablePhoneVerification, _ := memorystore.Provider.GetBoolStoreEnvVariable(constants.EnvKeyDisablePhoneVerification)
-	if disablePhoneVerification {
+	if disablePhoneVerification && isMobileSignup {
 		now := time.Now().Unix()
 		user.PhoneNumberVerifiedAt = &now
 	}

server/resolvers/verify_otp.go

@@ -36,24 +36,29 @@ func VerifyOtpResolver(ctx context.Context, params model.VerifyOTPRequest) (*mod
 		return res, fmt.Errorf(`invalid session: %s`, err.Error())
 	}
 
-	if refs.StringValue(params.Email) == "" && refs.StringValue(params.PhoneNumber) == "" {
+	email := strings.TrimSpace(refs.StringValue(params.Email))
+	phoneNumber := strings.TrimSpace(refs.StringValue(params.PhoneNumber))
+	if email == "" && phoneNumber == "" {
 		log.Debug("Email or phone number is required")
-		return res, fmt.Errorf(`email or phone_number is required`)
-	}
-	currentField := models.FieldNameEmail
-	if refs.StringValue(params.Email) == "" {
-		currentField = models.FieldNamePhoneNumber
+		return res, fmt.Errorf(`email or phone number is required`)
 	}
+	isEmailVerification := email != ""
+	isMobileVerification := phoneNumber != ""
 	// Get user by email or phone number
 	var user *models.User
-	if currentField == models.FieldNameEmail {
+	if isEmailVerification {
 		user, err = db.Provider.GetUserByEmail(ctx, refs.StringValue(params.Email))
+		if err != nil {
+			log.Debug("Failed to get user by email: ", err)
+		}
 	} else {
 		user, err = db.Provider.GetUserByPhoneNumber(ctx, refs.StringValue(params.PhoneNumber))
+		if err != nil {
+			log.Debug("Failed to get user by phone number: ", err)
+		}
 	}
 	if user == nil || err != nil {
-		log.Debug("Failed to get user by email or phone number: ", err)
-		return res, err
+		return res, fmt.Errorf(`user not found`)
 	}
 	// Verify OTP based on TOPT or OTP
 	if refs.BoolValue(params.IsTotp) {
@@ -78,14 +83,19 @@ func VerifyOtpResolver(ctx context.Context, params model.VerifyOTPRequest) (*mod
 		}
 	} else {
 		var otp *models.OTP
-		if currentField == models.FieldNameEmail {
+		if isEmailVerification {
 			otp, err = db.Provider.GetOTPByEmail(ctx, refs.StringValue(params.Email))
+			if err != nil {
+				log.Debug(`Failed to get otp request for email: `, err.Error())
+			}
 		} else {
 			otp, err = db.Provider.GetOTPByPhoneNumber(ctx, refs.StringValue(params.PhoneNumber))
+			if err != nil {
+				log.Debug(`Failed to get otp request for phone number: `, err.Error())
+			}
 		}
 		if otp == nil && err != nil {
-			log.Debugf("Failed to get otp request for %s: %s", currentField, err.Error())
-			return res, fmt.Errorf(`invalid %s: %s`, currentField, err.Error())
+			return res, fmt.Errorf(`OTP not found`)
 		}
 		if params.Otp != otp.Otp {
 			log.Debug("Failed to verify otp request: Incorrect value")
@@ -104,10 +114,26 @@ func VerifyOtpResolver(ctx context.Context, params model.VerifyOTPRequest) (*mod
 		return res, fmt.Errorf(`invalid session: %s`, err.Error())
 	}
 
-	isSignUp := user.EmailVerifiedAt == nil && user.PhoneNumberVerifiedAt == nil
-	// TODO - Add Login method in DB when we introduce OTP for social media login
+	isSignUp := false
+	if user.EmailVerifiedAt == nil && isEmailVerification {
+		isSignUp = true
+		now := time.Now().Unix()
+		user.EmailVerifiedAt = &now
+	}
+	if user.PhoneNumberVerifiedAt == nil && isMobileVerification {
+		isSignUp = true
+		now := time.Now().Unix()
+		user.PhoneNumberVerifiedAt = &now
+	}
+	if isSignUp {
+		user, err = db.Provider.UpdateUser(ctx, user)
+		if err != nil {
+			log.Debug("Failed to update user: ", err)
+			return res, err
+		}
+	}
 	loginMethod := constants.AuthRecipeMethodBasicAuth
-	if currentField == models.FieldNamePhoneNumber {
+	if isMobileVerification {
 		loginMethod = constants.AuthRecipeMethodMobileOTP
 	}
 	roles := strings.Split(user.Roles, ",")

server/test/mobile_signup_test.go

@@ -98,12 +98,17 @@ func mobileSingupTest(t *testing.T, s TestSetup) {
 		})
 		assert.Nil(t, err)
 		assert.NotEmpty(t, otpRes.Message)
+		// Check if phone number is verified
+		user, err = db.Provider.GetUserByPhoneNumber(ctx, phoneNumber)
+		assert.NoError(t, err)
+		assert.NotNil(t, user)
+		assert.NotNil(t, user.PhoneNumberVerifiedAt)
 		res, err = resolvers.SignupResolver(ctx, model.SignUpInput{
 			PhoneNumber:     refs.NewStringRef(phoneNumber),
 			Password:        s.TestInfo.Password,
 			ConfirmPassword: s.TestInfo.Password,
 		})
-		assert.Error(t, err)
+		assert.Error(t, err, "should throw duplicate error")
 		assert.Nil(t, res)
 		cleanData("[email protected]")
 	})

server/test/verify_email_test.go

@@ -35,7 +35,11 @@ func verifyEmailTest(t *testing.T, s TestSetup) {
 		})
 		assert.Nil(t, err)
 		assert.NotEqual(t, verifyRes.AccessToken, "", "access token should not be empty")
-
+		// Check if phone number is verified
+		user1, err := db.Provider.GetUserByEmail(ctx, email)
+		assert.NoError(t, err)
+		assert.NotNil(t, user1)
+		assert.NotNil(t, user1.EmailVerifiedAt)
 		cleanData(email)
 	})
 }