Add sql schema and pgtap tests #6269
Conversation
mikemountain
changed the title
mikemountain
changed the title
| -- Create the enumeration table for app token global grant scope | ||
| create table app_token_global_grant_scope_enm ( | ||
| name text primary key | ||
| constraint only_predefined_app_token_global_grant_scope_allowed | ||
| check( | ||
| name in ( | ||
| 'individual', | ||
| 'children', | ||
| 'descendants' | ||
| ) | ||
| ) | ||
| ); |
There was a problem hiding this comment.
Why create a table & FK constraint for these enums instead of creating an enum type? e.g.
create type app_token_global_grant_scope_enum as enum ('individual', 'children', 'descendants');By using an enum type, we'd have one less table to join on
__
EDIT: I noticed we use this pattern elsewhere, so I don't expect it to change here - just curious if you knew why
| if old.revoked is distinct from new.revoked then | ||
| -- Only allow change from false to true | ||
| if not (old.revoked = false and new.revoked = true) then | ||
| raise exception 'App token cannot be unrevoked. revoked value. Current: %, Attempted: %', |
There was a problem hiding this comment.
It looks like we don't need the "revoked value." part of this exception message
| raise exception 'App token cannot be unrevoked. revoked value. Current: %, Attempted: %', | |
| raise exception 'App token cannot be unrevoked. Current: %, Attempted: %', |
| primary key(permission_id, canonical_grant) | ||
| ); | ||
| comment on table app_token_permission_grant is | ||
| 'app_token_permission_grant contains grants assigned to app tokens in project scope'; |
There was a problem hiding this comment.
It looks like this table can apply to app tokens at any scope, not just the project scope. Should this instead say,
'app_token_permission_grant contains grants assigned to app tokens'
| token bytea not null unique | ||
| ); | ||
| comment on table app_token_cipher is | ||
| 'app_token_cipher is the table for application token encryption keys. ' |
There was a problem hiding this comment.
Assuming the intent here was to say "app token encryption keys" instead of "application token encryption keys":
| 'app_token_cipher is the table for application token encryption keys. ' | |
| 'app_token_cipher is the table for app token encryption keys. ' |
There was a problem hiding this comment.
Maybe not -- feel free to ignore this if "application tokens" was intended
| references app_token_global_grant_scope_enm(name) | ||
| on delete restrict | ||
| on update cascade, | ||
| create_time wt_timestamp, |
There was a problem hiding this comment.
Is this field supposed to be on this table? The Whimsical DB diagram doesn't have a create_time field on this table
| check( | ||
| grant_scope = 'individual' | ||
| ), | ||
| create_time wt_timestamp, |
There was a problem hiding this comment.
This field isn't on Whimsical, but it's here -- intentional?
| end; | ||
| $$ language plpgsql; | ||
| comment on function validate_global_permission_org_scope() is | ||
| 'validate_global_permission_org_scope is used to enforced that scope ID added to app_token_permission_global_individual_project_grant_scope' |
There was a problem hiding this comment.
Small grammar nitpick -
| 'validate_global_permission_org_scope is used to enforced that scope ID added to app_token_permission_global_individual_project_grant_scope' | |
| 'validate_global_permission_org_scope is used to enforce that scope ID added to app_token_permission_global_individual_project_grant_scope' |
| grant_scope text not null | ||
| constraint app_token_global_grant_scope_enm_fkey | ||
| references app_token_global_grant_scope_enm(name) | ||
| on delete restrict | ||
| on update cascade, |
There was a problem hiding this comment.
Why reference another table here instead of referncing an enum type or doing something like this:
grant_scope text not null
constraint only_predefined_app_token_global_grant_scopes_allowed
check(
grant_scope in ('individual', 'children', 'descendents')
),2 participants
Description
Closes ICU-17908. Add the app token database schema + pgtap tests to validate the schema.
PCI review checklist
Examples of changes to security controls include using new access control methods, adding or removing logging pipelines, etc.