Skip to content
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

solve GitHub login two-factor auth problem and add new node package p… #35

Merged
merged 9 commits into from
Dec 30, 2019
Merged

Conversation

yihong0618
Copy link

…rompt-sync
Solve two-factor auth problem and format some code.

Please help me check if this way can use in vscode-leetcode or not, because I did not find a way, thank you very much.

Copy link

@jdneo jdneo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just tried with my GitHub account and it works!!

lib/plugins/leetcode.js Outdated Show resolved Hide resolved
lib/plugins/leetcode.js Outdated Show resolved Hide resolved
lib/plugins/leetcode.js Outdated Show resolved Hide resolved
lib/plugins/leetcode.js Outdated Show resolved Hide resolved
lib/plugins/leetcode.js Outdated Show resolved Hide resolved
@yihong0618
Copy link
Author

yihong0618 commented Dec 25, 2019

@jdneo
Hi~ I fixed the code by your so kind review, thank you for your help.
And I have not solved the two-factor in vscode-leetcode maybe the sync-prompt way doesn't support I don't konw~, so this pr only support GitHub and LinkedIn way without two-factor.
By the way I found a way to add "+86" phone number with two-factor in GitHub because of this pr~
see this, kind of interesting, share with you~
Thank you for your patience again.

@jdneo
Copy link

jdneo commented Dec 26, 2019

@yihong0618 could you please share more information about the errors you observed from the vs code extension side?

Since the third-party login can unblock a lot of users, how about we release a new CLI without the two-factor auth support at first. Then we revisit this issue after that?

@yihong0618
Copy link
Author

yihong0618 commented Dec 26, 2019

@jdneo
OK, the formation about the errors will send here maybe tonight.
You are right, maybe you can release the last pr and this, the vscode-leetcode side for two-factor later will be best.
And I vscode-leetcode pr has been raised~

@yihong0618
Copy link
Author

yihong0618 commented Dec 27, 2019

@jdneo
I try to add two-factor in vsocde-leetcode but when prompt the two input the errors are as below

login: pass: Please enter your two-factor code:
fs.js:114
throw err;
^

Error: EOF: end of file, read
at Object.readSync (fs.js:498:3)
at prompt (c:\Users\hongyi\Documents\projects\new-vscode-leetcode\vscode-leetcode\node_modules\prompt-sync\index.js:85:17)
at Request._callback (c:\Users\hongyi\Documents\projects\new-vscode-leetcode\vscode-leetcode\node_modules\vsc-leetcode-cli\lib\plugins\leetcode.js:623:55)
at Request.self.callback (c:\Users\hongyi\Documents\projects\new-vscode-leetcode\vscode-leetcode\node_modules\request\request.js:185:22)
at Request.emit (events.js:198:13)
at Request. (c:\Users\hongyi\Documents\projects\new-vscode-leetcode\vscode-leetcode\node_modules\request\request.js:1161:10)
at Request.emit (events.js:198:13)
at IncomingMessage. (c:\Users\hongyi\Documents\projects\new-vscode-leetcode\vscode-leetcode\node_modules\request\request.js:1083:12)
at Object.onceWrapper (events.js:286:20)
at IncomingMessage.emit (events.js:203:15)

And no problem for no two-factor
image
image

I think vscode-leetcode only support GitHub login without two-factor will be good for now.
And this pr has no side effect in vscode-leetcode side.
So I think the pr can support two-factor in leetcode-cli and we can find other way to support vscode-leetcodet in the future.
Thank you~

@jdneo
Copy link

jdneo commented Dec 27, 2019

I see. I'll release the CLI first to unblock the vscode-leetcode's PR. Just leave this PR here for now until we release the VS Code LeetCode.

@yihong0618
Copy link
Author

yihong0618 commented Dec 27, 2019

@jdneo
Thank you very much.
And can you help me to solve the ci problem, because every time I failed and I don't find the infomation why.

@yihong0618
Copy link
Author

@jdneo
Just support GitHub and LinkedIn login for leetcode-cn.com

lib/plugins/leetcode.js Outdated Show resolved Hide resolved
return cb(`${party} login failed or ${party} did not connect to LeetCode`);
}
let cookieData = {};
if (leetcodeUrl.includes('cn')) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cn -> leetcode-cn.com to avoid mismatch

lib/plugins/leetcode.js Outdated Show resolved Hide resolved
requestLeetcodeAndSave(_request, leetcodeUrl, user, cb, config.sys.login_methods.GitHub);
});
} else {
requestLeetcodeAndSave(_request, leetcodeUrl, user, cb, config.sys.login_methods.GitHub);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the calling of requestLeetcodeAndSave in line 648 and line 651 merged into one call -- move it out of the if-else block?

Copy link
Author

@yihong0618 yihong0618 Dec 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, these two are with different session(_request), the first one is two-factor, the second one is normal. Both need.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. How about

if (resp.request.uri.href !== urls.github_tf_redirect) {
  return requestLeetcodeAndSave(_request, leetcodeUrl, user, cb, config.sys.login_methods.GitHub);
}

cb('Your GitHub are using two-factor authentication');
// read two-factor code must be sync.
const twoFactorcode = require('prompt-sync')()('Please enter your two-factor code: ');
const authenticityTokenTwoFactor = body.match(/name="authenticity_token" value="(.*?)"/);
if (authenticityTokenTwoFactor === null) {
  return cb('Get GitHub two-factor token failed');
}
const optionsTwoFactor = {
  url:     urls.github_tf_session_request,
  method:  'POST',
  headers: {
    'Content-Type': 'application/x-www-form-urlencoded',
  },
  followAllRedirects: true,
  form:               {
    'otp':                twoFactorcode,
    'authenticity_token': authenticityTokenTwoFactor[1],
    'utf8':               encodeURIComponent('✓'),
  },
};
_request(optionsTwoFactor, function(e, resp, body) {
  if (resp.request.uri.href === urls.github_tf_session_request) {
    return cb('Invalid two-factor code please check');
  }
  requestLeetcodeAndSave(_request, leetcodeUrl, user, cb, config.sys.login_methods.GitHub);
});

which can save some indentions.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! Very clean code. Thank you.

lib/plugins/leetcode.js Outdated Show resolved Hide resolved
…e if cn test) to make it more clear, also fixed a tiny bug before
lib/config.js Outdated
plugin: 'https://raw.githubusercontent.com/leetcode-tools/leetcode-cli-plugins/master/plugins/$name.js'
},
// login methods enum
login_methods: {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This field can also be removed

Copy link

@jdneo jdneo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done!

@jdneo jdneo merged commit a11e137 into leetcode-tools:master Dec 30, 2019
@E011011101001
Copy link

Did this PR make cookie login unusable?

In leetcode.js, function parseCookie (Line 541) is changed and cb becomes the third parameter. However when it is called at Line 577, cb remains to be the second argument. This problem remains in the latest commit today

When I was running this tool trying to login by cookie, I got a error says TypeError: cb is not a function. I think this bug was imported here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants