From 364f8789cd26a305cdaeb89e60c2f1eb6408fe8f Mon Sep 17 00:00:00 2001 From: Hamid Ghaf <83242695+hghaf099@users.noreply.github.com> Date: Tue, 17 May 2022 14:54:16 -0400 Subject: [PATCH] Globally scoped MFA method Get/List endpoints (#15248) * Globally scoped MFA method Get/List endpoints * Adding CL * minor changes * removing unwanted information from an error msg --- changelog/15248.txt | 3 + vault/external_tests/mfa/login_mfa_test.go | 125 +++++++++++++++++++++ vault/identity_store.go | 32 +++++- vault/login_mfa.go | 58 +++++++++- 4 files changed, 209 insertions(+), 9 deletions(-) create mode 100644 changelog/15248.txt diff --git a/changelog/15248.txt b/changelog/15248.txt new file mode 100644 index 000000000..9726d0770 --- /dev/null +++ b/changelog/15248.txt @@ -0,0 +1,3 @@ +```release-note:improvement +auth: Globally scoped Login MFA method Get/List endpoints +``` diff --git a/vault/external_tests/mfa/login_mfa_test.go b/vault/external_tests/mfa/login_mfa_test.go index 8a971ea63..427818640 100644 --- a/vault/external_tests/mfa/login_mfa_test.go +++ b/vault/external_tests/mfa/login_mfa_test.go @@ -45,6 +45,7 @@ func TestLoginMFA_Method_CRUD(t *testing.T) { testCases := []struct { methodName string + invalidType string configData map[string]interface{} keyToUpdate string valueToUpdate string @@ -53,6 +54,7 @@ func TestLoginMFA_Method_CRUD(t *testing.T) { }{ { "totp", + "duo", map[string]interface{}{ "issuer": "yCorp", "period": 10, @@ -70,6 +72,7 @@ func TestLoginMFA_Method_CRUD(t *testing.T) { }, { "duo", + "totp", map[string]interface{}{ "mount_accessor": mountAccessor, "secret_key": "lol-secret", @@ -83,6 +86,7 @@ func TestLoginMFA_Method_CRUD(t *testing.T) { }, { "okta", + "pingid", map[string]interface{}{ "mount_accessor": mountAccessor, "base_url": "example.com", @@ -96,6 +100,7 @@ func TestLoginMFA_Method_CRUD(t *testing.T) { }, { "pingid", + "okta", map[string]interface{}{ "mount_accessor": mountAccessor, "settings_file_base64": "I0F1dG8tR2VuZXJhdGVkIGZyb20gUGluZ09uZSwgZG93bmxvYWRlZCBieSBpZD1bU1NPXSBlbWFpbD1baGFtaWRAaGFzaGljb3JwLmNvbV0KI1dlZCBEZWMgMTUgMTM6MDg6NDQgTVNUIDIwMjEKdXNlX2Jhc2U2NF9rZXk9YlhrdGMyVmpjbVYwTFd0bGVRPT0KdXNlX3NpZ25hdHVyZT10cnVlCnRva2VuPWxvbC10b2tlbgppZHBfdXJsPWh0dHBzOi8vaWRweG55bDNtLnBpbmdpZGVudGl0eS5jb20vcGluZ2lkCm9yZ19hbGlhcz1sb2wtb3JnLWFsaWFzCmFkbWluX3VybD1odHRwczovL2lkcHhueWwzbS5waW5naWRlbnRpdHkuY29tL3BpbmdpZAphdXRoZW50aWNhdG9yX3VybD1odHRwczovL2F1dGhlbnRpY2F0b3IucGluZ29uZS5jb20vcGluZ2lkL3BwbQ==", @@ -165,6 +170,23 @@ func TestLoginMFA_Method_CRUD(t *testing.T) { } } + // read the id on another MFA type endpoint should fail + invalidPath := fmt.Sprintf("identity/mfa/method/%s/%s", tc.invalidType, methodId) + resp, err = client.Logical().Read(invalidPath) + if err == nil { + t.Fatal(err) + } + + // read the id globally should succeed + globalPath := fmt.Sprintf("identity/mfa/method/%s", methodId) + resp, err = client.Logical().Read(globalPath) + if err != nil { + t.Fatal(err) + } + if resp.Data["id"] != methodId { + t.Fatal("expected response id to match existing method id but it didn't") + } + // delete it _, err = client.Logical().Delete(myNewPath) if err != nil { @@ -180,6 +202,109 @@ func TestLoginMFA_Method_CRUD(t *testing.T) { } } +// TestLoginMFA_ListAllMFAConfigs tests listing all configs globally +func TestLoginMFA_ListAllMFAConfigsGlobally(t *testing.T) { + cluster := vault.NewTestCluster(t, &vault.CoreConfig{ + CredentialBackends: map[string]logical.Factory{ + "userpass": userpass.Factory, + }, + }, &vault.TestClusterOptions{ + HandlerFunc: vaulthttp.Handler, + }) + cluster.Start() + defer cluster.Cleanup() + + core := cluster.Cores[0].Core + vault.TestWaitActive(t, core) + client := cluster.Cores[0].Client + + // Enable userpass authentication + err := client.Sys().EnableAuthWithOptions("userpass", &api.EnableAuthOptions{ + Type: "userpass", + }) + if err != nil { + t.Fatalf("failed to enable userpass auth: %v", err) + } + + auths, err := client.Sys().ListAuth() + if err != nil { + t.Fatal(err) + } + mountAccessor := auths["userpass/"].Accessor + + mfaConfigs := []struct { + methodType string + configData map[string]interface{} + }{ + { + "totp", + map[string]interface{}{ + "issuer": "yCorp", + "period": 10, + "algorithm": "SHA1", + "digits": 6, + "skew": 1, + "key_size": uint(10), + "qr_size": 100, + "max_validation_attempts": 1, + }, + }, + { + "duo", + map[string]interface{}{ + "mount_accessor": mountAccessor, + "secret_key": "lol-secret", + "integration_key": "integration-key", + "api_hostname": "some-hostname", + }, + }, + { + "okta", + map[string]interface{}{ + "mount_accessor": mountAccessor, + "base_url": "example.com", + "org_name": "my-org", + "api_token": "lol-token", + }, + }, + { + "pingid", + map[string]interface{}{ + "mount_accessor": mountAccessor, + "settings_file_base64": "I0F1dG8tR2VuZXJhdGVkIGZyb20gUGluZ09uZSwgZG93bmxvYWRlZCBieSBpZD1bU1NPXSBlbWFpbD1baGFtaWRAaGFzaGljb3JwLmNvbV0KI1dlZCBEZWMgMTUgMTM6MDg6NDQgTVNUIDIwMjEKdXNlX2Jhc2U2NF9rZXk9YlhrdGMyVmpjbVYwTFd0bGVRPT0KdXNlX3NpZ25hdHVyZT10cnVlCnRva2VuPWxvbC10b2tlbgppZHBfdXJsPWh0dHBzOi8vaWRweG55bDNtLnBpbmdpZGVudGl0eS5jb20vcGluZ2lkCm9yZ19hbGlhcz1sb2wtb3JnLWFsaWFzCmFkbWluX3VybD1odHRwczovL2lkcHhueWwzbS5waW5naWRlbnRpdHkuY29tL3BpbmdpZAphdXRoZW50aWNhdG9yX3VybD1odHRwczovL2F1dGhlbnRpY2F0b3IucGluZ29uZS5jb20vcGluZ2lkL3BwbQ==", + }, + }, + } + + var methodIDs []interface{} + for _, method := range mfaConfigs { + // create a new method config + myPath := fmt.Sprintf("identity/mfa/method/%s", method.methodType) + resp, err := client.Logical().Write(myPath, method.configData) + if err != nil { + t.Fatal(err) + } + + methodId := resp.Data["method_id"] + if methodId == "" { + t.Fatal("method id is empty") + } + methodIDs = append(methodIDs, methodId) + } + // listing should show it + resp, err := client.Logical().List("identity/mfa/method") + if err != nil || resp == nil { + t.Fatal(err) + } + + if len(resp.Data["keys"].([]interface{})) != len(methodIDs) { + t.Fatalf("global list request did not return all MFA method IDs") + } + if len(resp.Data["key_info"].(map[string]interface{})) != len(methodIDs) { + t.Fatal("global list request did not return all MFA method configurations") + } +} + // TestLoginMFA_LoginEnforcement_CRUD tests creating/reading/updating/deleting a login enforcement config func TestLoginMFA_LoginEnforcement_CRUD(t *testing.T) { cluster := vault.NewTestCluster(t, &vault.CoreConfig{ diff --git a/vault/identity_store.go b/vault/identity_store.go index c434bf9dc..f42c31999 100644 --- a/vault/identity_store.go +++ b/vault/identity_store.go @@ -141,6 +141,30 @@ func (i *IdentityStore) paths() []*framework.Path { func mfaPaths(i *IdentityStore) []*framework.Path { return []*framework.Path{ + { + Pattern: "mfa/method" + genericOptionalUUIDRegex("method_id"), + Fields: map[string]*framework.FieldSchema{ + "method_id": { + Type: framework.TypeString, + Description: `The unique identifier for this MFA method.`, + }, + }, + Operations: map[logical.Operation]framework.OperationHandler{ + logical.ReadOperation: &framework.PathOperation{ + Callback: i.handleMFAMethodReadGlobal, + Summary: "Read the current configuration for the given ID regardless of the MFA method type", + }, + }, + }, + { + Pattern: "mfa/method/?$", + Operations: map[logical.Operation]framework.OperationHandler{ + logical.ListOperation: &framework.PathOperation{ + Callback: i.handleMFAMethodListGlobal, + Summary: "List MFA method configurations for all MFA methods", + }, + }, + }, { Pattern: "mfa/method/totp" + genericOptionalUUIDRegex("method_id"), Fields: map[string]*framework.FieldSchema{ @@ -189,7 +213,7 @@ func mfaPaths(i *IdentityStore) []*framework.Path { }, Operations: map[logical.Operation]framework.OperationHandler{ logical.ReadOperation: &framework.PathOperation{ - Callback: i.handleMFAMethodRead, + Callback: i.handleMFAMethodTOTPRead, Summary: "Read the current configuration for the given MFA method", }, logical.UpdateOperation: &framework.PathOperation{ @@ -303,7 +327,7 @@ func mfaPaths(i *IdentityStore) []*framework.Path { }, Operations: map[logical.Operation]framework.OperationHandler{ logical.ReadOperation: &framework.PathOperation{ - Callback: i.handleMFAMethodRead, + Callback: i.handleMFAMethodOKTARead, Summary: "Read the current configuration for the given MFA method", }, logical.UpdateOperation: &framework.PathOperation{ @@ -359,7 +383,7 @@ func mfaPaths(i *IdentityStore) []*framework.Path { }, Operations: map[logical.Operation]framework.OperationHandler{ logical.ReadOperation: &framework.PathOperation{ - Callback: i.handleMFAMethodRead, + Callback: i.handleMFAMethodDuoRead, Summary: "Read the current configuration for the given MFA method", }, logical.UpdateOperation: &framework.PathOperation{ @@ -399,7 +423,7 @@ func mfaPaths(i *IdentityStore) []*framework.Path { }, Operations: map[logical.Operation]framework.OperationHandler{ logical.ReadOperation: &framework.PathOperation{ - Callback: i.handleMFAMethodRead, + Callback: i.handleMFAMethodPingIDRead, Summary: "Read the current configuration for the given MFA method", }, logical.UpdateOperation: &framework.PathOperation{ diff --git a/vault/login_mfa.go b/vault/login_mfa.go index 6d6163ced..aab6dbe42 100644 --- a/vault/login_mfa.go +++ b/vault/login_mfa.go @@ -182,6 +182,15 @@ func (i *IdentityStore) handleMFAMethodListPingID(ctx context.Context, req *logi return i.handleMFAMethodList(ctx, req, d, mfaMethodTypePingID) } +func (i *IdentityStore) handleMFAMethodListGlobal(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + keys, configInfo, err := i.mfaBackend.mfaMethodList(ctx, "") + if err != nil { + return nil, err + } + + return logical.ListResponseWithInfo(keys, configInfo), nil +} + func (i *IdentityStore) handleMFAMethodList(ctx context.Context, req *logical.Request, d *framework.FieldData, methodType string) (*logical.Response, error) { keys, configInfo, err := i.mfaBackend.mfaMethodList(ctx, methodType) if err != nil { @@ -191,7 +200,27 @@ func (i *IdentityStore) handleMFAMethodList(ctx context.Context, req *logical.Re return logical.ListResponseWithInfo(keys, configInfo), nil } -func (i *IdentityStore) handleMFAMethodRead(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { +func (i *IdentityStore) handleMFAMethodTOTPRead(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + return i.handleMFAMethodReadCommon(ctx, req, d, mfaMethodTypeTOTP) +} + +func (i *IdentityStore) handleMFAMethodOKTARead(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + return i.handleMFAMethodReadCommon(ctx, req, d, mfaMethodTypeOkta) +} + +func (i *IdentityStore) handleMFAMethodDuoRead(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + return i.handleMFAMethodReadCommon(ctx, req, d, mfaMethodTypeDuo) +} + +func (i *IdentityStore) handleMFAMethodPingIDRead(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + return i.handleMFAMethodReadCommon(ctx, req, d, mfaMethodTypePingID) +} + +func (i *IdentityStore) handleMFAMethodReadGlobal(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + return i.handleMFAMethodReadCommon(ctx, req, d, "") +} + +func (i *IdentityStore) handleMFAMethodReadCommon(ctx context.Context, req *logical.Request, d *framework.FieldData, methodType string) (*logical.Response, error) { methodID := d.Get("method_id").(string) if methodID == "" { return logical.ErrorResponse("missing method ID"), nil @@ -220,6 +249,11 @@ func (i *IdentityStore) handleMFAMethodRead(ctx context.Context, req *logical.Re if !(ns.ID == mfaNs.ID || mfaNs.HasParent(ns) || ns.HasParent(mfaNs)) { return logical.ErrorResponse("request namespace does not match method namespace"), logical.ErrPermissionDenied } + + if methodType != "" && respData["type"] != methodType { + return logical.ErrorResponse("failed to find the method ID under MFA type %s.", methodType), nil + } + return &logical.Response{ Data: respData, }, nil @@ -460,6 +494,10 @@ func (i *IdentityStore) handleLoginMFAAdminDestroyUpdate(ctx context.Context, re return nil, fmt.Errorf("configuration for method ID %q does not contain an identifier", methodID) } + if mConfig.Type != mfaMethodTypeTOTP { + return nil, fmt.Errorf("method ID does not match TOTP type") + } + ns, err := namespace.FromContext(ctx) if err != nil { return logical.ErrorResponse("failed to retrieve the namespace"), nil @@ -1188,10 +1226,20 @@ func (b *LoginMFABackend) mfaMethodList(ctx context.Context, methodType string) ws := memdb.NewWatchSet() txn := b.db.Txn(false) - // get all the configs for the given type - iter, err := txn.Get(b.methodTable, "type", methodType) - if err != nil { - return nil, nil, fmt.Errorf("failed to fetch iterator for login mfa method configs in memdb: %w", err) + var iter memdb.ResultIterator + switch { + case methodType == "": + // get all the configs + iter, err = txn.Get(b.methodTable, "id") + if err != nil { + return nil, nil, fmt.Errorf("failed to fetch iterator for login mfa method configs in memdb: %w", err) + } + default: + // get all the configs for the given type + iter, err = txn.Get(b.methodTable, "type", methodType) + if err != nil { + return nil, nil, fmt.Errorf("failed to fetch iterator for login mfa method configs in memdb: %w", err) + } } ws.Add(iter.WatchCh())