Skip to content

Commit

Permalink
add session authn
Browse files Browse the repository at this point in the history
  • Loading branch information
ezekg committed Feb 24, 2025
1 parent d485239 commit 5cc3e01
Show file tree
Hide file tree
Showing 16 changed files with 224 additions and 42 deletions.
13 changes: 10 additions & 3 deletions app/controllers/api/v1/tokens_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,16 @@ def generate
context: { bearer: current_bearer },
with: TokenPolicy

# TODO(ezekg) make default session expiry configurable
session = token.sessions.build(
expiry: token.expiry.presence || (1.week + 12.hours).from_now,
user_agent: request.user_agent,
ip: request.remote_ip,
)

if token.save
cookies.encrypted[:session_id] = { value: session.id, httponly: true, secure: true, same_site: :none, expires: session.expiry, domain: Keygen::DOMAIN }

BroadcastEventService.call(
event: 'token.generated',
account: current_account,
Expand All @@ -93,11 +102,9 @@ def generate
else
render_unprocessable_resource token
end
rescue ArgumentError # Catch null bytes (Postgres throws an argument error)
render_bad_request
end

# FIXME(ezekg) Deprecate this route.
# FIXME(ezekg) deprecate this route
def regenerate_current
raise Keygen::Error::NotFoundError.new(model: Token.name) unless
current_token.present?
Expand Down
8 changes: 7 additions & 1 deletion app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,14 @@ class ApplicationController < ActionController::API
attr_accessor :current_http_token
attr_accessor :current_account
attr_accessor :current_environment
attr_accessor :current_session
attr_accessor :current_bearer
attr_accessor :current_token

# Action policy authz contexts
authorize :account, through: :current_account
authorize :environment, through: :current_environment
authorize :session, through: :current_session
authorize :bearer, through: :current_bearer
authorize :token, through: :current_token

Expand Down Expand Up @@ -118,6 +120,9 @@ def render_unauthorized(**kwargs)

response.headers['WWW-Authenticate'] = challenge

# clear current session cookie
cookies.delete(:session_id)

respond_to do |format|
format.any {
render status: :unauthorized, json: {
Expand Down Expand Up @@ -450,12 +455,13 @@ def rescue_from_exceptions
kwargs[:source] = e.source if
e.source.present?

# Add additional properties based on code
case e.code
when 'LICENSE_NOT_ALLOWED'
kwargs[:links] = { about: 'https://keygen.sh/docs/api/authentication/#license-authentication' }
when 'TOKEN_NOT_ALLOWED'
kwargs[:links] = { about: 'https://keygen.sh/docs/api/authentication/#token-authentication' }
when 'USER_BANNED'
cookies.delete(:session_id) # clear session
end

render_forbidden(**kwargs)
Expand Down
50 changes: 49 additions & 1 deletion app/controllers/concerns/authentication.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ module Authentication

include ActionController::HttpAuthentication::Token::ControllerMethods
include ActionController::HttpAuthentication::Basic::ControllerMethods
include ActionController::Cookies

def authenticate_with_token!
case
Expand All @@ -14,6 +15,8 @@ def authenticate_with_token!
authenticate_or_request_with_http_basic(&method(:http_basic_authenticator))
when has_license_credentials?
authenticate_or_request_with_http_license(&method(:http_license_authenticator))
when has_cookie_credentials?
authenticate_or_request_with_http_cookie(&method(:http_cookie_authenticator))
else
authenticate_or_request_with_query_token(&method(:query_token_authenticator))
end
Expand All @@ -27,6 +30,8 @@ def authenticate_with_token
authenticate_with_http_basic(&method(:http_basic_authenticator))
when has_license_credentials?
authenticate_with_http_license(&method(:http_license_authenticator))
when has_cookie_credentials?
authenticate_with_http_cookie(&method(:http_cookie_authenticator))
else
authenticate_with_query_token(&method(:query_token_authenticator))
end
Expand Down Expand Up @@ -58,6 +63,14 @@ def authenticate_with_password_or_token

private

def authenticate_or_request_with_http_cookie(&auth_procedure)
authenticate_with_http_cookie(&auth_procedure) || request_http_token_authentication
end

def authenticate_with_http_cookie(&auth_procedure)
auth_procedure.call(cookies.encrypted)
end

def authenticate_or_request_with_query_token(&auth_procedure)
authenticate_with_query_token(&auth_procedure) || request_http_token_authentication
end
Expand Down Expand Up @@ -96,10 +109,43 @@ def query_token_authenticator(query_token)
end
end

def http_cookie_authenticator(cookie_jar)
session = current_account.sessions.for_environment(current_environment, strict: current_environment.nil?)
.find_by(id: cookie_jar[:session_id])

@current_http_scheme = :cookie
@current_http_token = nil

raise Keygen::Error::UnauthorizedError.new(code: 'SESSION_INVALID') if
session.nil? || session.token.nil? || session.bearer.nil?

raise Keygen::Error::UnauthorizedError.new(code: 'SESSION_EXPIRED', detail: 'Session is expired') if
session.expired?

raise Keygen::Error::ForbiddenError.new(code: 'USER_BANNED', detail: 'User is banned') if
session.bearer.respond_to?(:banned?) && session.bearer.banned?

if session.last_used_at.nil? || session.last_used_at.before?(1.hour.ago)
session.update(
last_used_at: Time.current,
user_agent: request.user_agent,
ip: request.remote_ip,
)
end

# FIXME(ezekg) use Current everywhere instead of current ivars
Current.session = @current_session = session
Current.token = @current_token = session.token
Current.bearer = @current_bearer = session.bearer
end

def http_password_authenticator(username = nil, password = nil)
user = current_account.users.for_environment(current_environment, strict: current_environment.nil?)
.find_by(email: "#{username}".downcase)

@current_http_scheme = :password
@current_http_token = nil

unless user.present?
raise Keygen::Error::UnauthorizedError.new(
detail: 'email and password must be valid',
Expand Down Expand Up @@ -135,7 +181,7 @@ def http_password_authenticator(username = nil, password = nil)
)
end

@current_bearer = user
Current.bearer = @current_bearer = user
end

def http_basic_authenticator(username = nil, password = nil)
Expand Down Expand Up @@ -271,6 +317,8 @@ def request_http_basic_authentication(realm = 'keygen', message = nil)
raise Keygen::Error::UnauthorizedError.new(code: 'TOKEN_INVALID')
end

def has_cookie_credentials? = cookies.encrypted[:session_id].present?

def has_bearer_credentials?
authentication_scheme == 'bearer' || authentication_scheme == 'token'
end
Expand Down
1 change: 1 addition & 0 deletions app/models/account.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ class Account < ApplicationRecord
has_many :request_logs, dependent: :destroy_async
has_many :metrics, dependent: :destroy_async
has_many :tokens, dependent: :destroy_async
has_many :sessions, dependent: :destroy_async
has_many :users, index_errors: true, dependent: :destroy_async
has_many :second_factors, dependent: :destroy_async
has_many :products, dependent: :destroy_async
Expand Down
18 changes: 9 additions & 9 deletions app/models/concerns/denormalizable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,15 @@ module Denormalizable
class_methods do
cattr_accessor :denormalized_attributes, default: Set.new

# TODO(ezekg) Active Record's normalizes method accepts an array of names. Should we?
def denormalizes(attribute_name, with: nil, from: nil, to: nil, prefix: nil)
def denormalizes(*attribute_names, with: nil, from: nil, to: nil, prefix: nil)
raise ArgumentError, 'must provide :from, :to, or :with (but not multiple)' unless
from.present? ^ to.present? ^ with.present?

# TODO(ezekg) Should we store more information, such as :to or :from?
denormalized_attributes << attribute_name

case
when from.present?
instrument_denormalized_attribute_from(attribute_name, from:, prefix:)
attribute_names.each { instrument_denormalized_attribute_from(_1, from:, prefix:) }
when to.present?
instrument_denormalized_attribute_to(attribute_name, to:, prefix:)
attribute_names.each { instrument_denormalized_attribute_to(_1, to:, prefix:) }
when with.present?
raise NotImplementedError, 'denormalizes :with is not supported yet'
else
Expand Down Expand Up @@ -51,8 +47,10 @@ def instrument_denormalized_attribute_from(attribute_name, from:, prefix:)
before_validation -> { write_denormalized_attribute_from_schrodingers_record(association_name, attribute_name, prefixed_attribute_name) }, if: -> { send(:"#{reflection.foreign_key}_changed?") || send(:"#{reflection.name}_changed?") }, on: :create
before_update -> { write_denormalized_attribute_from_persisted_record(association_name, attribute_name, prefixed_attribute_name) }, if: -> { send(:"#{reflection.foreign_key}_changed?") || send(:"#{reflection.name}_changed?") }

# Make sure validation fails if our denormalized column is modified directly
# make sure validation fails if our denormalized column is modified directly
validate -> { validate_denormalized_attribute_from_persisted_record(association_name, attribute_name, prefixed_attribute_name) }, if: :"#{prefixed_attribute_name}_changed?", on: :update

denormalized_attributes << attribute_name
else
raise ArgumentError, "invalid :from association: #{from.inspect}"
end
Expand All @@ -71,7 +69,7 @@ def instrument_denormalized_attribute_to(attribute_name, to:, prefix:)
attribute_name.to_s
end

# FIXME(ezekg) Set to nil on destroy unless the association is dependent?
# FIXME(ezekg) set to nil on destroy unless the association is dependent?
if reflection.collection?
after_initialize -> { write_denormalized_attribute_to_unpersisted_relation(association_name, prefixed_attribute_name, attribute_name) }, if: :"#{attribute_name}_changed?", unless: :persisted?
before_validation -> { write_denormalized_attribute_to_unpersisted_relation(association_name, prefixed_attribute_name, attribute_name) }, if: :"#{attribute_name}_changed?", on: :create
Expand All @@ -81,6 +79,8 @@ def instrument_denormalized_attribute_to(attribute_name, to:, prefix:)
before_validation -> { write_denormalized_attribute_to_unpersisted_record(association_name, prefixed_attribute_name, attribute_name) }, if: :"#{attribute_name}_changed?", on: :create
after_update -> { write_denormalized_attribute_to_persisted_record(association_name, prefixed_attribute_name, attribute_name) }, if: :"#{attribute_name}_previously_changed?"
end

denormalized_attributes << attribute_name
else
raise ArgumentError, "invalid :to association: #{to.inspect}"
end
Expand Down
5 changes: 4 additions & 1 deletion app/models/current.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ class Current < ActiveSupport::CurrentAttributes
attribute :account,
:environment,
:bearer,
:session,
:token,
:resource

Expand All @@ -13,16 +14,18 @@ class Current < ActiveSupport::CurrentAttributes
def account=(account)
super

# Ensure these are always reset when account is changed
# ensure these are always reset when account is changed
self.environment = nil
self.bearer = nil
self.session = nil
self.token = nil
self.resource = nil
end

def account_id = account&.id
def environment_id = environment&.id
def bearer_id = bearer&.id
def session_id = session&.id
def token_id = token&.id
def resource_id = resource&.id
end
19 changes: 19 additions & 0 deletions app/models/session.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# frozen_string_literal: true

class Session < ApplicationRecord
include Denormalizable
include Environmental
include Accountable

belongs_to :token
belongs_to :bearer,
polymorphic: true

has_environment default: -> { token&.environment_id }
has_account default: -> { token&.account_id }, inverse_of: :sessions

denormalizes :bearer_type, :bearer_id,
from: :token

def expired? = expiry < Time.current
end
5 changes: 5 additions & 0 deletions app/models/token.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ class Token < ApplicationRecord
belongs_to :bearer,
polymorphic: true

# FIXME(ezekg) sessions must come before permissions otherwise autosave breaks
has_many :sessions,
dependent: :delete_all,
autosave: true

has_many :token_permissions,
dependent: :delete_all,
autosave: true
Expand Down
41 changes: 41 additions & 0 deletions app/workers/prune_expired_sessions_worker.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
class PruneExpiredSessionsWorker < BaseWorker
STATEMENT_TIMEOUT = ENV.fetch('KEYGEN_PRUNE_STATEMENT_TIMEOUT') { '1min' }
BATCH_SIZE = ENV.fetch('KEYGEN_PRUNE_BATCH_SIZE') { 1_000 }.to_i
BATCH_WAIT = ENV.fetch('KEYGEN_PRUNE_BATCH_WAIT') { 1 }.to_f

sidekiq_options queue: :cron,
cronitor_disabled: false

def perform
sessions = Session.where(<<~SQL.squish, max_age: 90.days.ago)
(created_at < :max_age AND last_used_at IS NULL) OR last_used_at < :max_age OR expiry < :max_age
SQL
.reorder(created_at: :asc)

total = sessions.count
sum = 0

batches = (total / BATCH_SIZE) + 1
batch = 0

Keygen.logger.info "[workers.prune-expired-sessions] Starting"
Keygen.logger.info "[workers.prune-expired-sessions] Pruning #{total} rows: batches=#{batches}"

loop do
count = sessions.statement_timeout(STATEMENT_TIMEOUT) do
sessions.limit(BATCH_SIZE).delete_all
end

sum += count
batch += 1

Keygen.logger.info "[workers.prune-expired-sessions] Pruned #{sum}/#{total} rows: batch=#{batch}/#{batches}"

sleep BATCH_WAIT

break if count < BATCH_SIZE
end

Keygen.logger.info "[workers.prune-expired-sessions] Done"
end
end
7 changes: 5 additions & 2 deletions config/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,18 @@ class Application < Rails::Application
# Skip views, helpers and assets when generating a new resource.
config.api_only = true

# Remove unneeded Rack middleware
# remove unneeded Rack middleware
config.middleware.delete Rack::ConditionalGet
config.middleware.delete Rack::ETag
config.middleware.delete Rack::Sendfile
config.middleware.delete Rack::Runtime

# remove unneeded Rails middlware
# remove unneeded Rails middleware
config.middleware.delete ActionDispatch::ShowExceptions

# readd support for cookies
config.middleware.use ActionDispatch::Cookies

# ignore X-Forwarded-For header
config.middleware.insert_before 0, Keygen::Middleware::IgnoreForwardedHost

Expand Down
2 changes: 2 additions & 0 deletions config/initializers/lograge.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
bearer_type = controller.current_bearer&.class&.name&.underscore
bearer_id = controller.current_bearer&.id
token_id = controller.current_token&.id
session_id = controller.current_session&.id
authn = controller.current_http_scheme
authz = controller.current_bearer&.role&.name
api_version = controller.current_api_version
Expand Down Expand Up @@ -98,6 +99,7 @@
bearer_type: bearer_type || 'N/A',
bearer_id: bearer_id || 'N/A',
token_id: token_id || 'N/A',
session_id: session_id || 'N/A',
authn: authn || 'N/A',
authz: authz || 'N/A',
ip: req.remote_ip,
Expand Down
Loading

0 comments on commit 5cc3e01

Please sign in to comment.