Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Policy API: Enable admins to perform unsafe mutations on data in their platform #115

Open
jrschumacher opened this issue Feb 1, 2024 · 6 comments
Assignees
Labels
comp:policy Policy Configuration ( attributes, subject mappings, resource mappings, kas registry)

Comments

@jrschumacher
Copy link
Member

jrschumacher commented Feb 1, 2024

Implement the unsafe mutation RPCs so that admins can have an audit trail when they need to make unsafe changes to their platform

Depends on

Acceptance Criteria

  • Create an UnsafeService which captures the unsafe update RPC to empower platform admins
    • e.g. UnsafeService.UnsafeUpdateAttributeName()
    • e.g. UnsafeService.UnsafeDeleteAttribute()
  • Every unsafe property should require passing a value that would provoke the user to question what they are doing or why the platform requires it
@jrschumacher jrschumacher added the comp:policy Policy Configuration ( attributes, subject mappings, resource mappings, kas registry) label Feb 1, 2024
@strantalis strantalis added this to the POC to Alpha milestone Feb 1, 2024
@jrschumacher jrschumacher removed this from the POC to Alpha milestone Feb 1, 2024
@jakedoublev jakedoublev added this to the Finalize protos and SDK milestone Feb 6, 2024
@jakedoublev
Copy link
Contributor

jakedoublev commented Feb 14, 2024

Suggesting we add the following rpc's, which are also dangerous if a deactivation was intentional and restoration could give new access where it had previously been blocked (see discussion in 108)

  • UnsafeService.RestoreDeactivatedNamespace()
  • UnsafeService.RestoreDeletedAttribute()
  • UnsafeService.RestoreDeletedAttributeValue()

They will intentionally not bubble up or cascade down to avoid unintended consequences. In other words, restoring a deactivated namespace will not restore any of its deactivated child attributes, and restoring an attribute will not restore any of its deactivated values.

@jakedoublev
Copy link
Contributor

Along with those:

  • UnsafeService.UpdateNamespace() // name is the only value that can be mutated and it should be dangerous to mutate
  • UnsafeService.UpdateAttributeValue()

Any of these should be prevented and/or removed if currently implemented and reserved for specific calls by the unsafe service:

  1. reactivation of any inactive state by update
  2. mutation of attribute name via update
  3. mutation of attribute value via update

@jakedoublev
Copy link
Contributor

For future discussion

Scenario:
I create the following: namespace: namespace.com, attribute: attr-xyz on that namespace, value: value-123 on that attribute. I then deactivate attribute attr-xyz, which cascades down to value-123. Then I create value: value-456 on deactivated attribute attr-xyz. What is the API contract a PEP will need so that a subject mapping or resource mapping with that newly created active value value-456 on inactive attribute attr-xyz will be properly entitled/enforced?

Questions:

  1. is creation/update of an attribute on a namespace that is inactive something to prevent?
  2. likewise, is creation/update of an attribute value on an attribute that is inactive something to prevent?
  3. is that a user error considered out of scope?
  4. should we enable simple boolean rpc's on the resources that check their entire "validity chain" for active state? (i.e. getIsValidSubjectMapping which checks value -> attribute -> namespace for active state?

The goal is to prevent data loss and prevent a blocking experience via user error, but these flows probably need to be considered.

jrschumacher added a commit that referenced this issue Feb 17, 2024
…efinitions, attribute values #96 #108 (#191)

This work encompasses the following (including multiple breaking changes):
1. Tables `namespaces`, `attribute_definitions`, `attribute_values` updated via migration to add `active` boolean state
2. cascading deactivation from namespace -> attr -> values (in DB implementation via SQL trigger function provided in migration up/down)
3. integration tests:
    - cascading behavior ns -> attr -> val
    - integration tests proving no deactivation bubbling up behavior val -> attr -> ns
4. protos:
    - updated to provide a state enum back on all 3 resources (with helpful comments about defaults)
    - new example grpcurl requests/responses with these updates
    - all three LIST rpc's filterable by state as a common Message type (including an ANY enum option that returns both active TRUE and FALSE rows) and defaulting to ACTIVE if not specified
5. preservation of DB delete functionality/tests which will be exposed in newly separate rpc's

This PR does _not_ include:
1. Unsafe RPCs for actual deletion: #115
2. Unsafe RPCs for dangerous mutations (same issue)
3. Prevention of update mutation of INACTIVE namespaces/attributes/attributeValues
4. Prevention of creation of new attributes/values on a prior created then deactivated namespace
5. Provision of parent or child state in GET responses beyond that of the resource requested (i.e. namespace & value state are not given in a GET for an attribute, even though the attribute's state is given)

---------

Co-authored-by: Ryan Schumacher <jschumacher@virtru.com>
@jrschumacher jrschumacher modified the milestones: Finalize protos and SDK , Platform hardening Mar 6, 2024
@jrschumacher
Copy link
Member Author

With this work, we will need to see how to remain DRY in our DB funcs. The unsafe properties were removed in the update funcs for the time being.

@jakedoublev
Copy link
Contributor

Unsafe updates should also include:

  1. reactivation (probably without cascades, but could be discussed)
  2. attribute definition update with full replacement of values order on an attribute (enabled by feat(policy): preserve values order within an attribute #496), which will require validation that the supplied list of value UUIDs contains only ids for values on that attribute and that the quantity includes all values under the definition (both active & inactive)

@jakedoublev jakedoublev self-assigned this Apr 29, 2024
@jakedoublev
Copy link
Contributor

Moved back to todo and unassigned myself as other priorities consistently took precedence this week.

@jakedoublev jakedoublev removed their assignment May 3, 2024
@jrschumacher jrschumacher self-assigned this May 21, 2024
@jrschumacher jrschumacher reopened this May 21, 2024
tech-guru42 added a commit to tech-guru42/TDF that referenced this issue Jun 3, 2024
…efinitions, attribute values #96 #108 (#191)

This work encompasses the following (including multiple breaking changes):
1. Tables `namespaces`, `attribute_definitions`, `attribute_values` updated via migration to add `active` boolean state
2. cascading deactivation from namespace -> attr -> values (in DB implementation via SQL trigger function provided in migration up/down)
3. integration tests:
    - cascading behavior ns -> attr -> val
    - integration tests proving no deactivation bubbling up behavior val -> attr -> ns
4. protos:
    - updated to provide a state enum back on all 3 resources (with helpful comments about defaults)
    - new example grpcurl requests/responses with these updates
    - all three LIST rpc's filterable by state as a common Message type (including an ANY enum option that returns both active TRUE and FALSE rows) and defaulting to ACTIVE if not specified
5. preservation of DB delete functionality/tests which will be exposed in newly separate rpc's

This PR does _not_ include:
1. Unsafe RPCs for actual deletion: opentdf/platform#115
2. Unsafe RPCs for dangerous mutations (same issue)
3. Prevention of update mutation of INACTIVE namespaces/attributes/attributeValues
4. Prevention of creation of new attributes/values on a prior created then deactivated namespace
5. Provision of parent or child state in GET responses beyond that of the resource requested (i.e. namespace & value state are not given in a GET for an attribute, even though the attribute's state is given)

---------

Co-authored-by: Ryan Schumacher <jschumacher@virtru.com>
passion-127 added a commit to passion-127/TDF that referenced this issue Jun 6, 2024
…efinitions, attribute values #96 #108 (#191)

This work encompasses the following (including multiple breaking changes):
1. Tables `namespaces`, `attribute_definitions`, `attribute_values` updated via migration to add `active` boolean state
2. cascading deactivation from namespace -> attr -> values (in DB implementation via SQL trigger function provided in migration up/down)
3. integration tests:
    - cascading behavior ns -> attr -> val
    - integration tests proving no deactivation bubbling up behavior val -> attr -> ns
4. protos:
    - updated to provide a state enum back on all 3 resources (with helpful comments about defaults)
    - new example grpcurl requests/responses with these updates
    - all three LIST rpc's filterable by state as a common Message type (including an ANY enum option that returns both active TRUE and FALSE rows) and defaulting to ACTIVE if not specified
5. preservation of DB delete functionality/tests which will be exposed in newly separate rpc's

This PR does _not_ include:
1. Unsafe RPCs for actual deletion: opentdf/platform#115
2. Unsafe RPCs for dangerous mutations (same issue)
3. Prevention of update mutation of INACTIVE namespaces/attributes/attributeValues
4. Prevention of creation of new attributes/values on a prior created then deactivated namespace
5. Provision of parent or child state in GET responses beyond that of the resource requested (i.e. namespace & value state are not given in a GET for an attribute, even though the attribute's state is given)

---------

Co-authored-by: Ryan Schumacher <jschumacher@virtru.com>
@cassandrabailey293 cassandrabailey293 removed this from the Platform hardening milestone Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:policy Policy Configuration ( attributes, subject mappings, resource mappings, kas registry)
Projects
None yet
Development

No branches or pull requests

4 participants