diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index bc0e4082..6314ee10 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -10,6 +10,7 @@ def current_user = nil # TODO: this is a temp hack to fix partials until /backen helper_method :detected_country_alpha2 + prepend_before_action :set_current_identity_session before_action :invalidate_v1_sessions, :authenticate_identity!, :set_honeybadger_context before_action :set_paper_trail_whodunnit @@ -115,6 +116,10 @@ def current_onboarding_step private + def set_current_identity_session + Current.identity_session = current_session + end + def touch_session_last_seen_at current_session&.touch_last_seen_at end diff --git a/app/models/current.rb b/app/models/current.rb new file mode 100644 index 00000000..5873e111 --- /dev/null +++ b/app/models/current.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +class Current < ActiveSupport::CurrentAttributes + attribute :identity_session +end diff --git a/config/initializers/doorkeeper_oidc_patches.rb b/config/initializers/doorkeeper_oidc_patches.rb new file mode 100644 index 00000000..04cbe4d8 --- /dev/null +++ b/config/initializers/doorkeeper_oidc_patches.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +# These patches thread the current identity session through doorkeeper-openid_connect's +# ID token generation, so auth_time reflects the actual session that authorized the +# request — not just the most recently created session for the identity. +# +# The flow: +# 1. Authorization endpoint (user in browser): Current.identity_session is set by +# ApplicationController. The grant is stamped with source_session_id. +# 2. Token endpoint (RP server exchanging auth code): No user session cookie, but +# we load the source session from the grant and set it directly on the IdToken. +# 3. IdToken checks @source_session (from grant) then Current.identity_session +# (from cookie). Returns nil if neither is available — we don't guess. + +Rails.application.config.to_prepare do + # Stamp source_session_id on the grant at authorization time + Doorkeeper::OAuth::Authorization::Code.prepend(Module.new do + private + + def access_grant_attributes + super.merge(source_session_id: Current.identity_session&.id) + end + end) + + # At code exchange, set the source session directly on the IdToken from the grant. + # We call super first (openid_connect creates the IdToken), then attach the session. + # Current.identity_session stays honest — it only means "the session behind this request." + Doorkeeper::OAuth::AuthorizationCodeRequest.prepend(Module.new do + private + + def after_successful_response + super + if grant.source_session_id && @response.id_token + session = IdentitySession.find_by(id: grant.source_session_id) + @response.id_token.instance_variable_set(:@source_session, session) + end + end + end) + + # Use the real session for auth_time: @source_session (from grant, set during code + # exchange) or Current.identity_session (from cookie, set during controller flows). + # Returns nil if neither is available — don't guess, don't lie. + Doorkeeper::OpenidConnect::IdToken.prepend(Module.new do + private + + def auth_time + session = @source_session || Current.identity_session + return nil unless session + + [ session.created_at, session.last_step_up_at ].compact.max.to_i + end + end) +end diff --git a/config/initializers/doorkeeper_openid_connect.rb b/config/initializers/doorkeeper_openid_connect.rb index 3f0ad8d0..c4d7a52b 100644 --- a/config/initializers/doorkeeper_openid_connect.rb +++ b/config/initializers/doorkeeper_openid_connect.rb @@ -26,15 +26,15 @@ end auth_time_from_resource_owner do |resource_owner| - session = resource_owner.sessions.not_expired.order(created_at: :desc).first - return nil unless session + session = Current.identity_session + next nil unless session [ session.created_at, session.last_step_up_at ].compact.max end reauthenticate_resource_owner do |resource_owner, return_to| - session = resource_owner.sessions.not_expired.order(created_at: :desc).first - return if session&.last_step_up_at&.after?(60.seconds.ago) + session = Current.identity_session + next if session&.last_step_up_at&.after?(60.seconds.ago) redirect_to new_step_up_path(action_type: "oidc_reauth", return_to: return_to) end diff --git a/db/migrate/20260328000001_add_source_session_id_to_oauth_access_grants.rb b/db/migrate/20260328000001_add_source_session_id_to_oauth_access_grants.rb new file mode 100644 index 00000000..504dc88f --- /dev/null +++ b/db/migrate/20260328000001_add_source_session_id_to_oauth_access_grants.rb @@ -0,0 +1,6 @@ +class AddSourceSessionIdToOAuthAccessGrants < ActiveRecord::Migration[8.0] + def change + add_column :oauth_access_grants, :source_session_id, :bigint + add_foreign_key :oauth_access_grants, :identity_sessions, column: :source_session_id, on_delete: :nullify + end +end diff --git a/db/schema.rb b/db/schema.rb index 4c54b03c..60f0fe39 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.0].define(version: 2026_03_24_000001) do +ActiveRecord::Schema[8.0].define(version: 2026_03_28_000001) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" enable_extension "pg_trgm" @@ -477,6 +477,7 @@ t.datetime "created_at", null: false t.datetime "revoked_at" t.string "resource_owner_type", null: false + t.bigint "source_session_id" t.index ["application_id"], name: "index_oauth_access_grants_on_application_id" t.index ["resource_owner_id", "resource_owner_type"], name: "polymorphic_owner_oauth_access_grants" t.index ["resource_owner_id"], name: "index_oauth_access_grants_on_resource_owner_id" @@ -629,6 +630,7 @@ add_foreign_key "login_attempts", "identities" add_foreign_key "login_attempts", "identity_sessions", column: "session_id" add_foreign_key "oauth_access_grants", "identities", column: "resource_owner_id" + add_foreign_key "oauth_access_grants", "identity_sessions", column: "source_session_id", on_delete: :nullify add_foreign_key "oauth_access_grants", "oauth_applications", column: "application_id" add_foreign_key "oauth_access_tokens", "identities", column: "resource_owner_id" add_foreign_key "oauth_access_tokens", "oauth_applications", column: "application_id" diff --git a/spec/factories/identity_sessions.rb b/spec/factories/identity_sessions.rb new file mode 100644 index 00000000..9febf674 --- /dev/null +++ b/spec/factories/identity_sessions.rb @@ -0,0 +1,16 @@ +FactoryBot.define do + factory :identity_session do + association :identity + session_token { SecureRandom.urlsafe_base64 } + expires_at { 1.month.from_now } + + trait :expired do + expires_at { 1.hour.ago } + end + + trait :stepped_up do + last_step_up_at { Time.current } + last_step_up_action { "oidc_reauth" } + end + end +end diff --git a/spec/requests/oidc_auth_time_spec.rb b/spec/requests/oidc_auth_time_spec.rb new file mode 100644 index 00000000..37c24866 --- /dev/null +++ b/spec/requests/oidc_auth_time_spec.rb @@ -0,0 +1,152 @@ +require "rails_helper" + +RSpec.describe "OIDC auth_time", type: :request do + let(:identity) { create(:identity) } + let(:program) { create(:program, scopes: "openid email") } + let(:session) { create(:identity_session, identity: identity) } + + # Stub current_session; current_identity and identity_signed_in? derive from it + def sign_in_as(identity_session) + allow_any_instance_of(SessionsHelper).to receive(:current_session).and_return(identity_session) + end + + before do + sign_in_as(session) + + # ensure a signing key exists for ID token generation (may not be set in CI) + unless ENV["OIDC_SIGNING_KEY"].present? + key = OpenSSL::PKey::RSA.generate(2048).to_pem + Doorkeeper::OpenidConnect.configuration.instance_variable_set(:@signing_key, key) + end + end + + after { Current.reset_all } + + def expected_auth_time(s) + [ s.created_at, s.last_step_up_at ].compact.max.to_i + end + + def decode_id_token(jwt) + JWT.decode(jwt, nil, false).first + end + + # POST to /oauth/authorize to create the grant, returns [grant, code_string] + def authorize!(extra_params = {}) + post "/oauth/authorize", params: { + client_id: program.uid, + redirect_uri: program.redirect_uri, + response_type: "code", + scope: "openid email", + nonce: "test-nonce" + }.merge(extra_params) + + # Extract the code from the redirect location + location = response.headers["Location"] + code = CGI.parse(URI.parse(location).query)["code"].first + grant = Doorkeeper::AccessGrant.order(:created_at).last + + [ grant, code ] + end + + def exchange_code!(code) + post "/oauth/token", params: { + grant_type: "authorization_code", + code: code, + redirect_uri: program.redirect_uri, + client_id: program.uid, + client_secret: program.secret + } + + JSON.parse(response.body) + end + + describe "authorization code flow" do + it "stamps source_session_id on the grant" do + grant, _code = authorize! + expect(grant).to be_present + expect(grant.source_session_id).to eq(session.id) + end + + it "returns correct auth_time after code exchange" do + _grant, code = authorize! + body = exchange_code!(code) + + expect(response).to have_http_status(:ok) + expect(body["id_token"]).to be_present + + claims = decode_id_token(body["id_token"]) + expect(claims["auth_time"]).to eq(expected_auth_time(session)) + end + + it "uses the authorizing session, not the newest session" do + newer_session = identity.sessions.create!( + session_token: SecureRandom.hex(32), + expires_at: 1.week.from_now, + created_at: 1.hour.from_now + ) + + _grant, code = authorize! + body = exchange_code!(code) + + claims = decode_id_token(body["id_token"]) + expect(claims["auth_time"]).to eq(expected_auth_time(session)) + expect(claims["auth_time"]).not_to eq(expected_auth_time(newer_session)) + end + + it "uses the original auth_time even if the source session has since expired" do + _grant, code = authorize! + session.update!(expires_at: 1.hour.ago) + + body = exchange_code!(code) + + claims = decode_id_token(body["id_token"]) + expect(claims["auth_time"]).to eq(expected_auth_time(session)) + end + + it "reflects last_step_up_at when more recent than created_at" do + session.update!(last_step_up_at: 1.minute.from_now) + + _grant, code = authorize! + body = exchange_code!(code) + + claims = decode_id_token(body["id_token"]) + expect(claims["auth_time"]).to eq(session.last_step_up_at.to_i) + end + end + + describe "IdToken without session context" do + it "returns nil auth_time when neither source is available" do + Current.identity_session = nil + + token = create(:oauth_token, resource_owner: identity, application: program, scopes: "openid email") + id_token = Doorkeeper::OpenidConnect::IdToken.new(token, "nonce") + + expect(id_token.as_json[:auth_time]).to be_nil + end + end + + describe "config blocks" do + describe "auth_time_from_resource_owner" do + let(:config_block) { Doorkeeper::OpenidConnect.configuration.auth_time_from_resource_owner } + + it "returns auth_time from Current.identity_session" do + Current.identity_session = session + + # The block uses `return`, so we need instance_exec (same as doorkeeper does) + controller = Object.new + result = controller.instance_exec(identity, &config_block) + + expect(result).to eq([ session.created_at, session.last_step_up_at ].compact.max) + end + + it "returns nil when Current.identity_session is not set" do + Current.identity_session = nil + + controller = Object.new + result = controller.instance_exec(identity, &config_block) + + expect(result).to be_nil + end + end + end +end