Fix Clerk SSO auto-admin: fail closed + explicit admin allowlist (BLO-287)

clerk_email_allowed? failed OPEN on an empty CLERK_ALLOWED_EMAIL_DOMAINS
(empty env = anyone could sign in), and both SSO entrypoints
(from_clerk_oidc and the apex-cookie ClerkDeviseBridge) auto-provisioned
every first-time user as ADMIN_ROLE.

- Fail closed: an unset/empty allowlist now matches no one.
- New CLERK_ADMIN_EMAIL_DOMAINS allowlist gates admin provisioning; SSO
  never silently mints an admin.
- Single chokepoint User.provision_clerk_admin used by both SSO paths.
- Add RSpec coverage (none existed).
pull/688/head
mohammed-guiga 2 weeks ago
parent 2ba9dc2ec1
commit 2e18084a04

@ -47,20 +47,10 @@ module ClerkDeviseBridge
end end
def provision_user_for_clerk(email, clerk_user) def provision_user_for_clerk(email, clerk_user)
account = Account.first
return nil unless account
first_name = clerk_user.respond_to?(:first_name) ? clerk_user.first_name : nil first_name = clerk_user.respond_to?(:first_name) ? clerk_user.first_name : nil
last_name = clerk_user.respond_to?(:last_name) ? clerk_user.last_name : nil last_name = clerk_user.respond_to?(:last_name) ? clerk_user.last_name : nil
User.create!( User.provision_clerk_admin(email: email, first_name: first_name, last_name: last_name)
account: account,
email: email,
first_name: first_name.presence,
last_name: last_name.presence,
role: User::ADMIN_ROLE,
password: Devise.friendly_token(40)
)
rescue ActiveRecord::RecordInvalid, ActiveRecord::RecordNotUnique => e rescue ActiveRecord::RecordInvalid, ActiveRecord::RecordNotUnique => e
Rails.logger.warn("[clerk-bridge] provision failed for #{email}: #{e.message}") Rails.logger.warn("[clerk-bridge] provision failed for #{email}: #{e.message}")
User.active.find_by(email: email) User.active.find_by(email: email)

@ -132,19 +132,37 @@ class User < ApplicationRecord
return nil if email.blank? return nil if email.blank?
return nil unless Docuseal.clerk_email_allowed?(email) return nil unless Docuseal.clerk_email_allowed?(email)
account = Account.first
return nil if account.blank?
user = active.find_by(email:) user = active.find_by(email:)
return user if user return user if user
first, *rest = auth.info.name.to_s.split(' ', 2) first, *rest = auth.info.name.to_s.split(' ', 2)
provision_clerk_admin(
email:,
first_name: auth.info.first_name.presence || first,
last_name: auth.info.last_name.presence || rest.join(' ')
)
end
# The single chokepoint for auto-provisioning a Devise user from Clerk SSO,
# used by every SSO entrypoint (the OIDC callback and the apex-cookie bridge).
# Fails closed: it refuses to create anyone unless their domain is on the
# explicit admin allowlist, so SSO can never silently mint an admin. docuseal
# has no non-admin console role, so an allowed-but-not-admin email gets no
# account rather than an elevated one.
def self.provision_clerk_admin(email:, first_name:, last_name:)
email = email.to_s.downcase
return nil if email.blank?
return nil unless Docuseal.clerk_email_admin?(email)
account = Account.first
return nil if account.blank?
create!( create!(
account:, account:,
email:, email:,
first_name: auth.info.first_name.presence || first, first_name: first_name.presence,
last_name: auth.info.last_name.presence || rest.join(' '), last_name: last_name.presence,
role: ADMIN_ROLE, role: ADMIN_ROLE,
password: Devise.friendly_token(40) password: Devise.friendly_token(40)
) )

@ -38,11 +38,15 @@ module Docuseal
CERTS = JSON.parse(ENV.fetch('CERTS', '{}')) CERTS = JSON.parse(ENV.fetch('CERTS', '{}'))
TIMESERVER_URL = ENV.fetch('TIMESERVER_URL', nil) TIMESERVER_URL = ENV.fetch('TIMESERVER_URL', nil)
def self.parse_email_domains(env_key)
ENV.fetch(env_key, '').split(',').map { |d| d.strip.downcase }.compact_blank.freeze
end
CLERK_DISCOVERY_URL = ENV.fetch('CLERK_DISCOVERY_URL', nil) CLERK_DISCOVERY_URL = ENV.fetch('CLERK_DISCOVERY_URL', nil)
CLERK_CLIENT_ID = ENV.fetch('CLERK_CLIENT_ID', nil) CLERK_CLIENT_ID = ENV.fetch('CLERK_CLIENT_ID', nil)
CLERK_CLIENT_SECRET = ENV.fetch('CLERK_CLIENT_SECRET', nil) CLERK_CLIENT_SECRET = ENV.fetch('CLERK_CLIENT_SECRET', nil)
CLERK_ALLOWED_EMAIL_DOMAINS = ENV.fetch('CLERK_ALLOWED_EMAIL_DOMAINS', '') CLERK_ALLOWED_EMAIL_DOMAINS = parse_email_domains('CLERK_ALLOWED_EMAIL_DOMAINS')
.split(',').map { |d| d.strip.downcase }.compact_blank.freeze CLERK_ADMIN_EMAIL_DOMAINS = parse_email_domains('CLERK_ADMIN_EMAIL_DOMAINS')
VERSION_FILE_PATH = Rails.root.join('.version') VERSION_FILE_PATH = Rails.root.join('.version')
VERSION_FILE2_PATH = Rails.public_path.join('version') VERSION_FILE2_PATH = Rails.public_path.join('version')
@ -71,11 +75,24 @@ module Docuseal
CLERK_DISCOVERY_URL.present? && CLERK_CLIENT_ID.present? && CLERK_CLIENT_SECRET.present? CLERK_DISCOVERY_URL.present? && CLERK_CLIENT_ID.present? && CLERK_CLIENT_SECRET.present?
end end
def clerk_email_allowed?(email) # Fail closed: an unset/empty allowlist matches no one rather than everyone.
return true if CLERK_ALLOWED_EMAIL_DOMAINS.empty? def email_in_domains?(email, allowed_domains)
return false if allowed_domains.empty?
domain = email.to_s.split('@').last.to_s.downcase domain = email.to_s.split('@').last.to_s.downcase
CLERK_ALLOWED_EMAIL_DOMAINS.include?(domain) return false if domain.blank?
allowed_domains.include?(domain)
end
def clerk_email_allowed?(email)
email_in_domains?(email, CLERK_ALLOWED_EMAIL_DOMAINS)
end
# Admin must be granted explicitly via its own allowlist; never inferred from
# mere sign-in eligibility.
def clerk_email_admin?(email)
email_in_domains?(email, CLERK_ADMIN_EMAIL_DOMAINS)
end end
def advanced_formats? def advanced_formats?

@ -0,0 +1,108 @@
# frozen_string_literal: true
require 'rails_helper'
RSpec.describe User do
describe '.from_clerk_oidc' do
let!(:account) { create(:account) }
def auth_for(email, name: 'Jane Doe')
OmniAuth::AuthHash.new(
provider: 'clerk_oidc',
info: { email:, name:, first_name: nil, last_name: nil }
)
end
before do
stub_const('Docuseal::CLERK_ALLOWED_EMAIL_DOMAINS', ['bloombilt.com'])
stub_const('Docuseal::CLERK_ADMIN_EMAIL_DOMAINS', ['bloombilt.com'])
end
it 'returns nil for a blank email' do
expect(described_class.from_clerk_oidc(auth_for(''))).to be_nil
end
it 'rejects an email whose domain is not on the allowlist' do
expect { described_class.from_clerk_oidc(auth_for('intruder@evil.com')) }
.not_to change(described_class, :count)
expect(described_class.from_clerk_oidc(auth_for('intruder@evil.com'))).to be_nil
end
it 'provisions a first-time admin-allowlisted user as admin' do
user = described_class.from_clerk_oidc(auth_for('founder@bloombilt.com'))
expect(user).to be_persisted
expect(user.role).to eq(User::ADMIN_ROLE)
expect(user.email).to eq('founder@bloombilt.com')
end
it 'refuses to provision an allowed-but-not-admin user instead of minting an admin' do
stub_const('Docuseal::CLERK_ADMIN_EMAIL_DOMAINS', ['admins.bloombilt.com'])
expect { described_class.from_clerk_oidc(auth_for('staff@bloombilt.com')) }
.not_to change(described_class, :count)
expect(described_class.from_clerk_oidc(auth_for('staff@bloombilt.com'))).to be_nil
end
it 'returns an existing active user without creating a new one' do
existing = create(:user, account:, email: 'founder@bloombilt.com', role: User::ADMIN_ROLE)
expect { described_class.from_clerk_oidc(auth_for('FOUNDER@bloombilt.com')) }
.not_to change(described_class, :count)
expect(described_class.from_clerk_oidc(auth_for('FOUNDER@bloombilt.com'))).to eq(existing)
end
end
describe '.provision_clerk_admin' do
before { stub_const('Docuseal::CLERK_ADMIN_EMAIL_DOMAINS', ['bloombilt.com']) }
it 'creates an admin for an admin-allowlisted email' do
account = create(:account)
user = described_class.provision_clerk_admin(
email: 'Founder@Bloombilt.com', first_name: 'Founder', last_name: 'One'
)
expect(user).to be_persisted
expect(user.role).to eq(User::ADMIN_ROLE)
expect(user.email).to eq('founder@bloombilt.com')
expect(user.account).to eq(account)
end
it 'refuses to provision an email that is not admin-allowlisted' do
create(:account)
expect { described_class.provision_clerk_admin(email: 'rando@evil.com', first_name: 'R', last_name: 'O') }
.not_to change(described_class, :count)
end
it 'returns nil when no account exists' do
expect(described_class.provision_clerk_admin(email: 'founder@bloombilt.com', first_name: 'F', last_name: 'O'))
.to be_nil
end
end
describe '.clerk_email_allowed? (via Docuseal)' do
it 'fails closed when the allowlist is empty' do
stub_const('Docuseal::CLERK_ALLOWED_EMAIL_DOMAINS', [])
expect(Docuseal.clerk_email_allowed?('anyone@anywhere.com')).to be(false)
end
it 'allows a listed domain and rejects an unlisted one' do
stub_const('Docuseal::CLERK_ALLOWED_EMAIL_DOMAINS', ['bloombilt.com'])
expect(Docuseal.clerk_email_allowed?('a@bloombilt.com')).to be(true)
expect(Docuseal.clerk_email_allowed?('a@evil.com')).to be(false)
end
end
describe '.clerk_email_admin? (via Docuseal)' do
it 'grants no one when the admin allowlist is empty' do
stub_const('Docuseal::CLERK_ADMIN_EMAIL_DOMAINS', [])
expect(Docuseal.clerk_email_admin?('founder@bloombilt.com')).to be(false)
end
it 'grants only listed domains' do
stub_const('Docuseal::CLERK_ADMIN_EMAIL_DOMAINS', ['bloombilt.com'])
expect(Docuseal.clerk_email_admin?('founder@bloombilt.com')).to be(true)
expect(Docuseal.clerk_email_admin?('founder@other.com')).to be(false)
end
end
end
Loading…
Cancel
Save