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
4 changes: 2 additions & 2 deletions app/controllers/api/v1/request_logs_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class RequestLogsController < Api::V1::BaseController
before_action :set_request_log, only: %i[show]

def index
request_logs = apply_pagination(authorized_scope(apply_scopes(current_account.request_logs.ordered.without_blobs)).preload(:account))
request_logs = apply_pagination(authorized_scope(apply_scopes(current_account.request_logs.external.ordered.without_blobs)).preload(:account))
authorize! request_logs

render jsonapi: request_logs
Expand All @@ -36,7 +36,7 @@ def show
attr_reader :request_log

def set_request_log
scoped_request_logs = authorized_scope(current_account.request_logs)
scoped_request_logs = authorized_scope(current_account.request_logs.external)

@request_log = scoped_request_logs.find_by!(id: params[:id])

Expand Down
4 changes: 4 additions & 0 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,10 @@ def current_api_version
RequestMigrations.config.request_version_resolver.call(request)
end

# NB(ezekg) an internal request is one authenticated via session cookie (i.e. from Portal)
def internal_request? = has_cookie_credentials?
def external_request? = !internal_request?

private

# NOTE(ezekg) Remove memoization of authorization context. This allows us
Expand Down
4 changes: 1 addition & 3 deletions app/controllers/concerns/current_account_constraints.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# frozen_string_literal: true

module CurrentAccountConstraints
CURRENT_ACCOUNT_IGNORED_ORIGINS = %w[https://app.keygen.sh https://dist.keygen.sh].freeze

extend ActiveSupport::Concern

def require_active_subscription!
Expand All @@ -13,7 +11,7 @@ def require_active_subscription!
detail: "must have an active subscription to access this resource"
)
return false
when !CURRENT_ACCOUNT_IGNORED_ORIGINS.include?(request.headers['origin']) &&
when external_request? &&
current_account.trialing_or_free? &&
current_account.daily_request_limit_exceeded?
render_payment_required(
Expand Down
4 changes: 1 addition & 3 deletions app/controllers/concerns/request_counter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
module RequestCounter
extend ActiveSupport::Concern

REQUEST_COUNT_IGNORED_ORIGINS = %w[https://app.keygen.sh https://dist.keygen.sh].freeze

included do
prepend_around_action :count_request!

Expand All @@ -18,7 +16,7 @@ def count_request!

def count_request?
return false if
REQUEST_COUNT_IGNORED_ORIGINS.include?(request.headers['Origin'])
internal_request?

Current.account.present?
end
Expand Down
20 changes: 11 additions & 9 deletions app/controllers/concerns/request_logger.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
module RequestLogger
extend ActiveSupport::Concern

REQUEST_LOG_IGNORED_ORIGINS = %w[https://app.keygen.sh https://dist.keygen.sh].freeze
REQUEST_LOG_IGNORED_HOSTS = %w[get.keygen.sh bin.keygen.sh].freeze
REQUEST_LOG_IGNORED_RESOURCES = %w[
webhook_endpoints
webhook_events
Expand Down Expand Up @@ -64,12 +62,6 @@ def log_request!
end

def log_request?
return false if
REQUEST_LOG_IGNORED_ORIGINS.include?(request.headers['Origin'])

return false if
REQUEST_LOG_IGNORED_HOSTS.include?(request.host)

controller = request.path_parameters[:controller]
return false if
REQUEST_LOG_IGNORED_RESOURCES.any? { controller&.include?(it) }
Expand Down Expand Up @@ -98,6 +90,7 @@ def queue_request_log_worker
'ip' => request_log_ip,
'method' => request_log_method,
'url' => request_log_url,
'origin' => request_log_origin,
'request_headers' => request_log_request_headers,
'request_body' => request_log_request_body,
'response_signature' => request_log_signature,
Expand All @@ -106,7 +99,7 @@ def queue_request_log_worker
'status' => request_log_status,
'queue_time' => request_log_request_queue_time,
'run_time' => request_log_request_run_time,
'ttl' => Current.account.request_log_retention_duration&.to_i,
'ttl' => request_log_ttl,
)
rescue => e
Keygen.logger.exception(e)
Expand Down Expand Up @@ -160,6 +153,10 @@ def request_log_url
end
end

def request_log_origin
internal_request? ? 'ui' : 'api'
end

def request_log_request_headers
headers = REQUEST_LOG_REQUEST_HEADERS.reduce({}) do |hash, header|
value = request.headers[header]
Expand Down Expand Up @@ -226,5 +223,10 @@ def request_log_status
def request_log_signature
response.headers['Keygen-Signature']
end

# TODO(ezekg) expire internal requests daily?
def request_log_ttl
Current.account.request_log_retention_duration&.to_i
end
end
end
3 changes: 3 additions & 0 deletions app/models/request_log.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ class RequestLog < ClickhouseRecord
has_environment
has_account

scope :external, -> { where(origin: :api) }
scope :internal, -> { where(origin: :ui) }

# NB(ezekg) this is based on the clickhouse table's ordering key
scope :ordered, -> (dir = :desc) {
case dir
Expand Down
1 change: 1 addition & 0 deletions app/workers/record_request_spark_worker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ class RecordRequestSparkWorker < BaseWorker

def perform(account_id)
logs_cte = RequestLog.where(account_id:, created_date: Date.yesterday)
.external
.select(
:account_id,
:environment_id,
Expand Down
1 change: 1 addition & 0 deletions config/initializers/lograge.rb
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@
db_name: ReadYourOwnWrites.current_database,
db_role: ReadYourOwnWrites.current_role,
host: req.host,
request_type: controller.internal_request? ? 'ui' : 'api',
request_id: req.request_id,
api_revision: api_revision || 'N/A',
api_version: api_version || 'N/A',
Expand Down
11 changes: 11 additions & 0 deletions db/clickhouse/20260420091847_add_origin_to_request_logs.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# frozen_string_literal: true

class AddOriginToRequestLogs < ActiveRecord::Migration[8.1]
verbose!

def change
add_column :request_logs, :origin, :string, low_cardinality: true, null: false, default: 'api'

add_index :request_logs, :origin, name: 'idx_origin', type: 'set(10)', granularity: 4
end
end
6 changes: 4 additions & 2 deletions db/clickhouse_schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[8.1].define(version: 2026_03_23_160228) do
ActiveRecord::Schema[8.1].define(version: 2026_04_20_091847) do
# TABLE: active_licensed_user_sparks
# SQL: CREATE TABLE active_licensed_user_sparks ( `account_id` UUID, `environment_id` Nullable(UUID), `count` UInt64 DEFAULT 0, `created_date` Date, `created_at` DateTime64(3), INDEX idx_environment environment_id TYPE bloom_filter GRANULARITY 4 ) ENGINE = MergeTree PARTITION BY toYYYYMM(created_date) ORDER BY (account_id, created_date) SETTINGS index_granularity = 8192
create_table "active_licensed_user_sparks", id: false, options: "MergeTree PARTITION BY toYYYYMM(created_date) ORDER BY (account_id, created_date) SETTINGS index_granularity = 8192", force: :cascade do |t|
Expand Down Expand Up @@ -137,7 +137,7 @@
end

# TABLE: request_logs
# SQL: CREATE TABLE request_logs ( `id` UUID, `account_id` UUID, `environment_id` Nullable(UUID), `created_at` DateTime64(3), `updated_at` DateTime64(3), `created_date` Date, `method` LowCardinality(Nullable(String)), `status` LowCardinality(Nullable(String)), `url` Nullable(String), `ip` Nullable(String) TTL created_at + toIntervalDay(30), `user_agent` Nullable(String) TTL created_at + toIntervalDay(30), `requestor_type` LowCardinality(Nullable(String)), `requestor_id` Nullable(UUID), `resource_type` LowCardinality(Nullable(String)), `resource_id` Nullable(UUID), `request_body` Nullable(String) CODEC(ZSTD(1)) TTL created_at + toIntervalDay(30), `request_headers` Nullable(JSON) TTL created_at + toIntervalDay(30), `response_body` Nullable(String) CODEC(ZSTD(1)) TTL created_at + toIntervalDay(30), `response_headers` Nullable(JSON) TTL created_at + toIntervalDay(30), `response_signature` Nullable(String) CODEC(ZSTD(1)) TTL created_at + toIntervalDay(30), `queue_time` Nullable(Float32), `run_time` Nullable(Float32), `is_deleted` UInt8 DEFAULT 0, `ver` DateTime64(3) DEFAULT now(), `ttl` UInt32 DEFAULT 2592000, INDEX idx_status status TYPE set(100) GRANULARITY 4, INDEX idx_method method TYPE set(20) GRANULARITY 4, INDEX idx_requestor (requestor_type, requestor_id) TYPE bloom_filter GRANULARITY 4, INDEX idx_resource (resource_type, resource_id) TYPE bloom_filter GRANULARITY 4, INDEX idx_ip ip TYPE bloom_filter GRANULARITY 4, INDEX idx_environment environment_id TYPE bloom_filter GRANULARITY 4, INDEX idx_id id TYPE bloom_filter GRANULARITY 4 ) ENGINE = ReplacingMergeTree(ver, is_deleted) PARTITION BY toYYYYMM(created_date) ORDER BY (account_id, created_date, UUIDToNum(id)) TTL created_at + toIntervalSecond(ttl) SETTINGS index_granularity = 8192
# SQL: CREATE TABLE request_logs ( `id` UUID, `account_id` UUID, `environment_id` Nullable(UUID), `created_at` DateTime64(3), `updated_at` DateTime64(3), `created_date` Date, `method` LowCardinality(Nullable(String)), `status` LowCardinality(Nullable(String)), `url` Nullable(String), `ip` Nullable(String) TTL created_at + toIntervalDay(30), `user_agent` Nullable(String) TTL created_at + toIntervalDay(30), `requestor_type` LowCardinality(Nullable(String)), `requestor_id` Nullable(UUID), `resource_type` LowCardinality(Nullable(String)), `resource_id` Nullable(UUID), `request_body` Nullable(String) CODEC(ZSTD(1)) TTL created_at + toIntervalDay(30), `request_headers` Nullable(JSON) TTL created_at + toIntervalDay(30), `response_body` Nullable(String) CODEC(ZSTD(1)) TTL created_at + toIntervalDay(30), `response_headers` Nullable(JSON) TTL created_at + toIntervalDay(30), `response_signature` Nullable(String) CODEC(ZSTD(1)) TTL created_at + toIntervalDay(30), `queue_time` Nullable(Float32), `run_time` Nullable(Float32), `is_deleted` UInt8 DEFAULT 0, `ver` DateTime64(3) DEFAULT now(), `ttl` UInt32 DEFAULT 2592000, `origin` LowCardinality(String) DEFAULT 'api', INDEX idx_status status TYPE set(100) GRANULARITY 4, INDEX idx_method method TYPE set(20) GRANULARITY 4, INDEX idx_requestor (requestor_type, requestor_id) TYPE bloom_filter GRANULARITY 4, INDEX idx_resource (resource_type, resource_id) TYPE bloom_filter GRANULARITY 4, INDEX idx_ip ip TYPE bloom_filter GRANULARITY 4, INDEX idx_environment environment_id TYPE bloom_filter GRANULARITY 4, INDEX idx_id id TYPE bloom_filter GRANULARITY 4, INDEX idx_origin origin TYPE set(10) GRANULARITY 4 ) ENGINE = ReplacingMergeTree(ver, is_deleted) PARTITION BY toYYYYMM(created_date) ORDER BY (account_id, created_date, UUIDToNum(id)) TTL created_at + toIntervalSecond(ttl) SETTINGS index_granularity = 8192
create_table "request_logs", id: :uuid, options: "ReplacingMergeTree(ver, is_deleted) PARTITION BY toYYYYMM(created_date) ORDER BY (account_id, created_date, UUIDToNum(id)) TTL created_at + toIntervalSecond(ttl) SETTINGS index_granularity = 8192", force: :cascade do |t|
t.uuid "id", null: false
t.uuid "account_id", null: false
Expand All @@ -164,6 +164,7 @@
t.integer "is_deleted", limit: 1, default: 0, null: false
t.datetime "ver", precision: 3, default: -> { "now()" }, null: false
t.integer "ttl", default: 2592000, null: false
t.string "origin", low_cardinality: true, default: "api", null: false

t.index "status", name: "idx_status", type: "set(100)", granularity: 4
t.index "method", name: "idx_method", type: "set(20)", granularity: 4
Expand All @@ -172,6 +173,7 @@
t.index "ip", name: "idx_ip", type: "bloom_filter", granularity: 4
t.index "environment_id", name: "idx_environment", type: "bloom_filter", granularity: 4
t.index "id", name: "idx_id", type: "bloom_filter", granularity: 4
t.index "origin", name: "idx_origin", type: "set(10)", granularity: 4
end

# TABLE: request_logs_tmp
Expand Down
2 changes: 1 addition & 1 deletion features/api/v1/licenses/create.feature
Original file line number Diff line number Diff line change
Expand Up @@ -6143,7 +6143,7 @@ Feature: Create license
"""
And sidekiq should have 0 "webhook" jobs
And sidekiq should have 0 "event-log" jobs
And sidekiq should have 0 "request-log" jobs
And sidekiq should have 1 "request-log" job

Scenario: Admin uses an invalid token while attempting to create a license
Given the current account is "test1"
Expand Down
12 changes: 6 additions & 6 deletions features/api/v1/request_limits/limits.feature
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,16 @@ Feature: Request limits
When I send a POST request to "/accounts/test1/licenses"
Then the response status should not be "402"

Scenario: Endpoint should be accessible when account is trialing and has exceeded its daily request limit but the request came from the dashboard
Scenario: Endpoint should be accessible when account is trialing and has exceeded its daily request limit but the request came from a portal session
Given the account "test1" has exceeded its daily request limit
And the current account is "test1"
And the account "test1" does not have a card on file
And the account "test1" is trialing
And I am an admin of account "test1"
And I use an authentication token
And I authenticate with a valid session
And I send the following headers:
"""
{ "Origin": "https://app.keygen.sh" }
{ "Origin": "https://portal.keygen.sh" }
"""
When I send a POST request to "/accounts/test1/licenses"
Then the response status should not be "402"
Expand All @@ -61,16 +61,16 @@ Feature: Request limits
When I send a POST request to "/accounts/test1/licenses"
Then the response status should be "402"

Scenario: Endpoint should be accessible when account is on a free tier and has exceeded its daily request limit but the request came from the dashboard
Scenario: Endpoint should be accessible when account is on a free tier and has exceeded its daily request limit but the request came from a portal session
Given the account "test1" has exceeded its daily request limit
And the account "test1" is subscribed
And the account "test1" is on a free tier
And I am an admin of account "test1"
And the current account is "test1"
And I use an authentication token
And I authenticate with a valid session
And I send the following headers:
"""
{ "Origin": "https://app.keygen.sh" }
{ "Origin": "https://portal.keygen.sh" }
"""
When I send a POST request to "/accounts/test1/licenses"
Then the response status should not be "402"
13 changes: 13 additions & 0 deletions features/api/v1/request_logs/index.feature
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,19 @@ Feature: List request logs
Then the response status should be "200"
And the response body should be an array with 3 "request-logs"

Scenario: Admin retrieves logs with internal logs filtered out
Given I am an admin of account "test1"
And the current account is "test1"
And the current account has 2 "request-logs"
And the current account has 3 "request-logs" with the following:
"""
{ "origin": "ui" }
"""
And I use an authentication token
When I send a GET request to "/accounts/test1/request-logs"
Then the response status should be "200"
And the response body should be an array with 2 "request-logs"

Scenario: Admin retrieves a list of logs that is automatically limited
Given I am an admin of account "test1"
And the current account is "test1"
Expand Down
11 changes: 11 additions & 0 deletions features/api/v1/request_logs/show.feature
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,17 @@ Feature: Show request logs
}
"""

Scenario: Admin attempts to retrieve an internal log for their account
Given I am an admin of account "test1"
And the current account is "test1"
And the current account has 1 "request-log" with the following:
"""
{ "origin": "ui" }
"""
And I use an authentication token
When I send a GET request to "/accounts/test1/request-logs/$0"
Then the response status should be "404"

Scenario: Admin attempts to retrieve a log for another account
Given I am an admin of account "test2"
But the current account is "test1"
Expand Down
55 changes: 55 additions & 0 deletions spec/controllers/concerns/request_counter_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
# frozen_string_literal: true

require 'rails_helper'
require 'spec_helper'

describe RequestCounter, type: :concern do
controller_class = Class.new(ActionController::API) do
include RequestCounter

def internal_request? = false
end

let(:controller) { controller_class.new }
let(:request) { instance_double(ActionDispatch::Request) }
let(:response) { instance_double(ActionDispatch::Response) }

before do
allow(controller).to receive(:request).and_return(request)
allow(controller).to receive(:response).and_return(response)
end

describe '#count_request?' do
let(:account) { build(:account) }

after { Current.reset }

it 'returns false for an external request with no account' do
Current.account = nil

expect(controller.send(:count_request?)).to be false
end

it 'returns false for an internal request with no account' do
allow(controller).to receive(:internal_request?).and_return(true)

Current.account = nil

expect(controller.send(:count_request?)).to be false
end

it 'returns true for an external request with an account' do
Current.account = account

expect(controller.send(:count_request?)).to be true
end

it 'returns false for an internal request with an account' do
allow(controller).to receive(:internal_request?).and_return(true)

Current.account = account

expect(controller.send(:count_request?)).to be false
end
end
end
Loading
Loading