Skip to content

Commit 7f94da7

Browse files
chore: clean up whitespace and improve code readability across multiple files
- Removed unnecessary blank lines and adjusted formatting for consistency in the ApiKey module, ApiKeys context, authentication pipeline, and various controllers and views. - Enhanced readability in the API key controller and tests by streamlining code structure and ensuring consistent formatting. - Improved error handling in the API key authentication plug for better clarity and maintainability.
1 parent 28aebf7 commit 7f94da7

12 files changed

Lines changed: 93 additions & 90 deletions

File tree

lib/wraft_doc/api_keys/api_key.ex

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -25,20 +25,20 @@ defmodule WraftDoc.ApiKeys.ApiKey do
2525
field(:name, :string)
2626
field(:key_hash, :string)
2727
field(:key_prefix, :string)
28-
28+
2929
# Security settings
3030
field(:expires_at, :utc_datetime)
3131
field(:is_active, :boolean, default: true)
3232
field(:rate_limit, :integer, default: 1000)
3333
field(:ip_whitelist, {:array, :string}, default: [])
34-
34+
3535
# Usage tracking
3636
field(:last_used_at, :utc_datetime)
3737
field(:usage_count, :integer, default: 0)
38-
38+
3939
# Additional metadata
4040
field(:metadata, :map, default: %{})
41-
41+
4242
# Virtual field to store the unhashed key (only available during creation)
4343
field(:key, :string, virtual: true)
4444

@@ -102,8 +102,7 @@ defmodule WraftDoc.ApiKeys.ApiKey do
102102
Changeset for updating usage statistics.
103103
"""
104104
def usage_changeset(api_key, attrs \\ %{}) do
105-
api_key
106-
|> cast(attrs, [:last_used_at, :usage_count])
105+
cast(api_key, attrs, [:last_used_at, :usage_count])
107106
end
108107

109108
# Private functions
@@ -129,7 +128,7 @@ defmodule WraftDoc.ApiKeys.ApiKey do
129128
prefix = generate_prefix()
130129
random_part = generate_random_string(32)
131130
full_key = "wraft_#{prefix}_#{random_part}"
132-
131+
133132
# Hash the key for storage
134133
key_hash = Bcrypt.hash_pwd_salt(full_key)
135134

@@ -144,14 +143,12 @@ defmodule WraftDoc.ApiKeys.ApiKey do
144143

145144
defp generate_prefix do
146145
# Generate a 8-character prefix for easy identification
147-
:crypto.strong_rand_bytes(4)
148-
|> Base.encode16(case: :lower)
146+
Base.encode16(:crypto.strong_rand_bytes(4), case: :lower)
149147
end
150148

151149
defp generate_random_string(length) do
152-
:crypto.strong_rand_bytes(length)
153-
|> Base.url_encode64(padding: false)
154-
|> binary_part(0, length)
150+
encoded = Base.url_encode64(:crypto.strong_rand_bytes(length), padding: false)
151+
binary_part(encoded, 0, length)
155152
end
156153

157154
@doc """
@@ -182,4 +179,3 @@ defmodule WraftDoc.ApiKeys.ApiKey do
182179
ip in whitelist
183180
end
184181
end
185-

lib/wraft_doc/api_keys/api_keys.ex

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ defmodule WraftDoc.ApiKeys do
33
The ApiKeys context - handles all API key management operations.
44
"""
55
import Ecto.Query
6-
6+
77
alias WraftDoc.Account.{Role, User}
88
alias WraftDoc.ApiKeys.ApiKey
99
alias WraftDoc.Documents.InstanceApprovalSystem
@@ -42,7 +42,7 @@ defmodule WraftDoc.ApiKeys do
4242
def get_api_key_by_key("wraft_" <> rest = full_key) do
4343
# Extract prefix (first 8 characters after "wraft_")
4444
prefix = String.slice(rest, 0..7)
45-
45+
4646
# Find all keys with this prefix and check each one
4747
ApiKey
4848
|> where([k], k.key_prefix == ^prefix)
@@ -224,7 +224,7 @@ defmodule WraftDoc.ApiKeys do
224224
@doc """
225225
Check if rate limit is exceeded for an API key.
226226
Returns {:ok, api_key} if within limit, {:error, :rate_limit_exceeded} otherwise.
227-
227+
228228
This is a simple hourly rate limit check.
229229
For production, consider using a more sophisticated rate limiting solution.
230230
"""
@@ -267,4 +267,3 @@ defmodule WraftDoc.ApiKeys do
267267
end
268268
end
269269
end
270-
Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
defmodule WraftDocWeb.Guardian.FlexibleAuthPipeline do
22
@moduledoc """
33
A flexible authentication pipeline that supports both API Key and JWT authentication.
4-
4+
55
This pipeline:
66
1. First attempts API Key authentication (X-API-Key header)
77
2. If no API key, falls back to JWT authentication (Authorization: Bearer header)
88
3. If either succeeds, sets current_user and current_organisation
99
4. If both fail, returns 401 Unauthorized
10-
10+
1111
This allows the same endpoints to work with both authentication methods.
1212
"""
1313
use Guardian.Plug.Pipeline,
@@ -17,16 +17,15 @@ defmodule WraftDocWeb.Guardian.FlexibleAuthPipeline do
1717

1818
# Try API key authentication first
1919
plug(WraftDocWeb.Plug.ApiKeyAuth)
20-
20+
2121
# Then try JWT authentication (only if API key didn't succeed)
2222
plug(Guardian.Plug.VerifyHeader, claims: %{})
2323
plug(Guardian.Plug.LoadResource, allow_blank: true)
24-
24+
2525
# Load user context (works for both auth methods)
2626
plug(WraftDocWeb.CurrentUser)
2727
plug(WraftDocWeb.CurrentOrganisation)
28-
28+
2929
# Finally, ensure we have authentication from either method
3030
plug(WraftDocWeb.Plug.EnsureAuthenticated)
3131
end
32-

lib/wraft_doc_web/controllers/api_key_controller.ex

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,11 @@ defmodule WraftDocWeb.Api.V1.ApiKeyController do
3131

3232
properties do
3333
name(:string, "Descriptive name for the API key", required: true)
34-
user_id(:string, "User ID for authentication (defaults to current user)", required: false)
34+
35+
user_id(:string, "User ID for authentication (defaults to current user)",
36+
required: false
37+
)
38+
3539
expires_at(:string, "Optional: Expiration datetime (ISO-8601)", required: false)
3640
rate_limit(:integer, "Requests per hour limit", required: false)
3741
ip_whitelist(:array, "Optional: List of allowed IP addresses", required: false)
@@ -308,4 +312,3 @@ defmodule WraftDocWeb.Api.V1.ApiKeyController do
308312
end
309313
end
310314
end
311-

lib/wraft_doc_web/plugs/api_key_auth.ex

Lines changed: 47 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,18 @@
11
defmodule WraftDocWeb.Plug.ApiKeyAuth do
22
@moduledoc """
33
Plug for API Key authentication.
4-
4+
55
This plug attempts to authenticate requests using an API key from the X-API-Key header.
66
If a valid API key is found:
77
- Sets conn.assigns.current_user to the user associated with the API key
88
- Sets conn.assigns.current_organisation to the organisation
99
- Sets conn.assigns.api_key to the API key struct (for audit purposes)
1010
- Sets conn.assigns.authenticated_via to :api_key
11-
11+
1212
If no API key is provided or invalid, the plug does nothing and lets the
1313
request continue to the next authentication method (JWT).
1414
"""
15-
15+
1616
import Plug.Conn
1717
require Logger
1818

@@ -52,44 +52,56 @@ defmodule WraftDocWeb.Plug.ApiKeyAuth do
5252

5353
case ApiKeys.verify_api_key(api_key_string, remote_ip) do
5454
{:ok, %{api_key: api_key, user: user, organisation: organisation}} ->
55-
# Check rate limit
56-
case ApiKeys.check_rate_limit(api_key) do
57-
{:ok, _} ->
58-
conn
59-
|> assign(:current_user, user)
60-
|> assign(:current_organisation, organisation)
61-
|> assign(:api_key, api_key)
62-
|> assign(:authenticated_via, :api_key)
63-
64-
{:error, :rate_limit_exceeded} ->
65-
Logger.warning("API key rate limit exceeded: #{api_key.id}")
66-
send_error_response(conn, 429, "Rate limit exceeded")
67-
end
55+
handle_rate_limit_check(conn, api_key, user, organisation)
6856

69-
{:error, :invalid_api_key} ->
70-
Logger.warning("Invalid API key provided")
71-
send_error_response(conn, 401, "Invalid API key")
57+
{:error, error_type} ->
58+
handle_authentication_error(conn, error_type)
59+
end
60+
end
7261

73-
{:error, :api_key_expired} ->
74-
Logger.warning("Expired API key used")
75-
send_error_response(conn, 401, "API key has expired")
62+
defp handle_rate_limit_check(conn, api_key, user, organisation) do
63+
case ApiKeys.check_rate_limit(api_key) do
64+
{:ok, _} ->
65+
conn
66+
|> assign(:current_user, user)
67+
|> assign(:current_organisation, organisation)
68+
|> assign(:api_key, api_key)
69+
|> assign(:authenticated_via, :api_key)
70+
71+
{:error, :rate_limit_exceeded} ->
72+
Logger.warning("API key rate limit exceeded: #{api_key.id}")
73+
send_error_response(conn, 429, "Rate limit exceeded")
74+
end
75+
end
7676

77-
{:error, :api_key_inactive} ->
78-
Logger.warning("Inactive API key used")
79-
send_error_response(conn, 401, "API key is inactive")
77+
defp handle_authentication_error(conn, :invalid_api_key) do
78+
Logger.warning("Invalid API key provided")
79+
send_error_response(conn, 401, "Invalid API key")
80+
end
8081

81-
{:error, :ip_not_whitelisted} ->
82-
Logger.warning("API key used from non-whitelisted IP")
83-
send_error_response(conn, 403, "IP address not authorized for this API key")
82+
defp handle_authentication_error(conn, :api_key_expired) do
83+
Logger.warning("Expired API key used")
84+
send_error_response(conn, 401, "API key has expired")
85+
end
8486

85-
{:error, :user_not_found} ->
86-
Logger.warning("API key user not found")
87-
send_error_response(conn, 401, "User associated with API key not found")
87+
defp handle_authentication_error(conn, :api_key_inactive) do
88+
Logger.warning("Inactive API key used")
89+
send_error_response(conn, 401, "API key is inactive")
90+
end
8891

89-
{:error, reason} ->
90-
Logger.warning("API key authentication failed: #{inspect(reason)}")
91-
send_error_response(conn, 401, "API key authentication failed")
92-
end
92+
defp handle_authentication_error(conn, :ip_not_whitelisted) do
93+
Logger.warning("API key used from non-whitelisted IP")
94+
send_error_response(conn, 403, "IP address not authorized for this API key")
95+
end
96+
97+
defp handle_authentication_error(conn, :user_not_found) do
98+
Logger.warning("API key user not found")
99+
send_error_response(conn, 401, "User associated with API key not found")
100+
end
101+
102+
defp handle_authentication_error(conn, reason) do
103+
Logger.warning("API key authentication failed: #{inspect(reason)}")
104+
send_error_response(conn, 401, "API key authentication failed")
93105
end
94106

95107
defp get_remote_ip(conn) do
@@ -119,4 +131,3 @@ defmodule WraftDocWeb.Plug.ApiKeyAuth do
119131
|> halt()
120132
end
121133
end
122-

lib/wraft_doc_web/plugs/ensure_authenticated.ex

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
defmodule WraftDocWeb.Plug.EnsureAuthenticated do
22
@moduledoc """
33
Ensures that a request is authenticated via either API Key or JWT.
4-
4+
55
This plug checks if current_user is set in conn.assigns.
66
If not, it returns a 401 Unauthorized response.
7-
7+
88
This works with both API Key and JWT authentication methods.
99
"""
10-
10+
1111
import Plug.Conn
1212

1313
def init(opts), do: opts
@@ -29,4 +29,3 @@ defmodule WraftDocWeb.Plug.EnsureAuthenticated do
2929
|> halt()
3030
end
3131
end
32-

lib/wraft_doc_web/views/api_key_view.ex

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ defmodule WraftDocWeb.Api.V1.ApiKeyView do
1818
end
1919

2020
def render("api_key.json", %{api_key: api_key}) do
21-
%{
21+
base_map = %{
2222
id: api_key.id,
2323
name: api_key.name,
2424
key_prefix: api_key.key_prefix,
@@ -34,7 +34,8 @@ defmodule WraftDocWeb.Api.V1.ApiKeyView do
3434
user: render_user(api_key),
3535
created_by: render_created_by(api_key)
3636
}
37-
|> maybe_add_key(api_key)
37+
38+
maybe_add_key(base_map, api_key)
3839
end
3940

4041
def render("api_key_list.json", %{api_key: api_key}) do
@@ -85,4 +86,3 @@ defmodule WraftDocWeb.Api.V1.ApiKeyView do
8586
}
8687
end
8788
end
88-

priv/repo/migrations/20251118100000_create_api_keys.exs

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,28 +7,28 @@ defmodule WraftDoc.Repo.Migrations.CreateApiKeys do
77
add(:name, :string, null: false)
88
add(:key_hash, :string, null: false)
99
add(:key_prefix, :string, null: false)
10-
10+
1111
add(:organisation_id, references(:organisation, type: :uuid, on_delete: :delete_all),
1212
null: false
1313
)
14-
14+
1515
add(:user_id, references(:user, type: :uuid, on_delete: :delete_all), null: false)
16-
16+
1717
add(:created_by_id, references(:user, type: :uuid, on_delete: :nilify_all))
18-
18+
1919
# Security settings
2020
add(:expires_at, :utc_datetime)
2121
add(:is_active, :boolean, default: true, null: false)
2222
add(:rate_limit, :integer, default: 1000)
2323
add(:ip_whitelist, {:array, :string}, default: [])
24-
24+
2525
# Usage tracking
2626
add(:last_used_at, :utc_datetime)
2727
add(:usage_count, :integer, default: 0, null: false)
28-
28+
2929
# Additional metadata
3030
add(:metadata, :map, default: %{})
31-
31+
3232
timestamps()
3333
end
3434

@@ -41,4 +41,3 @@ defmodule WraftDoc.Repo.Migrations.CreateApiKeys do
4141
create(unique_index(:api_keys, [:name, :organisation_id]))
4242
end
4343
end
44-

test/support/factory.ex

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -726,10 +726,13 @@ defmodule WraftDoc.Factory do
726726
user = build(:user)
727727

728728
# Generate a sample API key
729-
prefix = :crypto.strong_rand_bytes(4) |> Base.encode16(case: :lower)
730-
random_part = :crypto.strong_rand_bytes(32) |> Base.url_encode64(padding: false) |> binary_part(0, 32)
729+
prefix = Base.encode16(:crypto.strong_rand_bytes(4), case: :lower)
730+
731+
encoded = Base.url_encode64(:crypto.strong_rand_bytes(32), padding: false)
732+
random_part = binary_part(encoded, 0, 32)
733+
731734
full_key = "wraft_#{prefix}_#{random_part}"
732-
735+
733736
%ApiKey{
734737
name: sequence(:api_key_name, &"API Key #{&1}"),
735738
key_hash: Bcrypt.hash_pwd_salt(full_key),

test/wraft_doc/api_keys/api_key_test.exs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ defmodule WraftDoc.ApiKeys.ApiKeyTest do
3535
assert get_change(changeset, :key_hash) != nil
3636
assert get_change(changeset, :key_prefix) != nil
3737
assert get_change(changeset, :key) != nil
38-
38+
3939
# Check key format
4040
key = get_change(changeset, :key)
4141
assert String.starts_with?(key, "wraft_")
@@ -240,4 +240,3 @@ defmodule WraftDoc.ApiKeys.ApiKeyTest do
240240
end
241241
end
242242
end
243-

0 commit comments

Comments
 (0)