fix: do not inject GIT_USERNAME and GIT_PASSWORD into git clone URL #141
Conversation
| } | ||
| srv := httptest.NewServer(opts.authMW(createGitHandler(t, opts))) | ||
| return srv.URL | ||
| } | ||
|
|
||
| func checkBasicAuth(username, password string) func(http.Handler) http.Handler { |
There was a problem hiding this comment.
review: moved to gittest
There was a problem hiding this comment.
is it intended to be used anywhere else in envbuilder? I'm considering moving it to integration/gittest.go
Ok, I found the place. Speaking of consistency with coder/coder that should land in testutil.
There was a problem hiding this comment.
Will do in a follow-up PR 👍
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestCloneRepo(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| t.Run("Clones", func(t *testing.T) { | ||
| t.Parallel() | ||
| for _, tc := range []struct { |
There was a problem hiding this comment.
review: trying to cover all cases outlined here:
https://github.com/coder/envbuilder?tab=readme-ov-file#git-authentication
johnstcn
changed the title
| } | ||
| srv := httptest.NewServer(opts.authMW(createGitHandler(t, opts))) | ||
| return srv.URL | ||
| } | ||
|
|
||
| func checkBasicAuth(username, password string) func(http.Handler) http.Handler { |
There was a problem hiding this comment.
is it intended to be used anywhere else in envbuilder? I'm considering moving it to integration/gittest.go
Ok, I found the place. Speaking of consistency with coder/coder that should land in testutil.
| return string(content) | ||
| } | ||
|
|
||
| func setupGit(t *testing.T, user, pass string) (url string) { |
There was a problem hiding this comment.
What about moving this to gittest?
There was a problem hiding this comment.
I'll open a a separate PR to refactor this 👍
Fixes #126
We had been writing the
GIT_USERNAMEandGIT_PASSWORDinto the git auth URL as well as setting basic HTTP auth credentials.This causes
GIT_USERNAMEandGIT_PASSWORDto be present in the.git/configof the clond repo, which is not desirable.This PR removes that behaviour and modifies existing unit tests to assert that the git clone URL is not modified from what the user passes in.
The previous behaviour can be preserved by explicitly setting basic auth in the git clone URL, if required.