diff --git a/changelog/15482.txt b/changelog/15482.txt new file mode 100644 index 000000000..0dcfd6f34 --- /dev/null +++ b/changelog/15482.txt @@ -0,0 +1,3 @@ +```release-note:bug +auth: Prevent deleting a valid MFA method ID using the endpoint for a different MFA method type +``` diff --git a/vault/external_tests/mfa/login_mfa_test.go b/vault/external_tests/mfa/login_mfa_test.go index 427818640..8a2bdb5b2 100644 --- a/vault/external_tests/mfa/login_mfa_test.go +++ b/vault/external_tests/mfa/login_mfa_test.go @@ -187,6 +187,12 @@ func TestLoginMFA_Method_CRUD(t *testing.T) { t.Fatal("expected response id to match existing method id but it didn't") } + // delete with invalid path should fail + _, err = client.Logical().Delete(invalidPath) + if err == nil { + t.Fatal("expected deleting an MFA method ID with invalid path to fail") + } + // delete it _, err = client.Logical().Delete(myNewPath) if err != nil { diff --git a/vault/identity_store.go b/vault/identity_store.go index b3b01bb5e..3d8626b6d 100644 --- a/vault/identity_store.go +++ b/vault/identity_store.go @@ -221,7 +221,7 @@ func mfaPaths(i *IdentityStore) []*framework.Path { Summary: "Update or create a configuration for the given MFA method", }, logical.DeleteOperation: &framework.PathOperation{ - Callback: i.handleMFAMethodDelete, + Callback: i.handleMFAMethodTOTPDelete, Summary: "Delete a configuration for the given MFA method", }, }, @@ -335,7 +335,7 @@ func mfaPaths(i *IdentityStore) []*framework.Path { Summary: "Update or create a configuration for the given MFA method", }, logical.DeleteOperation: &framework.PathOperation{ - Callback: i.handleMFAMethodDelete, + Callback: i.handleMFAMethodOKTADelete, Summary: "Delete a configuration for the given MFA method", }, }, @@ -391,7 +391,7 @@ func mfaPaths(i *IdentityStore) []*framework.Path { Summary: "Update or create a configuration for the given MFA method", }, logical.DeleteOperation: &framework.PathOperation{ - Callback: i.handleMFAMethodDelete, + Callback: i.handleMFAMethodDUODelete, Summary: "Delete a configuration for the given MFA method", }, }, @@ -431,7 +431,7 @@ func mfaPaths(i *IdentityStore) []*framework.Path { Summary: "Update or create a configuration for the given MFA method", }, logical.DeleteOperation: &framework.PathOperation{ - Callback: i.handleMFAMethodDelete, + Callback: i.handleMFAMethodPingIDDelete, Summary: "Delete a configuration for the given MFA method", }, }, diff --git a/vault/login_mfa.go b/vault/login_mfa.go index cb30e5aab..d92cd6304 100644 --- a/vault/login_mfa.go +++ b/vault/login_mfa.go @@ -384,12 +384,28 @@ func (i *IdentityStore) handleMFAMethodPingIDUpdate(ctx context.Context, req *lo return i.handleMFAMethodUpdateCommon(ctx, req, d, mfaMethodTypePingID) } -func (i *IdentityStore) handleMFAMethodDelete(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { +func (i *IdentityStore) handleMFAMethodTOTPDelete(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + return i.handleMFAMethodDeleteCommon(ctx, req, d, mfaMethodTypeTOTP) +} + +func (i *IdentityStore) handleMFAMethodOKTADelete(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + return i.handleMFAMethodDeleteCommon(ctx, req, d, mfaMethodTypeOkta) +} + +func (i *IdentityStore) handleMFAMethodDUODelete(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + return i.handleMFAMethodDeleteCommon(ctx, req, d, mfaMethodTypeDuo) +} + +func (i *IdentityStore) handleMFAMethodPingIDDelete(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + return i.handleMFAMethodDeleteCommon(ctx, req, d, mfaMethodTypePingID) +} + +func (i *IdentityStore) handleMFAMethodDeleteCommon(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 } - return nil, i.mfaBackend.deleteMFAConfigByMethodID(ctx, methodID, memDBLoginMFAConfigsTable, loginMFAConfigPrefix) + return nil, i.mfaBackend.deleteMFAConfigByMethodID(ctx, methodID, methodType, memDBLoginMFAConfigsTable, loginMFAConfigPrefix) } func (i *IdentityStore) handleLoginMFAGenerateUpdate(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { @@ -2559,7 +2575,7 @@ func (b *LoginMFABackend) MemDBDeleteMFALoginEnforcementConfigByNameAndNamespace return nil } -func (b *LoginMFABackend) deleteMFAConfigByMethodID(ctx context.Context, configID, tableName, prefix string) error { +func (b *LoginMFABackend) deleteMFAConfigByMethodID(ctx context.Context, configID, methodType, tableName, prefix string) error { var err error if configID == "" { @@ -2601,6 +2617,10 @@ func (b *LoginMFABackend) deleteMFAConfigByMethodID(ctx context.Context, configI return nil } + if mConfig.Type != methodType { + return fmt.Errorf("method type does not match the MFA config type") + } + mfaNs, err := b.Core.NamespaceByID(ctx, mConfig.NamespaceID) if err != nil { return err