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

[SHACK-212] [GH #175] Always expand host to include full user info #176

Merged
merged 1 commit into from
May 30, 2018

Conversation

marcparadise
Copy link
Member

@marcparadise marcparadise commented May 23, 2018

Always expand host to include full user info

To provide a consistent experience with multi- and single-target,
we will always construct a fully qualified url in the form:

proto://user-info@target

Where user-info is "username:password" (ref: RFC 3928[1])

We no longer pass the user or password option into train, so that there
is no conflict between those values and the values embedded in the url.
This ensures that train will only have one set of credentials to work
with per host, and won't do surprising things.

A blank username will be expanded in the form ":password@host.com" and
a blank password will be supplied in the form "username:@host.com", both
of which are valid under the RFC.

This fixes github issue #175

Signed-off-by: Marc A. Paradise marc.paradise@gmail.com

@marcparadise marcparadise requested a review from a team May 23, 2018 18:30
@marcparadise marcparadise changed the title WIP - always put full creds in url passed to train Always expand host to include full user info May 23, 2018
Copy link
Contributor

@jonsmorrow jonsmorrow left a comment

Choose a reason for hiding this comment

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

LGTM

@marcparadise marcparadise changed the title Always expand host to include full user info [SHACK-212] [GH #175] Always expand host to include full user info May 30, 2018
To provide a consistent experience with multi- and single-target,
we will always construct a fully qualified url in the form:

proto://user-info@target

Where user-info is "username:password" (ref: RFC 3928[1] s. 3.2.1)

We no longer pass the user or password option into train, so that there
is no conflict between those values and the values embedded in the url.
This ensures that train will only have one set of credentials to work
with per host, and won't do surprising things.

A blank username will be expanded in the form ":password@host.com" and
a blank password will be supplied in the form "username:@host.com", both
of which are valid under the RFC.

This fixes github issue #175
[1] https://www.ietf.org/rfc/rfc3986.txt
Signed-off-by: Marc A. Paradise <marc.paradise@gmail.com>Always expand host to include full user info
Signed-off-by: Marc A. Paradise <marc.paradise@gmail.com>
Copy link
Contributor

@tyler-ball tyler-ball left a comment

Choose a reason for hiding this comment

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

tenor-120917471

@tyler-ball tyler-ball merged commit 45e9b23 into master May 30, 2018
@tyler-ball tyler-ball deleted the mp/password-flag-fix branch May 30, 2018 22:30
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