Support View Protected Fields

Created on 2 December 2024, 2 months ago

Problem/Motivation

We have a way to natively specify that some fields should be patch protected. However, we don't have the same for view protected. In my case a field may supplied during POST or PATCH but not viewable after.

Other areas of the ResourceTestBase allow the user to customize the expected document enough to remove the view protected fields, however some areas escape developer control.

Steps to reproduce

Allow POST or PATCH of a resource with a field that will not be returned on the response. Tests that validate the fields will fail.

Proposed resolution

Add a static property for the develop to specify that a field should not be expected on a response. Similar to the property for patch protected fields.

Remaining tasks

None

User interface changes

None

Introduced terminology

Introduces the view protected fields to the JSON API testing suite.

API changes

Should be none. As far as I can tell the addition of this property as provided should not conflict with any existing work.

Data model changes

None

Release notes snippet

Added '$viewProtectedFieldNames' to describe view protected field for JSON API Resources.

✨ Feature request
Status

Needs work

Version

11.1 πŸ”₯

Component

jsonapi.module

Created by

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @rowrowrowrow
  • Pipeline finished with Failed
    2 months ago
    Total: 137s
    #356708
  • πŸ‡³πŸ‡ΏNew Zealand quietone

    Changes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies.

  • πŸ‡³πŸ‡ΏNew Zealand quietone

    Changes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies.

  • I'm working on the code changes. These are done and ready for review.

  • Pipeline finished with Success
    2 months ago
    Total: 1441s
    #358419
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Change should have test coverage.

  • πŸ‡³πŸ‡±Netherlands bbrala Netherlands

    Thanks for opening this issue, you've been on fire lately.

    I was thinking, wouldn't this "just work" (tm) if you set field permissions? Although i'm a little weary if this is possible to allow post/path but then not view. But this might just work because of entity api.

    function YOUR_MODULE_NAME_entity_field_access($operation, FieldDefinitionInterface $field_definition, AccountInterface $account, FieldItemListInterface $items = NULL) {
      $result = AccessResult::neutral();
      if ($field_definition->getName() == 'field_we_care_about') {
        if ($operation == 'edit' && !in_array('administrator', $account->getRoles())) {
          $result = AccessResult::forbidden();
        }
      }
      return $result->addCacheContexts(['user.roles:administrator']);
    }
    
  • Issue was unassigned.
  • Status changed to Needs review 6 days ago
  • @bbrala, Ya modifying the field access would be one of the ways to protect a field from view. I guess the naming of the issue is misleading. I'll update the name to "Support Testing View Protected Fields". This update is to provide the same sort of patch protected testing that's used on the User entity, and maybe others, to view protected fields.

  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Would see if there is an existing test in jsonapi that can be expanded or would have to write a brand new one.

  • Pipeline finished with Failed
    6 days ago
    Total: 145s
    #413846
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    I don't see anything that calls that variable to testing.

    Before moving to review please check that the pipeline is passes the code standard checks.
    If a test fails that's fine just shows the fix needs to be tweaked usually
    When adding test coverage, if the pipeline could run, there's a test-only feature that checks if the test covers the change.

  • Pipeline finished with Failed
    6 days ago
    Total: 156s
    #413887
  • Pipeline finished with Success
    6 days ago
    Total: 1090s
    #413911
  • Yep apologies, I marked it ready for review while still working on updates. I opted to move the view protected field checking to its own method as we would want to test in various places, namely after a successful get, post, and patch. I then applied this method after the expected successful requests

  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Thanks for working on it.

    So the summary and or title don’t really make sense. The summary seems like it wants to add a new feature but I just see changes to test files. Yes feature requests need test coverage but also needs the new feature added to the MR so we can see that the tests pass.

  • Yep, I can add a description to the method. I don't think we are currently testing that the User's "pass" field is unavailable on any get request so it's been implemented on the UserTest.

    If we need a test for a test I'm not sure how to implement that seeing as it would require an entity that has exactly what the User entity has, a field that is modifiable but not viewable. Please let me know how best to proceed.

Production build 0.71.5 2024