Refactor redirect uri validation

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## v0.5.7 (TBA)
+
+* Permit native application redirect uri
+
 ## v0.5.6 (2020-01-07)
 
 * Permit associations to be overridden

lib/ex_oauth2_provider/redirect_uri.ex

@@ -5,7 +5,12 @@ defmodule ExOauth2Provider.RedirectURI do
   alias ExOauth2Provider.{Config, Utils}
 
   @doc """
-  Validates if a url can be used as a redirect_uri
+  Validates if a url can be used as a redirect_uri.
+
+  Validates according to [RFC 6749 3.1.2](https://tools.ietf.org/html/rfc6749#section-3.1.2)
+  and [RFC 8252 7.1](https://tools.ietf.org/html/rfc8252#section-7.1). The validation is
+  skipped if the redirect uri is the same as the `:native_redirect_uri` configuration
+  setting.
   """
   @spec validate(binary() | nil, keyword()) :: {:ok, binary()} | {:error, binary()}
   def validate(nil, config), do: validate("", config)
@@ -26,25 +31,17 @@ defmodule ExOauth2Provider.RedirectURI do
 
   defp do_validate(_url, %{fragment: fragment}, _config) when not is_nil(fragment),
     do: {:error, "Redirect URI cannot contain fragments"}
-  defp do_validate(_url, %{scheme: scheme, host: _host}, _config) when is_nil(scheme),
-    do: {:error, "Redirect URI must be an absolute URI"}
-  defp do_validate(_url, %{scheme: "https", host: host}, _config) when is_nil(host) or host == "",
-    do: {:error, "Redirect URI must be an absolute URI"}
-  defp do_validate(_url, %{scheme: "http", host: host}, _config) when is_nil(host) or host == "",
-    do: {:error, "Redirect URI must be an absolute URI"}
-  defp do_validate(url, %{scheme: "https", host: _host}, _config),
-    do: {:ok, url}
-
-  defp do_validate(url, %{scheme: "http", host: _host}, config) do
-    if Config.force_ssl_in_redirect_uri?(config) do
-      {:error, "Redirect URI must be an HTTPS/SSL URI"}
-    else
-      {:ok, url}
+  defp do_validate(url, %{scheme: scheme} = uri, config) when not is_nil(scheme) do
+    case invalid_ssl_uri?(uri, config) do
+      true  -> {:error, "Redirect URI must be an HTTPS/SSL URI"}
+      false -> {:ok, url}
     end
   end
+  defp do_validate(_url, _uri, _config),
+    do: {:error, "Redirect URI must be an absolute URI"}
 
-  defp do_validate(url, _uri, _config),
-    do: {:ok, url}
+  defp invalid_ssl_uri?(%{scheme: "http"}, config), do: Config.force_ssl_in_redirect_uri?(config)
+  defp invalid_ssl_uri?(_uri, _config), do: false
 
   @doc false
   @deprecated "Use `matches?/3` instead"

test/ex_oauth2_provider/redirect_uri_test.exs

@@ -2,45 +2,53 @@ defmodule ExOauth2Provider.RedirectURITest do
   use ExUnit.Case
   alias ExOauth2Provider.{Config, RedirectURI}
 
-  test "validate native url" do
+  test "validate/2 native url" do
     uri = Config.native_redirect_uri(otp_app: :ex_oauth2_provider)
     assert RedirectURI.validate(uri, []) == {:ok, uri}
   end
 
-  test "validate rejects blank" do
+  test "validate/2 rejects blank" do
     assert RedirectURI.validate("", []) == {:error, "Redirect URI cannot be blank"}
     assert RedirectURI.validate(nil, []) == {:error, "Redirect URI cannot be blank"}
     assert RedirectURI.validate("  ", []) == {:error, "Redirect URI cannot be blank"}
   end
 
-  test "validate rejects with fragment" do
+  test "validate/2 rejects uri with fragment" do
     assert RedirectURI.validate("https://app.co/test#fragment", []) == {:error, "Redirect URI cannot contain fragments"}
   end
 
-  test "validate rejects with missing scheme" do
+  test "validate/2 rejects uri with missing scheme" do
     assert RedirectURI.validate("app.co", []) == {:error, "Redirect URI must be an absolute URI"}
   end
 
-  test "validate rejects relative url" do
+  test "validate/2 rejects relative uri" do
     assert RedirectURI.validate("/abc/123", []) == {:error, "Redirect URI must be an absolute URI"}
   end
 
-  test "validate rejects scheme only" do
-    assert RedirectURI.validate("https://", []) == {:error, "Redirect URI must be an absolute URI"}
+  test "validate/2 requires https scheme with `:force_ssl_in_redirect_uri` setting" do
+    uri = "http://app.co/"
+    assert RedirectURI.validate(uri, []) == {:error, "Redirect URI must be an HTTPS/SSL URI"}
+    assert RedirectURI.validate(uri, [force_ssl_in_redirect_uri: false]) == {:ok, uri}
   end
 
-  test "validate https scheme" do
-    assert RedirectURI.validate("http://app.co/", []) == {:error, "Redirect URI must be an HTTPS/SSL URI"}
+  test "validate/2 accepts absolute uri" do
+    uri = "https://app.co"
+    assert RedirectURI.validate(uri, []) == {:ok, uri}
+    uri = "https://app.co/path"
+    assert RedirectURI.validate(uri, []) == {:ok, uri}
+    uri = "https://app.co/?query=1"
+    assert RedirectURI.validate(uri, []) == {:ok, uri}
   end
 
-  test "validate http scheme when not required" do
-    uri = "http://app.co/"
-    assert RedirectURI.validate(uri, [force_ssl_in_redirect_uri: false]) == {:ok, uri}
+  test "validate/2 with wild card subdomain" do
+    uri = "https://*.app.co/"
+    assert RedirectURI.validate(uri, []) == {:ok, uri}
   end
 
-  test "validate custom url scheme" do
+  test "validate/2 with private-use uri" do
     # RFC Spec - OAuth 2.0 for Native Apps
     # https://tools.ietf.org/html/rfc8252#section-7.1
+
     uri = "com.example.app:/oauth2redirect/example-provider"
     assert RedirectURI.validate(uri, []) == {:ok, uri}
     uri = "com.example.app://oauth2redirect/example-provider"
@@ -53,20 +61,6 @@ defmodule ExOauth2Provider.RedirectURITest do
     assert RedirectURI.validate(uri, []) == {:ok, uri}
   end
 
-  test "validate" do
-    uri = "https://app.co"
-    assert RedirectURI.validate(uri, []) == {:ok, uri}
-    uri = "https://app.co/path"
-    assert RedirectURI.validate(uri, []) == {:ok, uri}
-    uri = "https://app.co/?query=1"
-    assert RedirectURI.validate(uri, []) == {:ok, uri}
-  end
-
-  test "validates wild card subdomain" do
-    uri = "https://*.app.co/"
-    assert RedirectURI.validate(uri, []) == {:ok, uri}
-  end
-
   test "matches?#true" do
     uri = "https://app.co/aaa"
     assert RedirectURI.matches?(uri, uri, [])