test(19.4-03): pin OIDC code binding, credential-free logs and random subjects

- a code is refused for another client or redirect URI and burned
- authorize refuses plain and malformed challenges; verifier rules; 60 s code TTL
- rejected token exchanges log client id, IP and reason only; key parse errors quote no key
- a public sub is a random UUIDv4

modules/wristband/oidc_security_test.go

@@ -0,0 +1,153 @@
+package wristband
+
+import (
+	"bytes"
+	"log/slog"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"regexp"
+	"strings"
+	"testing"
+	"time"
+)
+
+// TestOIDCCodeBindingSmoke pins T-19.4-02: a code is bound to the client
+// and the redirect URI it was issued for, a failed redemption burns it, the
+// authorize endpoint accepts only an S256 challenge of 43 to 128 base64url
+// characters, the token endpoint only a verifier of 43 to 128 unreserved
+// characters, and a code lives 60 seconds by default.
+func TestOIDCCodeBindingSmoke(t *testing.T) {
+	f := newOIDCFixture(t, testOIDCIssuer, nil)
+	other := f.backend.addClient(OIDCClientRecord{
+		ClientID: "oas_other", Kind: ClientKindOIDC, RedirectURIs: []string{testOIDCRedirect},
+		Scopes: []string{"openid"}, IsActive: true,
+	}, testOIDCSecret)
+
+	wantInvalidGrant := func(what string, rec *httptest.ResponseRecorder) {
+		t.Helper()
+		if rec.Code != http.StatusBadRequest || !strings.Contains(rec.Body.String(), `"error":"invalid_grant"`) {
+			t.Fatalf("%s = %d %s, want 400 invalid_grant", what, rec.Code, rec.Body.String())
+		}
+	}
+
+	// A code issued to one client is refused for another and is burned.
+	code := f.obtainCode(t, f.client, 7, "openid")
+	wantInvalidGrant("code of another client", f.exchange(other, testOIDCSecret, code, oidcTestVerifier))
+	wantInvalidGrant("code after a client mismatch", f.exchange(f.client, f.clientSecret, code, oidcTestVerifier))
+
+	// A code presented with another redirect URI is refused and burned.
+	code = f.obtainCode(t, f.client, 7, "openid")
+	wantInvalidGrant("redirect mismatch", f.tokenCall(url.Values{
+		"grant_type": {"authorization_code"}, "code": {code},
+		"redirect_uri": {testOIDCRedirect + "/elsewhere"}, "code_verifier": {oidcTestVerifier},
+	}, f.client.ClientID, f.clientSecret))
+	wantInvalidGrant("code after a redirect mismatch", f.exchange(f.client, f.clientSecret, code, oidcTestVerifier))
+
+	// Authorize refuses the plain method and a short challenge.
+	for _, tc := range []struct {
+		name   string
+		mutate func(url.Values)
+		desc   string
+	}{
+		{"plain method", func(q url.Values) {
+			q.Set("code_challenge_method", "plain")
+			q.Set("code_challenge", oidcTestVerifier)
+		}, "The+code+challenge+method+is+required+and+must+be+S256."},
+		{"short challenge", func(q url.Values) { q.Set("code_challenge", strings.Repeat("a", 42)) },
+			"A+PKCE+code+challenge+is+required+and+must+be+a+base64url-encoded+SHA-256+digest."},
+		{"challenge outside base64url", func(q url.Values) { q.Set("code_challenge", strings.Repeat("a", 42)+"+") },
+			"A+PKCE+code+challenge+is+required+and+must+be+a+base64url-encoded+SHA-256+digest."},
+	} {
+		rec := f.get("/oauth/authorize?" + oidcAuthorizeParams(f, tc.mutate).Encode())
+		if rec.Code != http.StatusFound || !strings.Contains(rec.Header().Get("Location"), "error=invalid_request&error_description="+tc.desc) {
+			t.Fatalf("%s: %d %q", tc.name, rec.Code, rec.Header().Get("Location"))
+		}
+	}
+
+	// The verifier rules: S256 only, 43 to 128 unreserved characters.
+	short := strings.Repeat("v", 42)
+	long := strings.Repeat("v", 129)
+	odd := strings.Repeat("v", 42) + "!"
+	for _, tc := range []struct {
+		verifier, challenge, method string
+		want                        bool
+	}{
+		{oidcTestVerifier, s256Challenge(oidcTestVerifier), "S256", true},
+		{oidcTestVerifier, oidcTestVerifier, "plain", false},
+		{oidcTestVerifier, s256Challenge(oidcTestVerifier), "plain", false},
+		{short, s256Challenge(short), "S256", false},
+		{long, s256Challenge(long), "S256", false},
+		{odd, s256Challenge(odd), "S256", false},
+		{oidcTestVerifier, "", "S256", false},
+	} {
+		if got := verifyOIDCPkce(tc.verifier, tc.challenge, tc.method); got != tc.want {
+			t.Fatalf("verifyOIDCPkce(%d chars, %s) = %v, want %v", len(tc.verifier), tc.method, got, tc.want)
+		}
+	}
+
+	if ttl := DefaultOIDCOptions().CodeTTL; ttl != 60*time.Second {
+		t.Fatalf("default CodeTTL = %s, want 60s", ttl)
+	}
+}
+
+// TestOIDCLogsCarryNoCredentials pins T-19.4-09: a rejected token exchange
+// logs the client id, the client IP and a fixed reason, never the secret,
+// the code or the verifier; key parsing errors never quote the key.
+func TestOIDCLogsCarryNoCredentials(t *testing.T) {
+	var buf bytes.Buffer
+	prev := slog.Default()
+	slog.SetDefault(slog.New(slog.NewTextHandler(&buf, nil)))
+	t.Cleanup(func() { slog.SetDefault(prev) })
+
+	f := newOIDCFixture(t, testOIDCIssuer, nil)
+	const wrongSecret = "oas_cs_wrong-guess-0123456789abcdefghijklmnopqrstuv"
+	const wrongVerifier = "wrong-verifier-0123456789abcdefghijklmnopqrstuvwxyz"
+
+	// Wrong secret in Basic and in the form.
+	f.tokenCall(url.Values{"grant_type": {"authorization_code"}}, f.client.ClientID, wrongSecret)
+	f.tokenCall(url.Values{
+		"grant_type": {"authorization_code"}, "client_id": {f.client.ClientID}, "client_secret": {wrongSecret},
+	}, "", "")
+	// A wrong verifier burns the code, the replay revokes.
+	code := f.obtainCode(t, f.client, 7, "openid")
+	f.exchange(f.client, f.clientSecret, code, wrongVerifier)
+	f.exchange(f.client, f.clientSecret, code, oidcTestVerifier)
+	// A code that never existed.
+	f.exchange(f.client, f.clientSecret, "oas_ac_never-issued-0123456789abcdefghijklmnop", oidcTestVerifier)
+
+	logs := buf.String()
+	for _, want := range []string{"reason=invalid_client", "reason=invalid_grant:pkce", "reason=invalid_grant:replay", "client_id=" + f.client.ClientID} {
+		if !strings.Contains(logs, want) {
+			t.Fatalf("log lacks %q:\n%s", want, logs)
+		}
+	}
+	for _, secret := range []string{wrongSecret, testOIDCSecret, code, wrongVerifier, oidcTestVerifier, "oas_ac_", "oas_cs_", "oas_at_"} {
+		if strings.Contains(logs, secret) {
+			t.Fatalf("log carries a credential (%q):\n%s", secret, logs)
+		}
+	}
+
+	// Key parsing errors are fixed texts.
+	const marker = "U0VDUkVUTUFSS0VS"
+	if _, err := ParsePrivateKeyPEM([]byte("-----BEGIN PRIVATE KEY-----\n" + marker + "\n-----END PRIVATE KEY-----\n")); err == nil || strings.Contains(err.Error(), marker) {
+		t.Fatalf("private key error = %v", err)
+	}
+	if _, err := ParsePublicKeyPEM([]byte("-----BEGIN PUBLIC KEY-----\n" + marker + "\n-----END PUBLIC KEY-----\n")); err == nil || strings.Contains(err.Error(), marker) {
+		t.Fatalf("public key error = %v", err)
+	}
+}
+
+// uuidV4 is the shape of a random subject.
+var uuidV4 = regexp.MustCompile(`^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$`)
+
+// TestOIDCSubjectIsRandomSmoke pins T-19.4-13: a public sub is a random
+// UUIDv4, never the user id, the email or the name.
+func TestOIDCSubjectIsRandomSmoke(t *testing.T) {
+	f := newOIDCFixture(t, testOIDCIssuer, nil)
+	tok := decodeToken(t, f.exchange(f.client, f.clientSecret, f.obtainCode(t, f.client, 7, "openid"), oidcTestVerifier))
+	sub, _ := f.parseIDToken(t, tok.IDToken)["sub"].(string)
+	if !uuidV4.MatchString(sub) || sub == "7" || strings.Contains(sub, "ada") {
+		t.Fatalf("sub = %q, want a random UUIDv4", sub)
+	}
+}