Support OIDC form_post, require loopback listener, redact request URLs in errors
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline was successful

This commit is contained in:
2026-10-23 22:01:42 +11:00
parent 588aea7858
commit ddc25139f7
4 changed files with 78 additions and 14 deletions
+2 -1
View File
@@ -77,7 +77,8 @@ For each context, every field is resolved with the chain
- `namespace` — no built-in default (unset means the root namespace).
- `path` — the auth mount path; defaults to the resolved `method`.
- `role` — OIDC role; unset uses the mount's `default_role`.
- `oidc_listen` — OIDC callback listener; defaults to `localhost:8250`.
- `oidc_listen` — OIDC callback listener on a loopback host (`localhost`,
`127.0.0.1` or `[::1]`); defaults to `localhost:8250`.
`--method` / `--user` on the command line override the resolved values. When
`--method` changes the method and the context did not pin an explicit `path`,
+22 -7
View File
@@ -32,6 +32,13 @@ type oidcResult struct {
// fetch auth_url, open the browser, receive code+state on a local callback,
// and exchange them via auth/<path>/oidc/callback.
func LoginOIDC(rc ResolvedContext) (*Token, error) {
host, _, err := net.SplitHostPort(rc.OIDCListen)
if err != nil {
return nil, fmt.Errorf("oidc listen address %q: %w", rc.OIDCListen, err)
}
if host != "localhost" && host != "127.0.0.1" && host != "::1" {
return nil, fmt.Errorf("oidc listen address %q must be on localhost, 127.0.0.1 or [::1]", rc.OIDCListen)
}
ln, err := net.Listen("tcp", rc.OIDCListen)
if err != nil {
return nil, fmt.Errorf("oidc callback listener: %w", err)
@@ -90,17 +97,25 @@ func LoginOIDC(rc ResolvedContext) (*Token, error) {
}
}
// oidcCallback exchanges the provider's state+code for a Vault token.
// oidcCallback exchanges the provider's state+code for a Vault token. A POST
// is the form_post response mode: forward the form to Vault, then GET as usual.
func oidcCallback(rc ResolvedContext, r *http.Request, nonce string) oidcResult {
q := r.URL.Query()
if e := q.Get("error"); e != "" {
return oidcResult{err: fmt.Errorf("provider returned %s: %s", e, q.Get("error_description"))}
if e := r.FormValue("error"); e != "" {
return oidcResult{err: fmt.Errorf("provider returned %s: %s", e, r.FormValue("error_description"))}
}
if q.Get("code") == "" || q.Get("state") == "" {
state, code := r.FormValue("state"), r.FormValue("code")
if state == "" || (code == "" && r.Method != http.MethodPost) {
return oidcResult{err: errors.New("callback missing code or state")}
}
v := url.Values{"state": {q.Get("state")}, "code": {q.Get("code")}, "client_nonce": {nonce}}
data, err := doRequest(http.MethodGet, rc.Address, "auth/"+rc.Path+"/oidc/callback?"+v.Encode(), rc.Namespace, "", nil)
cb := "auth/" + rc.Path + "/oidc/callback"
if r.Method == http.MethodPost {
if _, err := doRequest(http.MethodPost, rc.Address, cb, rc.Namespace, "",
map[string]string{"state": state, "code": code, "id_token": r.FormValue("id_token"), "client_nonce": nonce}); err != nil {
return oidcResult{err: err}
}
}
v := url.Values{"state": {state}, "code": {code}, "client_nonce": {nonce}}
data, err := doRequest(http.MethodGet, rc.Address, cb+"?"+v.Encode(), rc.Namespace, "", nil)
if err != nil {
return oidcResult{err: err}
}
+43 -3
View File
@@ -27,7 +27,7 @@ func freeListen(t *testing.T) string {
// nonce/redirect round-trip, and records the redirect_uri it was given.
func fakeOIDCVault(t *testing.T, wantRole string) (*httptest.Server, *string) {
t.Helper()
var redirect, nonce string
var redirect, nonce, idToken string
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Header.Get("X-Vault-Namespace") != "team-a" {
t.Errorf("namespace header = %q", r.Header.Get("X-Vault-Namespace"))
@@ -41,9 +41,16 @@ func fakeOIDCVault(t *testing.T, wantRole string) (*httptest.Server, *string) {
}
redirect, nonce = body["redirect_uri"], body["client_nonce"]
_, _ = io.WriteString(w, `{"data":{"auth_url":"https://idp.example/authorize?state=st1"}}`)
case r.Method == http.MethodPost && r.URL.Path == "/v1/auth/sso/oidc/callback":
var body map[string]string
_ = json.NewDecoder(r.Body).Decode(&body)
if body["state"] != "st1" || body["client_nonce"] != nonce {
t.Errorf("form_post body = %v", body)
}
idToken = body["id_token"]
case r.Method == http.MethodGet && r.URL.Path == "/v1/auth/sso/oidc/callback":
q := r.URL.Query()
if q.Get("state") != "st1" || q.Get("code") != "c0de" || q.Get("client_nonce") != nonce {
if q.Get("state") != "st1" || (q.Get("code") != "c0de" && idToken != "idt") || q.Get("client_nonce") != nonce {
w.WriteHeader(http.StatusBadRequest)
_, _ = io.WriteString(w, `{"errors":["bad callback"]}`)
return
@@ -60,6 +67,11 @@ func fakeOIDCVault(t *testing.T, wantRole string) (*httptest.Server, *string) {
// fakeBrowser stands in for the IdP: it hits the local callback with query.
func fakeBrowser(t *testing.T, redirect *string, query string) {
t.Helper()
fakeBrowserDo(t, func() (*http.Response, error) { return http.Get(*redirect + "?" + query) })
}
func fakeBrowserDo(t *testing.T, do func() (*http.Response, error)) {
t.Helper()
prev := OpenBrowser
OpenBrowser = func(u string) error {
@@ -67,7 +79,7 @@ func fakeBrowser(t *testing.T, redirect *string, query string) {
t.Errorf("browser url = %q", u)
}
go func() {
resp, err := http.Get(*redirect + "?" + query)
resp, err := do()
if err == nil {
_ = resp.Body.Close()
}
@@ -136,3 +148,31 @@ func TestLoginOIDCNoAuthURL(t *testing.T) {
t.Errorf("want no auth_url error, got %v", err)
}
}
func TestLoginOIDCFormPost(t *testing.T) {
srv, redirect := fakeOIDCVault(t, "")
fakeBrowserDo(t, func() (*http.Response, error) {
return http.PostForm(*redirect, url.Values{"state": {"st1"}, "id_token": {"idt"}})
})
rc := ResolvedContext{Name: "sso", Address: srv.URL, Method: "oidc", Path: "sso", Namespace: "team-a", OIDCListen: freeListen(t)}
tok, err := LoginOIDC(rc)
if err != nil || tok.Token != "s.oidc" {
t.Fatalf("form_post login: tok=%+v err=%v", tok, err)
}
}
func TestLoginOIDCRejectsNonLoopbackListen(t *testing.T) {
for _, l := range []string{":8250", "0.0.0.0:8250", "192.0.2.1:8250", "localhost"} {
rc := ResolvedContext{Name: "sso", Address: "http://unused", Method: "oidc", Path: "sso", OIDCListen: l}
if _, err := LoginOIDC(rc); err == nil || !strings.Contains(err.Error(), "oidc listen address") {
t.Errorf("%q: want listen error, got %v", l, err)
}
}
}
func TestDoRequestErrorOmitsQuery(t *testing.T) {
_, err := doRequest(http.MethodGet, "http://127.0.0.1:1", "auth/sso/oidc/callback?code=secretcode&state=st", "", "", nil)
if err == nil || strings.Contains(err.Error(), "secretcode") || !strings.Contains(err.Error(), "/v1/auth/sso/oidc/callback") {
t.Errorf("error = %v", err)
}
}
+11 -3
View File
@@ -3,9 +3,11 @@ package shared
import (
"bytes"
"encoding/json"
"errors"
"fmt"
"io"
"net/http"
"net/url"
"strings"
"time"
)
@@ -79,8 +81,8 @@ func doRequest(method, address, path, namespace, token string, payload any) ([]b
body = bytes.NewReader(b)
}
url := strings.TrimRight(address, "/") + "/v1/" + strings.TrimLeft(path, "/")
req, err := http.NewRequest(method, url, body)
reqURL := strings.TrimRight(address, "/") + "/v1/" + strings.TrimLeft(path, "/")
req, err := http.NewRequest(method, reqURL, body)
if err != nil {
return nil, fmt.Errorf("building request: %w", err)
}
@@ -96,7 +98,13 @@ func doRequest(method, address, path, namespace, token string, payload any) ([]b
resp, err := httpClient.Do(req)
if err != nil {
return nil, fmt.Errorf("request to %s failed: %w", url, err)
// Query strings can carry OIDC code/state/nonce; keep them out of errors.
bare, _, _ := strings.Cut(reqURL, "?")
var ue *url.Error
if errors.As(err, &ue) {
ue.URL = bare
}
return nil, fmt.Errorf("request to %s failed: %w", bare, err)
}
defer func() { _ = resp.Body.Close() }()