Skip to content

Commit

Permalink
Improve error message for unauthenticated error (#3093)
Browse files Browse the repository at this point in the history
This improves the error message when a BSR token is found in netrc but
not valid, distinguishing it from the case where it's not found.

Fixes #2500

Message for no `netrc` entry and no `BUF_TOKEN`:
```
Failure: you are not authenticated for bufbuild.internal. Set the BUF_TOKEN environment variable or run "buf registry login bufbuild.internal", using a Buf API token as the password. For details, visit https://buf.build/docs/bsr/authentication
```
Invalid netrc entry:
```
Failure: your Buf API token for buf.build is invalid. Run "buf registry login" using a valid Buf API token. For details, visit https://buf.build/docs/bsr/authentication
```
Invalid `BUF_TOKEN`:
```
Failure: the BUF_TOKEN environment variable is set, but is not valid. Set BUF_TOKEN to a valid Buf API token, or unset it. For details, visit https://buf.build/docs/bsr/authentication
```
  • Loading branch information
oliversun9 committed Jun 21, 2024
1 parent 0ad8677 commit 28408f4
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 17 deletions.
46 changes: 40 additions & 6 deletions private/buf/cmd/buf/buf.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,14 +317,48 @@ func wrapError(err error) error {
connectCode := connectErr.Code()
switch {
case connectCode == connect.CodeUnauthenticated, isEmptyUnknownError(err):
if authErr, ok := bufconnect.AsAuthError(err); ok && authErr.TokenEnvKey() != "" {
loginCommand := "buf registry login"
authErr, ok := bufconnect.AsAuthError(err)
if !ok {
// This code should be unreachable.
return fmt.Errorf("Failure: you are not authenticated. "+
"Set the %[1]s environment variable or run %q, using a Buf API token as the password. "+
"If you have set the %[1]s or run the login command, "+
"your token may have expired. "+
"For details, visit https://buf.build/docs/bsr/authentication",
bufconnect.TokenEnvKey,
loginCommand,
)
}
// Invalid token found in env var.
if authErr.HasToken() && authErr.TokenEnvKey() != "" {
return fmt.Errorf("Failure: the %[1]s environment variable is set, but is not valid. "+
"Set %[1]s to a valid Buf API key, or unset it. For details, "+
"visit https://docs.buf.build/bsr/authentication", authErr.TokenEnvKey())
"Set %[1]s to a valid Buf API token, or unset it. "+
"For details, visit https://buf.build/docs/bsr/authentication",
authErr.TokenEnvKey(),
)
}
if authErr.Remote() != bufconnect.DefaultRemote {
loginCommand = fmt.Sprintf("%s %s", loginCommand, authErr.Remote())
}
return errors.New("Failure: you are not authenticated. Create a new entry in your netrc, " +
"using a Buf API Key as the password. If you already have an entry in your netrc, check " +
"to see that your token is not expired. For details, visit https://docs.buf.build/bsr/authentication")
// Invalid token found in netrc.
if authErr.HasToken() {
return fmt.Errorf("Failure: your Buf API token for %s is invalid. "+
"Run %q using a valid Buf API token. "+
"For details, visit https://buf.build/docs/bsr/authentication",
authErr.Remote(),
loginCommand,
)
}
// No token found.
return fmt.Errorf("Failure: you are not authenticated for %s. "+
"Set the %s environment variable or run %q, "+
"using a Buf API token as the password. "+
"For details, visit https://buf.build/docs/bsr/authentication",
authErr.Remote(),
bufconnect.TokenEnvKey,
loginCommand,
)
case connectCode == connect.CodeUnavailable:
msg := `Failure: the server hosted at that remote is unavailable.`
// If the returned error is Unavailable, then determine if this is a DNS error. If so,
Expand Down
15 changes: 13 additions & 2 deletions private/bufpkg/bufconnect/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@ import "errors"

// AuthError wraps the error returned in the auth provider to add additional context.
type AuthError struct {
cause error

cause error
remote string
hasToken bool
tokenEnvKey string
}

Expand All @@ -36,6 +37,16 @@ func (e *AuthError) Error() string {
return e.cause.Error()
}

// Remote returns the remote the request was sent to.
func (e *AuthError) Remote() string {
return e.remote
}

// HasToken returns whether a token was included in the request.
func (e *AuthError) HasToken() bool {
return e.hasToken
}

// TokenEnvKey returns the environment variable used, if any, for authentication.
func (e *AuthError) TokenEnvKey() string {
return e.tokenEnvKey
Expand Down
19 changes: 15 additions & 4 deletions private/bufpkg/bufconnect/interceptors.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ import (
)

const (
// tokenEnvKey is the environment variable key for the auth token
tokenEnvKey = "BUF_TOKEN"
// TokenEnvKey is the environment variable key for the auth token
TokenEnvKey = "BUF_TOKEN"
)

// NewAugmentedConnectErrorInterceptor returns a new Connect Interceptor that wraps
Expand Down Expand Up @@ -112,16 +112,27 @@ func NewAuthorizationInterceptorProvider(tokenProviders ...TokenProvider) func(s
interceptor := func(next connect.UnaryFunc) connect.UnaryFunc {
return connect.UnaryFunc(func(ctx context.Context, req connect.AnyRequest) (connect.AnyResponse, error) {
usingTokenEnvKey := false
hasToken := false
for _, tf := range tokenProviders {
if token := tf.RemoteToken(address); token != "" {
req.Header().Set(AuthenticationHeader, AuthenticationTokenPrefix+token)
usingTokenEnvKey = tf.IsFromEnvVar()
hasToken = true
break
}
}
response, err := next(ctx, req)
if err != nil && usingTokenEnvKey {
err = &AuthError{cause: err, tokenEnvKey: tokenEnvKey}
if err != nil {
var envKey string
if usingTokenEnvKey {
envKey = TokenEnvKey
}
err = &AuthError{
cause: err,
remote: address,
hasToken: hasToken,
tokenEnvKey: envKey,
}
}
return response, err
})
Expand Down
4 changes: 2 additions & 2 deletions private/bufpkg/bufconnect/interceptors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,15 +97,15 @@ func TestNewAuthorizationInterceptorProvider(t *testing.T) {
assert.NoError(t, err)

tokenSet, err = NewTokenProviderFromContainer(app.NewEnvContainer(map[string]string{
tokenEnvKey: "default",
TokenEnvKey: "default",
}))
assert.NoError(t, err)
_, err = NewAuthorizationInterceptorProvider(tokenSet)("default")(func(ctx context.Context, req connect.AnyRequest) (connect.AnyResponse, error) {
return nil, errors.New("underlying cause")
})(context.Background(), connect.NewRequest(&bytes.Buffer{}))
authErr, ok := AsAuthError(err)
assert.True(t, ok)
assert.Equal(t, tokenEnvKey, authErr.tokenEnvKey)
assert.Equal(t, TokenEnvKey, authErr.tokenEnvKey)
}

func TestCLIWarningInterceptor(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion private/bufpkg/bufconnect/static_token_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (

// NewTokenProviderFromContainer creates a singleTokenProvider from the BUF_TOKEN environment variable
func NewTokenProviderFromContainer(container app.EnvContainer) (TokenProvider, error) {
return newTokenProviderFromString(container.Env(tokenEnvKey), true)
return newTokenProviderFromString(container.Env(TokenEnvKey), true)
}

// NewTokenProviderFromString creates a singleTokenProvider by the token provided
Expand Down
4 changes: 2 additions & 2 deletions private/bufpkg/bufconnect/static_token_provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
func TestNewTokenProviderFromContainer(t *testing.T) {
t.Parallel()
tokenSet, err := NewTokenProviderFromContainer(app.NewEnvContainer(map[string]string{
tokenEnvKey: "default",
TokenEnvKey: "default",
}))
assert.NoError(t, err)
token := tokenSet.RemoteToken("fake")
Expand Down Expand Up @@ -68,7 +68,7 @@ func TestInvalidTokens(t *testing.T) {
_, err := NewTokenProviderFromString(token)
assert.Error(t, err, "expected %s to be an invalid token, but it wasn't", token)
_, err = NewTokenProviderFromContainer(app.NewEnvContainer(map[string]string{
tokenEnvKey: token,
TokenEnvKey: token,
}))
assert.Error(t, err, "expected %s to be an invalid token, but it wasn't", token)
}
Expand Down

0 comments on commit 28408f4

Please sign in to comment.