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

Check for ssh clients when logging in #180

Merged
merged 2 commits into from Sep 19, 2019
Merged

Conversation

tomer-stripe
Copy link
Collaborator

Reviewers

r? @ob-stripe @brandur-stripe
cc @stripe/dev-platform

Summary

This checks if the CLI is possibly running in an SSH client and if it is, prints the url instead of trying to open the browser (pulled from here: https://unix.stackexchange.com/questions/9605/how-can-i-detect-if-the-shell-is-controlled-from-ssh)

I tried to see if there was a good test to add here but since we're not catching stdout I wasn't sure of a decent way to validate this. Open to ideas!

Copy link

@brandur-stripe brandur-stripe left a comment

Choose a reason for hiding this comment

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

Is the general idea here that if SSHed, the open command (or Linux equivalent) will still work, but it'll just open it on the wrong system?

Looks like the build wants you to reorder your import statements, but LGTM.

I tried to see if there was a good test to add here but since we're not catching stdout I wasn't sure of a decent way to validate this. Open to ideas!

Yeah, going to be tricky without output capture. It's probably okay.

@tomer-stripe
Copy link
Collaborator Author

tomer-stripe commented Sep 18, 2019

Is the general idea here that if SSHed, the open command (or Linux equivalent) will still work, but it'll just open it on the wrong system?

Yea, that's a possibility here! It's come up a few times that people have been ssh'd on a box and tried to login but got the "opening browser" prompt.

@tomer-stripe tomer-stripe merged commit 8d1e862 into master Sep 19, 2019
@tomer-stripe tomer-stripe deleted the tomer/detect-ssh branch September 19, 2019 03:27
@ob-stripe
Copy link
Contributor

Yeah, we need to come up with a standard way of passing stdin/out/err handles to all CLI commands so we can replace them with buffers in test.

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

Successfully merging this pull request may close these issues.

None yet

4 participants