Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VAULT-5885: Fix erroneous success message in case of two-phase MFA, and provide MFA information in table format #15428

Merged
merged 7 commits into from
May 17, 2022
3 changes: 3 additions & 0 deletions changelog/15428.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: i think you can have multiple blocks here to make it a little easier to parse (maybe one block per thing?)

see changelog/_1622.txt for an example

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, good suggestion, I didn't see any example of this in any changelogs that I looked at, I'll make this change :)

auth: Fixed erroneous success message and token info when using vault login in case of two-phase MFA, and fixed MFA information missing from table format when using vault login.
```
41 changes: 26 additions & 15 deletions command/format.go
Original file line number Diff line number Diff line change
Expand Up @@ -385,21 +385,32 @@ func (t TableFormatter) OutputSecret(ui cli.Ui, secret *api.Secret) error {
}

if secret.Auth != nil {
hghaf099 marked this conversation as resolved.
Show resolved Hide resolved
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 {
hghaf099 marked this conversation as resolved.
Show resolved Hide resolved
out = append(out, fmt.Sprintf("mfa_request_id %s %s", hopeDelim, secret.Auth.MFARequirement.MFARequestID))

for k, constraintSet := range secret.Auth.MFARequirement.MFAConstraints {
hghaf099 marked this conversation as resolved.
Show resolved Hide resolved
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))
hghaf099 marked this conversation as resolved.
Show resolved Hide resolved
}
}
} 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))
}
}
}

Expand Down
25 changes: 16 additions & 9 deletions command/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
swayne275 marked this conversation as resolved.
Show resolved Hide resolved
c.checkForAndWarnAboutLoginToken()
hghaf099 marked this conversation as resolved.
Show resolved Hide resolved
return OutputSecret(c.UI, secret)
}

// Unset any previous token wrapping functionality. If the original request
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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() {
swayne275 marked this conversation as resolved.
Show resolved Hide resolved
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")
}
}