v1.0.0-rc.1

CHANGELOG.md

@@ -1,5 +1,16 @@
 # CHANGELOG
 
+### v1.0.0-rc.1
+
++   `target_url` query parameter for the sign-in/sign-out requests must be
+    `x-www-form-urlencoded`.
+
++   Redirect URLs are properly encoded.
+
++   Switched to `report-to` in content security policy.
+
++   `cache-control` header value updated.
+
 ### v1.0.0-rc.0
 
 +   Issue: #33 - Content Security Policy

README.md

@@ -15,8 +15,8 @@ plug enabled routes.
 defp deps() do
   [
     # ...
-    {:samly, "~> 1.0.0-rc.0"},
-    # v1.0.0-rc.0 uses esaml v4.2 which in turn relies on cowboy 2.x
+    {:samly, "~> 1.0.0-rc.1"},
+    # v1.0.0-rc.1 uses esaml v4.2 which in turn relies on cowboy 2.x
     # If you need to work with cowboy 1.x, you need the following override:
     # {:esaml, "~> 3.7", override: true}
   ]
@@ -199,8 +199,8 @@ config :samly, Samly.Provider,
 | `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 this is specified, `Samly` will put the value in the `Format` attribute of the `NameIDPolicy` element of the login request. Value may be a string, a character list, or one of the following atoms: `:email`, `:x509`, `:windows`, `:krb`, `:persistent`, `:transient`.                                                                                                |
+| `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
 

lib/samly/auth_handler.ex

@@ -40,16 +40,12 @@ defmodule Samly.AuthHandler do
   def initiate_sso_req(conn) do
     import Plug.CSRFProtection, only: [get_csrf_token: 0]
 
-    target_url =
-      case conn.params["target_url"] do
-        nil -> nil
-        url -> URI.decode_www_form(url)
-      end
+    target_url = conn.private[:samly_target_url] || "/"
 
     opts = [
       nonce: conn.private[:samly_nonce],
-      action: conn.request_path,
-      target_url: target_url,
+      action: URI.encode(conn.request_path),
+      target_url: URI.encode_www_form(target_url),
       csrf_token: get_csrf_token()
     ]
 
@@ -63,7 +59,7 @@ defmodule Samly.AuthHandler do
     %IdpData{esaml_idp_rec: idp_rec, esaml_sp_rec: sp_rec} = idp
     sp = ensure_sp_uris_set(sp_rec, conn)
 
-    target_url = (conn.params["target_url"] || "/") |> URI.decode_www_form()
+    target_url = conn.private[:samly_target_url] || "/"
     assertion_key = get_session(conn, "samly_assertion_key")
 
     case State.get_assertion(conn, assertion_key) do
@@ -85,7 +81,7 @@ defmodule Samly.AuthHandler do
           idp_signin_url,
           idp.use_redirect_for_req,
           req_xml_frag,
-          relay_state |> URI.encode_www_form()
+          relay_state
         )
     end
 
@@ -100,7 +96,7 @@ defmodule Samly.AuthHandler do
     %IdpData{esaml_idp_rec: idp_rec, esaml_sp_rec: sp_rec} = idp
     sp = ensure_sp_uris_set(sp_rec, conn)
 
-    target_url = (conn.params["target_url"] || "/") |> URI.decode_www_form()
+    target_url = conn.private[:samly_target_url] || "/"
     assertion_key = get_session(conn, "samly_assertion_key")
 
     case State.get_assertion(conn, assertion_key) do
@@ -123,7 +119,7 @@ defmodule Samly.AuthHandler do
           idp_signout_url,
           idp.use_redirect_for_req,
           req_xml_frag,
-          relay_state |> URI.encode_www_form()
+          relay_state
         )
 
       _ ->

lib/samly/auth_router.ex

@@ -3,12 +3,13 @@ defmodule Samly.AuthRouter do
 
   use Plug.Router
   import Plug.Conn
-  import Samly.RouterUtil, only: [check_idp_id: 2]
+  import Samly.RouterUtil, only: [check_idp_id: 2, check_target_url: 2]
 
   plug :fetch_session
   plug Plug.CSRFProtection
   plug :match
   plug :check_idp_id
+  plug :check_target_url
   plug :dispatch
 
   get "/signin/*idp_id_seg" do

lib/samly/helper.ex

@@ -72,6 +72,7 @@ defmodule Samly.Helper do
          {:ok, assertion_rec} <- :esaml_sp.validate_assertion(xml_frag, sp) do
       {:ok, Assertion.from_rec(assertion_rec)}
     else
+      {:error, reason} -> {:error, reason}
       error -> {:error, {:invalid_request, "#{inspect(error)}"}}
     end
   end

lib/samly/router.ex

@@ -19,7 +19,7 @@ defmodule Samly.Router do
        default-src 'none';
        script-src 'self' 'nonce-<%= nonce %>' 'report-sample';
        img-src 'self' 'report-sample';
-       report-uri /sso/csp-report;
+       report-to /sso/csp-report;
        """
        |> String.replace("\n", " ")
 
@@ -30,7 +30,7 @@ defmodule Samly.Router do
       nonce = connection.private[:samly_nonce]
 
       connection
-      |> put_resp_header("cache-control", "no-cache")
+      |> put_resp_header("cache-control", "no-cache, no-store, must-revalidate")
       |> put_resp_header("pragma", "no-cache")
       |> put_resp_header("x-frame-options", "SAMEORIGIN")
       |> put_resp_header("content-security-policy", EEx.eval_string(@csp, nonce: nonce))

lib/samly/router_util.ex

@@ -2,6 +2,7 @@ defmodule Samly.RouterUtil do
   @moduledoc false
 
   alias Plug.Conn
+  require Logger
   require Samly.Esaml
   alias Samly.{Esaml, IdpData, Helper}
 
@@ -32,6 +33,20 @@ defmodule Samly.RouterUtil do
     end
   end
 
+  def check_target_url(conn, _opts) do
+    try do
+      target_url = conn.params["target_url"] && URI.decode_www_form(conn.params["target_url"])
+      conn |> Conn.put_private(:samly_target_url, target_url)
+    rescue
+      ArgumentError ->
+        Logger.error(
+          "[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()
+    end
+  end
+
   # generate URIs using the idp_id
   @spec ensure_sp_uris_set(tuple, Conn.t()) :: tuple
   def ensure_sp_uris_set(sp, conn) do
@@ -85,7 +100,7 @@ defmodule Samly.RouterUtil do
 
   def redirect(conn, status_code, dest) do
     conn
-    |> Conn.put_resp_header("location", dest)
+    |> Conn.put_resp_header("location", URI.encode(dest))
     |> Conn.send_resp(status_code, "")
     |> Conn.halt()
   end

lib/samly/sp_handler.ex

@@ -50,7 +50,7 @@ defmodule Samly.SPHandler do
       conn
       |> configure_session(renew: true)
       |> put_session("samly_assertion_key", assertion_key)
-      |> redirect(302, target_url |> URI.decode_www_form())
+      |> redirect(302, target_url)
     else
       {:halted, conn} -> conn
       {:error, reason} -> conn |> send_resp(403, "access_denied #{inspect(reason)}")
@@ -133,7 +133,7 @@ defmodule Samly.SPHandler do
          target_url when target_url != nil <- get_session(conn, "target_url") do
       conn
       |> configure_session(drop: true)
-      |> redirect(302, target_url |> URI.decode_www_form())
+      |> redirect(302, target_url)
     else
       error -> conn |> send_resp(403, "invalid_request #{inspect(error)}")
     end
@@ -152,7 +152,7 @@ defmodule Samly.SPHandler do
 
     saml_encoding = conn.body_params["SAMLEncoding"]
     saml_request = conn.body_params["SAMLRequest"]
-    relay_state = conn.body_params["RelayState"]
+    relay_state = conn.body_params["RelayState"] |> URI.decode_www_form()
 
     with {:ok, payload} <- Helper.decode_idp_signout_req(sp, saml_encoding, saml_request) do
       Esaml.esaml_logoutreq(name: nameid, issuer: _issuer) = payload

mix.exs

@@ -1,7 +1,7 @@
 defmodule Samly.Mixfile do
   use Mix.Project
 
-  @version "1.0.0-rc.0"
+  @version "1.0.0-rc.1"
   @description "SAML SP SSO made easy"
   @source_url "https://github.com/handnot2/samly"
   @blog_url "https://handnot2.github.io/blog/auth/saml-auth-for-phoenix"
@@ -31,7 +31,7 @@ defmodule Samly.Mixfile do
     [
       {:plug, "~> 1.6"},
       {:esaml, "~> 4.2"},
-      {:sweet_xml, "~> 0.6"},
+      {:sweet_xml, "~> 0.6.6"},
       {:ex_doc, "~> 0.19.0", only: :dev, runtime: false},
       {:inch_ex, "~> 1.0", only: [:dev, :test]}
     ]

mix.lock

@@ -13,5 +13,5 @@
   "plug_crypto": {:hex, :plug_crypto, "1.0.0", "18e49317d3fa343f24620ed22795ec29d4a5e602d52d1513ccea0b07d8ea7d4d", [:mix], [], "hexpm"},
   "poison": {:hex, :poison, "3.1.0", "d9eb636610e096f86f25d9a46f35a9facac35609a7591b3be3326e99a0484665", [:mix], [], "hexpm"},
   "ranch": {:hex, :ranch, "1.7.1", "6b1fab51b49196860b733a49c07604465a47bdb78aa10c1c16a3d199f7f8c881", [:rebar3], [], "hexpm"},
-  "sweet_xml": {:hex, :sweet_xml, "0.6.5", "dd9cde443212b505d1b5f9758feb2000e66a14d3c449f04c572f3048c66e6697", [:mix], [], "hexpm"},
+  "sweet_xml": {:hex, :sweet_xml, "0.6.6", "fc3e91ec5dd7c787b6195757fbcf0abc670cee1e4172687b45183032221b66b8", [:mix], [], "hexpm"},
 }