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 http auth to static warm #7115

Merged
merged 6 commits into from
Nov 29, 2022

Conversation

moritzlang
Copy link

Problem:
Staging systems are often behind a HTTP basic authentication. Calling the current static warm command on a site behind HTTP basic authentication (e.g. from a CI job) produces a "401 Authorization Required" error.

Solution:
This pull request adds the possibility to pass the authentication credentials via the parameters --u (HTTP authentication user) and --p (HTTP authentication password) to the static warm command.

@what-the-diff
Copy link

what-the-diff bot commented Nov 24, 2022

  • Added the --user and --password options to the command signature.
  • Updated client configuration with authentication credentials if provided, otherwise null is passed in as auth option value (to avoid passing an empty array).

@edalzell
Copy link
Contributor

Oh I like this one, nicely done

@arthurperton
Copy link
Contributor

Nice one! The convention for command line arguments is to use full words with double dashes, and short (often single letter) with single dashes. So instead of --u and --p I'd use either -u and -p or for example --user and --pass or even --username and --password.

@moritzlang
Copy link
Author

Thanks @arthurperton for pointing this out! I adjusted the arguments in order to comply with the convention.

Other commands in this repository generally use full words for the arguments, therefore I also used this form here.
For the naming of the arguments I followed the well-known wget command which uses --user and --password.

@arthurperton
Copy link
Contributor

Nice 👌🏻

@jasonvarga
Copy link
Member

You could add shortcuts if you want.

https://laravel.com/docs/9.x/artisan#option-shortcuts

e.g.

{--u|user=}
{--p|password=}

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