You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The reason will be displayed to describe this comment to others. Learn more.
We discussed this and I decided to leave as-is.
When an authURL is given, which is a cross-domain request, then the client is responsible for setting the CORS headers appropriately in the authURL response. If they wish to get an Authorization header, then they have to ensure that there is no wildcard origin in the CORS response. So I think this check is correct.
The side-effects of omitting withCredentials also include no cookies being sent. I can see a use-case for the authURL request expecting a cookie in the same-origin case, but I don't think it is reasonable to expect anything in the cross-origin case.
f5187ec
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.
We discussed this and I decided to leave as-is.
When an authURL is given, which is a cross-domain request, then the client is responsible for setting the CORS headers appropriately in the authURL response. If they wish to get an
Authorization
header, then they have to ensure that there is no wildcard origin in the CORS response. So I think this check is correct.The side-effects of omitting
withCredentials
also include no cookies being sent. I can see a use-case for theauthURL
request expecting a cookie in the same-origin case, but I don't think it is reasonable to expect anything in the cross-origin case.f5187ec
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.
Yup makes sense, and later if anyone asks, we could add another Auth option if need be