Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for two OpenID Connect key-pairs #11626

Merged
merged 11 commits into from
Dec 13, 2024
27 changes: 17 additions & 10 deletions app/forms/openid_connect_logout_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -86,22 +86,29 @@ def load_identity
if reject_id_token_hint?
identity_from_client_id
else
identity_from_token_hint(id_token_hint) || identity_from_client_id
end
end

def identity_from_token_hint(id_token_hint)
return nil if id_token_hint.blank?
payload, _headers = nil

Rails.application.config.oidc_public_key_queue.compact.find do |key|
payload, _headers = JWT.decode(
id_token_hint, AppArtifacts.store.oidc_public_key, true,
id_token_hint, key, true,
algorithm: 'RS256',
leeway: Float::INFINITY
).map(&:with_indifferent_access)

identity_from_payload(payload) || identity_from_client_id
rescue JWT::DecodeError
next
end
rescue JWT::DecodeError
nil
end

def identity_from_payload(payload)
uuid = payload[:sub]
sp = payload[:aud]
AgencyIdentityLinker.sp_identity_from_uuid_and_sp(uuid, sp)
if payload
uuid = payload[:sub]
sp = payload[:aud]
AgencyIdentityLinker.sp_identity_from_uuid_and_sp(uuid, sp)
end
end

def id_token_hint_or_client_id_present
Expand Down
20 changes: 8 additions & 12 deletions app/presenters/openid_connect_certs_presenter.rb
Original file line number Diff line number Diff line change
@@ -1,20 +1,16 @@
# frozen_string_literal: true

class OpenidConnectCertsPresenter
KEYS = Rails.application.config.oidc_public_key_queue.map do |key|
{
alg: 'RS256',
use: 'sig',
}.merge(JWT::JWK.new(key).export)
end.freeze

def certs
{
keys: keys,
keys: KEYS,
}
end

private

def keys
[AppArtifacts.store.oidc_public_key].map do |key|
{
alg: 'RS256',
use: 'sig',
}.merge(JWT::JWK.new(key).export)
end
end
end
4 changes: 2 additions & 2 deletions app/services/id_token_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ def initialize(identity:, code:, custom_expiration: nil, now: Time.zone.now)
def id_token
JWT.encode(
jwt_payload,
AppArtifacts.store.oidc_private_key,
AppArtifacts.store.oidc_primary_private_key,
'RS256',
kid: JWT::JWK.new(AppArtifacts.store.oidc_private_key).kid,
kid: JWT::JWK.new(AppArtifacts.store.oidc_primary_private_key).kid,
)
end

Expand Down
4 changes: 2 additions & 2 deletions app/services/push_notification/http_push.rb
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,10 @@ def jwt(service_provider)

JWT.encode(
payload,
AppArtifacts.store.oidc_private_key,
AppArtifacts.store.oidc_primary_private_key,
'RS256',
typ: 'secevent+jwt',
kid: JWT::JWK.new(AppArtifacts.store.oidc_private_key).kid,
kid: JWT::JWK.new(AppArtifacts.store.oidc_primary_private_key).kid,
)
end

Expand Down
30 changes: 30 additions & 0 deletions config/artifacts.example/local/oidc_secondary.key
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
This is a public example key used for testing and local development.
Even though it appears to be a private key, it is not secret and is intended to be public.

-----BEGIN RSA PRIVATE KEY-----
MIIEpQIBAAKCAQEA1a4Vx1KKS9ByDlNgX+Z7hiojyA9U5f8iu4wNesiOdtWmHG9O
d7QsIpPV2A/yf7Y0X2nEWAABdHtAmrvmmJbP1aofmhaxAmYQXYJSloACZFA5qVdT
cV2jKA3+ZCCgZDj6bXWhERFQRIBbKIc7VWtxM/M5lhpnTEsy55XIN0iXoq1cM539
GtKp5XfGTQ1Pv9ovCJ1//WH1vBnhXwd62kExk+Snp/9X+eLBoguQcNofphQOsLIu
b7t9YuqI9xbQZDCfy280R3jV6K9g/ZlIoKfE18q9MI5n4U7L8W0/DopImyYCJltM
zcJtcvSUVCapOK+bnwnnaDvke67lA1kyReZnmwIDAQABAoIBAQC1xR9VrctjbvB3
a0nCishtew9xMkmgVYdwT1VwK4e1Y02pRq5TeftJdsUkxXweVBr9R3X0/hw+wFJW
zpz9FN27/rpfVApD5hrp0OD8kex29R/4BAdBmsweWLkc5/xJBYdS8guP/1Bu1Vm2
gkNhCMMF1FQacl+JMTcedfYZwTDs+kjX4i5ccY9xXdHQSYikCSZOcrcqD5949FZd
3SjIMrYaY8Mf6WFsWXMzsXNef7eVoeK7MLPbe9+jsdyrswk/3qMrUSmYG+hVSkCp
lO0S3ZD+INbOHQe0x3v88rNMJVe6VrwFMauDDXs3s7TVelqDjsyA8uAKCUR+amVj
9/6e7d4RAoGBAP8RZlOVTkfKxgZPjoYEOVq9kMRCbF+4S/6ZGflpNBEQAPyegooh
5GP7k/QzqA+D9wHIl7ATslW3i3CNOnhIX77TuRanErtQ6KMSKoX4I+FBJyZWpFml
FF9t5xkRrUHLk3dP925y/ZguuQHWOsO4jZOn0lSYGYSCYTwoR+X4jX7PAoGBANZ1
+DECFAA6R+/0/neHuzHOFukrIJicekBm0mkIq333DjXAZBDZsqNG6lbfpNCe5Z/J
zv0BCL7m50ypmRh6jp8lqndm8A/ONy4XkB9QrYd+ftTIZGm3bvFdjgbqNUGplfwQ
yFF/mcvY/2y48M9Uwr+SDPJZNzhGAxQQ/3zjwh11AoGAXziNrNJmYOLQPnbgzCMz
ji6KptntP2a76BYb3kJqD5yb4bMDJLI2YyT+PQIz0WcAhYfvKOqRfvKAecofc9wA
8mp0BILmuUshLg+QFGdobaU3Clb6EAVSr7WFupQgzBlFuhr+UhtXlMKMiqUBVyPE
psTV/oKxtAhAaIbZIH9Dw30CgYEAwgMG/I78uRgAbDwe2NOZrXzbjSTO4EDu98QN
JagKPHJ8EHR0EipfSQamiODZoUGeSeevsYJ1/v200c28CkEVNTRF+q7NDf9oO/Jl
F29NDP6KjsSa0mh3nTMdgXPvqe9ZGCe2kMP2xksRB7JnZ6kuZVAjFjtPkUEFF+oo
tzr0KOECgYEA7jLXEmtJZtdaHRzgEb2DxwNiHzfUg7mrBn3tbfB5V3yDdRtMzXRk
ASMMc7n+wh553Ms9A8X/75yX/Lt2+viA0q8L/ideZl7b1Of5I4E4c92WGQ0P6x6E
9JV6vfZzsKjmOBux52vvxjCUHLaBEL6y8e4mteMgvQJbY8aimU19dgU=
-----END RSA PRIVATE KEY-----
9 changes: 9 additions & 0 deletions config/artifacts.example/local/oidc_secondary.pub
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
-----BEGIN PUBLIC KEY-----
MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA1a4Vx1KKS9ByDlNgX+Z7
hiojyA9U5f8iu4wNesiOdtWmHG9Od7QsIpPV2A/yf7Y0X2nEWAABdHtAmrvmmJbP
1aofmhaxAmYQXYJSloACZFA5qVdTcV2jKA3+ZCCgZDj6bXWhERFQRIBbKIc7VWtx
M/M5lhpnTEsy55XIN0iXoq1cM539GtKp5XfGTQ1Pv9ovCJ1//WH1vBnhXwd62kEx
k+Snp/9X+eLBoguQcNofphQOsLIub7t9YuqI9xbQZDCfy280R3jV6K9g/ZlIoKfE
18q9MI5n4U7L8W0/DopImyYCJltMzcJtcvSUVCapOK+bnwnnaDvke67lA1kyReZn
mwIDAQAB
-----END PUBLIC KEY-----
52 changes: 46 additions & 6 deletions config/initializers/app_artifacts.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,52 @@
store.add_artifact(:saml_2024_cert, '/%<env>s/saml2024.crt')
store.add_artifact(:saml_2024_key, '/%<env>s/saml2024.key.enc')

store.add_artifact(:oidc_private_key, '/%<env>s/oidc.key') { |k| OpenSSL::PKey::RSA.new(k) }
store.add_artifact(:oidc_public_key, '/%<env>s/oidc.pub') { |k| OpenSSL::PKey::RSA.new(k) }
store.add_artifact(:oidc_primary_private_key, '/%<env>s/oidc.key') do |k|
OpenSSL::PKey::RSA.new(k)
end
store.add_artifact(:oidc_primary_public_key, '/%<env>s/oidc.pub') do |k|
OpenSSL::PKey::RSA.new(k)
end
store.add_artifact(
:oidc_secondary_private_key, '/%<env>s/oidc_secondary.key',
allow_missing: true
) do |k|
OpenSSL::PKey::RSA.new(k)
end
store.add_artifact(
:oidc_secondary_public_key, '/%<env>s/oidc_secondary.pub',
allow_missing: true
) do |k|
OpenSSL::PKey::RSA.new(k)
end
end

valid = OpenidConnectKeyValidation.valid?(
public_key: AppArtifacts.store.oidc_public_key,
private_key: AppArtifacts.store.oidc_private_key,
primary_valid = OpenidConnectKeyValidation.valid?(
public_key: AppArtifacts.store.oidc_primary_public_key,
private_key: AppArtifacts.store.oidc_primary_private_key,
)
raise 'OIDC Public/Private Keys do not match' if !valid
raise 'OIDC Primary Public/Private Keys do not match' if !primary_valid

secondary_valid =
(AppArtifacts.store.oidc_secondary_private_key.nil? &&
AppArtifacts.store.oidc_secondary_public_key.nil?) ||
OpenidConnectKeyValidation.valid?(
public_key: AppArtifacts.store.oidc_secondary_public_key,
private_key: AppArtifacts.store.oidc_secondary_private_key,
)
raise 'OIDC Secondary Public/Private Keys are invalid' if !secondary_valid

Rails.application.configure do
config.oidc_public_key = AppArtifacts.store.oidc_primary_public_key
config.oidc_private_key = AppArtifacts.store.oidc_primary_private_key

config.oidc_public_key_queue = [
AppArtifacts.store.oidc_primary_public_key,
AppArtifacts.store.oidc_secondary_public_key,
].compact.freeze

config.oidc_private_key_queue = [
AppArtifacts.store.oidc_primary_private_key,
AppArtifacts.store.oidc_secondary_private_key,
].compact.freeze
end
7 changes: 4 additions & 3 deletions lib/app_artifacts.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,11 @@ def build

# @param [Symbol] name
# @param [String] path
def add_artifact(name, path)
# @param [Boolean] allow_missing
def add_artifact(name, path, allow_missing: false)
value = read_artifact(path)
raise MissingArtifactError.new("missing artifact: #{path}") if value.nil?
value = yield(value) if block_given?
raise MissingArtifactError.new("missing artifact: #{path}") if value.nil? && !allow_missing
value = yield(value) if block_given? && value
@artifacts[name] = value
nil
end
Expand Down
18 changes: 17 additions & 1 deletion spec/forms/openid_connect_logout_form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,25 @@
end
end

context 'with a valid payload that was signed with the secondary OIDC key' do
let(:id_token_hint) do
JWT.encode(
{ sub: identity.uuid, aud: identity.service_provider },
AppArtifacts.store.oidc_secondary_private_key, 'RS256'
)
end

it 'is valid' do
expect(valid?).to eq(true)
end
end

context 'with a payload that does not correspond to an identity' do
let(:id_token_hint) do
JWT.encode({ sub: '123', aud: '456' }, AppArtifacts.store.oidc_private_key, 'RS256')
JWT.encode(
{ sub: '123', aud: '456' },
AppArtifacts.store.oidc_primary_private_key, 'RS256'
)
end

it 'is not valid' do
Expand Down
2 changes: 1 addition & 1 deletion spec/forms/openid_connect_token_form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
let(:client_private_key) do
OpenSSL::PKey::RSA.new(Rails.root.join('keys', 'saml_test_sp.key').read)
end
let(:server_public_key) { AppArtifacts.store.oidc_public_key }
let(:server_public_key) { Rails.application.config.oidc_public_key }

let(:user) { create(:user) }

Expand Down
2 changes: 1 addition & 1 deletion spec/forms/security_event_form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@

context 'when signed with a different key than registered to the SP' do
let(:rp_private_key) do
OpenSSL::PKey::RSA.new(AppArtifacts.store.oidc_private_key)
AppArtifacts.store.oidc_primary_private_key
end

it 'is invalid' do
Expand Down
14 changes: 14 additions & 0 deletions spec/lib/app_artifacts_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,20 @@
AppArtifacts::MissingArtifactError, 'missing artifact: /%<env>s/test_artifact'
)
end

context 'with allow_missing: true' do
it 'does not raise an error if an artifact is missing' do
expect(secrets_s3).to receive(:read_file).with(
'/%<env>s/test_artifact',
).and_return(nil)

store = instance.build do |store|
store.add_artifact(:test_artifact, '/%<env>s/test_artifact', allow_missing: true)
end

expect(store.test_artifact).to eq nil
end
end
end

context 'when running locally' do
Expand Down
16 changes: 11 additions & 5 deletions spec/presenters/openid_connect_certs_presenter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,23 @@
subject(:presenter) { OpenidConnectCertsPresenter.new }

describe '#certs' do
it 'renders the server public key as a JWK set' do
it 'renders the server public keys as a JWK set' do
json = presenter.certs

expect(json[:keys].size).to eq(1)
expect(json[:keys].size).to eq(2)
expect(json[:keys].all? { |k| k[:alg] == 'RS256' }).to eq(true)
expect(json[:keys].all? { |k| k[:use] == 'sig' }).to eq(true)

key_from_response = JWT::JWK.import(json[:keys].first).public_key
public_key = AppArtifacts.store.oidc_public_key
# Primary key should be first
primary_key_from_response, secondary_key_from_response = json[:keys].map do |key|
JWT::JWK.import(key).public_key
end

expect(key_from_response.to_pem).to eq(public_key.to_pem)
primary_public_key = Rails.application.config.oidc_public_key
expect(primary_key_from_response.to_pem).to eq(primary_public_key.to_pem)

secondary_public_key = Rails.application.config.oidc_public_key_queue.last
expect(secondary_key_from_response.to_pem).to eq(secondary_public_key.to_pem)
end
end
end
6 changes: 4 additions & 2 deletions spec/services/id_token_builder_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
let(:decoded_id_token) do
JWT.decode(
id_token,
AppArtifacts.store.oidc_public_key,
Rails.application.config.oidc_public_key,
true,
algorithm: 'RS256',
).map(&:with_indifferent_access)
Expand Down Expand Up @@ -224,7 +224,9 @@
end

it 'sets the kid for the signing key in the JWT headers' do
expect(decoded_headers[:kid]).to eq(JWT::JWK.new(AppArtifacts.store.oidc_private_key).kid)
expect(decoded_headers[:kid]).to eq(
JWT::JWK.new(AppArtifacts.store.oidc_primary_private_key).kid,
)
end
end
end
10 changes: 5 additions & 5 deletions spec/services/push_notification/http_push_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,14 @@

jwt_payload, headers = JWT.decode(
args[:jwt],
AppArtifacts.store.oidc_public_key,
Rails.application.config.oidc_public_key,
true,
algorithm: 'RS256',
kid: JWT::JWK.new(AppArtifacts.store.oidc_private_key).kid,
kid: JWT::JWK.new(AppArtifacts.store.oidc_primary_private_key).kid,
)

expect(headers['typ']).to eq('secevent+jwt')
expect(headers['kid']).to eq(JWT::JWK.new(AppArtifacts.store.oidc_private_key).kid)
expect(headers['kid']).to eq(JWT::JWK.new(AppArtifacts.store.oidc_primary_private_key).kid)

expect(jwt_payload['iss']).to eq(root_url)
expect(jwt_payload['iat']).to eq(now.to_i)
Expand All @@ -82,10 +82,10 @@
expect { deliver }.to have_enqueued_job(RiscDeliveryJob).with { |args|
jwt_payload, _headers = JWT.decode(
args[:jwt],
AppArtifacts.store.oidc_public_key,
Rails.application.config.oidc_public_key,
true,
algorithm: 'RS256',
kid: JWT::JWK.new(AppArtifacts.store.oidc_private_key).kid,
kid: JWT::JWK.new(AppArtifacts.store.oidc_primary_private_key).kid,
)
expect(jwt_payload['events'][event.event_type]['subject']['sub']).to eq(agency_uuid)
}
Expand Down
2 changes: 1 addition & 1 deletion spec/support/features/push_notifications_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ def stub_push_notification_request(sp_push_notification_endpoint:, event_type:,
stub_request(:post, sp_push_notification_endpoint).with do |request|
parsed_jwt, _jwt_headers = JWT.decode(
request.body,
AppArtifacts.store.oidc_public_key,
Rails.application.config.oidc_public_key,
true,
algorithm: 'RS256',
)
Expand Down
2 changes: 1 addition & 1 deletion spec/support/oidc_auth_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ def oidc_decoded_token
def oidc_decoded_id_token
@oidc_decoded_id_token ||= JWT.decode(
oidc_decoded_token[:id_token],
AppArtifacts.store.oidc_public_key,
Rails.application.config.oidc_public_key,
true,
algorithm: 'RS256',
).first.with_indifferent_access
Expand Down
4 changes: 2 additions & 2 deletions spec/support/private_key_file_helper.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
module PrivateKeyFileHelper
# Returns the private key in AppArtifacts.store.oidc_private_key if
# Returns the private key in AppArtifacts.store.oidc_primary_private_key if
# Identity::Hostdata.in_datacenter? or if the private key file does
# not exist; otherwise, the private key from the file is returned.
def private_key_from_store_or(file_name:)
Expand All @@ -12,7 +12,7 @@ def private_key_from_store_or(file_name:)
if File.exist?(file_name)
OpenSSL::PKey::RSA.new(File.read(file_name))
else
return AppArtifacts.store.oidc_private_key
return AppArtifacts.store.oidc_primary_private_key
end
end

Expand Down