From 032281091ea933b925f86b537cacfbaa791bacc3 Mon Sep 17 00:00:00 2001 From: handnot2 Date: Wed, 27 Feb 2019 22:14:44 -0800 Subject: [PATCH] v1.0.0-rc.1 --- CHANGELOG.md | 11 +++++++++++ README.md | 8 ++++---- lib/samly/auth_handler.ex | 18 +++++++----------- lib/samly/auth_router.ex | 3 ++- lib/samly/helper.ex | 1 + lib/samly/router.ex | 4 ++-- lib/samly/router_util.ex | 17 ++++++++++++++++- lib/samly/sp_handler.ex | 6 +++--- mix.exs | 4 ++-- mix.lock | 2 +- 10 files changed, 49 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c44400a..b3bde1c 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/README.md b/README.md index fd49e86..618020f 100644 --- a/README.md +++ b/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 diff --git a/lib/samly/auth_handler.ex b/lib/samly/auth_handler.ex index 92ce230..277d358 100644 --- a/lib/samly/auth_handler.ex +++ b/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 ) _ -> diff --git a/lib/samly/auth_router.ex b/lib/samly/auth_router.ex index 143aa09..ac64b9c 100644 --- a/lib/samly/auth_router.ex +++ b/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 diff --git a/lib/samly/helper.ex b/lib/samly/helper.ex index 6863070..aa1bf7e 100644 --- a/lib/samly/helper.ex +++ b/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 diff --git a/lib/samly/router.ex b/lib/samly/router.ex index 55fa62d..3986195 100644 --- a/lib/samly/router.ex +++ b/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)) diff --git a/lib/samly/router_util.ex b/lib/samly/router_util.ex index 3d1d908..2550b76 100644 --- a/lib/samly/router_util.ex +++ b/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 diff --git a/lib/samly/sp_handler.ex b/lib/samly/sp_handler.ex index f0287e7..fb77e2a 100644 --- a/lib/samly/sp_handler.ex +++ b/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 diff --git a/mix.exs b/mix.exs index cca3ab1..47576e1 100644 --- a/mix.exs +++ b/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]} ] diff --git a/mix.lock b/mix.lock index 132cad6..a7fcc4d 100644 --- a/mix.lock +++ b/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"}, }