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(ssh_tunnel): add support for authentication via ipv4 and ipv6 #121

Merged
merged 3 commits into from
Jun 13, 2024

Conversation

empwilli
Copy link
Contributor

Some users of the situation faced the situation where redirect link the oauth2 flow was not invoked correctly. The problem here was that localhost was resolved differently, depending on the respective user's machine (either as IPv4 only or as IPv6 only). If the redirect server, however, bound to the opposing protocol, the resolver link did not work.

To the best of our knowledge, this is an issue with actix (or underlying crates), as the "bind" method promises to bind to either IPv4 and IPv6 if "localhost" is specified.

With this fix we resolve the issue by extending the authentication info so that multiple bind addresses can be specified. We then explicitly specify both, the IPv4 and the IPv6 localhost addresses.

Some users of the situation faced the situation where redirect link the oauth2
flow was not invoked correctly. The problem here was that localhost was resolved
differently, depending on the respective user's machine (either as IPv4 only or
as IPv6 only). If the redirect server, however, bound to the opposing protocol,
the resolver link did not work.

To the best of our knowledge, this is an issue with actix (or underlying
crates), as the "bind" method promises to bind to either IPv4 and IPv6 if
"localhost" is specified.

With this fix we resolve the issue by extending the authentication info so that
multiple bind addresses can be specified. We then explicitly specify both, the
IPv4 and the IPv6 localhost addresses.
@empwilli
Copy link
Contributor Author

Note: this requires an update of the test config in our test plan

Copy link
Contributor

@mlilien mlilien left a comment

Choose a reason for hiding this comment

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

no version change in Config.{toml/lock}

@JanZachmann
Copy link
Contributor

JanZachmann commented Jun 11, 2024

Note: this requires an update of the test config in our test plan

do you mean the concourse pipeline test? can you explain why?

@empwilli
Copy link
Contributor Author

Note: this requires an update of the test config in our test plan

do you mean the concourse pipeline test? can you explain why?

In our test plans we provide a test configuration for the ssh connection environment. With this change we now provide a list of bind addresses instead of a single one, the test plan configuration must be updated accordingly. The note above was a reminder to update that config as soon as the PR is merged.

Copy link
Contributor

@JanZachmann JanZachmann left a comment

Choose a reason for hiding this comment

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

@empwilli
Imho we should change PR title. The value in brackets is supposed to be a feature.

  • fix(ssh_tunnel): add support for authentication via ipv4 and ipv6
  • feat(ssh_tunnel): add support for authentication via ipv4 and ipv6

@empwilli empwilli changed the title fix(auth): add support for authentication via ipv4 and ipv6 feat(ssh_tunnel): add support for authentication via ipv4 and ipv6 Jun 12, 2024
@empwilli empwilli merged commit e53d0d8 into omnect:main Jun 13, 2024
3 checks passed
@empwilli empwilli deleted the fix/ssh_redirect_address branch June 20, 2024 11:08
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.

4 participants