-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Prevent login if Git Gateway is disabled. #1295
Conversation
@@ -78,7 +78,11 @@ export default class API { | |||
if (contentType && contentType.match(/json/)) { | |||
return this.parseJsonResponse(response); | |||
} | |||
return response.text(); | |||
const text = response.text(); | |||
if (!response.ok) { |
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.
@Benaiah Is there any problem internally if we reject on not-OK responses for all content-types? We do on JSON responses already (see parseJsonResponse
).
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.
@tech4him1 yeah - I actually thought it was supposed to be doing so already. If the caller is expecting a non-ok response it can catch the APIError
.
Deploy preview for cms-demo ready! Built with commit 2a58992 |
Deploy preview for netlify-cms-www ready! Built with commit 2a58992 |
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.
@tech4him1 apologies for just now getting to this, lgtm.
- Summary
Fixes #1272.
If Git Gateway was disabled, we were still allowing the user to log in, which can cause them to lose their work when they cannot save it.
- Test plan
Manually tested with both scenarios: #1272 (comment).
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)