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.

Sign up for GitHub

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

Jump to bottom

http: Add support in hash-password for reading from terminals/stdin (#3365) #3373

Merged
merged 5 commits into from
May 11, 2020
Merged

http: Add support in hash-password for reading from terminals/stdin (#3365) #3373

merged 5 commits into from
May 11, 2020

Conversation

0az
Copy link
Contributor

@0az 0az commented May 7, 2020

Implements #3365.

@CLAassistant
Copy link

CLAassistant commented May 7, 2020 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

0az
0az commented May 7, 2020
View reviewed changes
Copy link
Contributor Author

@0az 0az left a comment

Choose a reason for hiding this comment

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

I'll squash later, since I still need to fix the lint.

modules/caddyhttp/caddyauth/command.go Outdated Show resolved Hide resolved
@mholt mholt added the under review 🧐 Review is pending before merging label May 7, 2020
mholt
mholt requested changes May 7, 2020
View reviewed changes
Copy link
Member

@mholt mholt left a comment

Choose a reason for hiding this comment

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

Thanks for the change! I like where it's going.

modules/caddyhttp/caddyauth/command.go Outdated Show resolved Hide resolved
modules/caddyhttp/caddyauth/command.go Outdated Show resolved Hide resolved
modules/caddyhttp/caddyauth/command.go Outdated Show resolved Hide resolved
modules/caddyhttp/caddyauth/command.go Outdated Show resolved Hide resolved
@mholt mholt added this to the 2.1 milestone May 7, 2020
@0az @mholt
FIXUP: Apply suggestions from code review
66d8c2f 
Co-authored-by: Matt Holt <mholt@users.noreply.github.com>
mholt
mholt requested changes May 7, 2020
View reviewed changes
Copy link
Member

@mholt mholt left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks, getting closer!

I know, I appreciate the idea of congruency between the flag and the prompt, but nobody is really used to a password prompt that asks for "Plaintext" -- they are used to prompts that ask for "Password".

modules/caddyhttp/caddyauth/command.go Outdated Show resolved Hide resolved
modules/caddyhttp/caddyauth/command.go Outdated Show resolved Hide resolved
@0az 0az requested a review from mholt May 8, 2020 02:27
mholt
mholt approved these changes May 11, 2020
View reviewed changes
Copy link
Member

@mholt mholt left a comment

Choose a reason for hiding this comment

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

Tried it out, works great! 🎉 Thanks for the contribution!

@mholt mholt merged commit 5bde8d7 into caddyserver:master May 11, 2020
@mholt mholt removed the under review 🧐 Review is pending before merging label May 11, 2020
@0az 0az deleted the improve-hash-password branch May 11, 2020 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@francislavoie francislavoie francislavoie left review comments

@mholt mholt mholt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v2.1.0
Development

Successfully merging this pull request may close these issues.
4 participants
@0az @CLAassistant @mholt @francislavoie