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

Add support for jump hosts. Resolves #17. #18

Merged
merged 2 commits into from
Mar 24, 2020

Conversation

cynthia
Copy link
Contributor

@cynthia cynthia commented Mar 13, 2020

It is worth noting that one drawback of this feature is that it will only work on OpenSSH version 7.3 and above. Given that 7.3 has been released in August 2016, I don't consider this a huge problem.

Could you please review and let me know if this is a feature that can be introduced in mainline?

@cynthia
Copy link
Contributor Author

cynthia commented Mar 13, 2020

한글 설명.

최근 코로나바이러스로 재택 근무를 시행하고 있는 곳이 많은데, 이 중에서 VPN 대신 외부에서 내부망 접근이 가능한 바스티온 서버를 통해서 작업을 많이 하고 있습니다. 바스티온 서버 자체는 헤드레스 환경이라 지오프론트 인증이 조금 어려운 점도 있지만, 그것 보다도 인증 스테이트를 공용 서버에 두는건 보안적으로 좋지 않을것으로 판단, 바스티온을 프락시로 이용하여 개인 PC에서 바로 접근하게 하려는 패치입니다.

내부적으로는 OpenSSH에서 제공하는 ProxyJump (-J) 인자를 추가하는 수준의 간단한 패치로, CLI상 부작용의 우려는 적습니다.

CC: @dahlia

@kanghyojun kanghyojun self-requested a review March 17, 2020 09:52
@kanghyojun
Copy link
Contributor

@pbzweihander 님 이 PR 리뷰해주실 수 있나요?

@pbzweihander pbzweihander self-requested a review March 17, 2020 09:54
@@ -329,6 +329,8 @@ def ssh(args, alias=None):
options = get_ssh_options(remote)
except ValueError as e:
ssh.error(str(e))
if args.jump_host:
options.extend(['-o', 'ProxyJump==%s' % (args.jump_host)])
Copy link
Contributor

Choose a reason for hiding this comment

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

python3에서도 퍼센트로 문자열 포매팅이 되긴합니다만, str.format을 사용하며 포매팅 해주실 수 있을까요?

Suggested change
options.extend(['-o', 'ProxyJump==%s' % (args.jump_host)])
options.extend(['-o', 'ProxyJump=={}'.format(args.jump_host)])

@@ -358,6 +360,8 @@ def scp(args):
options.extend(['-S', args.ssh])
if args.recursive:
options.append('-r')
if args.jump_host:
options.extend(['-o', 'ProxyJump==%s' % (args.jump_host)])
Copy link
Contributor

Choose a reason for hiding this comment

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

여기도 마찬가지입니다.

Suggested change
options.extend(['-o', 'ProxyJump==%s' % (args.jump_host)])
options.extend(['-o', 'ProxyJump=={}'.format(args.jump_host)])

@kanghyojun
Copy link
Contributor

kanghyojun commented Mar 18, 2020

@cynthia 지난번 PR(https://github.com/spoqa/geofront-cli/pull/16)과 이어서 PR 제안해주셔서 감사합니다. 사소하긴하지만 리뷰한 코드 살펴봐주시겠어요?

Copy link

@pbzweihander pbzweihander left a comment

Choose a reason for hiding this comment

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

@kanghyojun 님이 리뷰해주신 사항만 반영해주시면 될 것 같습니다!

@cynthia
Copy link
Contributor Author

cynthia commented Mar 22, 2020

모레 정도에 리뷰 커멘트 반영해서 팔로업 패치 보내보겠습니다.

@cynthia
Copy link
Contributor Author

cynthia commented Mar 24, 2020

@kanghyojun 요청 사항 픽스업 커밋에 반영해놨습니다.

여담입니다만, '{}'.format(str) 이 '%s' % (str) 보다 10배 정도 느립니다.

@kanghyojun
Copy link
Contributor

오! 그렇군요. 패치 감사합니다

@kanghyojun kanghyojun merged commit 15025cc into geofront-auth:master Mar 24, 2020
@kanghyojun
Copy link
Contributor

0.4.3으로 릴리스되었습니다.

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