Skip to content

Reject metadata push when IdP discovery entry is missing English name#1957

Merged
kayjoosten merged 2 commits intomainfrom
fix/1865-reject-push-missing-discovery-name
Apr 13, 2026
Merged

Reject metadata push when IdP discovery entry is missing English name#1957
kayjoosten merged 2 commits intomainfrom
fix/1865-reject-push-missing-discovery-name

Conversation

@kayjoosten
Copy link
Copy Markdown
Contributor

Summary

  • Fixes EB: Push metadata from Manage should ensure IdP has discovery information #1865: when Manage pushes IdP metadata with a discoveries entry that has no English name (name_en), EngineBlock now rejects the push with HTTP 400 instead of silently dropping the entry
  • Changed DiscoveryAssembler::assembleDiscoveries() to throw InvalidDiscoveryException instead of silently skipping; the existing ConnectionsController catch-all propagates this as HTTP 400
  • Added tests at three levels: unit (DiscoveryAssemblerTest), assembler integration (PushMetadataAssemblerTest), and HTTP functional (ConnectionsControllerTest)

Test plan

  • Run unit and integration tests: ./vendor/bin/phpunit --configuration=./tests/phpunit.xml --testsuite=unit,integration
  • Verify POST /api/connections with an IdP discovery missing name_en returns HTTP 400 with an error message containing name_en
  • Verify a normal metadata push with a valid discovery (including name_en) still succeeds

A discovery entry without name_en was silently dropped, which could
leave an IdP with no discoveries and cause a WAYF crash. Now throws
InvalidDiscoveryException, which propagates to ConnectionsController
as HTTP 400. Fixes #1865.
@kayjoosten kayjoosten force-pushed the fix/1865-reject-push-missing-discovery-name branch from 53fbc96 to ba9137a Compare March 27, 2026 12:09
@kayjoosten kayjoosten requested a review from johanib March 31, 2026 08:26
Copy link
Copy Markdown
Contributor

@johanib johanib left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, 1 improvement that we should still add imo

@kayjoosten kayjoosten requested a review from johanib April 13, 2026 10:08
@kayjoosten kayjoosten merged commit 33ba9a9 into main Apr 13, 2026
2 checks passed
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.

EB: Push metadata from Manage should ensure IdP has discovery information

2 participants