f814859ab2
Managed GitHub credentials compared a repository's scope identity verbatim across sides that spell it differently. A repository cloned over SSH stores an SSH remote_url, which kandev itself backfills for local-path repositories, and the strict scheme assertion added in #2117 then rejected the launch with "must use an HTTPS clone URL for managed credentials". Credential identity now comes from the persisted provider identity, whose host carries the real HTTPS origin including a non-default port, falling back to rewriting a non-HTTPS remote in place. Issuance and authorization derive it the same way, so they cannot disagree. Cloning is untouched. Scope paths are compared through one canonical form (no trailing slash, no ".git") at the shim, the broker redemption, and the backend authorizer, so the suffix never decides whether a lease matches. Lease scopes still carry the exact path their provider resolver was issued for, because plugin providers compare it verbatim. Closes #2673 Closes #2675
341 lines
12 KiB
Go
341 lines
12 KiB
Go
package main
|
|
|
|
import (
|
|
"bytes"
|
|
"context"
|
|
"encoding/json"
|
|
"io"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"strings"
|
|
"testing"
|
|
|
|
"github.com/kandev/kandev/internal/githubauth"
|
|
)
|
|
|
|
const (
|
|
envGitHubCredentialBrokerURL = githubauth.CredentialBrokerURLEnv
|
|
envGitHubCredentialLease = githubauth.CredentialLeaseEnv
|
|
envGitHubCredentialTaskID = githubauth.CredentialTaskIDEnv
|
|
envGitHubCredentialSessionID = githubauth.CredentialSessionIDEnv
|
|
envGitHubCredentialRepository = githubauth.CredentialRepositoryEnv
|
|
envGitHubCredentialOwner = githubauth.CredentialOwnerEnv
|
|
envGitHubCredentialRepo = githubauth.CredentialRepoEnv
|
|
envGitHubCredentialHost = githubauth.CredentialHostEnv
|
|
envGitHubCredentialScopes = githubauth.CredentialScopesEnv
|
|
)
|
|
|
|
func TestGitHubCredentialHelperGetsFreshCredential(t *testing.T) {
|
|
var got githubBrokerResolveRequest
|
|
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
if r.Method != http.MethodPost {
|
|
t.Errorf("method = %s, want POST", r.Method)
|
|
}
|
|
if err := json.NewDecoder(r.Body).Decode(&got); err != nil {
|
|
t.Errorf("decode request: %v", err)
|
|
}
|
|
w.Header().Set("Content-Type", "application/json")
|
|
_, _ = io.WriteString(w, `{"username":"x-access-token","password":"fresh-token"}`)
|
|
}))
|
|
t.Cleanup(server.Close)
|
|
env := githubCredentialTestEnv(server.URL)
|
|
var stdout bytes.Buffer
|
|
|
|
err := runGitHubCredentialHelper(
|
|
context.Background(),
|
|
[]string{"get"},
|
|
strings.NewReader("protocol=https\nhost=github.com\npath=acme/widgets.git\n\n"),
|
|
&stdout,
|
|
lookupEnv(env),
|
|
server.Client(),
|
|
)
|
|
if err != nil {
|
|
t.Fatalf("runGitHubCredentialHelper() error = %v", err)
|
|
}
|
|
if got.Lease != "opaque-lease" || got.TaskID != "task-1" || got.SessionID != "session-1" {
|
|
t.Fatalf("broker scope = %+v", got)
|
|
}
|
|
if got.RepositoryID != "repo-1" || got.Owner != "acme" || got.Repo != "widgets" || got.Host != "github.com" {
|
|
t.Fatalf("broker repository scope = %+v", got)
|
|
}
|
|
if want := "username=x-access-token\npassword=fresh-token\n\n"; stdout.String() != want {
|
|
t.Fatalf("credential helper output = %q, want %q", stdout.String(), want)
|
|
}
|
|
}
|
|
|
|
func TestGitHubCredentialHelperRejectsRepositoryScopeMismatch(t *testing.T) {
|
|
requests := 0
|
|
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
|
requests++
|
|
w.WriteHeader(http.StatusInternalServerError)
|
|
}))
|
|
t.Cleanup(server.Close)
|
|
env := githubCredentialTestEnv(server.URL)
|
|
|
|
err := runGitHubCredentialHelper(
|
|
context.Background(),
|
|
[]string{"get"},
|
|
strings.NewReader("protocol=https\nhost=github.com\npath=other/repository.git\n\n"),
|
|
io.Discard,
|
|
lookupEnv(env),
|
|
server.Client(),
|
|
)
|
|
if err == nil || !strings.Contains(err.Error(), "does not match credential lease scope") {
|
|
t.Fatalf("runGitHubCredentialHelper() error = %v, want scope mismatch", err)
|
|
}
|
|
if requests != 0 {
|
|
t.Fatalf("broker requests = %d, want 0", requests)
|
|
}
|
|
}
|
|
|
|
func TestGitHubCredentialHelperRejectsRepositoryPathCaseMismatch(t *testing.T) {
|
|
env := githubCredentialTestEnv("https://broker.example/resolve")
|
|
env[envGitHubCredentialOwner] = "Team"
|
|
env[envGitHubCredentialRepo] = "Repo"
|
|
err := runGitHubCredentialHelper(
|
|
context.Background(),
|
|
[]string{"get"},
|
|
strings.NewReader("protocol=https\nhost=github.com\npath=team/repo.git\n\n"),
|
|
io.Discard,
|
|
lookupEnv(env),
|
|
http.DefaultClient,
|
|
)
|
|
if err == nil || !strings.Contains(err.Error(), "does not match credential lease scope") {
|
|
t.Fatalf("runGitHubCredentialHelper() error = %v, want case-sensitive scope mismatch", err)
|
|
}
|
|
}
|
|
|
|
func TestGitHubCredentialHelperRequiresCompleteScope(t *testing.T) {
|
|
for name, input := range map[string]string{
|
|
"protocol": "host=github.com\npath=acme/widgets.git\n\n",
|
|
"host": "protocol=https\npath=acme/widgets.git\n\n",
|
|
"path": "protocol=https\nhost=github.com\n\n",
|
|
} {
|
|
t.Run(name, func(t *testing.T) {
|
|
env := githubCredentialTestEnv("https://broker.example/resolve")
|
|
err := runGitHubCredentialHelper(
|
|
context.Background(), []string{"get"}, strings.NewReader(input), io.Discard,
|
|
lookupEnv(env), http.DefaultClient,
|
|
)
|
|
if err == nil {
|
|
t.Fatal("runGitHubCredentialHelper() error = nil, want incomplete scope rejection")
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestGitHubCredentialHelperSelectsRepositoryLease(t *testing.T) {
|
|
var got githubBrokerResolveRequest
|
|
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
if err := json.NewDecoder(r.Body).Decode(&got); err != nil {
|
|
t.Errorf("decode request: %v", err)
|
|
}
|
|
_, _ = io.WriteString(w, `{"username":"x-access-token","password":"fresh-token"}`)
|
|
}))
|
|
t.Cleanup(server.Close)
|
|
env := githubCredentialTestEnv(server.URL)
|
|
env[envGitHubCredentialScopes] = `[
|
|
{"lease":"frontend-lease","task_id":"task-1","session_id":"session-1","repository_id":"repo-1","owner":"acme","repo":"frontend","host":"github.com"},
|
|
{"lease":"backend-lease","task_id":"task-1","session_id":"session-1","repository_id":"repo-2","owner":"acme","repo":"backend","host":"github.com"}
|
|
]`
|
|
|
|
err := runGitHubCredentialHelper(
|
|
context.Background(), []string{"get"},
|
|
strings.NewReader("protocol=https\nhost=github.com\npath=acme/backend.git\n\n"),
|
|
io.Discard, lookupEnv(env), server.Client(),
|
|
)
|
|
if err != nil {
|
|
t.Fatalf("runGitHubCredentialHelper() error = %v", err)
|
|
}
|
|
if got.Lease != "backend-lease" || got.RepositoryID != "repo-2" {
|
|
t.Fatalf("selected broker scope = %+v", got)
|
|
}
|
|
}
|
|
|
|
func TestGitHubCredentialHelperPreservesExactProviderPath(t *testing.T) {
|
|
var got githubBrokerResolveRequest
|
|
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
if err := json.NewDecoder(r.Body).Decode(&got); err != nil {
|
|
t.Errorf("decode request: %v", err)
|
|
}
|
|
_, _ = io.WriteString(w, `{"username":"x-token-auth","password":"fresh-token"}`)
|
|
}))
|
|
t.Cleanup(server.Close)
|
|
env := githubCredentialTestEnv(server.URL)
|
|
env[envGitHubCredentialScopes] = `[
|
|
{"lease":"opaque-lease","task_id":"task-1","session_id":"session-1","repository_id":"repo-1","owner":"context","repo":"scm/ENG/widgets","host":"bitbucket.example","path":"/context/scm/ENG/widgets"}
|
|
]`
|
|
|
|
err := runGitHubCredentialHelper(
|
|
context.Background(), []string{"get"},
|
|
strings.NewReader("protocol=https\nhost=bitbucket.example\npath=context/scm/ENG/widgets\n\n"),
|
|
io.Discard, lookupEnv(env), server.Client(),
|
|
)
|
|
if err != nil {
|
|
t.Fatalf("runGitHubCredentialHelper() error = %v", err)
|
|
}
|
|
if got.Path != "/context/scm/ENG/widgets" {
|
|
t.Fatalf("broker path = %q, want exact provider path", got.Path)
|
|
}
|
|
}
|
|
|
|
// git sends the remote path verbatim, so it may or may not carry the ".git"
|
|
// suffix the scope path was derived from. Neither spelling may decide whether
|
|
// the lease resolves.
|
|
func TestGitHubCredentialHelperMatchesScopePathAcrossGitSuffix(t *testing.T) {
|
|
for _, testCase := range []struct {
|
|
name string
|
|
scopePath string
|
|
inputPath string
|
|
}{
|
|
{name: "scope keeps suffix", scopePath: "/acme/widgets.git", inputPath: "acme/widgets"},
|
|
{name: "request keeps suffix", scopePath: "/acme/widgets", inputPath: "acme/widgets.git"},
|
|
{name: "both keep suffix", scopePath: "/acme/widgets.git", inputPath: "acme/widgets.git"},
|
|
} {
|
|
t.Run(testCase.name, func(t *testing.T) {
|
|
var got githubBrokerResolveRequest
|
|
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
if err := json.NewDecoder(r.Body).Decode(&got); err != nil {
|
|
t.Errorf("decode request: %v", err)
|
|
}
|
|
_, _ = io.WriteString(w, `{"username":"x-access-token","password":"fresh-token"}`)
|
|
}))
|
|
t.Cleanup(server.Close)
|
|
env := githubCredentialTestEnv(server.URL)
|
|
env[envGitHubCredentialScopes] = `[
|
|
{"lease":"scoped-lease","task_id":"task-1","session_id":"session-1","repository_id":"repo-1","owner":"acme","repo":"widgets","host":"github.com","path":"` +
|
|
testCase.scopePath + `"}
|
|
]`
|
|
|
|
err := runGitHubCredentialHelper(
|
|
context.Background(), []string{"get"},
|
|
strings.NewReader("protocol=https\nhost=github.com\npath="+testCase.inputPath+"\n\n"),
|
|
io.Discard, lookupEnv(env), server.Client(),
|
|
)
|
|
if err != nil {
|
|
t.Fatalf("runGitHubCredentialHelper() error = %v", err)
|
|
}
|
|
if got.Lease != "scoped-lease" {
|
|
t.Fatalf("selected broker scope = %+v", got)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestGitHubCredentialHelperRejectsDifferentScopedRepository(t *testing.T) {
|
|
env := githubCredentialTestEnv("https://broker.example/resolve")
|
|
env[envGitHubCredentialScopes] = `[
|
|
{"lease":"scoped-lease","task_id":"task-1","session_id":"session-1","repository_id":"repo-1","owner":"acme","repo":"widgets","host":"github.com","path":"/acme/widgets.git"}
|
|
]`
|
|
|
|
err := runGitHubCredentialHelper(
|
|
context.Background(), []string{"get"},
|
|
strings.NewReader("protocol=https\nhost=github.com\npath=acme/widgets-staging\n\n"),
|
|
io.Discard, lookupEnv(env), http.DefaultClient,
|
|
)
|
|
if err == nil || !strings.Contains(err.Error(), "does not match any credential lease scope") {
|
|
t.Fatalf("runGitHubCredentialHelper() error = %v, want scope mismatch", err)
|
|
}
|
|
}
|
|
|
|
func TestGitHubCredentialHelperIgnoresStoreAndErase(t *testing.T) {
|
|
for _, operation := range []string{"store", "erase"} {
|
|
t.Run(operation, func(t *testing.T) {
|
|
if err := runGitHubCredentialHelper(
|
|
context.Background(), []string{operation}, strings.NewReader("password=secret\n\n"),
|
|
io.Discard, lookupEnv(nil), http.DefaultClient,
|
|
); err != nil {
|
|
t.Fatalf("runGitHubCredentialHelper(%q) error = %v", operation, err)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestGitHubCredentialHelperSurfacesBrokerErrorBody(t *testing.T) {
|
|
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
|
w.WriteHeader(http.StatusUnauthorized)
|
|
_, _ = io.WriteString(w, `{"error":"authentication required"}`)
|
|
}))
|
|
t.Cleanup(server.Close)
|
|
env := githubCredentialTestEnv(server.URL)
|
|
|
|
err := runGitHubCredentialHelper(
|
|
context.Background(), []string{"get"},
|
|
strings.NewReader("protocol=https\nhost=github.com\npath=acme/widgets.git\n\n"),
|
|
io.Discard, lookupEnv(env), server.Client(),
|
|
)
|
|
if err == nil {
|
|
t.Fatal("runGitHubCredentialHelper() error = nil, want broker denial")
|
|
}
|
|
want := `resolve GitHub credential: broker returned HTTP 401: {"error":"authentication required"}`
|
|
if err.Error() != want {
|
|
t.Fatalf("error = %q, want %q", err.Error(), want)
|
|
}
|
|
}
|
|
|
|
func TestGitHubCredentialHelperTruncatesAndSanitizesBrokerErrorBody(t *testing.T) {
|
|
rawBody := "prefix\x00\x01\x1f control chars" + strings.Repeat("x", 600)
|
|
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
|
w.WriteHeader(http.StatusInternalServerError)
|
|
_, _ = io.WriteString(w, rawBody)
|
|
}))
|
|
t.Cleanup(server.Close)
|
|
env := githubCredentialTestEnv(server.URL)
|
|
|
|
err := runGitHubCredentialHelper(
|
|
context.Background(), []string{"get"},
|
|
strings.NewReader("protocol=https\nhost=github.com\npath=acme/widgets.git\n\n"),
|
|
io.Discard, lookupEnv(env), server.Client(),
|
|
)
|
|
if err == nil {
|
|
t.Fatal("runGitHubCredentialHelper() error = nil, want broker denial")
|
|
}
|
|
if !strings.HasPrefix(err.Error(), "resolve GitHub credential: broker returned HTTP 500: ") {
|
|
t.Fatalf("error = %q, want HTTP 500 prefix", err.Error())
|
|
}
|
|
appended := strings.TrimPrefix(err.Error(), "resolve GitHub credential: broker returned HTTP 500: ")
|
|
if len(appended) > 512 {
|
|
t.Fatalf("appended body length = %d, want <= 512", len(appended))
|
|
}
|
|
for _, r := range appended {
|
|
if r < 0x20 {
|
|
t.Fatalf("appended body contains control char %q: %q", r, appended)
|
|
}
|
|
}
|
|
}
|
|
|
|
func TestGitHubCredentialHelperOmitsAppendedBodyWhenEmpty(t *testing.T) {
|
|
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
|
w.WriteHeader(http.StatusBadGateway)
|
|
}))
|
|
t.Cleanup(server.Close)
|
|
env := githubCredentialTestEnv(server.URL)
|
|
|
|
err := runGitHubCredentialHelper(
|
|
context.Background(), []string{"get"},
|
|
strings.NewReader("protocol=https\nhost=github.com\npath=acme/widgets.git\n\n"),
|
|
io.Discard, lookupEnv(env), server.Client(),
|
|
)
|
|
want := "resolve GitHub credential: broker returned HTTP 502"
|
|
if err == nil || err.Error() != want {
|
|
t.Fatalf("error = %v, want %q", err, want)
|
|
}
|
|
}
|
|
|
|
func githubCredentialTestEnv(url string) map[string]string {
|
|
return map[string]string{
|
|
envGitHubCredentialBrokerURL: url,
|
|
envGitHubCredentialLease: "opaque-lease",
|
|
envGitHubCredentialTaskID: "task-1",
|
|
envGitHubCredentialSessionID: "session-1",
|
|
envGitHubCredentialRepository: "repo-1",
|
|
envGitHubCredentialOwner: "acme",
|
|
envGitHubCredentialRepo: "widgets",
|
|
envGitHubCredentialHost: "github.com",
|
|
}
|
|
}
|
|
|
|
func lookupEnv(env map[string]string) func(string) string {
|
|
return func(key string) string { return env[key] }
|
|
}
|