From 2c6bcbdeb57a1c069b4f09f8f532fbd620df9ac1 Mon Sep 17 00:00:00 2001 From: Violet Hynes Date: Tue, 17 May 2022 14:03:02 -0400 Subject: [PATCH] VAULT-5885: Fix erroneous success message in case of two-phase MFA, and provide MFA information in table format (#15428) * VAULT-5885: Fix erroneous success message in case of two-phase MFA, and provide MFA information in table format * VAULT-5885 Add changelog * VAULT-5885 Update changelog as per PR comments * VAULT-5885 Update changelog category to just 'auth' * VAULT-5885 Hide useless token info in two-phase MFA case * VAULT-5885 Update changelog to reflect token info now no longer present * VAULT-5885 split up changelog into three blocks --- changelog/15428.txt | 9 +++++++++ command/format.go | 41 ++++++++++++++++++++++++++--------------- command/login.go | 25 ++++++++++++++++--------- 3 files changed, 51 insertions(+), 24 deletions(-) create mode 100644 changelog/15428.txt diff --git a/changelog/15428.txt b/changelog/15428.txt new file mode 100644 index 000000000..2c4a4a5f0 --- /dev/null +++ b/changelog/15428.txt @@ -0,0 +1,9 @@ +```release-note:bug +auth: Fixed erroneous success message when using vault login in case of two-phase MFA +``` +```release-note:bug +auth: Fixed erroneous token information being displayed when using vault login in case of two-phase MFA +``` +```release-note:bug +auth: Fixed two-phase MFA information missing from table format when using vault login +``` \ No newline at end of file diff --git a/command/format.go b/command/format.go index 314090136..fd065db17 100644 --- a/command/format.go +++ b/command/format.go @@ -385,21 +385,32 @@ func (t TableFormatter) OutputSecret(ui cli.Ui, secret *api.Secret) error { } if secret.Auth != nil { - out = append(out, fmt.Sprintf("token %s %s", hopeDelim, secret.Auth.ClientToken)) - out = append(out, fmt.Sprintf("token_accessor %s %s", hopeDelim, secret.Auth.Accessor)) - // If the lease duration is 0, it's likely a root token, so output the - // duration as "infinity" to clear things up. - if secret.Auth.LeaseDuration == 0 { - out = append(out, fmt.Sprintf("token_duration %s %s", hopeDelim, "∞")) - } else { - out = append(out, fmt.Sprintf("token_duration %s %v", hopeDelim, humanDurationInt(secret.Auth.LeaseDuration))) - } - out = append(out, fmt.Sprintf("token_renewable %s %t", hopeDelim, secret.Auth.Renewable)) - out = append(out, fmt.Sprintf("token_policies %s %q", hopeDelim, secret.Auth.TokenPolicies)) - out = append(out, fmt.Sprintf("identity_policies %s %q", hopeDelim, secret.Auth.IdentityPolicies)) - out = append(out, fmt.Sprintf("policies %s %q", hopeDelim, secret.Auth.Policies)) - for k, v := range secret.Auth.Metadata { - out = append(out, fmt.Sprintf("token_meta_%s %s %v", k, hopeDelim, v)) + if secret.Auth.MFARequirement != nil { + out = append(out, fmt.Sprintf("mfa_request_id %s %s", hopeDelim, secret.Auth.MFARequirement.MFARequestID)) + + for k, constraintSet := range secret.Auth.MFARequirement.MFAConstraints { + for _, constraint := range constraintSet.Any { + out = append(out, fmt.Sprintf("mfa_constraint_%s_%s_id %s %s", k, constraint.Type, hopeDelim, constraint.ID)) + out = append(out, fmt.Sprintf("mfa_constraint_%s_%s_uses_passcode %s %t", k, constraint.Type, hopeDelim, constraint.UsesPasscode)) + } + } + } else { // Token information only makes sense if no further MFA requirement (i.e. if we actually have a token) + out = append(out, fmt.Sprintf("token %s %s", hopeDelim, secret.Auth.ClientToken)) + out = append(out, fmt.Sprintf("token_accessor %s %s", hopeDelim, secret.Auth.Accessor)) + // If the lease duration is 0, it's likely a root token, so output the + // duration as "infinity" to clear things up. + if secret.Auth.LeaseDuration == 0 { + out = append(out, fmt.Sprintf("token_duration %s %s", hopeDelim, "∞")) + } else { + out = append(out, fmt.Sprintf("token_duration %s %v", hopeDelim, humanDurationInt(secret.Auth.LeaseDuration))) + } + out = append(out, fmt.Sprintf("token_renewable %s %t", hopeDelim, secret.Auth.Renewable)) + out = append(out, fmt.Sprintf("token_policies %s %q", hopeDelim, secret.Auth.TokenPolicies)) + out = append(out, fmt.Sprintf("identity_policies %s %q", hopeDelim, secret.Auth.IdentityPolicies)) + out = append(out, fmt.Sprintf("policies %s %q", hopeDelim, secret.Auth.Policies)) + for k, v := range secret.Auth.Metadata { + out = append(out, fmt.Sprintf("token_meta_%s %s %v", k, hopeDelim, v)) + } } } diff --git a/command/login.go b/command/login.go index 4a56075e4..fc2a0f7e6 100644 --- a/command/login.go +++ b/command/login.go @@ -240,6 +240,10 @@ func (c *LoginCommand) Run(args []string) int { c.UI.Warn(wrapAtLength("A login request was issued that is subject to "+ "MFA validation. Please make sure to validate the login by sending another "+ "request to sys/mfa/validate endpoint.") + "\n") + + // We return early to prevent success message from being printed + c.checkForAndWarnAboutLoginToken() + return OutputSecret(c.UI, secret) } // Unset any previous token wrapping functionality. If the original request @@ -302,15 +306,7 @@ func (c *LoginCommand) Run(args []string) int { return 2 } - // Warn if the VAULT_TOKEN environment variable is set, as that will take - // precedence. We output as a warning, so piping should still work since it - // will be on a different stream. - if os.Getenv("VAULT_TOKEN") != "" { - c.UI.Warn(wrapAtLength("WARNING! The VAULT_TOKEN environment variable "+ - "is set! This takes precedence over the value set by this command. To "+ - "use the value set by this command, unset the VAULT_TOKEN environment "+ - "variable or set it to the token displayed below.") + "\n") - } + c.checkForAndWarnAboutLoginToken() } else if !c.flagTokenOnly { // If token-only the user knows it won't be stored, so don't warn c.UI.Warn(wrapAtLength( @@ -372,3 +368,14 @@ func (c *LoginCommand) extractToken(client *api.Client, secret *api.Secret, unwr return nil, false, fmt.Errorf("no auth or wrapping info in response") } } + +// Warn if the VAULT_TOKEN environment variable is set, as that will take +// precedence. We output as a warning, so piping should still work since it +// will be on a different stream. +func (c *LoginCommand) checkForAndWarnAboutLoginToken() { + if os.Getenv("VAULT_TOKEN") != "" { + c.UI.Warn(wrapAtLength("WARNING! The VAULT_TOKEN environment variable "+ + "is set! The value of this variable will take precedence; if this is unwanted "+ + "please unset VAULT_TOKEN or update its value accordingly.") + "\n") + } +}