Innkeeper: ability to delete tenants

[esune]

Tenants seem to proliferate quickly in lower environments (such as dev), generating a lot of extra entries in the management section for the Innkeeper.

The innkeeper should have the possibility to delete a tenant, multitenancy endpoints to perform the task are already available in ACA-Py.

AC (UI):

• On the Tenant list for the innkeeper there's a delete button for each tenant
• It pops up a screen that makes you type in that tenant name to confirm it
• Calls the soft delete when confirmed

AC (Traction innkeeper plugin):

• There's a delete endpoint that sets the state to deleted
• Only the innkeeper can call it
• When fetching tenants, or operating on a tenant it provides a state filter, or restricts to only active ones. TBD a bit, can discuss these points.

[loneil]

I'm paranoid about this one 😆 so if implemented it could be nice to have something a little stronger than the "are you sure? 1 click" confirm.

I'm a fan of the Github repo delete, helm uninstall, type-to-confirm style as a hardcore "are you sure?"

[WadeBarnes]

I'm a advocate of soft deletes, that way you can easily recover from a mistake. It also provides the opportunity for a scream test; e.g. delete the tenant and see if anyone complains. Hard deletes should be an administrative task that is performed only on records that have already been soft deleted for a given length of time. This could be automated if the numbers are significant; delete all deactivated (soft deleted) tenants older than x months.

I am also a fan of the type-to-confirm interfaces @loneil pointed out, even for soft deletes. It gives you some extra time to think about what you're doing. I especially like the interfaces that do not allow copy-paste, giving you extra time to think about what you're doing as you type.

[esune]

Agreed on both points. An extra safeguard to prevent problems (or two) is definitely a good idea.

[loneil]

Just discussing with Emiliano and think the following path could be one to look into. Wondering if @usingtechnology has any thoughts on this?

Tenant State Tenants records have a state

The model TenantRecord in innkeeper\models.py has a constant for these and uses it in the constructor. Expand calls that list tenant records, or operate on any tenant record to take the Active state into account (if it's not already doing that) or allow a filter choice on state.

Add a delete call for tenants in the innkeeper routes that sets the state to Deleted.

So this accomplishes a soft delete easily enough for the TENANT, which would take it out of the innkeeper listings and such and any calls that use the Tenant record would not find anything if the state is deleted to operate on.

Token When fetching a token with API key (ie Tenant ID and key) this would just work and fail out because the Tenant fetch shouldn't find anything.

When fetching with the underlying Wallet ID though, it doesn't really know anything about the Tenant, so I think marking the Tenant as Deleted would still let you get a token with your wallet creds if you had them... Could "fix" that by having the wallet token fetch just use the wallet ID to check the tenant is active before coming back, but we don't want to do that because the multitenancy provider is meant to be a standalone plugin that shouldn't know anything about Traction Tenants. IE someone can use it without using the Traction Innkeeper plugin.

Any operation that uses that token that then does need the tenant record after would of course fail, so Tenant UI would fail out on login, but we would want to think about API usage. So this might needs some more discussion (probably check in with Sherman see if he has ideas)... Might possibly just implement the Tenant delete first and create a subsequent issue to look at this?

Hard delete/delete wallet So the above is just about soft deleting a tenant. It could be used as a quick management feature for us I think.

There's also just deleting the subwallet itself

But I don't think this is what we want to invoke if we want a soft delete implemented, until we build a feature to purge a tenant/wallet entirely (which like Wade said above could be a task after the tenant is gone for some time)

[esune]

Thank you @loneil . @gurjmatharu to you, feel free to reach out if you have questions.

[usingtechnology]

I think @loneil has stated it quite well. Short-term and tenant-ui specific, soft-delete would be great. Long-term would be hard-delete including removing the (sub)wallet completely. Probably want to wait until an export function is available for hard-delete?