Conversation
ttaylorr
left a comment
There was a problem hiding this comment.
Few comments, otherwise looks good 👍
test/cmd/lfstest-gitserver.go
Outdated
| func writeLFSError(w http.ResponseWriter, code int, msg string) { | ||
| by, err := json.Marshal(&lfsError{Message: msg}) | ||
| if err != nil { | ||
| w.Header().Set("Content-Type", "text/plain") |
There was a problem hiding this comment.
You can use http.Error(), which I think would look very clean:
by, err := json.Marshal(&lfsError{Message: msg})
if err != nil {
http.Error(w, errors.Wrap(err, "json encoding error").Error(), 500)
return
}
// ...| w.WriteHeader(401) | ||
|
|
||
| // ntlmNegotiateMessage from httputil pkg | ||
| case "NTLM TlRMTVNTUAABAAAAB7IIogwADAAzAAAACwALACgAAAAKAAAoAAAAD1dJTExISS1NQUlOTk9SVEhBTUVSSUNB": |
There was a problem hiding this comment.
Could this be extracted to a const with some godoc?
There was a problem hiding this comment.
It could, but it's already in httputil (and will be moved to lfsapi. Could just export it there, and use here.
test/test-batch-transfer.sh
Outdated
| begin_test "batch transfer" | ||
| ( | ||
| set -e | ||
| exit 0 |
There was a problem hiding this comment.
Did you mean to delete this locally? 😁
There was a problem hiding this comment.
Doesn't look like anything to me.
| ) | ||
| end_test | ||
|
|
||
| begin_test "batch transfers with ntlm server" |
There was a problem hiding this comment.
Nice, this looks really clean 👍
| } | ||
|
|
||
| auth := authHeader[5:] // strip "ntlm " prefix | ||
| val, err := base64.StdEncoding.DecodeString(auth) |
There was a problem hiding this comment.
Should we be using base64.UrlEncoding here since this is from an HTTP request?
There was a problem hiding this comment.
No, this is going in an http header and not a url parameter.
ttaylorr
left a comment
There was a problem hiding this comment.
Thanks for addressing. Looks great!
|
Closing in favor of #1840 |
This adds an integration test for NTLM authentication.