diff --git a/Gemfile b/Gemfile index 8e1618e3..f52bd5bc 100644 --- a/Gemfile +++ b/Gemfile @@ -19,7 +19,7 @@ gem 'ipaddress' gem 'jaro_winkler', '~> 1.5.5' gem 'jquery-rails' gem 'jquery-ui-rails' -gem 'jwt', '~> 1.5', '>= 1.5.4' +gem 'jwt', '~> 2.5' gem 'lograge', '>=0.11.2' gem 'mutex_m' # Deprecation warning. gem 'netaddr', '~> 1.5', '>= 1.5.1' diff --git a/Gemfile.lock b/Gemfile.lock index d2086ede..4f0099b4 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -213,7 +213,8 @@ GEM json (2.18.1) jsonpath (0.5.8) multi_json - jwt (1.5.6) + jwt (2.10.2) + base64 language_server-protocol (3.17.0.5) lint_roller (1.1.0) listen (3.10.0) @@ -542,7 +543,7 @@ DEPENDENCIES jaro_winkler (~> 1.5.5) jquery-rails jquery-ui-rails - jwt (~> 1.5, >= 1.5.4) + jwt (~> 2.5) listen (~> 3.2) lograge (>= 0.11.2) mutex_m diff --git a/app/controllers/concerns/alma_jwt_validator.rb b/app/controllers/concerns/alma_jwt_validator.rb new file mode 100644 index 00000000..efee3ba5 --- /dev/null +++ b/app/controllers/concerns/alma_jwt_validator.rb @@ -0,0 +1,49 @@ +require 'jwt' +require 'net/http' +require 'json' + +module AlmaJwtValidator + JWKS_URL = 'https://api-na.hosted.exlibrisgroup.com/auth/01UCS_BER/jwks.json'.freeze + EXPECTED_ISS = 'https://api-na.hosted.exlibrisgroup.com/auth/01UCS_BER'.freeze + + module_function + + def jwk_set + Rails.cache.fetch('jwks_set', expires_in: 4.hour) do + jwks_raw = Net::HTTP.get(URI(JWKS_URL)) + jwks_keys = JSON.parse(jwks_raw)['keys'] + JWT::JWK::Set.new(jwks_keys) + end + end + + # rubocop:disable Metrics/MethodLength + def decode_and_verify_jwt(token) + # Decode header to get the 'kid' + header = JWT.decode(token, nil, false).last + kid = header['kid'] + + # Find the key from the JWK set + jwk = jwk_set.keys.find { |key| key.kid == kid } + raise JWT::VerificationError, 'Key not found in JWKS' unless jwk + + public_key = jwk.public_key + + options = { + algorithm: 'RS256', + verify_expiration: true, + verify_aud: false, + verify_iss: true, + iss: EXPECTED_ISS + } + + # Returns [payload, header] array if valid + JWT.decode(token, public_key, true, options) + rescue JWT::ExpiredSignature + raise JWT::VerificationError, 'Token has expired' + rescue JWT::InvalidIssuerError + raise JWT::VerificationError, 'Token issuer mismatch' + rescue JWT::DecodeError => e + raise JWT::VerificationError, "Invalid JWT: #{e.message}" + end + # rubocop:enable Metrics/MethodLength +end diff --git a/app/controllers/fees_controller.rb b/app/controllers/fees_controller.rb index 6309ffa9..4eabf0bc 100644 --- a/app/controllers/fees_controller.rb +++ b/app/controllers/fees_controller.rb @@ -5,16 +5,24 @@ class FeesController < ApplicationController # This will be needed for transaction_complete since Paypal will hit that protect_from_forgery with: :null_session + # rubocop:disable Metrics/MethodLength def index @jwt = params.require(:jwt) - decoded_token = JWT.decode @jwt, nil, false - @alma_id = decoded_token.first['userName'] - @fees = FeesPayment.new(alma_id: @alma_id) + payload = AlmaJwtValidator.decode_and_verify_jwt(@jwt) + @alma_id = payload.first['userName'] + begin + @fees = FeesPayment.new(alma_id: @alma_id) + rescue StandardError => e + Rails.logger.warn "FeesPayment failed: #{e.message}" + redirect_to(action: :transaction_error) and return + end rescue ActionController::ParameterMissing redirect_to 'https://www.lib.berkeley.edu/find/borrow-renew?section=pay-fees', allow_other_host: true - rescue JWT::DecodeError + rescue JWT::DecodeError => e + Rails.logger.warn "JWT verification failed: #{e.message}" redirect_to(action: :transaction_error) end + # rubocop:enable Metrics/MethodLength def efee @jwt = params.require(:jwt) diff --git a/spec/controllers/concerns/alma_jwt_validator_spec.rb b/spec/controllers/concerns/alma_jwt_validator_spec.rb new file mode 100644 index 00000000..13599e22 --- /dev/null +++ b/spec/controllers/concerns/alma_jwt_validator_spec.rb @@ -0,0 +1,100 @@ +require 'rails_helper' +require 'jwt' +require 'json' +require 'openssl' + +describe AlmaJwtValidator do + let(:alma_institution_code) { '01UCS_BER' } + let(:jwks_url) { "https://api-na.hosted.exlibrisgroup.com/auth/#{alma_institution_code}/jwks.json" } + let(:expected_iss) { "https://api-na.hosted.exlibrisgroup.com/auth/#{alma_institution_code}" } + + # Generate an RSA key pair for testing + let(:rsa_key) { OpenSSL::PKey::RSA.new(2048) } + let(:kid) { 'test-key-id' } + let(:test_payload) { { 'userName' => '10335026', 'iss' => expected_iss } } + + # Helper to create JWK from RSA public key + def create_jwk(public_key, kid) + n = Base64.urlsafe_encode64(public_key.n.to_s(2)).gsub(/=+$/, '') + e = Base64.urlsafe_encode64(public_key.e.to_s(2)).gsub(/=+$/, '') + { + 'kty' => 'RSA', + 'kid' => kid, + 'use' => 'sig', + 'alg' => 'RS256', + 'n' => n, + 'e' => e + } + end + + # Helper to generate a valid JWT + def generate_jwt(payload, key, kid, algorithm = 'RS256') + header = { 'kid' => kid, 'alg' => algorithm } + JWT.encode(payload, key, algorithm, header) + end + + before do + jwk = create_jwk(rsa_key.public_key, kid) + + stub_request(:get, jwks_url) + .to_return( + status: 200, + body: { 'keys' => [jwk] }.to_json, + headers: { 'Content-Type' => 'application/json' } + ) + end + + describe '.decode_and_verify_jwt' do + context 'with a valid JWT' do + it 'returns the decoded payload' do + token = generate_jwt(test_payload, rsa_key, kid) + result = AlmaJwtValidator.decode_and_verify_jwt(token) + + expect(result).to be_an(Array) + expect(result[0]['userName']).to eq('10335026') + expect(result[1]['kid']).to eq(kid) + end + end + + context 'with an invalid signature' do + it 'raises JWT::VerificationError' do + # Generate a token with a different key + different_key = OpenSSL::PKey::RSA.new(2048) + token = generate_jwt(test_payload, different_key, kid) + + expect do + AlmaJwtValidator.decode_and_verify_jwt(token) + end.to raise_error(JWT::VerificationError) + end + end + + context 'with an unknown key id' do + it 'raises JWT::VerificationError' do + token = generate_jwt(test_payload, rsa_key, 'unknown-kid') + + expect do + AlmaJwtValidator.decode_and_verify_jwt(token) + end.to raise_error(JWT::VerificationError) + end + end + + context 'with a malformed JWT' do + it 'raises JWT::DecodeError' do + expect do + AlmaJwtValidator.decode_and_verify_jwt('not.a.jwt') + end.to raise_error(JWT::DecodeError) + end + end + + context 'when JWKS endpoint is unreachable' do + it 'raises an error' do + stub_request(:get, jwks_url).to_return(status: 500) + token = generate_jwt(test_payload, rsa_key, kid) + + expect do + AlmaJwtValidator.decode_and_verify_jwt(token) + end.to raise_error(StandardError) + end + end + end +end diff --git a/spec/request/fees_request_spec.rb b/spec/request/fees_request_spec.rb index b54f9339..827f278c 100644 --- a/spec/request/fees_request_spec.rb +++ b/spec/request/fees_request_spec.rb @@ -9,7 +9,13 @@ def base_url_for(user_id = nil) let(:request_headers) { { 'Accept' => 'application/json', 'Authorization' => "apikey #{alma_api_key}" } } before do - allow(Rails.application.config).to receive(:alma_api_key).and_return(alma_api_key) + allow(AlmaJwtValidator).to receive(:decode_and_verify_jwt).and_return( + [{ 'userName' => '10335026' }] + ) + allow(Rails.application.config).to receive_messages( + alma_api_key: alma_api_key, + alma_jwt_secret: 'fake-jwt-secret' + ) end it 'redirects to the fallback URL if there is no jwt' do @@ -18,7 +24,8 @@ def base_url_for(user_id = nil) end it 'redirects to error page if request has a non-existant alma id' do - stub_request(:get, "#{base_url_for}fees") + user_id = '10335026' + stub_request(:get, "#{base_url_for(user_id)}/fees") .with(headers: request_headers) .to_return(status: 404, body: '') @@ -53,9 +60,10 @@ def base_url_for(user_id = nil) end it 'payments page redirects to index if no fee was selected for payment' do - post '/fees/payment', params: { jwt: File.read('spec/data/fees/alma-fees-jwt.txt') } + jwt = File.read('spec/data/fees/alma-fees-jwt.txt').strip + post '/fees/payment', params: { jwt: jwt } expect(response).to have_http_status(:found) - expect(response).to redirect_to("#{fees_path}?jwt=#{File.read('spec/data/fees/alma-fees-jwt.txt')}") + expect(response).to redirect_to("#{fees_path}?jwt=#{jwt}") end it 'successful transaction_complete returns status 200' do