[#923] OAuth consumer controller tests. Misc. improvements.

diff --git a/lib/pleroma/web/oauth/oauth_controller.ex b/lib/pleroma/web/oauth/oauth_controller.ex
index 1b467e983431dba229dd85d0cfa7cda07ac72d12..2dcaaabc157cb76db6d234571cebed6e3505aa21 100644 (file)
--- a/lib/pleroma/web/oauth/oauth_controller.ex
+++ b/lib/pleroma/web/oauth/oauth_controller.ex
@@ -253,6 +253,7 @@ defmodule Pleroma.Web.OAuth.OAuthController do
       auth_params = %{
         "client_id" => params["client_id"],
         "redirect_uri" => params["redirect_uri"],
+        "state" => params["state"],
         "scopes" => oauth_scopes(params, nil)
       }
 
@@ -289,6 +290,7 @@ defmodule Pleroma.Web.OAuth.OAuthController do
     render(conn, "register.html", %{
       client_id: params["client_id"],
       redirect_uri: params["redirect_uri"],
+      state: params["state"],
       scopes: oauth_scopes(params, []),
       nickname: params["nickname"],
       email: params["email"]
@@ -313,6 +315,8 @@ defmodule Pleroma.Web.OAuth.OAuthController do
       )
     else
       _ ->
+        params = Map.delete(params, "password")
+
         conn
         |> put_flash(:error, "Unknown error, please try again.")
         |> redirect(to: o_auth_path(conn, :registration_details, params))

diff --git a/lib/pleroma/web/templates/o_auth/o_auth/register.html.eex b/lib/pleroma/web/templates/o_auth/o_auth/register.html.eex
index f4547170cdb93258b197b23f6786286cef3e8749..2e806e5fbf9cb151989e239e8fa830615fe1e1f9 100644 (file)
--- a/lib/pleroma/web/templates/o_auth/o_auth/register.html.eex
+++ b/lib/pleroma/web/templates/o_auth/o_auth/register.html.eex
@@ -44,5 +44,6 @@ please provide the details below.</p>
 <%= hidden_input f, :client_id, value: @client_id %>
 <%= hidden_input f, :redirect_uri, value: @redirect_uri %>
 <%= hidden_input f, :scope, value: Enum.join(@scopes, " ") %>
+<%= hidden_input f, :state, value: @state %>
 
 <% end %>

diff --git a/lib/pleroma/web/templates/o_auth/o_auth/show.html.eex b/lib/pleroma/web/templates/o_auth/o_auth/show.html.eex
index e6cf1db457a75392075b21977ff452bdc44e1353..0144675ab98ba1883de9cf79786a20e3bf1fc1de 100644 (file)
--- a/lib/pleroma/web/templates/o_auth/o_auth/show.html.eex
+++ b/lib/pleroma/web/templates/o_auth/o_auth/show.html.eex
@@ -22,7 +22,7 @@
 <%= hidden_input f, :client_id, value: @client_id %>
 <%= hidden_input f, :response_type, value: @response_type %>
 <%= hidden_input f, :redirect_uri, value: @redirect_uri %>
-<%= hidden_input f, :state, value: @state%>
+<%= hidden_input f, :state, value: @state %>
 <%= submit "Authorize" %>
 <% end %>
 

diff --git a/test/support/factory.ex b/test/support/factory.ex
index e1a08315a2f059f5464e6aa7b6e2c60edcc6f451..67953931b0a88344a961ade8c85aa0bd1783188d 100644 (file)
--- a/test/support/factory.ex
+++ b/test/support/factory.ex
@@ -257,4 +257,20 @@ defmodule Pleroma.Factory do
       user: build(:user)
     }
   end
+
+  def registration_factory do
+    user = insert(:user)
+
+    %Pleroma.Registration{
+      user: user,
+      provider: "twitter",
+      uid: "171799000",
+      info: %{
+        "name" => "John Doe",
+        "email" => "john@doe.com",
+        "nickname" => "johndoe",
+        "description" => "My bio"
+      }
+    }
+  end
 end

diff --git a/test/web/oauth/oauth_controller_test.exs b/test/web/oauth/oauth_controller_test.exs
index a9a0b9ed4c6b8eb8fd19b8da6eff0210824afef7..e13f4700d6d58af6d65326324856bed25c416406 100644 (file)
--- a/test/web/oauth/oauth_controller_test.exs
+++ b/test/web/oauth/oauth_controller_test.exs
@@ -5,24 +5,339 @@
 defmodule Pleroma.Web.OAuth.OAuthControllerTest do
   use Pleroma.Web.ConnCase
   import Pleroma.Factory
+  import Mock
 
+  alias Pleroma.Registration
   alias Pleroma.Repo
   alias Pleroma.Web.OAuth.Authorization
   alias Pleroma.Web.OAuth.Token
 
-  describe "GET /oauth/authorize" do
+  @session_opts [
+    store: :cookie,
+    key: "_test",
+    signing_salt: "cooldude"
+  ]
+
+  describe "in OAuth consumer mode, " do
     setup do
-      session_opts = [
-        store: :cookie,
-        key: "_test",
-        signing_salt: "cooldude"
+      oauth_consumer_enabled_path = [:auth, :oauth_consumer_enabled]
+      oauth_consumer_strategies_path = [:auth, :oauth_consumer_strategies]
+      oauth_consumer_enabled = Pleroma.Config.get(oauth_consumer_enabled_path)
+      oauth_consumer_strategies = Pleroma.Config.get(oauth_consumer_strategies_path)
+
+      Pleroma.Config.put(oauth_consumer_enabled_path, true)
+      Pleroma.Config.put(oauth_consumer_strategies_path, ~w(twitter facebook))
+
+      on_exit(fn ->
+        Pleroma.Config.put(oauth_consumer_enabled_path, oauth_consumer_enabled)
+        Pleroma.Config.put(oauth_consumer_strategies_path, oauth_consumer_strategies)
+      end)
+
+      [
+        app: insert(:oauth_app),
+        conn:
+          build_conn()
+          |> Plug.Session.call(Plug.Session.init(@session_opts))
+          |> fetch_session()
       ]
+    end
+
+    test "GET /oauth/authorize also renders OAuth consumer form", %{
+      app: app,
+      conn: conn
+    } do
+      conn =
+        get(
+          conn,
+          "/oauth/authorize",
+          %{
+            "response_type" => "code",
+            "client_id" => app.client_id,
+            "redirect_uri" => app.redirect_uris,
+            "scope" => "read"
+          }
+        )
+
+      assert response = html_response(conn, 200)
+      assert response =~ "Sign in with Twitter"
+      assert response =~ o_auth_path(conn, :prepare_request)
+    end
+
+    test "GET /oauth/prepare_request encodes parameters as `state` and redirects", %{
+      app: app,
+      conn: conn
+    } do
+      conn =
+        get(
+          conn,
+          "/oauth/prepare_request",
+          %{
+            "provider" => "twitter",
+            "scope" => app.scopes,
+            "client_id" => app.client_id,
+            "redirect_uri" => app.redirect_uris,
+            "state" => "a_state"
+          }
+        )
+
+      assert response = html_response(conn, 302)
+      redirected_to = redirected_to(conn)
+      [state] = Regex.run(~r/(?<=state=).*?(?=\Z|&)/, redirected_to)
+      state = URI.decode(state)
+      assert {:ok, state_params} = Poison.decode(state)
+
+      expected_scope_param = Enum.join(app.scopes, "+")
+      expected_client_id_param = app.client_id
+      expected_redirect_uri_param = app.redirect_uris
+
+      assert %{
+               "scope" => ^expected_scope_param,
+               "client_id" => ^expected_client_id_param,
+               "redirect_uri" => ^expected_redirect_uri_param,
+               "state" => "a_state"
+             } = state_params
+    end
+
+    test "on authentication error, redirects to `redirect_uri`", %{app: app, conn: conn} do
+      state_params = %{
+        "scope" => Enum.join(app.scopes, " "),
+        "client_id" => app.client_id,
+        "redirect_uri" => app.redirect_uris,
+        "state" => ""
+      }
+
+      conn =
+        conn
+        |> assign(:ueberauth_failure, %{errors: [%{message: "unknown error"}]})
+        |> get(
+          "/oauth/twitter/callback",
+          %{
+            "oauth_token" => "G-5a3AAAAAAAwMH9AAABaektfSM",
+            "oauth_verifier" => "QZl8vUqNvXMTKpdmUnGejJxuHG75WWWs",
+            "provider" => "twitter",
+            "state" => Poison.encode!(state_params)
+          }
+        )
+
+      assert response = html_response(conn, 302)
+      assert redirected_to(conn) == app.redirect_uris
+    end
+
+    test "with user-bound registration, GET /oauth/<provider>/callback redirects to `redirect_uri` with `code`",
+         %{app: app, conn: conn} do
+      registration = insert(:registration)
+
+      state_params = %{
+        "scope" => Enum.join(app.scopes, " "),
+        "client_id" => app.client_id,
+        "redirect_uri" => app.redirect_uris,
+        "state" => ""
+      }
+
+      with_mock Pleroma.Web.Auth.Authenticator,
+        get_registration: fn _, _ -> {:ok, registration} end do
+        conn =
+          get(
+            conn,
+            "/oauth/twitter/callback",
+            %{
+              "oauth_token" => "G-5a3AAAAAAAwMH9AAABaektfSM",
+              "oauth_verifier" => "QZl8vUqNvXMTKpdmUnGejJxuHG75WWWs",
+              "provider" => "twitter",
+              "state" => Poison.encode!(state_params)
+            }
+          )
+
+        assert response = html_response(conn, 302)
+        assert redirected_to(conn) =~ ~r/#{app.redirect_uris}\?code=.+/
+      end
+    end
+
+    test "with user-unbound registration, GET /oauth/<provider>/callback redirects to registration_details page",
+         %{app: app, conn: conn} do
+      registration = insert(:registration, user: nil)
+
+      state_params = %{
+        "scope" => "read",
+        "client_id" => app.client_id,
+        "redirect_uri" => app.redirect_uris,
+        "state" => "a_state"
+      }
+
+      with_mock Pleroma.Web.Auth.Authenticator,
+        get_registration: fn _, _ -> {:ok, registration} end do
+        conn =
+          get(
+            conn,
+            "/oauth/twitter/callback",
+            %{
+              "oauth_token" => "G-5a3AAAAAAAwMH9AAABaektfSM",
+              "oauth_verifier" => "QZl8vUqNvXMTKpdmUnGejJxuHG75WWWs",
+              "provider" => "twitter",
+              "state" => Poison.encode!(state_params)
+            }
+          )
+
+        expected_redirect_params =
+          state_params
+          |> Map.delete("scope")
+          |> Map.merge(%{
+            "scopes" => ["read"],
+            "email" => Registration.email(registration),
+            "nickname" => Registration.nickname(registration)
+          })
+
+        assert response = html_response(conn, 302)
+
+        assert redirected_to(conn) ==
+                 o_auth_path(conn, :registration_details, expected_redirect_params)
+      end
+    end
+
+    test "GET /oauth/registration_details renders registration details form", %{
+      app: app,
+      conn: conn
+    } do
+      conn =
+        get(
+          conn,
+          "/oauth/registration_details",
+          %{
+            "scopes" => app.scopes,
+            "client_id" => app.client_id,
+            "redirect_uri" => app.redirect_uris,
+            "state" => "a_state",
+            "nickname" => nil,
+            "email" => "john@doe.com"
+          }
+        )
+
+      assert response = html_response(conn, 200)
+      assert response =~ ~r/name="op" type="submit" value="register"/
+      assert response =~ ~r/name="op" type="submit" value="connect"/
+    end
+
+    test "with valid params, POST /oauth/register?op=register redirects to `redirect_uri` with `code`",
+         %{
+           app: app,
+           conn: conn
+         } do
+      registration = insert(:registration, user: nil, info: %{"nickname" => nil, "email" => nil})
+
+      conn =
+        conn
+        |> put_session(:registration_id, registration.id)
+        |> post(
+          "/oauth/register",
+          %{
+            "op" => "register",
+            "scopes" => app.scopes,
+            "client_id" => app.client_id,
+            "redirect_uri" => app.redirect_uris,
+            "state" => "a_state",
+            "nickname" => "availablenick",
+            "email" => "available@email.com"
+          }
+        )
+
+      assert response = html_response(conn, 302)
+      assert redirected_to(conn) =~ ~r/#{app.redirect_uris}\?code=.+/
+    end
+
+    test "with invalid params, POST /oauth/register?op=register redirects to registration_details page",
+         %{
+           app: app,
+           conn: conn
+         } do
+      another_user = insert(:user)
+      registration = insert(:registration, user: nil, info: %{"nickname" => nil, "email" => nil})
+
+      params = %{
+        "op" => "register",
+        "scopes" => app.scopes,
+        "client_id" => app.client_id,
+        "redirect_uri" => app.redirect_uris,
+        "state" => "a_state",
+        "nickname" => another_user.nickname,
+        "email" => another_user.email
+      }
+
+      conn =
+        conn
+        |> put_session(:registration_id, registration.id)
+        |> post("/oauth/register", params)
+
+      assert response = html_response(conn, 302)
+
+      assert redirected_to(conn) ==
+               o_auth_path(conn, :registration_details, params)
+    end
+
+    test "with valid params, POST /oauth/register?op=connect redirects to `redirect_uri` with `code`",
+         %{
+           app: app,
+           conn: conn
+         } do
+      user = insert(:user, password_hash: Comeonin.Pbkdf2.hashpwsalt("testpassword"))
+      registration = insert(:registration, user: nil)
+
+      conn =
+        conn
+        |> put_session(:registration_id, registration.id)
+        |> post(
+          "/oauth/register",
+          %{
+            "op" => "connect",
+            "scopes" => app.scopes,
+            "client_id" => app.client_id,
+            "redirect_uri" => app.redirect_uris,
+            "state" => "a_state",
+            "auth_name" => user.nickname,
+            "password" => "testpassword"
+          }
+        )
 
+      assert response = html_response(conn, 302)
+      assert redirected_to(conn) =~ ~r/#{app.redirect_uris}\?code=.+/
+    end
+
+    test "with invalid params, POST /oauth/register?op=connect redirects to registration_details page",
+         %{
+           app: app,
+           conn: conn
+         } do
+      user = insert(:user)
+      registration = insert(:registration, user: nil)
+
+      params = %{
+        "op" => "connect",
+        "scopes" => app.scopes,
+        "client_id" => app.client_id,
+        "redirect_uri" => app.redirect_uris,
+        "state" => "a_state",
+        "auth_name" => user.nickname,
+        "password" => "wrong password"
+      }
+
+      conn =
+        conn
+        |> put_session(:registration_id, registration.id)
+        |> post("/oauth/register", params)
+
+      assert response = html_response(conn, 302)
+
+      assert redirected_to(conn) ==
+               o_auth_path(conn, :registration_details, Map.delete(params, "password"))
+    end
+  end
+
+  describe "GET /oauth/authorize" do
+    setup do
       [
         app: insert(:oauth_app, redirect_uris: "https://redirect.url"),
         conn:
           build_conn()
-          |> Plug.Session.call(Plug.Session.init(session_opts))
+          |> Plug.Session.call(Plug.Session.init(@session_opts))
           |> fetch_session()
       ]
     end