Browse Source

Azure: Refactor login errors

Signed-off-by: Christopher Crone <[email protected]>
Christopher Crone 5 years ago
parent
commit
e5335adedd
4 changed files with 22 additions and 22 deletions
  1. 1 4
      azure/backend.go
  2. 12 11
      azure/login/login.go
  3. 3 3
      azure/login/login_test.go
  4. 6 4
      cli/cmd/login/login.go

+ 1 - 4
azure/backend.go

@@ -280,8 +280,5 @@ type aciCloudService struct {
 }
 
 func (cs *aciCloudService) Login(ctx context.Context, params map[string]string) error {
-	if err := cs.loginService.Login(ctx); err != nil {
-		return errors.Wrap(errdefs.ErrLoginFailed, err.Error())
-	}
-	return nil
+	return cs.loginService.Login(ctx)
 }

+ 12 - 11
azure/login/login.go

@@ -17,9 +17,10 @@ import (
 	"github.com/Azure/go-autorest/autorest/adal"
 	"github.com/Azure/go-autorest/autorest/azure/cli"
 	"github.com/Azure/go-autorest/autorest/date"
+	"github.com/pkg/errors"
 	"golang.org/x/oauth2"
 
-	"github.com/pkg/errors"
+	"github.com/docker/api/errdefs"
 )
 
 //go login process, derived from code sample provided by MS at https://github.com/devigned/go-az-cli-stuff
@@ -88,20 +89,20 @@ func (login AzureLoginService) Login(ctx context.Context) error {
 
 	redirectURL := s.Addr()
 	if redirectURL == "" {
-		return errors.New("empty redirect URL")
+		return errors.Wrap(errdefs.ErrLoginFailed, "empty redirect URL")
 	}
 	login.apiHelper.openAzureLoginPage(redirectURL)
 
 	select {
 	case <-ctx.Done():
-		return nil
+		return ctx.Err()
 	case q := <-queryCh:
 		if q.err != nil {
-			return errors.Wrap(err, "unhandled local login server error")
+			return errors.Wrapf(errdefs.ErrLoginFailed, "unhandled local login server error: %s", err)
 		}
 		code, hasCode := q.values["code"]
 		if !hasCode {
-			return errors.New("no login code")
+			return errors.Wrap(errdefs.ErrLoginFailed, "no login code")
 		}
 		data := url.Values{
 			"grant_type":   []string{"authorization_code"},
@@ -112,32 +113,32 @@ func (login AzureLoginService) Login(ctx context.Context) error {
 		}
 		token, err := login.apiHelper.queryToken(data, "organizations")
 		if err != nil {
-			return errors.Wrap(err, "access token request failed")
+			return errors.Wrapf(errdefs.ErrLoginFailed, "access token request failed: %s", err)
 		}
 
 		bits, statusCode, err := login.apiHelper.queryAuthorizationAPI(authorizationURL, fmt.Sprintf("Bearer %s", token.AccessToken))
 		if err != nil {
-			return errors.Wrap(err, "check auth failed")
+			return errors.Wrapf(errdefs.ErrLoginFailed, "check auth failed: %s", err)
 		}
 
 		switch statusCode {
 		case http.StatusOK:
 			var t tenantResult
 			if err := json.Unmarshal(bits, &t); err != nil {
-				return errors.Wrap(err, "unable to unmarshal tenant")
+				return errors.Wrapf(errdefs.ErrLoginFailed, "unable to unmarshal tenant: %s", err)
 			}
 			tID := t.Value[0].TenantID
 			tToken, err := login.refreshToken(token.RefreshToken, tID)
 			if err != nil {
-				return errors.Wrap(err, "unable to refresh token")
+				return errors.Wrapf(errdefs.ErrLoginFailed, "unable to refresh token: %s", err)
 			}
 			loginInfo := TokenInfo{TenantID: tID, Token: tToken}
 
 			if err := login.tokenStore.writeLoginInfo(loginInfo); err != nil {
-				return errors.Wrap(err, "could not store login info")
+				return errors.Wrapf(errdefs.ErrLoginFailed, "could not store login info: %s", err)
 			}
 		default:
-			return fmt.Errorf("unable to login status code %d: %s", statusCode, bits)
+			return errors.Wrapf(errdefs.ErrLoginFailed, "unable to login status code %d: %s", statusCode, bits)
 		}
 	}
 	return nil

+ 3 - 3
azure/login/login_test.go

@@ -98,7 +98,7 @@ func (suite *LoginSuite) TestDoesNotRefreshValidToken() {
 func (suite *LoginSuite) TestInvalidLogin() {
 	suite.mockHelper.On("openAzureLoginPage", mock.AnythingOfType("string")).Run(func(args mock.Arguments) {
 		redirectURL := args.Get(0).(string)
-		err := queryKeyValue(redirectURL, "error", "access denied")
+		err := queryKeyValue(redirectURL, "error", "access denied: login failed")
 		Expect(err).To(BeNil())
 	})
 
@@ -107,7 +107,7 @@ func (suite *LoginSuite) TestInvalidLogin() {
 	Expect(err).To(BeNil())
 
 	err = azureLogin.Login(context.TODO())
-	Expect(err.Error()).To(BeEquivalentTo("no login code"))
+	Expect(err.Error()).To(BeEquivalentTo("no login code: login failed"))
 }
 
 func (suite *LoginSuite) TestValidLogin() {
@@ -192,7 +192,7 @@ func (suite *LoginSuite) TestLoginAuthorizationFailed() {
 	Expect(err).To(BeNil())
 
 	err = azureLogin.Login(context.TODO())
-	Expect(err.Error()).To(BeEquivalentTo("unable to login status code 400: [access denied]"))
+	Expect(err.Error()).To(BeEquivalentTo("unable to login status code 400: [access denied]: login failed"))
 }
 
 func refreshTokenData(refreshToken string) url.Values {

+ 6 - 4
cli/cmd/login/login.go

@@ -1,6 +1,7 @@
 package login
 
 import (
+	"context"
 	"fmt"
 	"strings"
 
@@ -9,6 +10,7 @@ import (
 
 	"github.com/docker/api/cli/dockerclassic"
 	"github.com/docker/api/client"
+	"github.com/docker/api/errdefs"
 )
 
 // Command returns the login command
@@ -46,15 +48,15 @@ func cloudLogin(cmd *cobra.Command, backendType string) error {
 	ctx := cmd.Context()
 	cs, err := client.GetCloudService(ctx, backendType)
 	if err != nil {
-		return errors.Wrap(err, "cannot connect to backend")
+		return errors.Wrap(errdefs.ErrLoginFailed, "cannot connect to backend")
 	}
 	err = cs.Login(ctx, nil)
+	if errors.Is(err, context.Canceled) {
+		return errors.New("login canceled")
+	}
 	if err != nil {
 		return err
 	}
-	if cmd.Context().Err() != nil {
-		return errors.New("login canceled")
-	}
 	fmt.Println("login successful")
 	return nil
 }