Merge pull request #482 from Azure/fix/secret-login-error-check

Hot fix 10.2.1: Correct missed error check in oauthTokenManager

ChangeLog.md

@@ -1,6 +1,12 @@
 
 # Change Log
 
+## Version 10.2.1
+
+### Bug fix
+
+1. Fixed outputting error message for SPN login failures.
+
 ## Version 10.2.0
 
 ### Bug fix

cmd/helpMessages.go

@@ -38,6 +38,7 @@ The built-in lookup table is small but on Unix it is augmented by the local syst
   - /etc/apache/mime.types
 
 On Windows, MIME types are extracted from the registry. This feature can be turned off with the help of a flag. Please refer to the flag section.
+
 ` + environmentVariableNotice
 
 const copyCmdExample = `Upload a single file using OAuth authentication. Please use 'azcopy login' command first if you aren't logged in yet:
@@ -115,6 +116,7 @@ Copy all buckets in a S3 region with access key to blob account with SAS:
 const envCmdShortDescription = "Shows the environment variables that can configure AzCopy's behavior"
 
 const envCmdLongDescription = `Shows the environment variables that can configure AzCopy's behavior.
+
 ` + environmentVariableNotice
 
 // ===================================== JOBS COMMAND ===================================== //
@@ -154,12 +156,13 @@ const loginCmdLongDescription = `Log in to Azure Active Directory to access Azur
 Note that, to be authorized to your Azure Storage account, you must assign your user 'Storage Blob Data Contributor' role on the Storage account.
 This command will cache encrypted login information for current user using the OS built-in mechanisms.
 Please refer to the examples for more information.
+
 ` + environmentVariableNotice
 
-const environmentVariableNotice = `
-(NOTICE FOR SETTING ENVIRONMENT VARIABLES: Bear in mind that setting an environment variable from the command line
-will be readable in your command line history. For variables that contain credentials, consider clearing these 
-entries from your history or using a small script of sorts to prompt for and set these variables.)`
+const environmentVariableNotice = "(NOTICE FOR SETTING ENVIRONMENT VARIABLES: Bear in mind that setting an environment variable from the command line " +
+	"will be readable in your command line history. " +
+	"For variables that contain credentials, consider clearing these entries from your history " +
+	"or using a small script of sorts to prompt for and set these variables.)"
 
 const loginCmdExample = `Log in interactively with default AAD tenant ID set to common:
 - azcopy login

cmd/login.go

@@ -23,9 +23,9 @@ package cmd
 import (
 	"context"
 	"errors"
-	"fmt"
 	"github.com/Azure/azure-storage-azcopy/common"
 	"github.com/spf13/cobra"
+	"strings"
 )
 
 func init() {
@@ -51,7 +51,10 @@ func init() {
 
 			err := loginCmdArgs.process()
 			if err != nil {
-				return fmt.Errorf("failed to perform login command, %v", err)
+				// the errors from adal contains \r\n in the body, get rid of them to make the error easier to look at
+				prettyErr := strings.Replace(err.Error(), `\r\n`, "\n", -1)
+				prettyErr += "\n\nNOTE: If your credential was created in the last 5 minutes, please wait a few minutes and try again."
+				glcm.Error("Failed to perform login command: \n" + prettyErr)
 			}
 			return nil
 		},

common/oauthTokenManager.go

@@ -206,6 +206,10 @@ func secretLoginNoUOTM(tenantID, activeDirectoryEndpoint, secret, applicationID
 func (uotm *UserOAuthTokenManager) SecretLogin(tenantID, activeDirectoryEndpoint, secret, applicationID string, persist bool) (*OAuthTokenInfo, error) {
 	oAuthTokenInfo, err := secretLoginNoUOTM(tenantID, activeDirectoryEndpoint, secret, applicationID)
 
+	if err != nil {
+		return nil, err
+	}
+
 	if persist {
 		err = uotm.credCache.SaveToken(*oAuthTokenInfo)
 		if err != nil {

common/version.go

@@ -1,5 +1,5 @@
 package common
 
-const AzcopyVersion = "10.2.0"
+const AzcopyVersion = "10.2.1"
 const UserAgent = "AzCopy/" + AzcopyVersion
 const S3ImportUserAgent = "S3Import " + UserAgent