Delete the base auth account when a concrete auth account is deleted (#2528)
This commit is contained in:
6 files changed
+295
-1
No files matched your search
+6
-1
@@ -2,7 +2,12 @@
|
||||
|
||||
Canonical reference for changes, improvements, and bugfixes for Boundary.
|
||||
|
||||
## Next
|
||||
## Next
|
||||
|
||||
### Bug Fixes
|
||||
|
||||
* accounts: Deleted auth accounts would still show up as being associated with
|
||||
a User when reading the User ([PR](https://github.com/hashicorp/boundary/pull/2528))
|
||||
|
||||
## 0.11.0 (2022/09/27)
|
||||
|
||||
|
||||
@@ -0,0 +1,33 @@
|
||||
begin;
|
||||
|
||||
-- This deletes the base type if the concrete types exist anymore.
|
||||
delete from auth_account
|
||||
where public_id not in
|
||||
(select public_id
|
||||
from auth_oidc_account)
|
||||
and public_id not in
|
||||
(select public_id
|
||||
from auth_password_account);
|
||||
|
||||
-- delete_auth_account_subtype() is an after trigger function for subytypes of
|
||||
-- auth_account.
|
||||
-- this function is similar to the delete_auth_method_subtype account created
|
||||
-- in 2/10_auth.up.sql
|
||||
create function delete_auth_account_subtype() returns trigger
|
||||
as $$
|
||||
begin
|
||||
delete from auth_account
|
||||
where public_id = old.public_id;
|
||||
return null; -- results are ignore since this is an after trigger.
|
||||
end;
|
||||
$$ language plpgsql;
|
||||
comment on function delete_auth_account_subtype is
|
||||
'delete_auth_account_subtype() is an after trigger function for subytypes of auth_account';
|
||||
|
||||
create trigger delete_auth_account_subtype after delete on auth_oidc_account
|
||||
for each row execute procedure delete_auth_account_subtype();
|
||||
|
||||
create trigger delete_auth_account_subtype after delete on auth_password_account
|
||||
for each row execute procedure delete_auth_account_subtype();
|
||||
|
||||
commit;
|
||||
@@ -0,0 +1,185 @@
|
||||
package oss_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"crypto/rand"
|
||||
"testing"
|
||||
|
||||
"github.com/hashicorp/boundary/internal/auth/oidc"
|
||||
"github.com/hashicorp/boundary/internal/auth/password"
|
||||
"github.com/hashicorp/boundary/internal/db"
|
||||
"github.com/hashicorp/boundary/internal/db/common"
|
||||
"github.com/hashicorp/boundary/internal/db/schema"
|
||||
"github.com/hashicorp/boundary/internal/iam"
|
||||
"github.com/hashicorp/boundary/internal/kms"
|
||||
"github.com/hashicorp/boundary/internal/types/scope"
|
||||
"github.com/hashicorp/boundary/testing/dbtest"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func TestMigrations_DeleteOrphanedAccounts(t *testing.T) {
|
||||
const (
|
||||
priorMigration = 55001
|
||||
currentMigration = 56001
|
||||
)
|
||||
|
||||
t.Parallel()
|
||||
ctx := context.Background()
|
||||
dialect := dbtest.Postgres
|
||||
|
||||
c, u, _, err := dbtest.StartUsingTemplate(dialect, dbtest.WithTemplate(dbtest.Template1))
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() {
|
||||
require.NoError(t, c())
|
||||
})
|
||||
d, err := common.SqlOpen(dialect, u)
|
||||
require.NoError(t, err)
|
||||
|
||||
// migration to the prior migration (before the one we want to test)
|
||||
m, err := schema.NewManager(ctx, schema.Dialect(dialect), d, schema.WithEditions(
|
||||
schema.TestCreatePartialEditions(schema.Dialect(dialect), schema.PartialEditions{"oss": priorMigration}),
|
||||
))
|
||||
require.NoError(t, err)
|
||||
|
||||
_, err = m.ApplyMigrations(ctx)
|
||||
require.NoError(t, err)
|
||||
state, err := m.CurrentState(ctx)
|
||||
require.NoError(t, err)
|
||||
want := &schema.State{
|
||||
Initialized: true,
|
||||
Editions: []schema.EditionState{
|
||||
{
|
||||
Name: "oss",
|
||||
BinarySchemaVersion: priorMigration,
|
||||
DatabaseSchemaVersion: priorMigration,
|
||||
DatabaseSchemaState: schema.Equal,
|
||||
},
|
||||
},
|
||||
}
|
||||
require.Equal(t, want, state)
|
||||
|
||||
// Get a connection
|
||||
dbType, err := db.StringToDbType(dialect)
|
||||
require.NoError(t, err)
|
||||
conn, err := db.Open(ctx, dbType, u)
|
||||
require.NoError(t, err)
|
||||
|
||||
wrapper := db.TestWrapper(t)
|
||||
rw := db.New(conn)
|
||||
kmsCache := kms.TestKms(t, conn, wrapper)
|
||||
err = kmsCache.CreateKeys(context.Background(), scope.Global.String(), kms.WithRandomReader(rand.Reader))
|
||||
require.NoError(t, err)
|
||||
|
||||
// Create the user we will associate accounts with
|
||||
iamRepo := iam.TestRepo(t, conn, wrapper)
|
||||
usr, err := iam.NewUser(scope.Global.String())
|
||||
require.NoError(t, err)
|
||||
usr, err = iamRepo.CreateUser(ctx, usr)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Create the accounts we will delete and assert their behavior
|
||||
pwRepo, err := password.NewRepository(rw, rw, kmsCache)
|
||||
require.NoError(t, err)
|
||||
|
||||
pwAm1, err := password.NewAuthMethod(scope.Global.String())
|
||||
require.NoError(t, err)
|
||||
pwAm1, err = pwRepo.CreateAuthMethod(ctx, pwAm1)
|
||||
require.NoError(t, err)
|
||||
pwAcct1, err := password.NewAccount(pwAm1.GetPublicId(), password.WithLoginName("account1"), password.WithPassword("password"))
|
||||
require.NoError(t, err)
|
||||
pwAcct1, err = pwRepo.CreateAccount(ctx, scope.Global.String(), pwAcct1)
|
||||
require.NoError(t, err)
|
||||
|
||||
pwAm2, err := password.NewAuthMethod(scope.Global.String())
|
||||
require.NoError(t, err)
|
||||
pwAm2, err = pwRepo.CreateAuthMethod(ctx, pwAm2)
|
||||
require.NoError(t, err)
|
||||
pwAcct2, err := password.NewAccount(pwAm2.GetPublicId(), password.WithLoginName("account2"), password.WithPassword("password"))
|
||||
require.NoError(t, err)
|
||||
pwAcct2, err = pwRepo.CreateAccount(ctx, scope.Global.String(), pwAcct2)
|
||||
require.NoError(t, err)
|
||||
|
||||
oidcRepo, err := oidc.NewRepository(ctx, rw, rw, kmsCache)
|
||||
require.NoError(t, err)
|
||||
databaseWrapper, err := kmsCache.GetWrapper(context.Background(), scope.Global.String(), kms.KeyPurposeDatabase)
|
||||
oidcAm1 := oidc.TestAuthMethod(t, conn, databaseWrapper, scope.Global.String(), oidc.InactiveState, "alice_rp1",
|
||||
"my-dogs-name", oidc.WithIssuer(oidc.TestConvertToUrls(t, "https://alice1.com")[0]), oidc.WithApiUrl(oidc.TestConvertToUrls(t, "https://api1.com")[0]))
|
||||
oidcAcct1, err := oidc.NewAccount(ctx, oidcAm1.GetPublicId(), "oidcAcct1")
|
||||
require.NoError(t, err)
|
||||
oidcAcct1, err = oidcRepo.CreateAccount(ctx, scope.Global.String(), oidcAcct1)
|
||||
require.NoError(t, err)
|
||||
oidcAm2 := oidc.TestAuthMethod(t, conn, databaseWrapper, scope.Global.String(), oidc.InactiveState, "alice_rp2",
|
||||
"my-dogs-name", oidc.WithIssuer(oidc.TestConvertToUrls(t, "https://alice2.com")[0]), oidc.WithApiUrl(oidc.TestConvertToUrls(t, "https://api2.com")[0]))
|
||||
oidcAcct2, err := oidc.NewAccount(ctx, oidcAm2.GetPublicId(), "oidcAcct2")
|
||||
require.NoError(t, err)
|
||||
oidcAcct2, err = oidcRepo.CreateAccount(ctx, scope.Global.String(), oidcAcct2)
|
||||
require.NoError(t, err)
|
||||
|
||||
_, err = iamRepo.AddUserAccounts(ctx, usr.GetPublicId(), usr.GetVersion(),
|
||||
[]string{
|
||||
pwAcct1.GetPublicId(), pwAcct2.GetPublicId(),
|
||||
oidcAcct1.GetPublicId(), oidcAcct2.GetPublicId(),
|
||||
})
|
||||
require.NoError(t, err)
|
||||
usr, accts, err := iamRepo.LookupUser(ctx, usr.GetPublicId())
|
||||
require.NoError(t, err)
|
||||
assert.ElementsMatch(t, []string{
|
||||
pwAcct1.GetPublicId(), pwAcct2.GetPublicId(),
|
||||
oidcAcct1.GetPublicId(), oidcAcct2.GetPublicId(),
|
||||
}, accts)
|
||||
|
||||
_, err = pwRepo.DeleteAccount(ctx, scope.Global.String(), pwAcct1.GetPublicId())
|
||||
require.NoError(t, err)
|
||||
_, err = oidcRepo.DeleteAccount(ctx, scope.Global.String(), oidcAcct1.GetPublicId())
|
||||
require.NoError(t, err)
|
||||
|
||||
// pwAcct1 is still listed as an account for this user! oh no!
|
||||
_, accts, err = iamRepo.LookupUser(ctx, usr.GetPublicId())
|
||||
require.NoError(t, err)
|
||||
assert.ElementsMatch(t, []string{
|
||||
pwAcct1.GetPublicId(), pwAcct2.GetPublicId(),
|
||||
oidcAcct1.GetPublicId(), oidcAcct2.GetPublicId(),
|
||||
}, accts)
|
||||
|
||||
// now we're ready for the migration we want to test.
|
||||
m, err = schema.NewManager(ctx, schema.Dialect(dialect), d, schema.WithEditions(
|
||||
schema.TestCreatePartialEditions(schema.Dialect(dialect), schema.PartialEditions{"oss": currentMigration}),
|
||||
))
|
||||
require.NoError(t, err)
|
||||
|
||||
_, err = m.ApplyMigrations(ctx)
|
||||
require.NoError(t, err)
|
||||
state, err = m.CurrentState(ctx)
|
||||
require.NoError(t, err)
|
||||
want = &schema.State{
|
||||
Initialized: true,
|
||||
Editions: []schema.EditionState{
|
||||
{
|
||||
Name: "oss",
|
||||
BinarySchemaVersion: currentMigration,
|
||||
DatabaseSchemaVersion: currentMigration,
|
||||
DatabaseSchemaState: schema.Equal,
|
||||
},
|
||||
},
|
||||
}
|
||||
require.Equal(t, want, state)
|
||||
|
||||
// pwAcct1 is no longer listed as an account for this user. Phew!
|
||||
_, accts, err = iamRepo.LookupUser(ctx, usr.GetPublicId())
|
||||
require.NoError(t, err)
|
||||
assert.ElementsMatch(t, []string{pwAcct2.GetPublicId(), oidcAcct2.GetPublicId()}, accts)
|
||||
|
||||
_, err = pwRepo.DeleteAccount(ctx, scope.Global.String(), pwAcct2.GetPublicId())
|
||||
require.NoError(t, err)
|
||||
_, accts, err = iamRepo.LookupUser(ctx, usr.GetPublicId())
|
||||
require.NoError(t, err)
|
||||
assert.ElementsMatch(t, []string{oidcAcct2.GetPublicId()}, accts)
|
||||
_, err = oidcRepo.DeleteAccount(ctx, scope.Global.String(), oidcAcct2.GetPublicId())
|
||||
require.NoError(t, err)
|
||||
|
||||
// Deleting the account now removes the association with the user
|
||||
_, accts, err = iamRepo.LookupUser(ctx, usr.GetPublicId())
|
||||
require.NoError(t, err)
|
||||
assert.Empty(t, accts)
|
||||
}
|
||||
@@ -23,6 +23,7 @@ TESTS ?= tests/setup/*.sql \
|
||||
tests/sentinel/*.sql \
|
||||
tests/credential/*/*.sql \
|
||||
tests/session/*.sql \
|
||||
tests/account/*/*.sql \
|
||||
tests/target/*.sql \
|
||||
tests/controller/*.sql
|
||||
|
||||
|
||||
@@ -0,0 +1,35 @@
|
||||
-- account tests triggers:
|
||||
-- insert_auth_account_subtype
|
||||
-- delete_auth_account_subtype
|
||||
|
||||
begin;
|
||||
select plan(8);
|
||||
select wtt_load('widgets', 'iam', 'kms', 'auth');
|
||||
|
||||
-- validate the setup data
|
||||
select is(count(*), 1::bigint) from auth_oidc_account where public_id = 'aoa___walter';
|
||||
select is(count(*), 1::bigint) from auth_account where public_id = 'aoa___walter';
|
||||
|
||||
-- validate the insert triggers
|
||||
prepare insert_oidc_account as
|
||||
insert into auth_oidc_account
|
||||
(auth_method_id, public_id, issuer, subject)
|
||||
values
|
||||
('aom___widget', 'aoa___tania', 'https://widget.test', 'tania');
|
||||
select lives_ok('insert_oidc_account');
|
||||
|
||||
select is(count(*), 1::bigint) from auth_oidc_account where public_id = 'aoa___tania';
|
||||
select is(count(*), 1::bigint) from auth_account where public_id = 'aoa___tania';
|
||||
|
||||
-- validate the delete triggers
|
||||
prepare delete_oidc_account as
|
||||
delete
|
||||
from auth_oidc_account
|
||||
where public_id = 'aoa___tania';
|
||||
select lives_ok('delete_oidc_account');
|
||||
|
||||
select is(count(*), 0::bigint) from auth_oidc_account where public_id = 'aoa___tania';
|
||||
select is(count(*), 0::bigint) from auth_account where public_id = 'aoa___tania';
|
||||
|
||||
select * from finish();
|
||||
rollback;
|
||||
@@ -0,0 +1,35 @@
|
||||
-- account tests triggers:
|
||||
-- insert_auth_account_subtype
|
||||
-- delete_auth_account_subtype
|
||||
|
||||
begin;
|
||||
select plan(8);
|
||||
select wtt_load('widgets', 'iam', 'kms', 'auth');
|
||||
|
||||
-- validate the setup data
|
||||
select is(count(*), 1::bigint) from auth_password_account where public_id = 'apa___walter';
|
||||
select is(count(*), 1::bigint) from auth_account where public_id = 'apa___walter';
|
||||
|
||||
-- validate the insert triggers
|
||||
prepare insert_password_account as
|
||||
insert into auth_password_account
|
||||
(auth_method_id, public_id, login_name)
|
||||
values
|
||||
('apm___widget', 'apa___tania', 'tania');
|
||||
select lives_ok('insert_password_account');
|
||||
|
||||
select is(count(*), 1::bigint) from auth_password_account where public_id = 'apa___tania';
|
||||
select is(count(*), 1::bigint) from auth_account where public_id = 'apa___tania';
|
||||
|
||||
-- validate the delete triggers
|
||||
prepare delete_password_account as
|
||||
delete
|
||||
from auth_password_account
|
||||
where public_id = 'apa___tania';
|
||||
select lives_ok('delete_password_account');
|
||||
|
||||
select is(count(*), 0::bigint) from auth_password_account where public_id = 'apa___tania';
|
||||
select is(count(*), 0::bigint) from auth_account where public_id = 'apa___tania';
|
||||
|
||||
select * from finish();
|
||||
rollback;
|
||||
Reference in new issue
Block a user