- Notifications
You must be signed in to change notification settings - Fork670
add thrid party login -- GitHub and LinkedIn#496
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
jdneo commentedDec 27, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Hi@yihong0618, Thank you for your contribution. I have one idea about the login experience, please let me know your thoughts on this: We still have only one entry(command) for the login, but when users trigger the command, a drop down list pops up, having the following options:
By doing this, the shortcut in the explorer would be applied to all kinds of login methods. And users won't need to remember different login commands. (Imagine that we might support more 3rd-party login in the future).
|
yihong0618 commentedDec 27, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@jdneo |
Sure, thank you! |
@jdneo vscode-leetcode 2.6.20 not found ~ |
My bad! It should be available right now! |
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.
Thank you! Just a few comments.
README.md Outdated
@@ -56,6 +56,9 @@ Thanks for [@yihong0618](https://github.com/yihong0618) provided a workaround wh | |||
- **LeetCode: Sign in (by cookie)** | |||
- **LeetCode: Sign out** | |||
- You can also use the following command to sign in by third party: |
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.
Please only reserve the common login command as it was before.
src/leetCodeManager.ts Outdated
{ | ||
label: "LeetCode Cookie", | ||
description: "", | ||
detail: "Use LeetCode cookie that copy from browser to login", |
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.
Use LeetCode cookie copied from browser to login
src/leetCodeManager.ts Outdated
{ | ||
label: "Third-Party: GitHub", | ||
description: "", | ||
detail: "Use third party GitHub account to login", |
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.
Use GitHub account to login
src/leetCodeManager.ts Outdated
{ | ||
label: "Third-Party: LinkedIn", | ||
description: "", | ||
detail: "Use third party LinkedIn account to login", |
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.
Use LinkedIn account to login
src/leetCodeManager.ts Outdated
picks.push( | ||
{ | ||
label: "LeetCode Account", | ||
description: "", |
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.
No need to set the fielddescription
to empty string if you do not want to render it. Just remove it from the Object.
if (!commandArg) { | ||
throw new Error(`The login method "${loginMethod}" is not supported.`); | ||
} | ||
const isByCookie: boolean = loginMethod === "Cookie"; | ||
const inMessage: string = isByCookie ? "sign in by cookie" : "sign in"; |
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.
Is this variableinMessage
used anywhere?
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.
Yes, it is for the difference of cookie login or other login message.
e.g.
promptForOpenOutputChannel(
Failed to ${inMessage}. Please open the output channel for details
src/shared.ts Outdated
@@ -12,6 +12,13 @@ export enum UserStatus { | |||
SignedOut = 2, | |||
} | |||
export const loginCommand: Map<string, string> = new Map([ |
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.
loginCommand -> loginArgsMapping
2.6.21 has been published to npm |
yihong0618 commentedDec 30, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Thanks@yihong0618, let me also take a look first. |
yihong0618 commentedDec 31, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Thank you, that would be wonderful.FYI if(!pwd){childProc.kill();returnresolve(undefined);}childProc.stdin.write(`${pwd}\n`);// childProc.stdin.end(); vscode side can receive the "two-factor" message from cli side. |
Sorry my laptop broke down last weekend and I have to send it to the store for repairment. I will try to resolve this PR in this week. |
You can write something like this: childProc.stdout.on("data",async(data:string|Buffer)=>{data=data.toString();leetCodeChannel.append(data);if(data.includes("two-factor")){consttwoFactor:string|undefined=awaitvscode.window.showInputBox({prompt:"Enter two-factor code.",validateInput:(s:string):string|undefined=>s&&s.trim() ?undefined :"The input must not be empty",});if(!twoFactor){childProc.kill();returnresolve(undefined);}childProc.stdin.write(`${twoFactor}\n`);}else{constmatch:RegExpMatchArray|null=data.match(/(?:.*)Successfully(login|cookielogin|thirdpartylogin)as(.*)/i);if(match&&match[2]){returnresolve(match[2]);}}}); But I found that there is some problems in the CLI side.
|
Because all request with session are in callback,and prompt must in the sync way. Otherwide it will fall through。 |
@jdneo |
@yihong0618, not quite get your point. Every async can should be able to change to sync from my understanding. You can simply change it to: if(resp.request.uri.href!==urls.github_tf_redirect){returnrequestLeetcodeAndSave(_request,leetcodeUrl,user,cb);}prompt.colors=false;prompt.message='';prompt.start();prompt.get([{name:'code',required:true}],function(e,result){if(e)returnlog.fail(e);constauthenticityTokenTwoFactor=body.match(/name="authenticity_token"value="(.*?)"/);if(authenticityTokenTwoFactor===null){returncb('Get GitHub two-factor token failed');}constoptionsTwoFactor={url:urls.github_tf_session_request,method:'POST',headers:{'Content-Type':'application/x-www-form-urlencoded',},followAllRedirects:true,form:{'otp':result.code,'authenticity_token':authenticityTokenTwoFactor[1],'utf8':encodeURIComponent('✓'),},};_request(optionsTwoFactor,function(e,resp,body){if(resp.request.uri.href===urls.github_tf_session_request){returncb('Invalid two-factor code please check');}requestLeetcodeAndSave(_request,leetcodeUrl,user,cb);});}); The By using the above code, then you can update the extension side, with following two callbacks change: childProc.stdout.on("data",async(data:string|Buffer)=>{data=data.toString();leetCodeChannel.append(data);if(data.includes("twoFactorCode")){consttwoFactor:string|undefined=awaitvscode.window.showInputBox({prompt:"Enter two-factor code.",validateInput:(s:string):string|undefined=>s&&s.trim() ?undefined :"The input must not be empty",});if(!twoFactor){childProc.kill();returnresolve(undefined);}childProc.stdin.write(`${twoFactor}\n`);childProc.stdin.end();}else{constmatch:RegExpMatchArray|null=data.match(/(?:.*)Successfully.*loginas(.*)/i);if(match&&match[1]){childProc.stdin.end();returnresolve(match[1]);}}}); & childProc.on("close",(code:number)=>{if(code!==0){reject(newError("Failed to login.`"));}}); |
@jdneo |
@yihong0618 No worry, I should thank you for all of the contributions you have made. Will take a look at the PR recently. |
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.
add thrid party login -- GitHub and LinkedIn.