Refactor the api access token query. Fixes #2902"

core/user/externalAPIUser.go

@@ -117,20 +117,73 @@ func GetExternalAPIUserForAccessTokenAndScope(token string, scope string) (*Exte
 	// so we can efficiently find if a token supports a single scope.
 	// This is SQLite specific, so if we ever support other database
 	// backends we need to support other methods.
-	query := `SELECT id, scopes, display_name, display_color, created_at, last_used FROM user_access_tokens, (
+	query := `SELECT
-		WITH RECURSIVE split(id, scopes, display_name, display_color, created_at, last_used, disabled_at, scope, rest) AS (
+  id,
-		  SELECT id, scopes, display_name, display_color, created_at, last_used, disabled_at, '', scopes || ',' FROM users
+	scopes,
+  display_name,
+  display_color,
+  created_at,
+  last_used
+FROM
+  user_access_tokens
+  INNER JOIN (
+    WITH RECURSIVE split(
+      id,
+      scopes,
+      display_name,
+      display_color,
+      created_at,
+      last_used,
+      disabled_at,
+      scope,
+      rest
+    ) AS (
+      SELECT
+        id,
+        scopes,
+        display_name,
+        display_color,
+        created_at,
+        last_used,
+        disabled_at,
+        '',
+        scopes || ','
+      FROM
+        users AS u
       UNION ALL
-		  SELECT id, scopes, display_name, display_color, created_at, last_used, disabled_at,
+      SELECT
+        id,
+        scopes,
+        display_name,
+        display_color,
+        created_at,
+        last_used,
+        disabled_at,
         substr(rest, 0, instr(rest, ',')),
-				 substr(rest, instr(rest, ',')+1)
+        substr(rest, instr(rest, ',') + 1)
-			FROM split
+      FROM
-		   WHERE rest <> '')
+        split
-		SELECT id, scopes, display_name, display_color, created_at, last_used, disabled_at, scope
+      WHERE
-		  FROM split
+        rest <> ''
-		 WHERE scope <> ''
+    )
-		 ORDER BY scope
+    SELECT
-	  ) AS token WHERE user_access_tokens.token = ? AND token.scope = ?`
+      id,
+      display_name,
+      display_color,
+      created_at,
+      last_used,
+      disabled_at,
+      scopes,
+      scope
+    FROM
+      split
+    WHERE
+      scope <> ''
+  ) ON user_access_tokens.user_id = id
+WHERE
+  disabled_at IS NULL
+  AND token = ?
+  AND scope = ?;`
 
 	row := _datastore.DB.QueryRow(query, token, scope)
 	integration, err := makeExternalAPIUserFromRow(row)
@@ -150,7 +203,6 @@ func GetIntegrationNameForAccessToken(token string) *string {
 
 // GetExternalAPIUser will return all API users with access tokens.
 func GetExternalAPIUser() ([]ExternalAPIUser, error) { //nolint
-	// Get all messages sent within the past day
 	query := "SELECT id, token, display_name, display_color, scopes, created_at, last_used FROM users, user_access_tokens WHERE user_access_tokens.user_id = id  AND type IS 'API' AND disabled_at IS NULL"
 
 	rows, err := _datastore.DB.Query(query)
@@ -170,7 +222,6 @@ func SetExternalAPIUserAccessTokenAsUsed(token string) error {
 	if err != nil {
 		return err
 	}
-	// stmt, err := tx.Prepare("UPDATE users SET last_used = CURRENT_TIMESTAMP WHERE access_token = ?")
 	stmt, err := tx.Prepare("UPDATE users SET last_used = CURRENT_TIMESTAMP WHERE id = (SELECT user_id FROM user_access_tokens WHERE token = ?)")
 	if err != nil {
 		return err

core/user/externalAPIUser_test.go

@@ -0,0 +1,93 @@
+package user
+
+import (
+	"testing"
+
+	"github.com/owncast/owncast/core/data"
+)
+
+const (
+	tokenName = "test token name"
+	token     = "test-token-123"
+)
+
+var testScopes = []string{"test-scope"}
+
+func TestMain(m *testing.M) {
+	if err := data.SetupPersistence(":memory:"); err != nil {
+		panic(err)
+	}
+
+	SetupUsers()
+
+	m.Run()
+}
+
+func TestCreateExternalAPIUser(t *testing.T) {
+	if err := InsertExternalAPIUser(token, tokenName, 0, testScopes); err != nil {
+		t.Fatal(err)
+	}
+
+	user := GetUserByToken(token)
+	if user == nil {
+		t.Fatal("api user not found after creating")
+	}
+
+	if user.DisplayName != tokenName {
+		t.Errorf("expected display name %q, got %q", tokenName, user.DisplayName)
+	}
+
+	if user.Scopes[0] != testScopes[0] {
+		t.Errorf("expected scopes %q, got %q", testScopes, user.Scopes)
+	}
+}
+
+func TestDeleteExternalAPIUser(t *testing.T) {
+	if err := DeleteExternalAPIUser(token); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestVerifyTokenDisabled(t *testing.T) {
+	users, err := GetExternalAPIUser()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if len(users) > 0 {
+		t.Fatal("disabled user returned in list of all API users")
+	}
+}
+
+func TestVerifyGetUserTokenDisabled(t *testing.T) {
+	user := GetUserByToken(token)
+	if user == nil {
+		t.Fatal("user not returned in GetUserByToken after disabling")
+	}
+
+	if user.DisabledAt == nil {
+		t.Fatal("user returned in GetUserByToken after disabling")
+	}
+}
+
+func TestVerifyGetExternalAPIUserForAccessTokenAndScopeTokenDisabled(t *testing.T) {
+	user, _ := GetExternalAPIUserForAccessTokenAndScope(token, testScopes[0])
+
+	if user != nil {
+		t.Fatal("user returned in GetExternalAPIUserForAccessTokenAndScope after disabling")
+	}
+}
+
+func TestCreateAdditionalAPIUser(t *testing.T) {
+	if err := InsertExternalAPIUser("ignore-me", "token-to-be-ignored", 0, testScopes); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestAgainVerifyGetExternalAPIUserForAccessTokenAndScopeTokenDisabled(t *testing.T) {
+	user, _ := GetExternalAPIUserForAccessTokenAndScope(token, testScopes[0])
+
+	if user != nil {
+		t.Fatal("user returned in TestAgainVerifyGetExternalAPIUserForAccessTokenAndScopeTokenDisabled after disabling")
+	}
+}