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 custom error handling pipeline for SAML authentication to replace technical error messages with secure, user-friendly pages. This change allows developers to configure an on_error_pipeline that intercepts authentication failures and provides context via the Plug connection.

Topic	Details
Error Pipeline Logic	Adds the on_error_pipeline field to the IdpData struct and implements the logic in sp_handler.ex to trigger the pipeline when an authentication error occurs. Modified files (2) lib/samly/idp_data.ex lib/samly/sp_handler.ex Latest Contributors(2) User Commit Date aj-foster Fix-record-types-in-Id... April 19, 2024 mm@idyll.io Add-debug-mode-that-di... March 20, 2024
Docs and Testing	Updates the README.md with instructions and examples for implementing a custom error pipeline and adds unit tests to verify the configuration loading. Modified files (2) README.md test/samly_idp_data_test.exs Latest Contributors(2) User Commit Date mm@idyll.io Get-attestation-from-e... January 29, 2024 rpylipow@gmail.com Add-a-custom-consume-url December 06, 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).