Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions lib/mydia/accounts.ex
Original file line number Diff line number Diff line change
Expand Up @@ -395,12 +395,18 @@ defmodule Mydia.Accounts do
@doc """
Verifies an API key and returns the associated user and API key.
Returns {:error, reason} if the key is invalid, expired, or revoked.

The candidate set is narrowed by the indexed `key_prefix` column, which is
derived from the presented key. A key whose prefix matches no row costs zero
Argon2 passes, which keeps this endpoint from amplifying CPU under a brute
force attempt. Keys predating the key_prefix column have a NULL prefix, match
nothing, and are revoked by the RevokePrefixlessApiKeys migration.
"""
def verify_api_key(key) when is_binary(key) do
# Find all API keys and verify against them
# This is not ideal for performance but works for small numbers of keys
# For production, consider using a more efficient lookup mechanism
prefix = extract_key_prefix(key)

ApiKey
|> where([k], k.key_prefix == ^prefix)
|> preload(:user)
|> Repo.all()
|> Enum.find(fn api_key ->
Expand Down
2 changes: 1 addition & 1 deletion lib/mydia_web/schema/common_types.ex
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ defmodule MydiaWeb.Schema.CommonTypes do
object :api_key do
field :id, non_null(:id), description: "API key ID"
field :name, non_null(:string), description: "User-given name"
field :key_prefix, non_null(:string), description: "Key prefix for identification"
field :key_prefix, :string, description: "Key prefix for identification"

field :permissions, non_null(list_of(non_null(:string))),
description: "List of granted permissions"
Expand Down
24 changes: 12 additions & 12 deletions lib/mydia_web/schema/resolvers/remote_access_resolver.ex
Original file line number Diff line number Diff line change
Expand Up @@ -67,18 +67,15 @@ defmodule MydiaWeb.Schema.Resolvers.RemoteAccessResolver do
end

def refresh_media_token(_parent, %{token: token}, _context) do
case MediaToken.refresh_token(token) do
{:ok, new_token, claims} ->
expires_at = DateTime.from_unix!(claims["exp"])
permissions = Map.get(claims, "permissions", [])

{:ok,
%{
token: new_token,
expires_at: expires_at,
permissions: permissions
}}

with {:ok, new_token, claims} <- MediaToken.refresh_token(token),
{:ok, expires_at} <- expires_at_from_claims(claims) do
{:ok,
%{
token: new_token,
expires_at: expires_at,
permissions: Map.get(claims, "permissions", [])
}}
else
{:error, :token_expired} ->
{:error, "Token has expired"}

Expand All @@ -91,6 +88,9 @@ defmodule MydiaWeb.Schema.Resolvers.RemoteAccessResolver do
{:error, :device_revoked} ->
{:error, "Device has been revoked"}

{:error, :missing_expiry} ->
{:error, "Failed to refresh token: missing expiry"}

{:error, reason} ->
{:error, "Failed to refresh token: #{inspect(reason)}"}
end
Expand Down
22 changes: 22 additions & 0 deletions priv/repo/migrations/20260804033644_revoke_prefixless_api_keys.exs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
defmodule Mydia.Repo.Migrations.RevokePrefixlessApiKeys do
use Ecto.Migration

# API keys created before 20251226020220 added the key_prefix column have no
# prefix, and it cannot be recovered from an Argon2 hash. Verification now
# narrows by prefix, so these keys can no longer authenticate. Revoke them so
# the key's state is inspectable via the API and explicable in release notes,
# instead of the key silently failing.
def up do
now = DateTime.utc_now() |> DateTime.truncate(:second)

execute("""
UPDATE api_keys SET revoked_at = '#{now}'
WHERE key_prefix IS NULL AND revoked_at IS NULL
""")
end

# Not reversible. Rolling back cannot distinguish keys this migration revoked
# from keys the operator revoked by hand, and un-revoking a key on rollback is
# the wrong default for a security action.
def down, do: :ok
end
62 changes: 62 additions & 0 deletions test/mydia/accounts/api_key_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,20 @@ defmodule Mydia.Accounts.ApiKeyTest do

assert {:error, :invalid_key} = Accounts.verify_api_key(plain_key)
end

test "rejects a key whose stored prefix is NULL", %{plain_key: plain_key, api_key: api_key} do
Mydia.Repo.update_all(
from(k in Mydia.Accounts.ApiKey, where: k.id == ^api_key.id),
set: [key_prefix: nil]
)

assert {:error, :invalid_key} = Accounts.verify_api_key(plain_key)
end

test "rejects a well-formed key whose prefix matches no row" do
unknown = "mydia_ak_" <> String.duplicate("Z", 32)
assert {:error, :invalid_key} = Accounts.verify_api_key(unknown)
end
end

describe "revoke_api_key/1" do
Expand Down Expand Up @@ -185,4 +199,52 @@ defmodule Mydia.Accounts.ApiKeyTest do
assert Accounts.list_api_keys(user.id) == []
end
end

describe "prefix-less key revocation statement" do
setup do
user = AccountsFixtures.user_fixture()
%{user: user}
end

test "revokes a prefix-less key and leaves a prefixed key alone", %{user: user} do
{:ok, legacy, _} = Accounts.create_api_key(user.id, %{name: "Legacy Key"})
{:ok, modern, _} = Accounts.create_api_key(user.id, %{name: "Modern Key"})

# Simulate a row created before the key_prefix column existed.
Mydia.Repo.update_all(
from(k in Mydia.Accounts.ApiKey, where: k.id == ^legacy.id),
set: [key_prefix: nil]
)

now = DateTime.utc_now() |> DateTime.truncate(:second)

Mydia.Repo.query!("""
UPDATE api_keys SET revoked_at = '#{now}'
WHERE key_prefix IS NULL AND revoked_at IS NULL
""")

assert Accounts.get_api_key!(legacy.id).revoked_at != nil
assert Accounts.get_api_key!(modern.id).revoked_at == nil
end

test "does not overwrite an existing revocation timestamp", %{user: user} do
{:ok, legacy, _} = Accounts.create_api_key(user.id, %{name: "Already Revoked"})
{:ok, revoked} = Accounts.revoke_api_key(legacy)
original_revoked_at = revoked.revoked_at

Mydia.Repo.update_all(
from(k in Mydia.Accounts.ApiKey, where: k.id == ^legacy.id),
set: [key_prefix: nil]
)

later = DateTime.utc_now() |> DateTime.add(3600, :second) |> DateTime.truncate(:second)

Mydia.Repo.query!("""
UPDATE api_keys SET revoked_at = '#{later}'
WHERE key_prefix IS NULL AND revoked_at IS NULL
""")

assert Accounts.get_api_key!(legacy.id).revoked_at == original_revoked_at
end
end
end
Loading