Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Merged
jdneo merged 13 commits intoLeetCode-OpenSource:masterfromyihong0618:master
Jan 12, 2020
Merged

add thrid party login -- GitHub and LinkedIn#496

jdneo merged 13 commits intoLeetCode-OpenSource:masterfromyihong0618:master
Jan 12, 2020

Conversation

yihong0618
Copy link
Contributor

add thrid party login -- GitHub and LinkedIn.

@jdneo
Copy link
Member

jdneo commentedDec 27, 2019
edited
Loading

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:

  • LeetCode Account
  • LeetCode Cookie
  • Third-Party: GitHub
  • Third-Party: LinkedIn

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).

BTW, the CLI with the 3rd-party login has been released, you can update the CLI to2.6.20 in this PR.

@yihong0618
Copy link
ContributorAuthor

yihong0618 commentedDec 27, 2019
edited
Loading

@jdneo
You are right I also thinked of that, can I work on it this weekend and update the code?

@jdneo
Copy link
Member

Sure, thank you!

@yihong0618
Copy link
ContributorAuthor

@jdneo vscode-leetcode 2.6.20 not found ~

@jdneo
Copy link
Member

My bad! It should be available right now!

Copy link
Member

@jdneojdneo left a 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:
Copy link
Member

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.

{
label: "LeetCode Cookie",
description: "",
detail: "Use LeetCode cookie that copy from browser to login",
Copy link
Member

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

{
label: "Third-Party: GitHub",
description: "",
detail: "Use third party GitHub account to login",
Copy link
Member

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

{
label: "Third-Party: LinkedIn",
description: "",
detail: "Use third party LinkedIn account to login",
Copy link
Member

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

picks.push(
{
label: "LeetCode Account",
description: "",
Copy link
Member

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";
Copy link
Member

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?

Copy link
ContributorAuthor

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([
Copy link
Member

Choose a reason for hiding this comment

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

loginCommand -> loginArgsMapping

@yihong0618
Copy link
ContributorAuthor

@jdneo
Thank you for your review, the naming problems had changed, and please note that it is not support for leetcode-cn.com for GitHub and LinkedIn login in leetcode-cli 2.6.20 and I raised a commit in thispr to support it.

@jdneo
Copy link
Member

2.6.21 has been published to npm

@yihong0618
Copy link
ContributorAuthor

yihong0618 commentedDec 30, 2019
edited
Loading

@jdneo

2.6.21 has been published to npm

Try again, two-factor seems can not support in our way in vscode-leetcode, because maybe the async to sync way in request is using read fs but in vscode terminal doesn't support. I will try to figure out another if i could. Thank you.
image
Can you just note users that we are not support two-factor in vscode-leetcode for now, if they want they can use cli to login first.
Thank you very much.

@jdneo
Copy link
Member

Thanks@yihong0618, let me also take a look first.

@yihong0618
Copy link
ContributorAuthor

yihong0618 commentedDec 31, 2019
edited
Loading

Thanks@yihong0618, let me also take a look first.

Thank you, that would be wonderful.

FYI
I am wrong that is zsh problem.
image
If I comment this line

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.
But I have problem with how to use thevscode.window.showInputBox with theondata

@jdneo
Copy link
Member

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.

@jdneo
Copy link
Member

@yihong0618

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.

  1. prompt-sync is declared in thedevDependencies, which means the module will be missing in the extension side
  2. Why not usingprompt butprompt-sync?

@yihong0618
Copy link
ContributorAuthor

@yihong0618

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.

  1. prompt-sync is declared in thedevDependencies, which means the module will be missing in the extension side
  2. Why not usingprompt butprompt-sync?

Because all request with session are in callback,and prompt must in the sync way. Otherwide it will fall through。
2. I will try tonight thank you
3. I also found that~ will also fix

@yihong0618
Copy link
ContributorAuthor

@jdneo
Sorry, maybe I missed somthing, it didn't work on my win10 with this way.

@jdneo
Copy link
Member

@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);});});

Theprompt-sync is too buggy. I'm not in favor of using that module.

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.`"));}});

@yihong0618
Copy link
ContributorAuthor

@jdneo
You are right. Because I put the GitHub login without two-factor in the end of the code block, so it would fall through using prompt. And I think you are right. Really learned a lot from your project and the JS(TS), thank you. I will fix these tonight.

@jdneo
Copy link
Member

@yihong0618 No worry, I should thank you for all of the contributions you have made.

Will take a look at the PR recently.

Copy link
Member

@jdneojdneo left a comment

Choose a reason for hiding this comment

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

:shipit:

@jdneojdneo merged commitc20a2d5 intoLeetCode-OpenSource:masterJan 12, 2020
@jdneojdneo added this to the0.15.9 milestoneJan 12, 2020
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@jdneojdneojdneo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.16.0
Development

Successfully merging this pull request may close these issues.

2 participants
@yihong0618@jdneo

[8]ページ先頭

©2009-2025 Movatter.jp