Skip to content

Conversation

@lukesneeringer
Copy link
Contributor

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

@lukesneeringer lukesneeringer requested a review from a team as a code owner September 3, 2020 00:02
@lukesneeringer lukesneeringer changed the title feat: AIP-131 feat: AIP-131 – GET for single resources Sep 3, 2020
@lukesneeringer lukesneeringer changed the title feat: AIP-131 – GET for single resources feat: AIP-131 – GET for individual resources Sep 3, 2020
lukesneeringer pushed a commit that referenced this pull request Sep 3, 2020
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.
Comment on lines 53 to 55
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.
Copy link

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 401 or a 403 would 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 a 404 even for requests that would succeed with proper authentication.

Copy link
Contributor Author

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.

Copy link

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.

Copy link
Contributor Author

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.

Copy link
Contributor Author

@lukesneeringer lukesneeringer Nov 19, 2020

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.

Copy link

@mkistler mkistler left a 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.

Luke Sneeringer and others added 7 commits October 20, 2020 15:08
@lukesneeringer lukesneeringer merged commit d5e8f5c into master Nov 19, 2020
@lukesneeringer lukesneeringer deleted the aip-131 branch November 19, 2020 23:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants