Repository: Versent/saml2aws (go)

Issue:
Output too verbose and misleading
The latest release has verbose output and behaves strangely when no username was saved before:

```
[hoegertn@MacBook-Pro Downloads]$ ./saml2aws login --role="arn:aws:iam::123456789012:role/ADFS-Test"
adding osx helper
LookupCredentials <ADFS-URL>
hostname <ADFS-URL>
Using saved username
Using saved password
savedUsername
Hostname [<ADFS-URL>]:
To use saved username and password just hit enter.
Username []:
Username []: someusername
Password: ********

Authenticating to ADFS with URL https://<ADFS-URL>
Selected role: arn:aws:iam::123456789012:role/ADFS-Test
Requesting AWS credentials using SAML assertion
Saving credentials
Logged in as: arn:aws:sts::123456789012:assumed-role/ADFS-Test/someusername

Your new access key pair has been stored in the AWS configuration
Note that it will expire at 2017-07-27 15:08:08 +0200 CEST
To use this credential, call the AWS CLI with the --profile option (e.g. aws --profile saml ec2 describe-instances).
Saving config: /Users/hoegertn/.aws2saml.config
```

Second try:
```
[hoegertn@MacBook-Pro Downloads]$ ./saml2aws login --role="arn:aws:iam::123456789012:role/ADFS-Test"
adding osx helper
LookupCredentials <ADFS-URL>
hostname <ADFS-URL>
Using saved username
Using saved password
savedUsername someusername
Hostname [<ADFS-URL>]:
To use saved username and password just hit enter.
Username [someusername]:
Password: ********

Authenticating to ADFS with URL https://<ADFS-URL>
Selected role: arn:aws:iam::123456789012:role/ADFS-Test
Requesting AWS credentials using SAML assertion
Saving credentials
Logged in as: arn:aws:sts::123456789012:assumed-role/ADFS-Test/someusername

Your new access key pair has been stored in the AWS configuration
Note that it will expire at 2017-07-27 15:08:24 +0200 CEST
To use this credential, call the AWS CLI with the --profile option (e.g. aws --profile saml ec2 describe-instances).
Saving config: /Users/hoegertn/.aws2saml.config
[hoegertn@MacBook-Pro Downloads]$
```

Pull request description:
fix(Logging) Reduced output and removed debugging.
* Fixed missing error case handler on password entry
* Fixed else which assumed a saved username/password was available

fixes #46

Patch:
diff --git a/cmd/saml2aws/commands/login.go b/cmd/saml2aws/commands/login.go
index efe3a05..e30259f 100644
--- a/cmd/saml2aws/commands/login.go
+++ b/cmd/saml2aws/commands/login.go
@@ -40,25 +40,31 @@ func Login(loginFlags *LoginFlags) error {
 		return errors.Wrap(err, "error loading config file")
 	}
 
-	fmt.Println("LookupCredentials", hostname)
+	// fmt.Println("LookupCredentials", hostname)
 
 	loginDetails, err := resolveLoginDetails(hostname, loginFlags)
 	if err != nil {
-		return errors.Wrap(err, "error accepting password")
+		fmt.Printf("%+v\n", err)
+		os.Exit(1)
 	}
 
-	fmt.Printf("Authenticating to %s with URL https://%s\n", loginFlags.Provider, loginDetails.Hostname)
+	fmt.Printf("Authenticating as %s to %s https://%s\n", loginDetails.Username, loginFlags.Provider, loginDetails.Hostname)
 
 	opts := &saml2aws.SAMLOptions{Provider: loginFlags.Provider, SkipVerify: loginFlags.SkipVerify}
 
 	provider, err := saml2aws.NewSAMLClient(opts)
 	if err != nil {
-		return errors.Wrap(err, "error building adfs client")
+		return errors.Wrap(err, "error building IdP client")
+	}
+
+	err = loginDetails.Validate()
+	if err != nil {
+		return errors.Wrap(err, "error validating login details")
 	}
 
 	samlAssertion, err := provider.Authenticate(loginDetails)
 	if err != nil {
-		return errors.Wrap(err, "error authenticating to adfs")
+		return errors.Wrap(err, "error authenticating to IdP")
 
 	}
 
@@ -95,8 +101,11 @@ func Login(loginFlags *LoginFlags) error {
 	}
 
 	role, err := resolveRole(awsRoles, samlAssertion, loginFlags)
+	if err != nil {
+		return errors.Wrap(err, "Failed to assume role, please check you are permitted to assume the given role for the AWS service")
+	}
 
-	fmt.Println("Selected role:", role.RoleARN)
+	// fmt.Println("Selected role:", role.RoleARN)
 
 	sess, err := session.NewSession()
 	if err != nil {
@@ -119,7 +128,7 @@ func Login(loginFlags *LoginFlags) error {
 		return errors.Wrap(err, "error retrieving STS credentials using SAML")
 	}
 
-	fmt.Println("Saving credentials")
+	// fmt.Println("Saving credentials")
 
 	sharedCreds := saml2aws.NewSharedCredentials(loginFlags.Profile)
 
@@ -145,7 +154,7 @@ func resolveLoginDetails(hostname string, loginFlags *LoginFlags) (*saml2aws.Log
 
 	loginDetails := new(saml2aws.LoginDetails)
 
-	fmt.Println("hostname", hostname)
+	// fmt.Printf("loginFlags %+v\n", loginFlags)
 
 	savedUsername, savedPassword, err := credentials.LookupCredentials(hostname)
 	if err != nil {
@@ -154,23 +163,23 @@ func resolveLoginDetails(hostname string, loginFlags *LoginFlags) (*saml2aws.Log
 		}
 	}
 
+	// fmt.Printf("%s %s\n", savedUsername, savedPassword)
+
 	// if you supply a username in a flag it takes precedence
 	if loginFlags.Username != "" {
 		loginDetails.Username = loginFlags.Username
-	} else {
-		fmt.Println("Using saved username")
+	} else if savedUsername != "" {
 		loginDetails.Username = savedUsername
 	}
 
 	// if you supply a password in a flag it takes precedence
 	if loginFlags.Password != "" {
 		loginDetails.Password = loginFlags.Password
-	} else {
-		fmt.Println("Using saved password")
+	} else if savedPassword != "" {
 		loginDetails.Password = savedPassword
 	}
 
-	fmt.Println("savedUsername", savedUsername)
+	// fmt.Printf("loginDetails %+v\n", loginDetails)
 
 	// if skip prompt was passed just pass back the flag values
 	if loginFlags.SkipPrompt {
@@ -212,7 +221,7 @@ func resolveRole(awsRoles []*saml2aws.AWSRole, samlAssertion string, loginFlags
 		if err == nil {
 			break
 		}
-		fmt.Println("error selecting role")
+		fmt.Println("error selecting role, try again")
 	}
 
 	return role, nil
diff --git a/cmd/saml2aws/commands/login_darwin.go b/cmd/saml2aws/commands/login_darwin.go
index 895c462..ec61ac6 100644
--- a/cmd/saml2aws/commands/login_darwin.go
+++ b/cmd/saml2aws/commands/login_darwin.go
@@ -8,6 +8,6 @@ import (
 )
 
 func init() {
-	fmt.Println("adding osx helper")
+	fmt.Println("Adding osx helper")
 	credentials.CurrentHelper = &osxkeychain.Osxkeychain{}
 }
diff --git a/input.go b/input.go
index f8d4e18..3df39b9 100644
--- a/input.go
+++ b/input.go
@@ -7,6 +7,7 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/pkg/errors"
 	"github.com/segmentio/go-prompt"
 )
 
@@ -17,6 +18,20 @@ type LoginDetails struct {
 	Hostname string
 }
 
+// Validate validate the login details
+func (ld *LoginDetails) Validate() error {
+	if ld.Hostname == "" {
+		return errors.New("Missing hostname")
+	}
+	if ld.Username == "" {
+		return errors.New("Missing username")
+	}
+	if ld.Password == "" {
+		return errors.New("Missing password")
+	}
+	return nil
+}
+
 // PromptForLoginDetails prompt the user to present their username, password and hostname
 func PromptForLoginDetails(username, hostname, password string) (*LoginDetails, error) {
 


Test patch:
diff --git a/input_test.go b/input_test.go
new file mode 100644
index 0000000..ee8283d
--- /dev/null
+++ b/input_test.go
@@ -0,0 +1,36 @@
+package saml2aws
+
+import (
+	"testing"
+)
+
+func TestLoginDetails_Validate(t *testing.T) {
+	type fields struct {
+		Username string
+		Password string
+		Hostname string
+	}
+	tests := []struct {
+		name    string
+		fields  fields
+		wantErr bool
+	}{
+		// TODO: Add test cases.
+		{name: "hostname missing error", fields: fields{}, wantErr: true},
+		{name: "username missing error", fields: fields{Hostname: "id.example.com"}, wantErr: true},
+		{name: "password missing error", fields: fields{Hostname: "id.example.com", Username: "test"}, wantErr: true},
+		{name: "ok", fields: fields{Hostname: "id.example.com", Username: "test", Password: "test"}, wantErr: false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ld := &LoginDetails{
+				Username: tt.fields.Username,
+				Password: tt.fields.Password,
+				Hostname: tt.fields.Hostname,
+			}
+			if err := ld.Validate(); (err != nil) != tt.wantErr {
+				t.Errorf("LoginDetails.Validate() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}