Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove the actual email value from the error message in the validateEmail function or return a handleable error instead #586

Open
pepnova-9 opened this issue Sep 27, 2023 · 0 comments
Assignees

Comments

@pepnova-9
Copy link

pepnova-9 commented Sep 27, 2023

[REQUIRED] Step 2: Describe your environment

  • Operating System version: Mac OS Ventura Version 13.0.1
  • Firebase SDK version: v4.12.0
  • Library version: v4.12.0
  • Firebase Product: auth (auth, database, storage, etc)

[REQUIRED] Step 3: Describe the problem

Currently, the validateEmail() function returns an error with an error message that includes the actual email value passed as the argument. The error generated is a simple wrapError, making it impossible to handle this error using errors.As() or errors.Is() to distinguish it from other potential errors.

In my country, Japan, email addresses are considered personal information, and we are prohibited from logging them without user confirmation. Additionally, for security reasons, we do not want to log any personal information. I believe that similar restrictions apply in other countries as well.

func validateEmail(email string) error {
	if email == "" {
		return fmt.Errorf("email must be a non-empty string")
	}
	if parts := strings.Split(email, "@"); len(parts) != 2 || parts[0] == "" || parts[1] == "" {
		return fmt.Errorf("malformed email string: %q", email)  // the actual email value is included in the error message
	}
	return nil
}

Suggestions:

(1) Simply remove the actual email value from the error message:

func validateEmail(email string) error {
	if email == "" {
		return fmt.Errorf("email must be a non-empty string")
	}
	if parts := strings.Split(email, "@"); len(parts) != 2 || parts[0] == "" || parts[1] == "" {
		return fmt.Errorf("malformed email string) // remove the email value from the error message
	}
	return nil
}

For instance, the validatePassword() function does not include the actual password value in its error message.

func validatePassword(val string) error {
	if len(val) < 6 {
		return fmt.Errorf("password must be a string at least 6 characters long")
	}
	return nil
}

This is because passwords are considered personal and highly sensitive information. Following the same logic, we can remove the actual email value from the error message.

(2) Define a specific error and make it handleable with errors.Is() or errors.As():

var ErrMalformedEmailFormat = errors.New("malformed email string")

func validateEmail(email string) error {
	if email == "" {
		return fmt.Errorf("email must be a non-empty string")
	}
	if parts := strings.Split(email, "@"); len(parts) != 2 || parts[0] == "" || parts[1] == "" {
		return ErrMalformedEmailFormat
	}
	return nil
}

In the above example code, I didn't include the actual email value in the defined ErrMalformedEmailFormat. Therefore, we need to modify the code. Nevertheless, I believe that, at least, I can convey the essence of my suggestion.

Steps to reproduce:

Pass an invalid email value to CreateUser() function. I added the code to reproduce below.

Relevant Code:

package main

import (
	"context"
	"log"

	firebase "firebase.google.com/go"
	"firebase.google.com/go/auth"
	"google.golang.org/api/option"
)

func main() {
	ctx := context.Background()
	opt := option.WithCredentialsFile("serviceAccountKey.json")
	app, err := firebase.NewApp(ctx, nil, opt)
	if err != nil {
		panic(err)
	}
	authClient, err := app.Auth(ctx)
	if err != nil {
		panic(err)
	}

	param := &auth.UserToCreate{}
	param.Email("foo@example.com") // this is malformed email address. Fullwidth form `@` is used, not halfwidth `@`. 
	_, err = authClient.CreateUser(ctx, param)
	log.Printf("%#v", err)  // malformed email string: "foo@example.com"
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants