Fix OAuth registration redirect_uris array support

changelog.d/oauth-registration-redirect_uris.fix

@@ -0,0 +1 @@
+Fix OAuth app registration to accept `redirect_uris` as an array of strings (RFC 7591), while keeping backwards compatibility with string input.

lib/pleroma/web/api_spec/operations/admin/o_auth_app_operation.ex

@@ -123,8 +123,10 @@ defmodule Pleroma.Web.ApiSpec.Admin.OAuthAppOperation do
         name: %Schema{type: :string, description: "Application Name"},
         scopes: %Schema{type: :array, items: %Schema{type: :string}, description: "oAuth scopes"},
         redirect_uris: %Schema{
-          type: :array,
+          oneOf: [
-          items: %Schema{type: :string},
+            %Schema{type: :string},
+            %Schema{type: :array, items: %Schema{type: :string}}
+          ],
           description:
             "Where the user should be redirected after authorization. To display the authorization code to the user instead of redirecting to a web page, use `urn:ietf:wg:oauth:2.0:oob` in this parameter."
         },
@@ -158,8 +160,10 @@ defmodule Pleroma.Web.ApiSpec.Admin.OAuthAppOperation do
         name: %Schema{type: :string, description: "Application Name"},
         scopes: %Schema{type: :array, items: %Schema{type: :string}, description: "oAuth scopes"},
         redirect_uris: %Schema{
-          type: :array,
+          oneOf: [
-          items: %Schema{type: :string},
+            %Schema{type: :string},
+            %Schema{type: :array, items: %Schema{type: :string}}
+          ],
           description:
             "Where the user should be redirected after authorization. To display the authorization code to the user instead of redirecting to a web page, use `urn:ietf:wg:oauth:2.0:oob` in this parameter."
         },

lib/pleroma/web/api_spec/operations/app_operation.ex

@@ -97,7 +97,10 @@ defmodule Pleroma.Web.ApiSpec.AppOperation do
       properties: %{
         client_name: %Schema{type: :string, description: "A name for your application."},
         redirect_uris: %Schema{
-          type: :string,
+          oneOf: [
+            %Schema{type: :string},
+            %Schema{type: :array, items: %Schema{type: :string}}
+          ],
           description:
             "Where the user should be redirected after authorization. To display the authorization code to the user instead of redirecting to a web page, use `urn:ietf:wg:oauth:2.0:oob` in this parameter."
         },

lib/pleroma/web/o_auth/app.ex

@@ -14,7 +14,7 @@ defmodule Pleroma.Web.OAuth.App do
 
   schema "apps" do
     field(:client_name, :string)
-    field(:redirect_uris, {:array, :string})
+    field(:redirect_uris, :string)
     field(:scopes, {:array, :string}, default: [])
     field(:website, :string)
     field(:client_id, :string)
@@ -31,9 +31,32 @@ defmodule Pleroma.Web.OAuth.App do
 
   @spec changeset(t(), map()) :: Ecto.Changeset.t()
   def changeset(struct, params) do
+    params = normalize_redirect_uris_param(params)
+
     cast(struct, params, [:client_name, :redirect_uris, :scopes, :website, :trusted, :user_id])
   end
 
+  defp normalize_redirect_uris_param(%{} = params) do
+    case params do
+      %{redirect_uris: redirect_uris} when is_list(redirect_uris) ->
+        Map.put(params, :redirect_uris, normalize_redirect_uris(redirect_uris))
+
+      %{"redirect_uris" => redirect_uris} when is_list(redirect_uris) ->
+        Map.put(params, "redirect_uris", normalize_redirect_uris(redirect_uris))
+
+      _ ->
+        params
+    end
+  end
+
+  defp normalize_redirect_uris(redirect_uris) when is_list(redirect_uris) do
+    redirect_uris
+    |> Enum.filter(&is_binary/1)
+    |> Enum.map(&String.trim/1)
+    |> Enum.reject(&(&1 == ""))
+    |> Enum.join("\n")
+  end
+
   @spec register_changeset(t(), map()) :: Ecto.Changeset.t()
   def register_changeset(struct, params \\ %{}) do
     changeset =

test/pleroma/web/admin_api/controllers/o_auth_app_controller_test.exs

@@ -57,6 +57,28 @@ defmodule Pleroma.Web.AdminAPI.OAuthAppControllerTest do
              } = response
     end
 
+    test "success with redirect_uris array", %{conn: conn} do
+      base_url = Endpoint.url()
+      app_name = "Trusted app"
+
+      response =
+        conn
+        |> put_req_header("content-type", "application/json")
+        |> post("/api/pleroma/admin/oauth_app", %{
+          name: app_name,
+          redirect_uris: [base_url]
+        })
+        |> json_response_and_validate_schema(200)
+
+      assert %{
+               "client_id" => _,
+               "client_secret" => _,
+               "name" => ^app_name,
+               "redirect_uri" => ^base_url,
+               "trusted" => false
+             } = response
+    end
+
     test "with trusted", %{conn: conn} do
       base_url = Endpoint.url()
       app_name = "Trusted app"

test/pleroma/web/mastodon_api/controllers/app_controller_test.exs

@@ -61,6 +61,33 @@ defmodule Pleroma.Web.MastodonAPI.AppControllerTest do
     assert app.user_id == nil
   end
 
+  test "creates an oauth app with redirect_uris array", %{conn: conn} do
+    app_attrs = build(:oauth_app)
+
+    conn =
+      conn
+      |> put_req_header("content-type", "application/json")
+      |> post("/api/v1/apps", %{
+        client_name: app_attrs.client_name,
+        redirect_uris: [app_attrs.redirect_uris]
+      })
+
+    [app] = Repo.all(App)
+
+    expected = %{
+      "name" => app.client_name,
+      "website" => app.website,
+      "client_id" => app.client_id,
+      "client_secret" => app.client_secret,
+      "id" => app.id |> to_string(),
+      "redirect_uri" => app.redirect_uris,
+      "vapid_key" => Push.vapid_config() |> Keyword.get(:public_key)
+    }
+
+    assert expected == json_response_and_validate_schema(conn, 200)
+    assert app.user_id == nil
+  end
+
   test "creates an oauth app with a user", %{conn: conn} do
     user = insert(:user)
     app_attrs = build(:oauth_app)

test/pleroma/web/o_auth/app_test.exs

@@ -10,20 +10,20 @@ defmodule Pleroma.Web.OAuth.AppTest do
 
   describe "get_or_make/2" do
     test "gets exist app" do
-      attrs = %{client_name: "Mastodon-Local", redirect_uris: ["."]}
+      attrs = %{client_name: "Mastodon-Local", redirect_uris: "."}
       app = insert(:oauth_app, Map.merge(attrs, %{scopes: ["read", "write"]}))
       {:ok, %App{} = exist_app} = App.get_or_make(attrs, [])
       assert exist_app == app
     end
 
     test "make app" do
-      attrs = %{client_name: "Mastodon-Local", redirect_uris: ["."]}
+      attrs = %{client_name: "Mastodon-Local", redirect_uris: "."}
       {:ok, %App{} = app} = App.get_or_make(attrs, ["write"])
       assert app.scopes == ["write"]
     end
 
     test "gets exist app and updates scopes" do
-      attrs = %{client_name: "Mastodon-Local", redirect_uris: ["."]}
+      attrs = %{client_name: "Mastodon-Local", redirect_uris: "."}
       app = insert(:oauth_app, Map.merge(attrs, %{scopes: ["read", "write"]}))
       {:ok, %App{} = exist_app} = App.get_or_make(attrs, ["read", "write", "follow", "push"])
       assert exist_app.id == app.id
@@ -31,10 +31,10 @@ defmodule Pleroma.Web.OAuth.AppTest do
     end
 
     test "has unique client_id" do
-      insert(:oauth_app, client_name: "", redirect_uris: [""], client_id: "boop")
+      insert(:oauth_app, client_name: "", redirect_uris: "", client_id: "boop")
 
       error =
-        catch_error(insert(:oauth_app, client_name: "", redirect_uris: [""], client_id: "boop"))
+        catch_error(insert(:oauth_app, client_name: "", redirect_uris: "", client_id: "boop"))
 
       assert %Ecto.ConstraintError{} = error
       assert error.constraint == "apps_client_id_index"
@@ -55,7 +55,7 @@ defmodule Pleroma.Web.OAuth.AppTest do
   end
 
   test "removes orphaned apps" do
-    attrs = %{client_name: "Mastodon-Local", redirect_uris: ["."]}
+    attrs = %{client_name: "Mastodon-Local", redirect_uris: "."}
     {:ok, %App{} = old_app} = App.get_or_make(attrs, ["write"])
 
     # backdate the old app so it's within the threshold for being cleaned up
@@ -66,7 +66,7 @@ defmodule Pleroma.Web.OAuth.AppTest do
       |> Pleroma.Repo.query([one_hour_ago, old_app.id])
 
     # Create the new app after backdating the old one
-    attrs = %{client_name: "PleromaFE", redirect_uris: ["."]}
+    attrs = %{client_name: "PleromaFE", redirect_uris: "."}
     {:ok, %App{} = app} = App.get_or_make(attrs, ["write"])
 
     # Ensure the new app has a recent timestamp

test/pleroma/web/o_auth/o_auth_controller_test.exs

@@ -406,7 +406,7 @@ defmodule Pleroma.Web.OAuth.OAuthControllerTest do
   describe "GET /oauth/authorize" do
     setup do
       [
-        app: insert(:oauth_app, redirect_uris: ["https://redirect.url"]),
+        app: insert(:oauth_app, redirect_uris: "https://redirect.url"),
         conn:
           build_conn()
           |> Plug.Session.call(Plug.Session.init(@session_opts))