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

feat(webapp): add ssh key authentication #796

Merged
merged 12 commits into from
Apr 5, 2024
Merged

feat(webapp): add ssh key authentication #796

merged 12 commits into from
Apr 5, 2024

Conversation

irvingoujAtDevolution
Copy link
Contributor

@irvingoujAtDevolution irvingoujAtDevolution commented Apr 3, 2024

demo video:

private.key.working.demo.mp4

I tried to use PrimeNG file upload component, but it does not suit our need.
Please let me know if by using extraData attribute a good idea when creating new session.

@irvingoujAtDevolution irvingoujAtDevolution marked this pull request as ready for review April 4, 2024 00:28
this.canConnectExtraCallback = () => callback();
}

resetCanConnectCallback(){
Copy link
Contributor

@kristahouse kristahouse Apr 4, 2024

Choose a reason for hiding this comment

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

I'll be picky here and ask to add a space after the parathesis 🧐 Everything else looks great though!
resetCanConnectCallback() {

@kristahouse
Copy link
Contributor

kristahouse commented Apr 4, 2024

Code looks good. Nice job @irvingoujAtDevolution! I agree adding the extraData attribute is a good idea when creating new session.

I will add Paul's comments so we can keep @CBenoit in the loop:

Following the UI in DVLS for the ssh key: We usually display the key in a text box, after the user uploads the key. The text box could also allow the user to type the key manually or paste it in quickly.

@CBenoit CBenoit changed the title feat(webapp):add ssh key authentication feat(webapp): add ssh key authentication Apr 4, 2024
@CBenoit
Copy link
Member

CBenoit commented Apr 4, 2024

I will add Paul's comments so we can keep @CBenoit in the loop:

Following the UI in DVLS for the ssh key: We usually display the key in a text box, after the user uploads the key. The text box could also allow the user to type the key manually or paste it in quickly.

Thank you! This is a sensible UX.

Code-wise, I have nothing more to add!

@irvingoujAtDevolution
Copy link
Contributor Author

GW.private.key.working.after.udpate.mp4

updated as described

Copy link
Member

@CBenoit CBenoit left a comment

Choose a reason for hiding this comment

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

Out of curiosity, is it still possible to drag and drop the file?

@irvingoujAtDevolution
Copy link
Contributor Author

irvingoujAtDevolution commented Apr 4, 2024

Out of curiosity, is it still possible to drag and drop the file?

No, it is not implemented for this text area, but it is possible to do it.

irvingoujAtDevolution and others added 2 commits April 4, 2024 12:53
…le-control/file-control.component.html

Co-authored-by: Benoît Cortier <bcortier@proton.me>
…le-control/file-control.component.html

Co-authored-by: Benoît Cortier <bcortier@proton.me>
@CBenoit
Copy link
Member

CBenoit commented Apr 4, 2024

Do you think you could check with Paul or Krista if this should be implemented?

@irvingoujAtDevolution
Copy link
Contributor Author

Do you think you could check with Paul or Krista if this should be implemented?

sure, that's a good idea.

@irvingoujAtDevolution
Copy link
Contributor Author

irvingoujAtDevolution commented Apr 4, 2024

GW.final.versio.private.key.mp4

@kristahouse
Copy link
Contributor

LGTM! Well done @irvingoujAtDevolution! 🔥

Copy link
Member

@CBenoit CBenoit left a comment

Choose a reason for hiding this comment

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

Good job!!

@CBenoit CBenoit merged commit a884cbb into master Apr 5, 2024
20 checks passed
@CBenoit CBenoit deleted the Add-SSH-Key-Auth branch April 5, 2024 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants