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 14 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
91 changes: 91 additions & 0 deletions command/write.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
package command

import (
"bufio"
"fmt"
"io"
"os"
"strings"
"syscall"

"golang.org/x/term"

"github.com/mitchellh/cli"
"github.com/posener/complete"
Expand Down Expand Up @@ -130,6 +134,7 @@ func (c *WriteCommand) Run(args []string) int {
return 2
}

WRITE:
secret, err := client.Logical().Write(path, data)
if err != nil {
c.UI.Error(fmt.Sprintf("Error writing data to %s: %s", path, err))
Expand All @@ -150,11 +155,97 @@ func (c *WriteCommand) 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 mfa/validate endpoint.") + "\n")
ok := YesNoPrompt("Would you like to interactively validate MFA methods?", true)
if ok {
mfaPayload := make(map[string][]string, 0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you extract this into a func so we can use return instead of goto?

for name, mfaConstraintAny := range secret.Auth.MFARequirement.MFAConstraints {
methodIDs := make([]string, 0)
for _, m := range mfaConstraintAny.Any {
methodIDs = append(methodIDs, m.ID)
}
mfaMethodID := StringPrompt(fmt.Sprintf("From MFARequirement %q, please select one of the following methodIDs %q:\n", name, methodIDs))
if mfaMethodID == "" {
c.UI.Warn("Invalid method ID detected, please validate the login by sending a request to mfa/validate")
goto OUTPUT
}
passcode, err := PasswordPrompt(fmt.Sprintf("Please insert the passcode for methodID %q: ", mfaMethodID))
if err != nil {
c.UI.Error(fmt.Sprintf("Failed to read the passcode with error %q. please validate the login by sending a request to mfa/validate.", err.Error()))
goto OUTPUT
}
// passcode could be an empty string
mfaPayload[mfaMethodID] = []string{passcode}
}

if len(mfaPayload) == 0 {
c.UI.Error("did not get any input, please validate the login by sending a request to mfa/validate")
goto OUTPUT
}

// updating data and path
data = map[string]interface{}{
"mfa_request_id": secret.Auth.MFARequirement.MFARequestID,
"mfa_payload": mfaPayload,
}
path = "sys/mfa/validate"
goto WRITE
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would rather see the write code repeated than have this goto.

}
}

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

return OutputSecret(c.UI, secret)
}

func StringPrompt(label string) string {
var s string
r := bufio.NewReader(os.Stdin)
for {
fmt.Fprint(os.Stderr, label+" ")
s, _ = r.ReadString('\n')
if s != "" {
break
}
}
return strings.TrimSpace(s)
}

func YesNoPrompt(label string, def bool) bool {
choices := "Y/n"
if !def {
choices = "y/N"
}

r := bufio.NewReader(os.Stdin)
var s string

for {
fmt.Fprintf(os.Stderr, "%s (%s) ", label, choices)
s, _ = r.ReadString('\n')
s = strings.TrimSpace(s)
if s == "" {
return def
}
s = strings.ToLower(s)
if s == "y" || s == "yes" {
return true
}
if s == "n" || s == "no" {
return false
}
}
}

func PasswordPrompt(label string) (string, error) {
fmt.Fprint(os.Stderr, label+" ")
b, err := term.ReadPassword(int(syscall.Stdin))
if err != nil {
return "", fmt.Errorf("failed to read the password")
}
fmt.Println()
return string(b), nil
}
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