From a072fd704d84392dda475e183281e85bbb61a561 Mon Sep 17 00:00:00 2001 From: Jan Zborowski Date: Thu, 29 Jan 2026 13:49:48 +0100 Subject: [PATCH 1/6] Add on error pipeline --- lib/samly/idp_data.ex | 13 +++++++++- lib/samly/sp_handler.ex | 49 +++++++++++++++++++++++++++++------- test/samly_idp_data_test.exs | 7 ++++++ 3 files changed, 59 insertions(+), 10 deletions(-) diff --git a/lib/samly/idp_data.ex b/lib/samly/idp_data.ex index c2c89cb..9dc416c 100644 --- a/lib/samly/idp_data.ex +++ b/lib/samly/idp_data.ex @@ -16,6 +16,7 @@ defmodule Samly.IdpData do custom_recipient_url: nil, metadata_file: nil, metadata: nil, + on_error_pipeline: nil, pre_session_create_pipeline: nil, use_redirect_for_req: false, sign_requests: true, @@ -45,6 +46,7 @@ defmodule Samly.IdpData do custom_recipient_url: nil | binary(), metadata_file: nil | binary(), metadata: nil | binary(), + on_error_pipeline: nil | module(), pre_session_create_pipeline: nil | module(), use_redirect_for_req: boolean(), sign_requests: boolean(), @@ -128,6 +130,7 @@ defmodule Samly.IdpData do %IdpData{idp_data | id: id, sp_id: sp_id, base_url: Map.get(opts_map, :base_url)} |> set_metadata(opts_map) |> set_pipeline(opts_map) + |> set_error_pipeline(opts_map) |> set_custom_recipient_url(opts_map) |> set_allowed_target_urls(opts_map) |> set_boolean_attr(opts_map, :use_redirect_for_req) @@ -217,6 +220,12 @@ defmodule Samly.IdpData do %IdpData{idp_data | pre_session_create_pipeline: pipeline} end + @spec set_error_pipeline(%IdpData{}, map()) :: %IdpData{} + defp set_error_pipeline(%IdpData{} = idp_data, %{} = opts_map) do + pipeline = Map.get(opts_map, :on_error_pipeline) + %IdpData{idp_data | on_error_pipeline: pipeline} + end + @spec set_custom_recipient_url(%IdpData{}, map()) :: %IdpData{} defp set_custom_recipient_url(%IdpData{} = idp_data, %{} = opts_map) do consume_url = @@ -383,7 +392,9 @@ defmodule Samly.IdpData do idp_signs_assertions: idp_data.signed_assertion_in_resp, trusted_fingerprints: idp_data.fingerprints, metadata_uri: Helper.get_metadata_uri(idp_data.base_url, path_segment_idp_id), - consume_uri: idp_data.custom_recipient_url || Helper.get_consume_uri(idp_data.base_url, path_segment_idp_id), + consume_uri: + idp_data.custom_recipient_url || + Helper.get_consume_uri(idp_data.base_url, path_segment_idp_id), logout_uri: Helper.get_logout_uri(idp_data.base_url, path_segment_idp_id), entity_id: sp_entity_id ) diff --git a/lib/samly/sp_handler.ex b/lib/samly/sp_handler.ex index 784815a..de2246c 100644 --- a/lib/samly/sp_handler.ex +++ b/lib/samly/sp_handler.ex @@ -27,7 +27,13 @@ defmodule Samly.SPHandler do def consume_signin_response(conn) do %IdpData{id: idp_id} = idp = conn.private[:samly_idp] - %IdpData{pre_session_create_pipeline: pipeline, esaml_sp_rec: sp_rec} = idp + + %IdpData{ + pre_session_create_pipeline: pipeline, + on_error_pipeline: error_pipeline, + esaml_sp_rec: sp_rec + } = idp + sp = ensure_sp_uris_set(sp_rec, conn) saml_encoding = conn.body_params["SAMLEncoding"] @@ -53,17 +59,42 @@ defmodule Samly.SPHandler do |> put_session_new("samly_assertion_key", assertion_key) |> redirect(302, target_url) else - {:halted, conn} -> conn + {:halted, conn} -> + conn + {:error, reason} -> - case idp do - %IdpData{debug_mode: true} -> + {_, assertion_or_error} = Helper.decode_idp_auth_resp(sp, saml_encoding, saml_response) + + conn + |> put_private(:samly_error, reason) + |> put_private(:samly_assertion, assertion_or_error) + |> then(fn conn -> + if idp.debug_mode do + put_private(conn, :samly_saml_response, saml_response) + else conn - |> put_resp_header("content-type", "text/html") - |> send_resp(403, "

access_denied

Error:

#{inspect(reason)}

Raw Response:

#{saml_response}

- conn |> send_resp(403, "access_denied #{inspect(reason)}") + end + end) + |> pipethrough(error_pipeline) + |> case do + %Conn{halted: true} = conn -> + {:halted, conn} + + conn -> + if idp.debug_mode do + conn + |> put_resp_header("content-type", "text/html") + |> send_resp( + 403, + "

access_denied

Error:

#{inspect(reason)}

Raw Response:

#{saml_response}

conn |> send_resp(403, "access_denied") + + _ -> + send_resp(conn, 403, "access_denied") end # rescue diff --git a/test/samly_idp_data_test.exs b/test/samly_idp_data_test.exs index ce28eac..09d1fb8 100644 --- a/test/samly_idp_data_test.exs +++ b/test/samly_idp_data_test.exs @@ -177,6 +177,13 @@ defmodule SamlyIdpDataTest do assert idp_data.custom_recipient_url == ~c"custom-recipient-url" end + test "valid-idp-config-13", %{sps: sps} do + idp_config = Map.put(@idp_config1, :on_error_pipeline, MyErrorPipeline) + %IdpData{} = idp_data = IdpData.load_provider(idp_config, sps) + assert idp_data.valid? + assert idp_data.on_error_pipeline == MyErrorPipeline + end + test "url-test-1", %{sps: sps} do idp_config = %{@idp_config1 | metadata_file: "test/data/shibboleth_idp_metadata.xml"} %IdpData{} = idp_data = IdpData.load_provider(idp_config, sps) From 292daed8b6bb24832937811a7ecabdc82f3527cd Mon Sep 17 00:00:00 2001 From: Jan Zborowski Date: Tue, 17 Feb 2026 12:17:52 +0100 Subject: [PATCH 2/6] Update README.md --- README.md | 187 +++++++++++++++++++++++++++++++++--------------------- 1 file changed, 116 insertions(+), 71 deletions(-) diff --git a/README.md b/README.md index 8d0a8a7..f49757c 100644 --- a/README.md +++ b/README.md @@ -4,14 +4,14 @@ A SAML 2.0 Service Provider Single-Sign-On Authentication library. This Plug lib This has been used in the wild with the following Identity Providers: -+ Okta -+ Ping Identity -+ OneLogin -+ ADFS -+ Nexus GO -+ Shibboleth -+ SimpleSAMLphp -+ Google +- Okta +- Ping Identity +- OneLogin +- ADFS +- Nexus GO +- Shibboleth +- SimpleSAMLphp +- Google This library uses Erlang [`esaml`](https://github.com/dropbox/esaml) to provide plug enabled routes. @@ -113,13 +113,13 @@ example URL: `https://do-good.org/sso/auth/signin/affiliates`. The idp_id in this URL is "affiliates". If you have more than one IdP, only this last part changes. The URLs for this model are: -| Description | URL | -|:----|:----| -| Sign-in button/link in Web UI | `/sso/auth/signin/affiliates` | -| Sign-out button/link in Web UI | `/sso/auth/signout/affiliates` | -| SP Metadata URL | `https://do-good.org/sso/sp/metadata/affiliates` | -| SAML Assertion Consumer Service | `https://do-good.org/sso/sp/consume/affiliates` | -| SAML SingleLogout Service | `https://do-good.org/sso/sp/logout/affiliates` | +| Description | URL | +| :------------------------------ | :----------------------------------------------- | +| Sign-in button/link in Web UI | `/sso/auth/signin/affiliates` | +| Sign-out button/link in Web UI | `/sso/auth/signout/affiliates` | +| SP Metadata URL | `https://do-good.org/sso/sp/metadata/affiliates` | +| SAML Assertion Consumer Service | `https://do-good.org/sso/sp/consume/affiliates` | +| SAML SingleLogout Service | `https://do-good.org/sso/sp/logout/affiliates` | The path segment model is the default one in `Samly`. If there is only one Identity Provider, use this mode. @@ -135,13 +135,13 @@ The path segment model is the default one in `Samly`. If there is only one Ident In this model, the subdomain name is used as the idp_id. Here is an example URL: `https://ngo.do-good.org/sso/auth/signin`. Here `ngo` is the idp_id. The URLs supported by `Samly` in this model look different. -| Description | URL | -|:----|:----| -| Sign-in button/link in Web UI | `/sso/auth/signin` | -| Sign-out button/link in Web UI | `/sso/auth/signout` | -| SP Metadata URL | `https://ngo.do-good.org/sso/sp/metadata` | -| SAML Assertion Consumer Service | `https://ngo.do-good.org/sso/sp/consume` | -| SAML SingleLogout Service | `https://ngo.do-good.org/sso/sp/logout` | +| Description | URL | +| :------------------------------ | :---------------------------------------- | +| Sign-in button/link in Web UI | `/sso/auth/signin` | +| Sign-out button/link in Web UI | `/sso/auth/signout` | +| SP Metadata URL | `https://ngo.do-good.org/sso/sp/metadata` | +| SAML Assertion Consumer Service | `https://ngo.do-good.org/sso/sp/consume` | +| SAML SingleLogout Service | `https://ngo.do-good.org/sso/sp/logout` | > Take a look at [`samly_howto`](https://github.com/dropbox/samly_howto) - a reference/demo > application on how to use this library. @@ -195,32 +195,32 @@ config :samly, Samly.Provider, ] ``` -| Parameters | Description | -|:------------|:-----------| -| `idp_id_from` | _(optional)_`:path_segment` or `:subdomain`. Default is `:path_segment`. | -| **Service Provider Parameters** | | -| `id` | _(mandatory)_ | -| `identity_id` | _(optional)_ If omitted, the metadata URL will be used | -| `certfile` | _(optional)_ This is needed when SAML requests/responses from `Samly` need to be signed. Make sure to **set this in a production deployment**. Could be omitted during development if your IDP is setup to not require signing. If that is the case, the following **Identity Provider Parameters** must be explicitly set to false: `sign_requests`, `sign_metadata`| -| `keyfile` | _(optional)_ Similar to `certfile` | -| `contact_name` | _(optional)_ Technical contact name for the Service Provider | -| `contact_email` | _(optional)_ Technical contact email address | -| `org_name` | _(optional)_ SAML Service Provider (your app) Organization name | -| `org_displayname` | _(optional)_ SAML SP Organization displayname | -| `org_url` | _(optional)_ Service Provider Organization web site URL | -| **Identity Provider Parameters** | | -| `id` | _(mandatory)_ This will be the idp_id in the URLs | -| `sp_id` | _(mandatory)_ The service provider definition to be used with this Identity Provider definition | -| `base_url` | _(optional)_ If missing `Samly` will use the current URL to derive this. It is better to define this in production deployment. | -| `metadata_file` | _(mandatory if `metadata` is not set)_ Path to the IdP metadata XML file obtained from the Identity Provider. This will be ignored if `metadata` is non-nil. | -| `metadata` | _(mandatory if `metadata_file` is not set))_ String containing IdP metadata XML obtained from the Identity Provider. | -| `pre_session_create_pipeline` | _(optional)_ Check the customization section. | -| `use_redirect_for_req` | _(optional)_ Default is `false`. When this is `false`, `Samly` will POST to the IdP SAML endpoints. | -| `sign_requests`, `sign_metadata` | _(optional)_ Default is `true`. | -| `signed_assertion_in_resp`, `signed_envelopes_in_resp` | _(optional)_ Default is `true`. When `true`, `Samly` expects the requests and responses from IdP to be signed. | -| `allow_idp_initiated_flow` | _(optional)_ Default is `false`. IDP initiated SSO is allowed only when this is set to `true`. | -| `allowed_target_urls` | _(optional)_ Default is `[]`. `Samly` uses this **only** when `allow_idp_initiated_flow` parameter is set to `true`. Make sure to set this to one or more exact URLs you want to allow (whitelist). The URL to redirect the user after completing the SSO flow is sent from IDP in auth response as `relay_state`. This `relay_state` target URL is matched against this URL list. Set the value to `nil` if you do not want this whitelist capability. | -| `nameid_format` | _(optional)_ When specified, `Samly` includes the value as the `NameIDPolicy` element's `Format` attribute in the login request. Value must either be a string or one of the following atoms: `:email`, `:x509`, `:windows`, `:krb`, `:persistent`, `:transient`. Use the string value when you need to specify a non-standard/custom nameid format supported by your IdP. | +| Parameters | Description | +| :----------------------------------------------------- | :------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `idp_id_from` | _(optional)_`:path_segment` or `:subdomain`. Default is `:path_segment`. | +| **Service Provider Parameters** | | +| `id` | _(mandatory)_ | +| `identity_id` | _(optional)_ If omitted, the metadata URL will be used | +| `certfile` | _(optional)_ This is needed when SAML requests/responses from `Samly` need to be signed. Make sure to **set this in a production deployment**. Could be omitted during development if your IDP is setup to not require signing. If that is the case, the following **Identity Provider Parameters** must be explicitly set to false: `sign_requests`, `sign_metadata` | +| `keyfile` | _(optional)_ Similar to `certfile` | +| `contact_name` | _(optional)_ Technical contact name for the Service Provider | +| `contact_email` | _(optional)_ Technical contact email address | +| `org_name` | _(optional)_ SAML Service Provider (your app) Organization name | +| `org_displayname` | _(optional)_ SAML SP Organization displayname | +| `org_url` | _(optional)_ Service Provider Organization web site URL | +| **Identity Provider Parameters** | | +| `id` | _(mandatory)_ This will be the idp_id in the URLs | +| `sp_id` | _(mandatory)_ The service provider definition to be used with this Identity Provider definition | +| `base_url` | _(optional)_ If missing `Samly` will use the current URL to derive this. It is better to define this in production deployment. | +| `metadata_file` | _(mandatory if `metadata` is not set)_ Path to the IdP metadata XML file obtained from the Identity Provider. This will be ignored if `metadata` is non-nil. | +| `metadata` | _(mandatory if `metadata_file` is not set))_ String containing IdP metadata XML obtained from the Identity Provider. | +| `pre_session_create_pipeline` | _(optional)_ Check the customization section. | +| `use_redirect_for_req` | _(optional)_ Default is `false`. When this is `false`, `Samly` will POST to the IdP SAML endpoints. | +| `sign_requests`, `sign_metadata` | _(optional)_ Default is `true`. | +| `signed_assertion_in_resp`, `signed_envelopes_in_resp` | _(optional)_ Default is `true`. When `true`, `Samly` expects the requests and responses from IdP to be signed. | +| `allow_idp_initiated_flow` | _(optional)_ Default is `false`. IDP initiated SSO is allowed only when this is set to `true`. | +| `allowed_target_urls` | _(optional)_ Default is `[]`. `Samly` uses this **only** when `allow_idp_initiated_flow` parameter is set to `true`. Make sure to set this to one or more exact URLs you want to allow (whitelist). The URL to redirect the user after completing the SSO flow is sent from IDP in auth response as `relay_state`. This `relay_state` target URL is matched against this URL list. Set the value to `nil` if you do not want this whitelist capability. | +| `nameid_format` | _(optional)_ When specified, `Samly` includes the value as the `NameIDPolicy` element's `Format` attribute in the login request. Value must either be a string or one of the following atoms: `:email`, `:x509`, `:windows`, `:krb`, `:persistent`, `:transient`. Use the string value when you need to specify a non-standard/custom nameid format supported by your IdP. | #### Authenticated SAML Assertion State Store @@ -236,9 +236,9 @@ config :samly, Samly.State, This state configuration is optional. If omitted, `Samly` uses `Samly.State.ETS` provider by default. -| Options | Description | -|:------------|:-----------| -| `opts` | _(optional)_ The `:table` option is the ETS table name for storing the assertions. This ETS table is created during the store provider initialization if it is not already present. Default is `samly_assertions_table`. | +| Options | Description | +| :------ | :----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `opts` | _(optional)_ The `:table` option is the ETS table name for storing the assertions. This ETS table is created during the store provider initialization if it is not already present. Default is `samly_assertions_table`. | > Use `Samly.State.Session` provider in a clustered deployment. This provider uses > the Plug Sessions to keep the authenticated SAML assertions. @@ -251,9 +251,9 @@ config :samly, Samly.State, opts: [key: :my_assertion_key] ``` -| Options | Description | -|:------------|:-----------| -| `opts` | _(optional)_ The `:key` is the name of the session key where assertion is stored. Default is `:samly_assertion`. | +| Options | Description | +| :------ | :--------------------------------------------------------------------------------------------------------------- | +| `opts` | _(optional)_ The `:key` is the name of the session key where assertion is stored. Default is `:samly_assertion`. | ## SAML Assertion @@ -359,29 +359,74 @@ config :samly, Samly.Provider, ] ``` +#### Error Pipeline + +It is also possible to provide an error Plug pipeline for custom error handling. + +This is a vanilla Plug Pipeline, similar to the `pre_session_create_pipeline`. The SAML error reason from +the IdP is made available in the Plug connection as a "private". The SAML assertion is available too, but +only if the error occurred after it was decoded. In `debug_mode` SAML response is available. + +If the error pipeline is not provided in the config, or the error pipeline does not halt the connection, default `Samly` behavior will follow. + +Here is a sample error pipeline: + +```elixir +defmodule MySamlyErrorPipeline do + use Plug.Builder + + plug :handle_error + + def handle_error(conn, _opts) do + error = conn.private[:samly_error] + # assertion = conn.private[:samly_assertion] + # saml_response = conn.private[:samly_saml_response] - only in debug mode + + #... Log the error and do other necessary things + + #... then redirect and halt + conn + |> redirect(to: ~p"/auth/error?reason=#{error}") + |> halt() +end +``` + +Make this pipeline available in your config: + +```elixir +config :samly, Samly.Provider, + identity_providers: [ + %{ + # ... + on_error_pipeline: MySamlyErrorPipeline, + # ... + } + ] +``` + #### State Store Take a look at the implementation of `Samly.State.ETS` or `Samly.State.Session` and use those as examples showing how to create your own state store (based on redis, memcached, database etc.). ## Security Related -+ `Samly` initiated sign-in/sign-out requests send `RelayState` to IdP and expect to get that back. Mismatched or missing `RelayState` in IdP responses to SP initiated requests will fail (with HTTP `403 access_denied`). -+ Besides the `RelayState`, the request and response `idp_id`s must match. Response is rejected if they don't. -+ `Samly` makes the original request ID that an auth response corresponds to -in `Samly.Subject.in_response_to` field. It is the responsibility of the consuming application to use this information along with the validity period in the assertion to check for **replay attacks**. The consuming application should use the `pre_session_create_pipeline` to perform this check. You may need a database or a distributed cache such as memcache in a clustered setup to keep track of these request IDs for their validity period to perform this check. Be aware that `in_response_to` field is **not** set when IDP initialized authorization flow is used. -+ OOTB SAML requests and responses are signed. -+ Signature digest method supported: `SHA256`. - > Some Identity Providers may be using `SHA1` by default. - > Make sure to configure the IdP to use `SHA256`. `Samly` - > will reject (`access_denied`) IdP responses using `SHA1`. -+ `esaml` provides additional checks such as trusted certificate verification, recipient verification among others. -+ By default, `Samly` signs the SAML requests it sends to the Identity Provider. It also - expects the SAML reqsponses to be signed (both assertion and envelopes). If your IdP is - not configured to sign, you will have to explicitly turn them off in the configuration. - It is highly recommended to turn signing on in production deployments. -+ Encrypted Assertions are supported in `Samly`. There are no explicit config settings for this. Decryption happens automatically when encrypted assertions are detected in the SAML response. - > [Supported Encryption algorithms](https://github.com/dropbox/esaml#assertion-encryption) -+ Make sure to use HTTPS URLs in production deployments. +- `Samly` initiated sign-in/sign-out requests send `RelayState` to IdP and expect to get that back. Mismatched or missing `RelayState` in IdP responses to SP initiated requests will fail (with HTTP `403 access_denied`). +- Besides the `RelayState`, the request and response `idp_id`s must match. Response is rejected if they don't. +- `Samly` makes the original request ID that an auth response corresponds to + in `Samly.Subject.in_response_to` field. It is the responsibility of the consuming application to use this information along with the validity period in the assertion to check for **replay attacks**. The consuming application should use the `pre_session_create_pipeline` to perform this check. You may need a database or a distributed cache such as memcache in a clustered setup to keep track of these request IDs for their validity period to perform this check. Be aware that `in_response_to` field is **not** set when IDP initialized authorization flow is used. +- OOTB SAML requests and responses are signed. +- Signature digest method supported: `SHA256`. + > Some Identity Providers may be using `SHA1` by default. + > Make sure to configure the IdP to use `SHA256`. `Samly` + > will reject (`access_denied`) IdP responses using `SHA1`. +- `esaml` provides additional checks such as trusted certificate verification, recipient verification among others. +- By default, `Samly` signs the SAML requests it sends to the Identity Provider. It also + expects the SAML reqsponses to be signed (both assertion and envelopes). If your IdP is + not configured to sign, you will have to explicitly turn them off in the configuration. + It is highly recommended to turn signing on in production deployments. +- Encrypted Assertions are supported in `Samly`. There are no explicit config settings for this. Decryption happens automatically when encrypted assertions are detected in the SAML response. + > [Supported Encryption algorithms](https://github.com/dropbox/esaml#assertion-encryption) +- Make sure to use HTTPS URLs in production deployments. ## FAQ From b382d08240f72cd2c9660d21e6331805f3ab585a Mon Sep 17 00:00:00 2001 From: Jan Zborowski Date: Tue, 17 Feb 2026 12:18:17 +0100 Subject: [PATCH 3/6] Fix halted connection return --- lib/samly/sp_handler.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/samly/sp_handler.ex b/lib/samly/sp_handler.ex index de2246c..cfb83f7 100644 --- a/lib/samly/sp_handler.ex +++ b/lib/samly/sp_handler.ex @@ -78,7 +78,7 @@ defmodule Samly.SPHandler do |> pipethrough(error_pipeline) |> case do %Conn{halted: true} = conn -> - {:halted, conn} + conn conn -> if idp.debug_mode do From 08bcd8b93f51dd8280e4322a76f6ec58a421dbde Mon Sep 17 00:00:00 2001 From: Jan Zborowski Date: Tue, 17 Feb 2026 13:34:10 +0100 Subject: [PATCH 4/6] Apply suggestions from code review Co-authored-by: baz-reviewer[bot] <174234987+baz-reviewer[bot]@users.noreply.github.com> --- lib/samly/sp_handler.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/samly/sp_handler.ex b/lib/samly/sp_handler.ex index cfb83f7..0442898 100644 --- a/lib/samly/sp_handler.ex +++ b/lib/samly/sp_handler.ex @@ -86,7 +86,7 @@ defmodule Samly.SPHandler do |> put_resp_header("content-type", "text/html") |> send_resp( 403, - "

access_denied

Error:

#{inspect(reason)}

Raw Response:

#{saml_response}

access_denied

Error:

#{inspect(reason)}

Raw Response:

#{saml_response}

" ) else send_resp(conn, 403, "access_denied") From 5a1a500e43dd1a4527ad6ee287166b1e8d24fb58 Mon Sep 17 00:00:00 2001 From: Jan Zborowski Date: Wed, 25 Feb 2026 17:08:15 +0100 Subject: [PATCH 5/6] Move error pipeline to global config and handle more errors --- README.md | 7 ++----- lib/samly/auth_handler.ex | 8 +++++++- lib/samly/helper.ex | 7 +++++++ lib/samly/idp_data.ex | 9 --------- lib/samly/provider.ex | 1 + lib/samly/router_util.ex | 21 +++++++++++++++++++-- lib/samly/sp_handler.ex | 20 ++++++++++++++++---- lib/samly/sp_router.ex | 9 ++++++++- test/samly_idp_data_test.exs | 7 ------- 9 files changed, 60 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index f49757c..4b5adae 100644 --- a/README.md +++ b/README.md @@ -395,12 +395,9 @@ Make this pipeline available in your config: ```elixir config :samly, Samly.Provider, + on_error_pipeline: MySamlyErrorPipeline, identity_providers: [ - %{ - # ... - on_error_pipeline: MySamlyErrorPipeline, - # ... - } + # ... ] ``` diff --git a/lib/samly/auth_handler.ex b/lib/samly/auth_handler.ex index a9cf580..78dce08 100644 --- a/lib/samly/auth_handler.ex +++ b/lib/samly/auth_handler.ex @@ -124,7 +124,13 @@ defmodule Samly.AuthHandler do ) _ -> - conn |> send_resp(403, "access_denied") + conn + |> put_private(:samly_error, :no_active_session) + |> Helper.run_error_pipeline() + |> case do + %Plug.Conn{halted: true} = conn -> conn + conn -> conn |> send_resp(403, "access_denied") + end end # rescue diff --git a/lib/samly/helper.ex b/lib/samly/helper.ex index 32b1374..c6f97c9 100644 --- a/lib/samly/helper.ex +++ b/lib/samly/helper.ex @@ -10,6 +10,13 @@ defmodule Samly.Helper do Map.get(idps, idp_id) end + def run_error_pipeline(conn) do + case Application.get_env(:samly, :on_error_pipeline) do + nil -> conn + pipeline -> pipeline.call(conn, []) + end + end + @spec get_metadata_uri(nil | binary, binary) :: nil | charlist def get_metadata_uri(nil, _idp_id), do: nil diff --git a/lib/samly/idp_data.ex b/lib/samly/idp_data.ex index 9dc416c..c3134d8 100644 --- a/lib/samly/idp_data.ex +++ b/lib/samly/idp_data.ex @@ -16,7 +16,6 @@ defmodule Samly.IdpData do custom_recipient_url: nil, metadata_file: nil, metadata: nil, - on_error_pipeline: nil, pre_session_create_pipeline: nil, use_redirect_for_req: false, sign_requests: true, @@ -46,7 +45,6 @@ defmodule Samly.IdpData do custom_recipient_url: nil | binary(), metadata_file: nil | binary(), metadata: nil | binary(), - on_error_pipeline: nil | module(), pre_session_create_pipeline: nil | module(), use_redirect_for_req: boolean(), sign_requests: boolean(), @@ -130,7 +128,6 @@ defmodule Samly.IdpData do %IdpData{idp_data | id: id, sp_id: sp_id, base_url: Map.get(opts_map, :base_url)} |> set_metadata(opts_map) |> set_pipeline(opts_map) - |> set_error_pipeline(opts_map) |> set_custom_recipient_url(opts_map) |> set_allowed_target_urls(opts_map) |> set_boolean_attr(opts_map, :use_redirect_for_req) @@ -220,12 +217,6 @@ defmodule Samly.IdpData do %IdpData{idp_data | pre_session_create_pipeline: pipeline} end - @spec set_error_pipeline(%IdpData{}, map()) :: %IdpData{} - defp set_error_pipeline(%IdpData{} = idp_data, %{} = opts_map) do - pipeline = Map.get(opts_map, :on_error_pipeline) - %IdpData{idp_data | on_error_pipeline: pipeline} - end - @spec set_custom_recipient_url(%IdpData{}, map()) :: %IdpData{} defp set_custom_recipient_url(%IdpData{} = idp_data, %{} = opts_map) do consume_url = diff --git a/lib/samly/provider.ex b/lib/samly/provider.ex index f31ad14..d2dc061 100644 --- a/lib/samly/provider.ex +++ b/lib/samly/provider.ex @@ -73,6 +73,7 @@ defmodule Samly.Provider do identity_providers = Samly.IdpData.load_providers(opts[:identity_providers] || [], service_providers) + Application.put_env(:samly, :on_error_pipeline, opts[:on_error_pipeline]) Application.put_env(:samly, :service_providers, service_providers) Application.put_env(:samly, :identity_providers, identity_providers) diff --git a/lib/samly/router_util.ex b/lib/samly/router_util.ex index 0f6208c..69d9f95 100644 --- a/lib/samly/router_util.ex +++ b/lib/samly/router_util.ex @@ -29,7 +29,13 @@ defmodule Samly.RouterUtil do if idp do conn |> Conn.put_private(:samly_idp, idp) else - conn |> Conn.send_resp(403, "invalid_request unknown IdP") |> Conn.halt() + conn + |> Conn.put_private(:samly_error, :unknown_idp) + |> Helper.run_error_pipeline() + |> case do + %Conn{halted: true} = conn -> conn + conn -> conn |> Conn.send_resp(403, "invalid_request unknown IdP") |> Conn.halt() + end end end @@ -43,7 +49,18 @@ defmodule Samly.RouterUtil do "[Samly] target_url must be x-www-form-urlencoded: #{inspect(conn.params["target_url"])}" ) - conn |> Conn.send_resp(400, "target_url must be x-www-form-urlencoded") |> Conn.halt() + conn + |> Conn.put_private(:samly_error, :invalid_target_url_encoding) + |> Helper.run_error_pipeline() + |> case do + %Conn{halted: true} = conn -> + conn + + conn -> + conn + |> Conn.send_resp(400, "target_url must be x-www-form-urlencoded") + |> Conn.halt() + end end end diff --git a/lib/samly/sp_handler.ex b/lib/samly/sp_handler.ex index 0442898..57ea0fa 100644 --- a/lib/samly/sp_handler.ex +++ b/lib/samly/sp_handler.ex @@ -30,7 +30,6 @@ defmodule Samly.SPHandler do %IdpData{ pre_session_create_pipeline: pipeline, - on_error_pipeline: error_pipeline, esaml_sp_rec: sp_rec } = idp @@ -75,7 +74,7 @@ defmodule Samly.SPHandler do conn end end) - |> pipethrough(error_pipeline) + |> Helper.run_error_pipeline() |> case do %Conn{halted: true} = conn -> conn @@ -94,7 +93,13 @@ defmodule Samly.SPHandler do end _ -> - send_resp(conn, 403, "access_denied") + conn + |> put_private(:samly_error, :access_denied) + |> Helper.run_error_pipeline() + |> case do + %Conn{halted: true} = conn -> conn + conn -> send_resp(conn, 403, "access_denied") + end end # rescue @@ -182,7 +187,14 @@ defmodule Samly.SPHandler do |> configure_session(drop: true) |> redirect(302, target_url) else - error -> conn |> send_resp(403, "invalid_request #{inspect(error)}") + error -> + conn + |> put_private(:samly_error, {:invalid_logout_response, error}) + |> Helper.run_error_pipeline() + |> case do + %Conn{halted: true} = conn -> conn + conn -> conn |> send_resp(403, "invalid_request #{inspect(error)}") + end end # rescue diff --git a/lib/samly/sp_router.ex b/lib/samly/sp_router.ex index 661e9c7..7322712 100644 --- a/lib/samly/sp_router.ex +++ b/lib/samly/sp_router.ex @@ -23,7 +23,14 @@ defmodule Samly.SPRouter do cond do conn.params["SAMLResponse"] != nil -> Samly.SPHandler.handle_logout_response(conn) conn.params["SAMLRequest"] != nil -> Samly.SPHandler.handle_logout_request(conn) - true -> conn |> send_resp(403, "invalid_request") + true -> + conn + |> Plug.Conn.put_private(:samly_error, :invalid_request) + |> Samly.Helper.run_error_pipeline() + |> case do + %Plug.Conn{halted: true} = conn -> conn + conn -> conn |> send_resp(403, "invalid_request") + end end end diff --git a/test/samly_idp_data_test.exs b/test/samly_idp_data_test.exs index 09d1fb8..ce28eac 100644 --- a/test/samly_idp_data_test.exs +++ b/test/samly_idp_data_test.exs @@ -177,13 +177,6 @@ defmodule SamlyIdpDataTest do assert idp_data.custom_recipient_url == ~c"custom-recipient-url" end - test "valid-idp-config-13", %{sps: sps} do - idp_config = Map.put(@idp_config1, :on_error_pipeline, MyErrorPipeline) - %IdpData{} = idp_data = IdpData.load_provider(idp_config, sps) - assert idp_data.valid? - assert idp_data.on_error_pipeline == MyErrorPipeline - end - test "url-test-1", %{sps: sps} do idp_config = %{@idp_config1 | metadata_file: "test/data/shibboleth_idp_metadata.xml"} %IdpData{} = idp_data = IdpData.load_provider(idp_config, sps) From b9101a1283545675b6e003049b4acf25076e3936 Mon Sep 17 00:00:00 2001 From: Jan Zborowski Date: Thu, 26 Feb 2026 12:51:08 +0100 Subject: [PATCH 6/6] Add error handling helper for less duplication --- lib/samly/auth_handler.ex | 8 +------- lib/samly/helper.ex | 20 +++++++++++++++++--- lib/samly/router_util.ex | 26 +++++++------------------- lib/samly/sp_handler.ex | 21 +++++++-------------- lib/samly/sp_router.ex | 8 +------- 5 files changed, 33 insertions(+), 50 deletions(-) diff --git a/lib/samly/auth_handler.ex b/lib/samly/auth_handler.ex index 78dce08..8400daf 100644 --- a/lib/samly/auth_handler.ex +++ b/lib/samly/auth_handler.ex @@ -124,13 +124,7 @@ defmodule Samly.AuthHandler do ) _ -> - conn - |> put_private(:samly_error, :no_active_session) - |> Helper.run_error_pipeline() - |> case do - %Plug.Conn{halted: true} = conn -> conn - conn -> conn |> send_resp(403, "access_denied") - end + Helper.handle_error_response(conn, :no_active_session, 403, "access_denied") end # rescue diff --git a/lib/samly/helper.ex b/lib/samly/helper.ex index c6f97c9..8ecf202 100644 --- a/lib/samly/helper.ex +++ b/lib/samly/helper.ex @@ -10,10 +10,24 @@ defmodule Samly.Helper do Map.get(idps, idp_id) end - def run_error_pipeline(conn) do + def handle_error_response(%Plug.Conn{} = conn, error, status, body) do + conn + |> Plug.Conn.put_private(:samly_error, error) + |> run_error_pipeline() + |> case do + %Plug.Conn{halted: true} = conn -> conn + conn -> conn |> Plug.Conn.send_resp(status, body) |> Plug.Conn.halt() + end + end + + def run_error_pipeline(%Plug.Conn{} = conn) do case Application.get_env(:samly, :on_error_pipeline) do - nil -> conn - pipeline -> pipeline.call(conn, []) + nil -> + conn + + pipeline -> + conn = pipeline.call(conn, []) + if conn.state in [:sent, :chunked, :set_chunked], do: %{conn | halted: true}, else: conn end end diff --git a/lib/samly/router_util.ex b/lib/samly/router_util.ex index 69d9f95..4d0936c 100644 --- a/lib/samly/router_util.ex +++ b/lib/samly/router_util.ex @@ -29,13 +29,7 @@ defmodule Samly.RouterUtil do if idp do conn |> Conn.put_private(:samly_idp, idp) else - conn - |> Conn.put_private(:samly_error, :unknown_idp) - |> Helper.run_error_pipeline() - |> case do - %Conn{halted: true} = conn -> conn - conn -> conn |> Conn.send_resp(403, "invalid_request unknown IdP") |> Conn.halt() - end + Helper.handle_error_response(conn, :unknown_idp, 403, "invalid_request unknown IdP") end end @@ -49,18 +43,12 @@ defmodule Samly.RouterUtil do "[Samly] target_url must be x-www-form-urlencoded: #{inspect(conn.params["target_url"])}" ) - conn - |> Conn.put_private(:samly_error, :invalid_target_url_encoding) - |> Helper.run_error_pipeline() - |> case do - %Conn{halted: true} = conn -> - conn - - conn -> - conn - |> Conn.send_resp(400, "target_url must be x-www-form-urlencoded") - |> Conn.halt() - end + Helper.handle_error_response( + conn, + :invalid_target_url_encoding, + 400, + "target_url must be x-www-form-urlencoded" + ) end end diff --git a/lib/samly/sp_handler.ex b/lib/samly/sp_handler.ex index 57ea0fa..662f901 100644 --- a/lib/samly/sp_handler.ex +++ b/lib/samly/sp_handler.ex @@ -93,13 +93,7 @@ defmodule Samly.SPHandler do end _ -> - conn - |> put_private(:samly_error, :access_denied) - |> Helper.run_error_pipeline() - |> case do - %Conn{halted: true} = conn -> conn - conn -> send_resp(conn, 403, "access_denied") - end + Helper.handle_error_response(conn, :access_denied, 403, "access_denied") end # rescue @@ -188,13 +182,12 @@ defmodule Samly.SPHandler do |> redirect(302, target_url) else error -> - conn - |> put_private(:samly_error, {:invalid_logout_response, error}) - |> Helper.run_error_pipeline() - |> case do - %Conn{halted: true} = conn -> conn - conn -> conn |> send_resp(403, "invalid_request #{inspect(error)}") - end + Helper.handle_error_response( + conn, + {:invalid_logout_response, error}, + 403, + "invalid_request #{inspect(error)}" + ) end # rescue diff --git a/lib/samly/sp_router.ex b/lib/samly/sp_router.ex index 7322712..19fa0ca 100644 --- a/lib/samly/sp_router.ex +++ b/lib/samly/sp_router.ex @@ -24,13 +24,7 @@ defmodule Samly.SPRouter do conn.params["SAMLResponse"] != nil -> Samly.SPHandler.handle_logout_response(conn) conn.params["SAMLRequest"] != nil -> Samly.SPHandler.handle_logout_request(conn) true -> - conn - |> Plug.Conn.put_private(:samly_error, :invalid_request) - |> Samly.Helper.run_error_pipeline() - |> case do - %Plug.Conn{halted: true} = conn -> conn - conn -> conn |> send_resp(403, "invalid_request") - end + Samly.Helper.handle_error_response(conn, :invalid_request, 403, "invalid_request") end end