-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Enables login over POST in addition to GET #4268
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4268 +/- ##
==========================================
+ Coverage 92.2% 92.43% +0.22%
==========================================
Files 116 118 +2
Lines 8124 8151 +27
==========================================
+ Hits 7491 7534 +43
+ Misses 633 617 -16
Continue to review full report at Codecov.
|
spec/ParseUser.spec.js
Outdated
'X-Parse-REST-API-Key': 'rest', | ||
}, | ||
json: { | ||
_method: 'POST', |
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.
You shouldn’t need that
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.
@flovilmart shouldn't need to indicate post?
@flovilmart yeah |
the _method prop is an internal parse thing so we can send everything as POST queries an interpolate as _method on the server. |
So this is a little something for #4052 . The change is relatively minor and really doesn't do anything on it's own. A single test is added to verify login works over POST using REST.
Most importantly this will allow future development on the sdks to eventually switch to using POST for login requests, instead of GET. Perhaps at some distant point in time we could even consider deprecating and eventually removing the GET approach, but for now keeping it is essential for reverse compatibility.
Just to make a point there really isn't anything too bad about the current approach using GET, so long as the connection is secure. The one thing that really stands out is that if logging is not done properly you risk including username/password combos in the log. Although this would really only happen if you were directly piping output from the process into a log or something like that. There is that chance that if you set up logging improperly and someone else has access to that log that you might have a problem. Although this doesn't really fix that this gives us the option to do so down the road.