Guard against OIDC Provider configuration being stale (#5296)

Fixes TOB-RGM-14

app/controllers/api/v1/oidc/api_key_roles_controller.rb

@@ -1,26 +1,20 @@
 class Api::V1::OIDC::ApiKeyRolesController < Api::BaseController
   include ApiKeyable
+  include JwtValidation
 
   before_action :authenticate_with_api_key, except: :assume_role
   before_action :verify_user_api_key, except: :assume_role
 
   with_options only: :assume_role do
     before_action :set_api_key_role
+    before_action :validate_provider
     before_action :decode_jwt
-    before_action :verify_jwt
+    before_action :validate_jwt_format
+    before_action :verify_jwt_time
+    before_action :verify_jwt_issuer
     before_action :verify_access
   end
 
-  class UnverifiedJWT < StandardError
-  end
-
-  rescue_from(
-    UnverifiedJWT,
-    JSON::JWT::VerificationFailed, JSON::JWK::Set::KidNotFound,
-    OIDC::AccessPolicy::AccessError,
-    with: :render_not_found
-  )
-
   rescue_from ActiveRecord::RecordInvalid do |err|
     render json: {
       errors: err.record.errors
@@ -67,18 +61,15 @@ def assume_role
 
   def set_api_key_role
     @api_key_role = OIDC::ApiKeyRole.active.find_by!(token: params.permit(:token).require(:token))
+    @provider = @api_key_role.provider
   end
 
-  def decode_jwt
-    raise UnverifiedJWT, "Provider missing JWKS" if @api_key_role.provider.jwks.blank?
-    @jwt = JSON::JWT.decode_compact_serialized(params.permit(:jwt).require(:jwt), @api_key_role.provider.jwks)
-  rescue JSON::ParserError
-    raise UnverifiedJWT, "Invalid JSON"
+  def jwt_key_or_secret
+    @provider.jwks
   end
 
-  def verify_jwt
-    raise UnverifiedJWT, "Issuer mismatch" unless @api_key_role.provider.issuer == @jwt["iss"]
-    raise UnverifiedJWT, "Invalid time" unless (@jwt["nbf"]..@jwt["exp"]).cover?(Time.now.to_i)
+  def verify_jwt_issuer
+    raise UnverifiedJWT, "Issuer mismatch" unless @provider.issuer == @jwt["iss"]
   end
 
   def verify_access

app/controllers/api/v1/oidc/trusted_publisher_controller.rb

@@ -1,26 +1,16 @@
 class Api::V1::OIDC::TrustedPublisherController < Api::BaseController
   include ApiKeyable
+  include JwtValidation
 
   before_action :decode_jwt
   before_action :validate_jwt_format
+  before_action :verify_jwt_time
   before_action :find_provider
+  before_action :validate_provider
   before_action :verify_signature
   before_action :find_trusted_publisher
   before_action :validate_claims
 
-  class UnsupportedIssuer < StandardError; end
-  class UnverifiedJWT < StandardError; end
-  class InvalidJWT < StandardError; end
-
-  rescue_from InvalidJWT, with: :render_bad_request
-
-  rescue_from(
-    UnsupportedIssuer, UnverifiedJWT,
-    JSON::JWT::VerificationFailed, JSON::JWK::Set::KidNotFound,
-    OIDC::AccessPolicy::AccessError,
-    with: :render_not_found
-  )
-
   def exchange_token
     key = generate_unique_rubygems_key
     iat = Time.at(@jwt[:iat].to_i, in: "UTC")
@@ -42,29 +32,15 @@ def exchange_token
 
   private
 
-  def decode_jwt
-    @jwt = JSON::JWT.decode_compact_serialized(params.permit(:jwt).require(:jwt), :skip_verification)
-  rescue JSON::JWT::InvalidFormat, JSON::ParserError, ArgumentError => e
-    # invalid base64 raises ArgumentError
-    render_bad_request(e)
-  end
-
-  def validate_jwt_format
-    %w[nbf iat exp].each do |claim|
-      raise InvalidJWT, "Missing/invalid #{claim}" unless @jwt[claim].is_a?(Integer)
-    end
-    %w[iss jti].each do |claim|
-      raise InvalidJWT, "Missing/invalid #{claim}" unless @jwt[claim].is_a?(String)
-    end
+  def jwt_key_or_secret
+    :skip_verification
   end
 
   def find_provider
     @provider = OIDC::Provider.find_by!(issuer: @jwt[:iss])
   end
 
   def verify_signature
-    raise UnsupportedIssuer, "Provider is missing jwks" if @provider.jwks.blank?
-    raise UnverifiedJWT, "Invalid time" unless (@jwt["nbf"]..@jwt["exp"]).cover?(Time.now.to_i)
     @jwt.verify!(@provider.jwks)
   end
 

app/controllers/concerns/jwt_validation.rb

@@ -0,0 +1,51 @@
+module JwtValidation
+  extend ActiveSupport::Concern
+
+  class UnsupportedIssuer < StandardError; end
+  class UnverifiedJWT < StandardError; end
+  class InvalidJWT < StandardError; end
+
+  included do
+    rescue_from InvalidJWT, with: :render_bad_request
+
+    rescue_from(
+      UnsupportedIssuer, UnverifiedJWT,
+      JSON::JWT::VerificationFailed, JSON::JWK::Set::KidNotFound,
+      OIDC::AccessPolicy::AccessError,
+      with: :render_not_found
+    )
+  end
+
+  def jwt_key_or_secret
+    raise NotImplementedError
+  end
+
+  def decode_jwt
+    @jwt = JSON::JWT.decode_compact_serialized(params.permit(:jwt).require(:jwt), jwt_key_or_secret)
+  rescue JSON::JWT::InvalidFormat, JSON::ParserError, ArgumentError => e
+    # invalid base64 raises ArgumentError
+    render_bad_request(e)
+  end
+
+  def validate_jwt_format
+    %w[nbf iat exp].each do |claim|
+      raise InvalidJWT, "Missing/invalid #{claim}" unless @jwt[claim].is_a?(Integer)
+    end
+    %w[iss jti].each do |claim|
+      raise InvalidJWT, "Missing/invalid #{claim}" unless @jwt[claim].is_a?(String)
+    end
+  end
+
+  def validate_provider
+    raise UnsupportedIssuer, "Provider is missing jwks" if @provider.jwks.blank?
+    # TODO: delete &. after all providers have updated their configuration
+    return unless @provider.configuration_updated_at&.before?(1.day.ago)
+    raise UnsupportedIssuer, "Configuration last updated too long ago: #{@provider.configuration_updated_at}"
+  end
+
+  def verify_jwt_time
+    now = Time.now.to_i
+    return if @jwt["nbf"] <= now && now < @jwt["exp"]
+    raise UnverifiedJWT, "Invalid time"
+  end
+end

app/jobs/refresh_oidc_provider_job.rb

@@ -21,6 +21,7 @@ def perform(provider:)
       raise JWKSURIMismatchError, "Invalid JWKS URI in OpenID Connect configuration #{provider.configuration.jwks_uri.inspect}"
     end
     provider.jwks = connection.get(provider.configuration.jwks_uri).body
+    provider.configuration_updated_at = Time.current
 
     provider.save!
   end

db/migrate/20240910152643_add_configuration_updated_at_to_oidc_providers.rb

@@ -0,0 +1,5 @@
+class AddConfigurationUpdatedAtToOIDCProviders < ActiveRecord::Migration[7.1]
+  def change
+    add_column :oidc_providers, :configuration_updated_at, :timestamp
+  end
+end

db/schema.rb

@@ -395,6 +395,7 @@
     t.jsonb "jwks"
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
+    t.datetime "configuration_updated_at", precision: nil
     t.index ["issuer"], name: "index_oidc_providers_on_issuer", unique: true
   end
 

test/integration/api/v1/oidc/trusted_publisher_controller_test.rb

@@ -168,6 +168,20 @@ def jwt(claims = @claims, key: @pkey)
       assert_response :not_found
     end
 
+    should "return not found when provider configuration is too old" do
+      OIDC::Provider.github_actions.update!(configuration_updated_at: 2.days.ago)
+      trusted_publisher = build(:oidc_trusted_publisher_github_action,
+        repository_name: "oidc-test",
+        repository_owner_id: "1946610",
+        workflow_filename: "token.yml")
+      trusted_publisher.repository_owner = "segiddins"
+      trusted_publisher.save!
+      post api_v1_oidc_trusted_publisher_exchange_token_path,
+        params: { jwt: jwt.to_s }
+
+      assert_response :not_found
+    end
+
     should "return not found when signature validation fails" do
       @claims["exp"] -= 1_000_000
       trusted_publisher = build(:oidc_trusted_publisher_github_action,