Add custom error for basic auth with HTTP #1176
Conversation
openshift-ci
bot
added
the
release-note
HeavyWombat
force-pushed
the
issue/1131
branch
3 times, most recently
from
25bd9c4
to
74fa43f
Compare
cmd/git/main.go
Outdated
| case hasUsername && hasPassword && !isHTTPSURL: | ||
| return typeUndef, &ExitError{ | ||
| Code: 110, | ||
| Message: shpgit.AuthUnexpectedHTTP.ToMessage(), | ||
| Reason: shpgit.AuthUnexpectedHTTP, | ||
| } |
There was a problem hiding this comment.
You're here checking if the URL is not HTTPS and then return an error stating that HTTP is unexpected. What happens if the protocol is neither http nor https but maybe htps ?
There was a problem hiding this comment.
Yeah, good point. I tried to be too minimalistic here I guess.
Change test cases for username and password to use same function style.
Fix linter warning by renaing variable or inlining them.
Introduce display URL that is a safe version of the provided URL to be used for logging. Refactor test case code in order to allow for more flexibility.
Do not write the SSH key in the test output.
HeavyWombat
force-pushed
the
issue/1131
branch
from
74fa43f
to
11ff2da
Compare
| @@ -127,13 +129,18 @@ func Execute(ctx context.Context) error { | |||
| return err | |||
| } | |||
|
|
|||
| checkGitVersionSpecificSettings(ctx) | |||
There was a problem hiding this comment.
Inlined the two checks from checkGitVersionSpecificSettings in the main Execute function to make it simpler and more readable.
|
|
||
| return nil | ||
| // Create clean version of the URL that should be safe to be displayed in logs | ||
| displayURL = cleanURL() |
There was a problem hiding this comment.
This line line is really new.
| func withLogOutput(out io.Writer) runOpts { return func(o *opts) { o.logOutput = out } } | ||
| func withArgs(args ...string) runOpts { return func(o *opts) { o.args = args } } | ||
|
|
||
| func run(o ...runOpts) error { |
There was a problem hiding this comment.
Moved the run function from within the Describe into the global context and changed it that it is more flexible.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: SaschaSchwarze0 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
Changes
Fixes #1131
Fixes #1133
Ref: #1116
Check that basic auth is not used in combination with a HTTP endpoint.
Redact log output in case basic auth is used with inline user fields, i.e.
https://user:pass@url.com.Submitter Checklist
See the contributor guide
for details on coding conventions, github and prow interactions, and the code review process.
Release Notes