From d0db3225505d34b85418b4d338e1182fcc08cc12 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gr=C3=A9goire=20Charvet=20=E9=BB=91=E7=93=9C?=
 <greg@geekingfrog.com>
Date: Mon, 5 Aug 2024 18:52:54 +0100
Subject: [PATCH] Forbid basic auth and query params at the same time

As per https://www.rfc-editor.org/rfc/rfc6749.html#section-2.3

> The client MUST NOT use more than one authentication method
> in each request.
---
 .../controllers/o_auth/code_controller.ex     | 47 ++++++++++++++-----
 .../o_auth/code_controller_test.exs           | 46 ++++++++++++++++++
 2 files changed, 80 insertions(+), 13 deletions(-)

diff --git a/lib/teiserver_web/controllers/o_auth/code_controller.ex b/lib/teiserver_web/controllers/o_auth/code_controller.ex
index 0b9eb76fb..a92eb147e 100644
--- a/lib/teiserver_web/controllers/o_auth/code_controller.ex
+++ b/lib/teiserver_web/controllers/o_auth/code_controller.ex
@@ -82,15 +82,21 @@ defmodule TeiserverWeb.OAuth.CodeController do
   end
 
   defp get_client_id(conn, params) do
-    case Map.get(params, "client_id") do
-      nil ->
-        case Plug.BasicAuth.parse_basic_auth(conn) do
-          {user, _pass} -> {:ok, user}
-          :error -> {:error, "missing client_id"}
-        end
+    query = Map.get(params, "client_id")
+    basic = Plug.BasicAuth.parse_basic_auth(conn)
+
+    case {basic, query} do
+      {:error, nil} ->
+        {:error, "missing client_id"}
 
-      cid ->
-        {:ok, cid}
+      {{user, _}, client_id} when client_id != nil and user != nil and user != "" ->
+        {:error, "cannot provide client_id through both basic auth header and query parameters"}
+
+      {{user, _}, nil} ->
+        {:ok, user}
+
+      {_, client_id} ->
+        {:ok, client_id}
     end
   end
 
@@ -99,11 +105,26 @@ defmodule TeiserverWeb.OAuth.CodeController do
     post_params = {Map.get(params, "client_id"), Map.get(params, "client_secret")}
 
     case {basic, post_params} do
-      {:error, {nil, nil}} -> {:error, "Invalid basic auth header"}
-      {{user, pass}, _} -> {:ok, user, pass}
-      {_, {nil, _}} -> {:error, "missing client_id"}
-      {_, {_, nil}} -> {:error, "missing client_secret"}
-      {_, {client_id, client_secret}} -> {:ok, client_id, client_secret}
+      {:error, {nil, nil}} ->
+        {:error, "Invalid basic auth header"}
+
+      {{user, _}, {client_id, _}} when user != nil and client_id != nil ->
+        {:error, "Must not provide client_id both in basic auth header and query parameter"}
+
+      {{_, pass}, {_, secret}} when pass != nil and secret != nil ->
+        {:error, "Must not provide client_secret both in basic auth header and query parameter"}
+
+      {{user, pass}, _} ->
+        {:ok, user, pass}
+
+      {_, {nil, _}} ->
+        {:error, "missing client_id"}
+
+      {_, {_, nil}} ->
+        {:error, "missing client_secret"}
+
+      {_, {client_id, client_secret}} ->
+        {:ok, client_id, client_secret}
     end
   end
 
diff --git a/test/teiserver_web/controllers/o_auth/code_controller_test.exs b/test/teiserver_web/controllers/o_auth/code_controller_test.exs
index db6fef8cc..42e82393a 100644
--- a/test/teiserver_web/controllers/o_auth/code_controller_test.exs
+++ b/test/teiserver_web/controllers/o_auth/code_controller_test.exs
@@ -98,6 +98,20 @@ defmodule TeiserverWeb.OAuth.CodeControllerTest do
       assert is_binary(json_resp["access_token"]), "has access_token"
     end
 
+    test "must not provide client_id twice", %{conn: conn} = setup_data do
+      data = get_valid_data(setup_data)
+
+      client_id = data.client_id
+      auth_header = Plug.BasicAuth.encode_basic_auth(client_id, "")
+
+      resp =
+        conn
+        |> put_req_header("authorization", auth_header)
+        |> post(~p"/oauth/token", data)
+
+      json_resp = json_response(resp, 400)
+    end
+
     test "must provide grant_type", %{conn: conn} = setup_data do
       data = get_valid_data(setup_data) |> Map.drop([:grant_type])
       resp = post(conn, ~p"/oauth/token", data)
@@ -206,6 +220,38 @@ defmodule TeiserverWeb.OAuth.CodeControllerTest do
       resp = post(conn, ~p"/oauth/token", data)
       json_response(resp, 400)
     end
+
+    test "cannot provide client_id twice", %{
+      conn: conn,
+      credential: credential,
+      credential_secret: secret
+    } do
+      data = %{grant_type: "client_credentials", client_id: credential.client_id}
+      auth_header = Plug.BasicAuth.encode_basic_auth(credential.client_id, secret)
+
+      conn =
+        conn
+        |> put_req_header("authorization", auth_header)
+
+      resp = post(conn, ~p"/oauth/token", data)
+      json_response(resp, 400)
+    end
+
+    test "cannot provide client_secret twice", %{
+      conn: conn,
+      credential: credential,
+      credential_secret: secret
+    } do
+      data = %{grant_type: "client_credentials", client_secret: secret}
+      auth_header = Plug.BasicAuth.encode_basic_auth(credential.client_id, secret)
+
+      conn =
+        conn
+        |> put_req_header("authorization", auth_header)
+
+      resp = post(conn, ~p"/oauth/token", data)
+      json_response(resp, 400)
+    end
   end
 
   describe "refresh token" do