actions/protector: improve errors

ErrProtectorName:
	Rename to ErrLoginProtectorName for clarity, and include the
	name and user.

ErrMissingProtectorName:
	Include the correct protector source.

ErrDuplicateName:
	Rename to ErrProtectorNameExists for clarity, and remove a level
	of wrapping by including the name directly.

ErrDuplicateUID:
	Rename to ErrLoginProtectorExists for clarity, and remove a
	level of wrapping by including the user directly.

5 files changed, 54 insertions, 20 deletions

diff --git a/actions/protector.go b/actions/protector.go
index dab9c27..3278e63 100644
--- a/actions/protector.go
+++ b/actions/protector.go
@@ -22,8 +22,7 @@ package actions
 import (
 	"fmt"
 	"log"
-
-	"github.com/pkg/errors"
+	"os/user"
 
 	"github.com/google/fscrypt/crypto"
 	"github.com/google/fscrypt/metadata"
@@ -34,13 +33,44 @@ import (
 // This can be overridden by the user of this package.
 var LoginProtectorMountpoint = "/"
 
-// Errors relating to Protectors
-var (
-	ErrProtectorName        = errors.New("login protectors do not need a name")
-	ErrMissingProtectorName = errors.New("custom protectors must have a name")
-	ErrDuplicateName        = errors.New("protector with this name already exists")
-	ErrDuplicateUID         = errors.New("login protector for this user already exists")
-)
+// ErrLoginProtectorExists indicates that a user already has a login protector.
+type ErrLoginProtectorExists struct {
+	User *user.User
+}
+
+func (err *ErrLoginProtectorExists) Error() string {
+	return fmt.Sprintf("user %q already has a login protector", err.User.Username)
+}
+
+// ErrLoginProtectorName indicates that a name was given for a login protector.
+type ErrLoginProtectorName struct {
+	Name string
+	User *user.User
+}
+
+func (err *ErrLoginProtectorName) Error() string {
+	return fmt.Sprintf(`cannot assign name %q to new login protector for
+	user %q because login protectors are identified by user, not by name.`,
+		err.Name, err.User.Username)
+}
+
+// ErrMissingProtectorName indicates that a protector name is needed.
+type ErrMissingProtectorName struct {
+	Source metadata.SourceType
+}
+
+func (err *ErrMissingProtectorName) Error() string {
+	return fmt.Sprintf("%s protectors must be named", err.Source)
+}
+
+// ErrProtectorNameExists indicates that a protector name already exists.
+type ErrProtectorNameExists struct {
+	Name string
+}
+
+func (err *ErrProtectorNameExists) Error() string {
+	return fmt.Sprintf("there is already a protector named %q", err.Name)
+}
 
 // checkForProtectorWithName returns an error if there is already a protector
 // on the filesystem with a specific name (or if we cannot read the necessary
@@ -52,7 +82,7 @@ func checkForProtectorWithName(ctx *Context, name string) error {
 	}
 	for _, option := range options {
 		if option.Name() == name {
-			return errors.Wrapf(ErrDuplicateName, "name %q", name)
+			return &ErrProtectorNameExists{name}
 		}
 	}
 	return nil
@@ -68,7 +98,7 @@ func checkIfUserHasLoginProtector(ctx *Context, uid int64) error {
 	}
 	for _, option := range options {
 		if option.Source() == metadata.SourceType_pam_passphrase && option.UID() == uid {
-			return errors.Wrapf(ErrDuplicateUID, "user %q", ctx.TargetUser.Username)
+			return &ErrLoginProtectorExists{ctx.TargetUser}
 		}
 	}
 	return nil
@@ -97,12 +127,12 @@ func CreateProtector(ctx *Context, name string, keyFn KeyFunc) (*Protector, erro
 	if ctx.Config.Source == metadata.SourceType_pam_passphrase {
 		// login protectors don't need a name (we use the username instead)
 		if name != "" {
-			return nil, ErrProtectorName
+			return nil, &ErrLoginProtectorName{name, ctx.TargetUser}
 		}
 	} else {
 		// non-login protectors need a name (so we can distinguish between them)
 		if name == "" {
-			return nil, ErrMissingProtectorName
+			return nil, &ErrMissingProtectorName{ctx.Config.Source}
 		}
 		// we don't want to duplicate naming
 		if err := checkForProtectorWithName(ctx, name); err != nil {
diff --git a/actions/recovery.go b/actions/recovery.go
index 1c55ec5..458349b 100644
--- a/actions/recovery.go
+++ b/actions/recovery.go
@@ -23,8 +23,6 @@ import (
 	"os"
 	"strconv"
 
-	"github.com/pkg/errors"
-
 	"github.com/google/fscrypt/crypto"
 	"github.com/google/fscrypt/metadata"
 )
@@ -72,7 +70,7 @@ func AddRecoveryPassphrase(policy *Policy, dirname string) (*crypto.Key, *Protec
 		if err == nil {
 			break
 		}
-		if errors.Cause(err) != ErrDuplicateName {
+		if _, ok := err.(*ErrProtectorNameExists); !ok {
 			return nil, nil, err
 		}
 		seq++
diff --git a/cli-tests/t_encrypt_custom.out b/cli-tests/t_encrypt_custom.out
index 572529a..e7b8656 100644
--- a/cli-tests/t_encrypt_custom.out
+++ b/cli-tests/t_encrypt_custom.out
@@ -46,7 +46,7 @@ PROTECTOR         LINKED  DESCRIPTION
 desc6  No      custom protector "prot"
 
 # Try to use a custom protector without a name
-[ERROR] fscrypt encrypt: custom protectors must have a name
+[ERROR] fscrypt encrypt: custom_passphrase protectors must be named
 
 Use --name=PROTECTOR_NAME to specify a protector name.
 ext4 filesystem "MNT" has 0 protectors and 0 policies
diff --git a/cli-tests/t_encrypt_login.out b/cli-tests/t_encrypt_login.out
index c6eb463..7ee66a2 100644
--- a/cli-tests/t_encrypt_login.out
+++ b/cli-tests/t_encrypt_login.out
@@ -130,7 +130,11 @@ POLICY                            UNLOCKED  PROTECTORS
 desc34  Yes       desc35
 
 # Try to give a login protector a name
-[ERROR] fscrypt encrypt: login protectors do not need a name
+[ERROR] fscrypt encrypt: cannot assign name "prot" to new login protector for
+                         user "fscrypt-test-user" because login protectors are
+                         identified by user, not by name.
+
+To fix this, don't specify the --name=PROTECTOR_NAME option.
 ext4 filesystem "MNT" has 0 protectors and 0 policies
 
 ext4 filesystem "MNT_ROOT" has 0 protectors and 0 policies
diff --git a/cmd/fscrypt/errors.go b/cmd/fscrypt/errors.go
index e7c025f..829a9d2 100644
--- a/cmd/fscrypt/errors.go
+++ b/cmd/fscrypt/errors.go
@@ -82,6 +82,10 @@ func getErrorSuggestions(err error) string {
 	switch err.(type) {
 	case *actions.ErrBadConfigFile:
 		return `Either fix this file manually, or run "sudo fscrypt setup" to recreate it.`
+	case *actions.ErrLoginProtectorName:
+		return fmt.Sprintf("To fix this, don't specify the %s option.", shortDisplay(nameFlag))
+	case *actions.ErrMissingProtectorName:
+		return fmt.Sprintf("Use %s to specify a protector name.", shortDisplay(nameFlag))
 	case *actions.ErrNoConfigFile:
 		return `Run "sudo fscrypt setup" to create this file.`
 	}
@@ -131,8 +135,6 @@ func getErrorSuggestions(err error) string {
 		return `The metadata for this encrypted directory is in an
 			inconsistent state. This most likely means the filesystem
 			metadata is corrupted.`
-	case actions.ErrMissingProtectorName:
-		return fmt.Sprintf("Use %s to specify a protector name.", shortDisplay(nameFlag))
 	case actions.ErrAccessDeniedPossiblyV2:
 		return fmt.Sprintf(`This may be caused by the directory using a v2
 		encryption policy and the current kernel not supporting it. If