[IMPROVE] Support for Google OAuth for mobile app#22014
[IMPROVE] Support for Google OAuth for mobile app#22014sampaiodiego merged 2 commits intodevelopfrom
Conversation
…pp, adding new information to the resulting request.
sampaiodiego
left a comment
sampaiodiego
left a comment
There was a problem hiding this comment.
I'm approving but I'll let it opened still since I added some comments, but them could be addressed in the future, would be better if we could address before merging though.
| // The code on this file was copied directly from Meteor and modified to support mobile google oauth | ||
| // https://github.com/meteor/meteor/blob/ffcfa5062cf1bf8a64ea64fef681ffcd99fe7939/packages/oauth/oauth_server.js | ||
|
|
||
| Meteor.startup(() => { |
There was a problem hiding this comment.
any special reason to have this in a Meteor.startup? Otherwise I'd remove it.
| redirectUrl = OAuth._stateFromQuery(details.query).redirectUrl; | ||
| const appHost = Meteor.absoluteUrl(); | ||
|
|
||
| if (redirectUrl.startsWith(appRedirectUrl)) { |
There was a problem hiding this comment.
we might need to think a way to have this custom code easier to spot and maintain. Let's say on next Meteor version this code changes and we need to copy the new code over here, it might be not that easy to spot our changes and keep them.
| import '../app/file-upload'; | ||
| import '../app/github-enterprise/server'; | ||
| import '../app/gitlab/server'; | ||
| import '../app/google-oauth/server'; |
There was a problem hiding this comment.
we should not create new things inside /app folder. we need to think where this file would fit in a new folder structure.
since this is overwriting Meteor's internal functions, maybe we could make that explicit making it easier to spot possible issues when updating Meteor.. maybe something like /server/overwrites/meteor/oauth.js .. idk, didn't like "overwrites" 🙈 wdyt?
There was a problem hiding this comment.
What about /server/custom/...
sampaiodiego
changed the title
3 participants
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments