-
Notifications
You must be signed in to change notification settings - Fork 4
feat: AIP-131 – GET for individual resources #2
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
Conversation
This adds a refactored AIP-165. I really like some of the precedents set in #2; for example, mirroring the structure there required me to add a full section on errors that I consider to be quite useful. Similarly, separating the behavior from the IDL still works well. There is an open question here about long-running operations, which I do not know how to properly handle in OAS and need to discuss with @mkistler.
aip/general/0131/aip.md.j2
Outdated
| If the user does not have permission to access the resource, regardless of | ||
| whether or not it exists, the service **must** reply with an HTTP 403 error. | ||
| Permission **must** be checked prior to checking if the resource exists. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is reversed from the more natural solution to the problem of leaking the existence of resources to users not authorized to know about them. Assuming granular permission where each user may have access to some resources in a collection, this would force the system to return 403 for any validly structured URI. I believe this should be solved, instead, by returning 404 for resources users are not authorized to know about, as we require in our API handbook:
There may be times when returning a
401or a403would reveal the presence of a resource on the server that a client should not even be allowed to know exists. In these cases, the server MUST respond with a404even for requests that would succeed with proper authentication.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great question and we should debate it when we meet next. (We found a spot where our handbooks disagree!)
We take the exact converse stance, for the same reason: essentially, "if the resource exists, and we are not saying it does, you would not have access to it, so 403".
Assuming granular permission where each user may have access to some resources in a collection, this would force the system to return 403 for any validly structured URI.
I see the point here, and I think our guidance is structured around the assumption that such granular permissions are less common.
We should discuss this more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. I happened to be reading the RFC for an unrelated reason this week and was reminded that it says this:
An origin server that wishes to "hide" the current existence of a forbidden target resource MAY instead respond with a status code of 404 (Not Found).
This is probably the reason for the direction we went.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find that rationale compelling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I still agree with the change @hudlow suggests, but I wanted to share some of the rationale from an internal Google discussion about this. The summary is that Google decided to use 403 because of the importance of telling users what permission they do not have (to give them an actionable way of rectifying the situation). Using 404 prevents this without leaking the existence of the resource.
Some choice quotes from our internal doc:
In most systems - you get permission denied in preference to not found until you have permission to determine the result is not found. Reversing this is not good in my view. You get not found until you have enough permission to get permission denied! This is far from intuitive.
Google has the best web security system, but we have poor auth experience (#1 cloud developer experience issue). Most Cloud APIs are used by real developers for real work. By making it hard to detect resource existence and not providing informative error message, we sacrifice the developer experience.
To be clear, the doc I am discussing was a specific proposal that brought up the same issue that Dan did -- contradicting the HTTP spec -- and deciding that, basically, "we think we are right" and that the HTTP recommendation does not scale well to large, distributed systems with complex permissions models.
mkistler
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good but left a few comments.
Thanks everyone! Co-authored-by: rhamiltonsf <38894273+rhamiltonsf@users.noreply.github.com> Co-authored-by: Mike Kistler <mkistler@us.ibm.com>
We should figure out how we want to handle resource names vs. tuples, but this is probably ready to go other than that.
Dependency: aip-dev/site-generator#9