agent-permissions: account root card#3595
Conversation
✅ Deploy Preview for golemcloud canceled.
|
| pub name: Option<String>, | ||
| } | ||
|
|
||
| pub struct AccountSetRoles { |
There was a problem hiding this comment.
Removed the option to set roles after creation as that would force rotation of the account root card -> all cards in the account become invalid.
Instead, roles need to be specified during creation and are considered when creating the account root card.
| @@ -1,3 +0,0 @@ | |||
| ALTER TABLE tokens ADD COLUMN card_id UUID NULL REFERENCES cards(card_id) ON DELETE SET NULL; | |||
There was a problem hiding this comment.
Leftover from last pr. I decided to only have a shared token root card (for now). If we ever need token specific cards later we can readd it (they would derive from both the token root card and the account root card)
| .create_account(&AccountCreation { | ||
| name: account_name, | ||
| email: AccountEmail::new(account_email), | ||
| roles: Vec::new(), |
There was a problem hiding this comment.
We may need to add something here to enable creation / promotion of accounts to admin / ops accounts that can at least impersonate (so we don't have to use admin for it). Will think about it some more in followups.
| .into_iter() | ||
| .map(|card| card.card_id) | ||
| .collect(); | ||
| EffectiveSurface::from_cards(&parent_cards, &account_holder) |
There was a problem hiding this comment.
Just a sanity check to ensure we are deriving legal cards
| let mut upper_positive = Vec::new(); | ||
| let mut upper_negative = Vec::new(); | ||
| for card in &parent_cards { | ||
| add_card_grants( |
There was a problem hiding this comment.
I'll try / will have to make this smarter in the future, this is more of a placeholder for now
| ToolResourcePattern, | ||
| }; | ||
|
|
||
| pub(super) fn account_root_card_record(account_id: AccountId, roles: &[AccountRole]) -> CardRecord { |
There was a problem hiding this comment.
I will see about optimizing this further later, more of a placeholder for now
| } | ||
| } | ||
|
|
||
| impl DbCardRepo<SqlitePool> { |
There was a problem hiding this comment.
The new per-card loop:
for row in rows { // rows from recursive CTE
DELETE FROM card_parents WHERE card_id = $1; // only this card's incoming-parent edges
DELETE FROM cards WHERE card_id = $1;
}
SQLite returns the recursive-CTE rows anchor-first. With PRAGMA foreign_keys = ON, deleting the root fails because card_parents still has rows where parent_id = root_id and that FK is non-cascading.
This deterministically breaks:
accounts.deletewhenever the account has issued a token (token root card is a child of the account root card)- the SQLite variant of
permission_share.invalidate_token_root_card_without_epoch_bump_in_txif the token root has any descendants (currently it doesn't, but the function is generic)
There was a problem hiding this comment.
Good catch, I forgot that we are enabling these for cli. Let me adjust our sqlite integration tests to have foreign keys.
| .account_service | ||
| .get(account_id, &AuthCtx::System) | ||
| .await | ||
| .map_err(|_| AuthError::CouldNotAuthenticate)?; |
There was a problem hiding this comment.
If we drop the error details, we should at least log it as a warning here, I think
2 participants
part of #3493