Prohibit following redirects whilst fetching Client Metadata#46
Prohibit following redirects whilst fetching Client Metadata#46ThisIsMissEm wants to merge 2 commits intooauth-wg:mainfrom
Conversation
|
I think a simpler solution is to require the response code 200 when serving the metadata document. |
|
@aaronpk the only reason to perhaps not limit to only response status code of 200, would be if the client is created dynamically and returns a response status code of 201 created or similar? I'm not sure if that's actually something we should support though. Limiting to just 200 is definitely simpler. |
|
yeah that's a real stretch, even if the CIMD resource is created on the fly, I think it's fine to require 200. It's not like that actually breaks any internet laws. |
|
Thanks for the edits, I like it. |
ThisIsMissEm
force-pushed
the
feat/fix-redirect-security-concerns
branch
from
3df960f to
f77abb9
Compare
| ## Metadata Discovery Errors | ||
|
|
||
| If fetching the metadata document fails, the authorization server SHOULD abort the | ||
| authorization request. |
There was a problem hiding this comment.
I think this is implied by the error response section.
| The client metadata document should be served with a 200 OK HTTP status code, | ||
| have the content type of `application/json` or a more specific content type that | ||
| conforms to `application/<AS-defined>+json`, and be a valid JSON object | ||
| {{RFC8259}}. |
There was a problem hiding this comment.
I'm not sure if this paragraph is really necessary as it just restates what's in the section above.
|
|
||
| ## Client Metadata Document | ||
|
|
||
| A Client Metadata Document is a JSON document {{RFC8259}} containing the client |
There was a problem hiding this comment.
Is there a better way to say this than "JSON document" since that's not exactly a defined term?
There was a problem hiding this comment.
The term you are looking for is "JSON text" https://datatracker.ietf.org/doc/html/rfc8259#section-1.2 ... containing an "object".... or directly https://datatracker.ietf.org/doc/html/rfc8259#section-4
"A Client Metadata Document is a JSON object as described in {{Section 4 of RFC8259}}."
|
@aaronpk have rebased the edits and corrected some wording a little more. (I actually had to recreate the branch) |
| If authorization server encounters an error response when retrieving the client | ||
| registration information, the authorization server SHOULD abort the | ||
| authorization request. The authorization server MAY use error responses to | ||
| inform their security policies. |
There was a problem hiding this comment.
Is this redundant? Or at worst contradictory? Above we say successful response MUST use.. and be a valid JSON object but here only a SHOULD?
| If authorization server encounters an error response when retrieving the client | ||
| registration information, the authorization server SHOULD abort the | ||
| authorization request. The authorization server MAY use error responses to | ||
| inform their security policies. |
There was a problem hiding this comment.
| If authorization server encounters an error response when retrieving the client | |
| registration information, the authorization server SHOULD abort the | |
| authorization request. The authorization server MAY use error responses to | |
| inform their security policies. |
|
|
||
| The client metadata document MUST be served with a 200 OK HTTP status code, | ||
| have the content type of `application/json` or a more specific content type that | ||
| conforms to `application/<AS-defined>+json`, and be a valid JSON object |
There was a problem hiding this comment.
<AS-defined> ...is really "AS supported"...
There was a problem hiding this comment.
I think this language comes from something to do with content types, basically trying to say that application/anything+json is valid as a content type. cc @aaronpk since this language came from you.
4 participants
This addresses the concerns raised in #44 and by Dick Hardt on the mailing list. Following redirects would likely give you a final document URI that is not equal to the supplied
client_id, and as such should not be trusted.We could potentially limit this further to only accepting 200 OK responses.