Feature: Hidden (aka Private) attributes

[vvcephei]

Github Issue

91

What Are We Doing Here?

I implemented the feature using the namespace strategy.

How to Test and Verify

1. Check out the project; run Emo
2. Write some documents with hidden attributes.
3. Verify that you cannot see the hidden fields unless you specify showHiddenFields=true
4. Verify you cannot specify showHiddenFields=true unless you have update permission to the table
5. Verify you cannot see hidden fields in databus events or in stash
6. Write some conditional deltas, conditioned on the values of hidden attributes.
7. Verify Emo correctly computes the conditions (hidenness has no impact on conditional logic).

Risk

Level

Medium

Required Testing

Smoke, Regression, or Manual. (All changes except documentation need smoke
testing at a minimum).

Not sure????

I wrote a bunch of unit tests, but I couldn't figure out how to test stash, other than maybe to use test.integration.sor.ScanUploadTest. But the docs are apparently wrong. it requires 3 args instead of 2, and I have no clue what a ddlFilePath might be.

Risk Summary

We are introducing a new concept to Emo. This is something we should do very, very carefully. Try to erase my use case from your mind and think about what a new user will make of this feature. Does the documentation make the intended usage obvious? Is the feature sufficiently intuitive?

Code Review Checklist

• Tests are included. If not, make sure you leave us a line or two for the reason.

• Pulled down the PR and performed verification of at least being able to
  build and run.

• Well documented, including updates to any necessary markdown files. When
  we inevitably come back to this code it will only take hours to figure out, not
  days.

• Consistent/Clear/Thoughtful? We are better with this code. We also aren't
  a victim of rampaging consistency, and should be using this course of action.
  We don't have coding standards out yet for this project, so please make sure to address any feedback regarding STYLE so the codebase remains consistent.

• PR has a valid summary, and a good description.

[billkalter]

Still reviewing, but got pulled off for an incident so I'll submit this now to get the ball rolling. Additional feedback to follow.

[billkalter]

I'm not sure if the ParamRequiresPermissions approach is the best fit for controlling access to the parameter. For example, if an API key without update permissions were to call /sor/1/...?showHiddenFields=false then the annotation-based approach would reject this request, even though it's really only showHiddenFields=true which needs restricting.

While it's not as baked into the authorization framework it is possible to have an @Authenticated Subject subject parameter and check the permission as part of the handler logic, such as here.

[vvcephei]

correct. I decided it was cleaner not to have to copy and paste an auth stanza requiring the subject everywhere but instead keep it with the parameter specification. I think it's fine to ask unauthorized users not to specify the restricted parameter at all.

In fact, in some ways it makes more sense this way. "showHiddenFields" is an admin feature, which you don't have access to as a mere reader. Why should you be able to specify any value for that parameter when you have no access to that feature?

[vvcephei]

I guess there's not much chance of this getting merged, two years after the fact ;)

Closing the PR.
Thanks for all the fish.

[bdevore17]

@vvcephei We actually had discussion about this the other day 😆 . Never say never....

[vvcephei]

Hey @bdevore17 !

Well, feel free to resurrect it! I may be a bit rusty, but I'm happy to help if I can.

Good to hear from you.
-John