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

Add a "writeOnly" (formerly "secret") annotation #433

Merged
merged 3 commits into from
Oct 20, 2017

Conversation

handrews
Copy link
Contributor

@handrews handrews commented Oct 8, 2017

This is for passwords and other senstive or write-only fields.

This addresses #249.

Copy link
Member

@dlax dlax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quite straightforward, so why not?

This is for passwords and other senstive or write-only fields.
@handrews handrews changed the base branch from applicability to master October 16, 2017 18:42
@handrews
Copy link
Contributor Author

Last call-ish on this one!

Given it's simplicity, and the fact that I'd like to move into the feature-freeze editorial review on Friday, I will probably merge this on Friday even though it will be one day early.

@philsturgeon
Copy link
Collaborator

I'm not sure if secret is the way to advertise the feature, even if it's what the keyword is being used for. I think writeOnly would be a tad more consistent?

Writing things but not being able to see them after is what they're asking for, no?

@handrews
Copy link
Contributor Author

@philsturgeon I'm open to writeOnly, the same thought had occurred to me but I figured I'd see if this got any reaction at all :-)

@dlax and/or @chrisdostert how do you feel about it?

Thinking a bit more, I can see writeOnly as being a general annotation here, which UIs may or may not interpret as using a "password" widget in the UI. The UI Schema proposal would then including a more detailed indicator of what kind of widget to use.

Does that seem like a good distinction?

This is more data-oriented and less UI-oriented, and allows
for further refinements to be handled in a UI vocabulary.
@handrews handrews changed the title Add a "secret" annotation Add a "writeOnly" (formerly "secret") annotation Oct 18, 2017
@handrews
Copy link
Contributor Author

I reworked this as writeOnly and I like it a lot. Not sure if it needs to sit longer for consideration? I am still inclined to merge it on Friday and include it in final review but definitely speak up if you would be concerned about that. I could add a CREF note about "secret" as an alternative if there are concerns about people missing this discussion.

Another use case for writeOnly is that I often find that triggering actions in a RESTful system is done using state manipulation fields that are often logically nulled out as soon as they are processed (not a literal JSON null, but removed entirely). They have no state representation appearance, they are purely buttons/triggers that can be activated. So that would be a writeOnly field that is not a secret field. It's not removed from responses because of sensitivity, just because it is not useful in responses at all.

@philsturgeon
Copy link
Collaborator

Fantastic.

@chrisdostert
Copy link

@philsturgeon @handrews I think both are useful & have specific semantics & use cases.

Knowing something is write only is not the same as knowing it's secret.

For example, think of logging; write-only instances might be included in logs whereas secret instances might not.

@handrews
Copy link
Contributor Author

@chrisdostert yes, agreed they are both useful. The proposal is that writeOnly is useful in general, and secret is useful in presentation (logging would be a presentation concern- I should be clear that I did not just mean "UI" as in "web form")

There is a similar separation for readOnly, where a read-only field might be managed as hidden form field (you can't change it so don't show it), or it might be managed by displaying the value as a non-editable field (you can't change it, but you should know about it).

So...
generic usage: readOnly, writeOnly
presentation: hidden, secret

@handrews
Copy link
Contributor Author

This spec is for generic usage, presentation ideas go in json-schema-org/json-schema-vocabularies#2

@handrews
Copy link
Contributor Author

Also, you can easily treat writeOnly as if it meant "secret" in your application without violating either the letter or spirit of the specification. The reverse is not true.

@handrews
Copy link
Contributor Author

@chrisdostert if you want to argue that we should not move forward with writeOnly please do so by tomorrow. Otherwise I will assume that you want to see secret added somewhere but are OK with that being tracked as json-schema-org/json-schema-vocabularies#6 which I filed for this as a presentation description request.

@handrews
Copy link
Contributor Author

Of course if you come back next week with new objections we can continue to discuss changes, I would just like to include this in the "final review" announcement for this weekend.

@handrews handrews merged commit 029de3f into json-schema-org:master Oct 20, 2017
@handrews handrews deleted the secret branch October 20, 2017 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants