Add on error pipeline [GRA-791]

[john-sonz]

User description

This PR adds the on_error_pipeline option to the IDP config data.

• The pipeline enables custom handling of SAML errors
• on_error_pipeline is a plug pipeline and follows the same behaviour as pre_session_create_pipeline
• If on_error_pipeline is not provided, then the default Samly behaviour is not affected

---

Generated description

Below is a concise technical summary of the changes proposed in this PR:
Introduces a configurable error handling pipeline within the Samly library to replace verbose technical SAML error messages with secure, user-friendly responses, addressing a critical security concern. This change centralizes error management by routing various authentication failures through a new on_error_pipeline for consistent and customizable error page rendering.

Topic	Details
Auth Flow Integration	Integrate the new centralized error handling mechanism across various SAML authentication and routing components, replacing direct error responses with calls to Samly.Helper.handle_error_response. This ensures consistent error processing for scenarios like invalid requests, unknown identity providers, and access denials, while also passing relevant error and assertion data to the custom pipeline for detailed logging or display. A minor code formatting adjustment in lib/samly/idp_data.ex is also included. Modified files (5) lib/samly/auth_handler.ex lib/samly/idp_data.ex lib/samly/router_util.ex lib/samly/sp_handler.ex lib/samly/sp_router.ex Latest Contributors(2) User Commit Date ryan.pylipow@dscout.com Isolate-IdP-loading-fa... February 20, 2026 aj-foster Fix-record-types-in-Id... April 19, 2024
Error Pipeline Core	Implement the core on_error_pipeline infrastructure in Samly.Helper to allow custom handling of SAML errors, including setting error context in the Plug connection. Configure this pipeline via Samly.Provider and document its usage and benefits in the README.md to guide developers in creating secure and user-friendly error experiences. Modified files (3) README.md lib/samly/helper.ex lib/samly/provider.ex Latest Contributors(2) User Commit Date mm@idyll.io Get-attestation-from-e... January 29, 2024 rpylipow@gmail.com Allow-for-setting-a-cu... May 22, 2023

This pull request is reviewed by Baz. Review like a pro on (Baz).

[baz-reviewer]

Spec Reviewer Report    📬

Checkout in Baz

0 / 2 Requirements met for ticket:

Make SSO error messages secure & user friendly (Best Buy committment)

2 unmet requirement(s)

#	Requirement	Explanation
1	SSO access_denied errors must render the default dscout Oops template - Not Met	When no error pipeline halts the response, the handler still returns the raw access_denied payload instead of the Oops page, and Samly’s configuration leaves `on_error_pipeline` nil by default so no automatic Oops renderer runs. evidence `lib/samly/sp_handler.ex:84-93` still falls back to sending a raw 403 response containing the technical error (debug or plain text) when the error pipeline does not halt the connection. `lib/samly/idp_data.ex:13-21` keeps `on_error_pipeline` as nil by default, so the new hook is never invoked unless manually configured and there is no default Oops renderer wired in.
2	SSO error responses hide technical details - Not Met	Even though an error pipeline hook was added, any request that escapes or omits that pipeline still hits the fallback branch and emits the inspected `reason` (and raw SAML payload in debug mode), so the user-visible page keeps leaking the technical failure details. evidence lib/samly/sp_handler.ex:78-89 → after invoking the optional on_error_pipeline, the fallback branch still reaches a 403 response whose body prints the inspected `reason` and, in debug mode, the raw SAML response, exposing the exact error details.

---

Used resources:
Hash: 30db459 | Ticket: link

[rpylipow]

Changes look good. Could you please also document this new functionality in the README.md?

[john-sonz]

@rpylipow README.md updated ✅

[john-sonz]

@rpylipow How does the release process work for this library? Do we need to do anything else besides merging?

[rpylipow]

@john-sonz Our axon application runs again the main branch of this

{:samly, git: "https://github.com/dscout/samly.git"},

So my suggestion is waiting to merge this until after you tested the axon integration with it incase you need to make changes. You should be able to test locally/staging on axon using something like

# I forget the exact syntax. It might be branch, or commit sha 
{:samly, git: "https://github.com/dscout/samly.git", branch: "your-branch-name"}, 

[rpylipow]

@john-sonz I was helping @madsmorrison test this today and we discovered that our error pipeline needs to be moved up a level (my fault, I gave you short-sided instructions).

Here's the general approach I discussed w/ Claude (I chopped off the implementation instructions bc I didn't have to time to vet them).

Context

 PR #3 added on_error_pipeline as a per-IdP field on IdpData, but it only
 covers SPHandler.consume_signin_response/1. The "invalid_request unknown
 IdP" error from RouterUtil.check_idp_id/2 fires before any IdP is resolved,
  so there's no IdpData to pull a pipeline from. Moving to a global config
 solves this and lets us cover all error paths uniformly.

 Since PR #3 hasn't been merged, there's no backward compatibility concern —
  we replace the per-IdP field entirely.

 Config

 config :samly, Samly.Provider,
   error_pipeline: MyApp.ErrorPipeline, 
   service_providers: [...],
   identity_providers: [...]

 Stored at runtime as Application.get_env(:samly, :error_pipeline) (same
 pattern as :idp_id_from).

[john-sonz]

@rpylipow Ooh, that makes sense, I'll rewrite it this way

[baz-reviewer]

Logout response fallback still renders send_resp(403, "invalid_request #{inspect(error)}") when the new on_error_pipeline is absent or doesn't halt, so SSO errors continue to leak internal reasons to the user despite the ticket's requirement for secure, user-friendly defaults; can we stop returning inspect(error) and use a generic response when the pipeline isn't handling the request?

_{Finding type: Basic Security Patterns}

---

• Apply fix with Baz

[baz-reviewer]

This block reruns the on-error pipeline + halted check that Helper.handle_error_response/4 already centralizes, so any future changes to pipeline/response flow require editing the helper and this branch separately; can we delegate the debug-HTML branch to that helper (e.g. by passing a custom responder) instead of copying the pipeline/case logic?

_{Finding type: Code Dedup and Conventions}

---

• Apply fix with Baz