-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
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)