Skip to content

fix(UT): pin validator clock in tests to avoid fixture cert expiry - #72

Merged
mridulgain merged 3 commits into
masterfrom
mg/fix-saml-test-cert-expiry
Aug 19, 2026
Merged

fix(UT): pin validator clock in tests to avoid fixture cert expiry#72
mridulgain merged 3 commits into
masterfrom
mg/fix-saml-test-cert-expiry

Conversation

@mridulgain

Copy link
Copy Markdown

Summary

  • TestVerifyUnsignedMessageAndSignedAssertionWithRootXmlNs started failing in CI because testdata/oam-ca.pem expired on 2026-06-28, causing the goxmldsig validator's real-time cert check to reject the signature.
  • Pin runVerify's dsig.ValidationContext.Clock to a fixed date (2020-01-01) that falls within the validity window of all fixture certs used in these tests, instead of relying on the system clock.
  • This also preempts okta-ca.pem (expires Dec 2026) from causing the same failure later.

Test plan

  • go test ./connector/saml/... passes locally, including the previously failing test

🤖 Generated with Claude Code

TestVerifyUnsignedMessageAndSignedAssertionWithRootXmlNs started
failing because testdata/oam-ca.pem expired 2026-06-28. Pin the
dsig.ValidationContext clock in runVerify to a fixed date within all
fixture certs' validity windows instead of relying on the real clock,
so this doesn't recur when okta-ca.pem expires in December 2026.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@mridulgain mridulgain self-assigned this Aug 18, 2026
@mridulgain mridulgain changed the title fix(saml): pin validator clock in tests to avoid fixture cert expiry fix(UT): pin validator clock in tests to avoid fixture cert expiry Aug 19, 2026
mridulgain added a commit that referenced this pull request Aug 19, 2026
The Trivy Filesystem Scan now runs successfully (after the install
fix) and surfaced real, pre-existing HIGH/CRITICAL CVEs in all three
Go modules (root, examples/, api/v2/):

- CVE-2026-33186 (CRITICAL) and GHSA-hrxh-6v49-42gf (HIGH) in
  google.golang.org/grpc -> bumped to v1.83.0
- CVE-2026-25681, CVE-2026-27136, CVE-2026-33814, CVE-2026-39821,
  CVE-2026-46600 in golang.org/x/net -> bumped to v0.58.0
- CVE-2026-56852 in golang.org/x/text -> bumped to v0.41.0
- CVE-2025-47913, CVE-2026-39828..39832, CVE-2026-39835,
  CVE-2026-42508, CVE-2026-46595, CVE-2026-46597 in
  golang.org/x/crypto -> bumped to v0.55.0
- CVE-2026-34986 in github.com/go-jose/go-jose/v4 -> bumped to v4.1.4
- CVE-2026-33487 in github.com/russellhaering/goxmldsig -> bumped to
  v1.6.0
- CVE-2026-29181 in go.opentelemetry.io/otel -> bumped to v1.45.0

grpc v1.82.1+ (needed for GHSA-hrxh-6v49-42gf) requires Go 1.25, so
this also bumps go.mod's go directive 1.24.0 -> 1.25.0 in all three
modules, ci.yaml's pinned go-version 1.24 -> 1.25, and the Dockerfile
builder base image to golang:1.25.13-alpine3.23.

The Go 1.25 toolchain's stricter vet printf check flagged two
existing non-constant-format-string calls to the newRedirectedErr
helper in server/oauth2.go (a real latent bug: a '%' in a redirect
URI or connector-derived value could have been misinterpreted as a
format verb); fixed by passing the value as a %s argument instead of
as the format string itself.

Verified: go build/vet/test pass across all three modules (root,
examples, api/v2); the one remaining test failure
(TestVerifyUnsignedMessageAndSignedAssertionWithRootXmlNs) is a
pre-existing, unrelated fixture-cert-expiry issue already fixed on a
separate branch (PR #72).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
hsri-pf9
hsri-pf9 previously approved these changes Aug 19, 2026
badrinath-pf9
badrinath-pf9 previously approved these changes Aug 19, 2026
mridulgain and others added 2 commits August 19, 2026 13:23
…nt cleanup

PR #74's golangci-lint v2 migration blanket-replaced .ObjectMeta.Name
with .Name across storage/kubernetes/storage.go and types.go to
resolve staticcheck's QF1008 "embedded field" suggestions. That's
safe for most types here, but storage/kubernetes.Client declares its
own explicit `Name string` field (the client's display name, e.g.
"dex client"), which shadows the embedded k8sapi.ObjectMeta.Name (the
actual Kubernetes resource name, a hash of the client ID). Go silently
resolves c.Name to the shallower, explicit field, so
DeleteClient/UpdateClient started PUTting/DELETEing using the display
name instead of the resource name -- which fails Kubernetes' DNS1123
name validation (or worse, could target the wrong resource if the
display name happened to be a valid k8s name).

This is exactly why staticcheck itself never flagged these two call
sites (client.go:426,496) even though the blanket sed touched them --
the quickfix check is shadow-aware and only flags genuinely
equivalent simplifications. This regression was introduced by
mechanically applying more replacements than the linter actually
flagged.

Reverts these two call sites to c.ObjectMeta.Name. Verified this was
the only affected type: Connector also declares its own Name field,
but its delete/put call sites already used the id parameter directly
and were untouched by the sed.

Fixes the storage/kubernetes TestStorage/ClientCRUD and
ClientConcurrentUpdate CI failures surfaced on this branch after
merging master (which includes #74).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@mridulgain
mridulgain merged commit f43c591 into master Aug 19, 2026
11 checks passed
@mridulgain
mridulgain deleted the mg/fix-saml-test-cert-expiry branch August 19, 2026 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants