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

readOnly=true is not working #350

Closed
MarcosEnevo opened this issue Nov 18, 2016 · 14 comments
Closed

readOnly=true is not working #350

MarcosEnevo opened this issue Nov 18, 2016 · 14 comments
Milestone

Comments

@MarcosEnevo
Copy link

Description

Connexion ignores readOnly=true being defined at OpenAPI properties.

Expected bahaviour

Properties with readOnly=true should not be passed as attributes to the operation function at request, and only used as validation for response.

Actual behaviour

Properties with readOnly=true are if fact passed as attributes to the operation function.

Steps to reproduce

  • Set a OpenAPI property with readOnly=true.
  • Set named parameter (or **kwargs) at target operation function.
  • Make a request to associated endpoint.

Additional info:

  • Python 3.5.1
  • Connexion version: 1.0.129
@rafaelcaricio
Copy link
Collaborator

Do you mean properties of objects schemas in #/definitions/*?

@andrey-k
Copy link

yes, readOnly in #/definitions/* like

  properties:
      name:
        type: string
        readOnly: true

jsonschema what in use by connexion RequestBodyValidator class does not support it. Not sure that readOnly validation should be done in jsonschema. Probably extra check in connexion or just exclude it from connexion.request.data?

@rafaelcaricio
Copy link
Collaborator

That is a good question. Would you expect Connexion, if present the readOnly: true property, to remove it from the payload or to refuse and return a 400 response to the client?

@dimbleby
Copy link
Contributor

dimbleby commented Nov 20, 2016

Either seems to be allowed, by my reading of the spec.

I'd (slightly) favour "rejected" - ignoring parts of a request and acting on others seems more likely to be confusing.

@rafaelcaricio
Copy link
Collaborator

@dimbleby I also would go with reject the whole request if a readOnly: true field is present.

@hjacobs
Copy link
Contributor

hjacobs commented Nov 20, 2016

@dimbleby @rafaelcaricio "rejecting" the whole message (e.g. with "Bad Request) is not a good idea I think --- a resource definition should IMHO be allowed to be used in a symmetric way, i.e. the resource from GET should be PUTable without modification. One example might be a created_by field which is auto-populated by the server (on creation) with the authenticated user (e.g. OAuth uid scope), as a user I want to be able to retrieve such resource, modify only known fields (the ones I care for) and PUT the resource again without additionally stripping readOnly fields.

I'm not saying that this use case always makes sense, but it should definitely be possible.

@dimbleby
Copy link
Contributor

dimbleby commented Nov 20, 2016

Hmm, swings and roundabouts:

  • I agree that it's reasonable to expect to be able to PUT eg user-with-identity-1 to /users/1
  • On the other hand, I think it's reasonable to expect that putting user-with-identity-2 to /users/1 should be rejected

Rejecting read-only fields annoyingly causes the first request to fail. Ignoring read-only fields confusingly allows the second request to succeed, sort of.

IMO the "reject" case is the lesser of the evils, but I don't feel all that strongly about it.

Edit to the edit: snip some rubbish!

@andrey-k
Copy link

@hjacobs why would you like to be able to PUT everything back from GET request? Just to have less headache cleaning up read only fields? Or are there any other possible points?
I also would prefer to have rejected status. If user will be able to send data like id, created_ts or fields pointing to other resources what should be defined by another methods and in the GET it is just information only, then that behaviour my create false expectation for user that it is possible to change provided fields/data.

@kmaillet
Copy link

Hi !
Any updates on this issue? We are facing the same problem here. I side with @hjacobs , I think it is easier if the read-only fields are just ignored

Thanks

@lucas03
Copy link

lucas03 commented Oct 17, 2018

Any update please? there is related stackoverflow question (there might be a better source for this discussion out there somewhere)

@cziebuhr
Copy link
Contributor

Support for readOnly has been added in dev-2.0 branch.

@jmcs
Copy link
Contributor

jmcs commented Oct 18, 2018

If you want to test it, there is a 2.0 Release Candidate you can install with pip install --pre

@hjacobs hjacobs added this to the 2.0 milestone Oct 18, 2018
@hjacobs
Copy link
Contributor

hjacobs commented Oct 18, 2018

Added this to the 2.0 milestone as it should be fixed with 2.0.

@hjacobs
Copy link
Contributor

hjacobs commented Nov 5, 2018

Can this be closed with the 2.0 release?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants