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

interactive CLI for mfa login #14131

Merged
merged 20 commits into from
Feb 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelog/14131.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
cli: interactive CLI for login mfa
```
8 changes: 8 additions & 0 deletions command/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ type BaseCommand struct {
flagFormat string
flagField string
flagOutputCurlString bool
flagNonInteractive bool

flagMFA []string

Expand Down Expand Up @@ -393,6 +394,13 @@ func (c *BaseCommand) flagSet(bit FlagSetBit) *FlagSets {
"This can be specified multiple times.",
})

f.BoolVar(&BoolVar{
Name: "non-interactive",
Target: &c.flagNonInteractive,
Default: false,
raskchanky marked this conversation as resolved.
Show resolved Hide resolved
Usage: "When set true, prevents asking the user for input via the terminal.",
})

}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we also use github.com/mattn/go-isatty to override the var to true when we can't ask the user for input (because there's no tty)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is less desirable, because then if the user didn't say -non-interactive, but they're running without a terminal, we'll simply block waiting for input on stdin. This will be a surprising change and it won't be obvious what the problem is.

if bit&(FlagSetOutputField|FlagSetOutputFormat) != 0 {
Expand Down
104 changes: 103 additions & 1 deletion command/write.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"os"
"strings"

"github.com/hashicorp/vault/sdk/logical"
"github.com/mattn/go-isatty"
"github.com/mitchellh/cli"
"github.com/posener/complete"
)
Expand All @@ -15,6 +17,13 @@ var (
_ cli.CommandAutocomplete = (*WriteCommand)(nil)
)

// MFAMethodInfo contains the information about an MFA method
type MFAMethodInfo struct {
methodID string
methodType string
usePasscode bool
}

// WriteCommand is a Command that puts data into the Vault.
type WriteCommand struct {
*BaseCommand
Expand Down Expand Up @@ -147,10 +156,103 @@ func (c *WriteCommand) Run(args []string) int {
}

if secret != nil && secret.Auth != nil && secret.Auth.MFARequirement != nil {
if c.isInteractiveEnabled(len(secret.Auth.MFARequirement.MFAConstraints)) {
// Currently, if there is only one MFA method configured, the login
// request is validated interactively
methodInfo := c.getMFAMethodInfo(secret.Auth.MFARequirement.MFAConstraints)
if methodInfo.methodID != "" {
return c.validateMFA(secret.Auth.MFARequirement.MFARequestID, methodInfo)
}
}
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 mfa/validate endpoint.") + "\n")
"request to sys/mfa/validate endpoint.") + "\n")
}

// Handle single field output
if c.flagField != "" {
return PrintRawField(c.UI, secret, c.flagField)
}

return OutputSecret(c.UI, secret)
}

func (c *WriteCommand) isInteractiveEnabled(mfaConstraintLen int) bool {
if mfaConstraintLen != 1 || !isatty.IsTerminal(os.Stdin.Fd()) {
return false
}

if !c.flagNonInteractive {
return true
}

return false
}

// getMFAMethodInfo returns MFA method information only if one MFA method is
// configured.
func (c *WriteCommand) getMFAMethodInfo(mfaConstraintAny map[string]*logical.MFAConstraintAny) MFAMethodInfo {
for _, mfaConstraint := range mfaConstraintAny {
if len(mfaConstraint.Any) != 1 {
return MFAMethodInfo{}
}

return MFAMethodInfo{
methodType: mfaConstraint.Any[0].Type,
methodID: mfaConstraint.Any[0].ID,
usePasscode: mfaConstraint.Any[0].UsesPasscode,
}
}

return MFAMethodInfo{}
}

func (c *WriteCommand) validateMFA(reqID string, methodInfo MFAMethodInfo) int {
var passcode string
var err error
if methodInfo.usePasscode {
passcode, err = c.UI.AskSecret(fmt.Sprintf("Enter the passphrase for methodID %q of type %q:", methodInfo.methodID, methodInfo.methodType))
if err != nil {
c.UI.Error(fmt.Sprintf("failed to read the passphrase with error %q. please validate the login by sending a request to sys/mfa/validate", err.Error()))
return 2
}
} else {
c.UI.Warn("Asking Vault to perform MFA validation with upstream service. " +
"You should receive a push notification in your authenticator app shortly")
}

// passcode could be an empty string
mfaPayload := map[string][]string{
methodInfo.methodID: {passcode},
}

client, err := c.Client()
if err != nil {
c.UI.Error(err.Error())
return 2
}

path := "sys/mfa/validate"

secret, err := client.Logical().Write(path, map[string]interface{}{
"mfa_request_id": reqID,
"mfa_payload": mfaPayload,
})
if err != nil {
c.UI.Error(err.Error())
if secret != nil {
OutputSecret(c.UI, secret)
}
return 2
}
if secret == nil {
// Don't output anything unless using the "table" format
if Format(c.UI) == "table" {
c.UI.Info(fmt.Sprintf("Success! Data written to: %s", path))
}
return 0
}

// Handle single field output
if c.flagField != "" {
return PrintRawField(c.UI, secret, c.flagField)
Expand Down
1 change: 0 additions & 1 deletion vault/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -2097,7 +2097,6 @@ func (s standardUnsealStrategy) unseal(ctx context.Context, logger log.Logger, c
if err := c.setupQuotas(ctx, false); err != nil {
return err
}

c.setupCachedMFAResponseAuth()

if err := c.setupHeaderHMACKey(ctx, false); err != nil {
Expand Down
4 changes: 0 additions & 4 deletions vault/core_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,6 @@ func (c *Core) collectNamespaces() []*namespace.Namespace {
}
}

func (c *Core) namepaceByPath(string) *namespace.Namespace {
return namespace.RootNamespace
}

func (c *Core) HasWALState(required *logical.WALState, perfStandby bool) bool {
return true
}
Expand Down
60 changes: 48 additions & 12 deletions vault/external_tests/identity/login_mfa_totp_test.go
Original file line number Diff line number Diff line change
@@ -1,42 +1,60 @@
package identity

import (
"context"
"fmt"
"strings"
"testing"
"time"

"github.com/hashicorp/vault/api"
"github.com/hashicorp/vault/audit"
"github.com/hashicorp/vault/builtin/credential/userpass"
"github.com/hashicorp/vault/builtin/logical/totp"
vaulthttp "github.com/hashicorp/vault/http"
"github.com/hashicorp/vault/sdk/logical"
"github.com/hashicorp/vault/vault"
)

var loginMFACoreConfig = &vault.CoreConfig{
CredentialBackends: map[string]logical.Factory{
"userpass": userpass.Factory,
},
LogicalBackends: map[string]logical.Factory{
"totp": totp.Factory,
func TestLoginMfaGenerateTOTPTestAuditIncluded(t *testing.T) {
var noop *vault.NoopAudit

cluster := vault.NewTestCluster(t, &vault.CoreConfig{
CredentialBackends: map[string]logical.Factory{
"userpass": userpass.Factory,
},
LogicalBackends: map[string]logical.Factory{
"totp": totp.Factory,
},
AuditBackends: map[string]audit.Factory{
"noop": func(ctx context.Context, config *audit.BackendConfig) (audit.Backend, error) {
noop = &vault.NoopAudit{
Config: config,
}
return noop, nil
},
},
},
}
&vault.TestClusterOptions{
HandlerFunc: vaulthttp.Handler,
})

func TestLoginMfaGenerateTOTPRoleTest(t *testing.T) {
cluster := vault.NewTestCluster(t, loginMFACoreConfig, &vault.TestClusterOptions{
HandlerFunc: vaulthttp.Handler,
})
cluster.Start()
defer cluster.Cleanup()

client := cluster.Cores[0].Client

// Enable the audit backend
err := client.Sys().EnableAuditWithOptions("noop", &api.EnableAuditOptions{Type: "noop"})
if err != nil {
t.Fatal(err)
}

// Mount the TOTP backend
mountInfo := &api.MountInput{
Type: "totp",
}
err := client.Sys().Mount("totp", mountInfo)
err = client.Sys().Mount("totp", mountInfo)
if err != nil {
t.Fatalf("failed to mount totp backend: %v", err)
}
Expand Down Expand Up @@ -254,6 +272,24 @@ func TestLoginMfaGenerateTOTPRoleTest(t *testing.T) {
t.Fatalf("MFA failed: %v", err)
}

if secret.Auth == nil || secret.Auth.ClientToken == "" {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm a bit confused by these test changes - are they related to the new rest of the PR? It looks like we're just checking in the audit log to see if the explicit write we did to sys/mfa/validate resulted in an audit record.

Copy link
Contributor Author

@hghaf099 hghaf099 Feb 24, 2022

Choose a reason for hiding this comment

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

no, I just did not want to push another PR for it. I have made a note in the PR description about it.

t.Fatalf("successful mfa validation did not return a client token")
}

if noop.Req == nil {
t.Fatalf("no request was logged in audit log")
}
var found bool
for _, req := range noop.Req {
if req.Path == "sys/mfa/validate" {
found = true
break
}
}
if !found {
t.Fatalf("mfa/validate was not logged in audit log")
}

// check for login request expiration
secret, err = user2Client.Logical().Write("auth/userpass/login/testuser", map[string]interface{}{
"password": "testpassword",
Expand Down