Extract JWT parsing into separate class

JWT parsing is rather involved, because we need to fetch
proper certificates first. We will need to parse JWTs in
a different context than authorization as well,
so it makes sense to have the parsing centralized.

This also allowed to add specs for this previously
not (unit) tested piece of code.

lib_static/open_project/authentication/strategies/warden/jwt_oidc.rb

@@ -5,12 +5,6 @@ module Warden
         class JwtOidc < ::Warden::Strategies::Base
           include FailWithHeader
 
-          SUPPORTED_ALG = %w[
-            RS256
-            RS384
-            RS512
-          ].freeze
-
           # The strategy is supposed to only handle JWT.
           # These tokens are supposed to be issued by configured OIDC.
           def valid?
@@ -19,60 +13,29 @@ def valid?
             )
             return false if @access_token.blank?
 
-            @unverified_payload, @unverified_header = JWT.decode(@access_token, nil, false)
-            @unverified_header.present? && @unverified_payload.present?
+            unverified_payload, unverified_header = JWT.decode(@access_token, nil, false)
+            unverified_payload.present? && unverified_header.present?
           rescue JWT::DecodeError
             false
           end
 
           def authenticate!
-            issuer = @unverified_payload["iss"]
-            provider = OpenProject::OpenIDConnect.providers.find { |p| p.configuration[:issuer] == issuer } if issuer.present?
-            if provider.blank?
-              return fail_with_header!(error: "invalid_token", error_description: "The access token issuer is unknown")
-            end
-
-            client_id = provider.configuration.fetch(:identifier)
-            alg = @unverified_header.fetch("alg")
-            if SUPPORTED_ALG.exclude?(alg)
-              return fail_with_header!(error: "invalid_token", error_description: "Token signature algorithm is not supported")
-            end
-
-            kid = @unverified_header.fetch("kid")
-            jwks_uri = provider.configuration[:jwks_uri]
-            begin
-              key = JSON::JWK::Set::Fetcher.fetch(jwks_uri, kid:).to_key
-            rescue JSON::JWK::Set::KidNotFound
-              return fail_with_header!(error: "invalid_token", error_description: "The access token signature kid is unknown")
-            end
-
-            begin
-              verified_payload, = JWT.decode(
-                @access_token,
-                key,
-                true,
-                {
-                  algorithm: alg,
-                  verify_iss: true,
-                  verify_aud: true,
-                  iss: issuer,
-                  aud: client_id,
-                  required_claims: ["sub", "iss", "aud"]
-                }
-              )
-            rescue JWT::ExpiredSignature
-              return fail_with_header!(error: "invalid_token", error_description: "The access token expired")
-            rescue JWT::ImmatureSignature
-              # happens when nbf time is less than current
-              return fail_with_header!(error: "invalid_token", error_description: "The access token is used too early")
-            rescue JWT::InvalidIssuerError
-              return fail_with_header!(error: "invalid_token", error_description: "The access token issuer is wrong")
-            rescue JWT::InvalidAudError
-              return fail_with_header!(error: "invalid_token", error_description: "The access token audience claim is wrong")
-            end
+            verified_payload, provider = ::OpenIDConnect::ProviderTokenParser.new(required_claims: ["sub"])
+                                                                             .parse(@access_token)
 
-            user = User.find_by(identity_url: "#{provider.name}:#{verified_payload['sub']}")
+            user = User.find_by(identity_url: "#{provider.slug}:#{verified_payload['sub']}")
             success!(user) if user
+          rescue JWT::ExpiredSignature
+            fail_with_header!(error: "invalid_token", error_description: "The access token expired")
+          rescue JWT::ImmatureSignature
+            # happens when nbf time is less than current
+            fail_with_header!(error: "invalid_token", error_description: "The access token is used too early")
+          rescue JWT::InvalidAudError
+            fail_with_header!(error: "invalid_token", error_description: "The access token audience claim is wrong")
+          rescue JSON::JWK::Set::KidNotFound
+            fail_with_header!(error: "invalid_token", error_description: "The access token signature kid is unknown")
+          rescue ::OpenIDConnect::ProviderTokenParser::Error => e
+            fail_with_header!(error: "invalid_token", error_description: e.message)
           end
         end
       end

modules/openid_connect/app/services/openid_connect/provider_token_parser.rb

@@ -0,0 +1,95 @@
+#-- copyright
+# OpenProject is a project management system.
+# Copyright (C) the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2017 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See COPYRIGHT and LICENSE files for more details.
+# +
+
+module OpenIDConnect
+  class ProviderTokenParser
+    class Error < StandardError; end
+
+    SUPPORTED_JWT_ALGORITHMS = %w[
+      RS256
+      RS384
+      RS512
+    ].freeze
+
+    def initialize(verify_audience: true, required_claims: [])
+      @verify_audience = verify_audience
+      @required_claims = required_claims
+    end
+
+    def parse(token)
+      issuer, alg, kid = parse_unverified_iss_alg_kid(token)
+      raise Error, "Token signature algorithm #{alg} is not supported" if SUPPORTED_JWT_ALGORITHMS.exclude?(alg)
+
+      provider = fetch_provider(issuer)
+      raise Error, "The access token issuer is unknown" if provider.blank?
+
+      jwks_uri = provider.jwks_uri
+      key = JSON::JWK::Set::Fetcher.fetch(jwks_uri, kid:).to_key
+
+      verified_payload, = JWT.decode(
+        token,
+        key,
+        true,
+        {
+          algorithm: alg,
+          verify_aud: @verify_audience,
+          aud: provider.client_id,
+          required_claims: all_required_claims
+        }
+      )
+
+      [verified_payload, provider]
+    end
+
+    private
+
+    def parse_unverified_iss_alg_kid(token)
+      unverified_payload, unverified_header = JWT.decode(token, nil, false)
+      raise Error, "The token's Key Identifier (kid) is missing" unless unverified_header.key?("kid")
+
+      [
+        unverified_payload["iss"],
+        unverified_header.fetch("alg"),
+        unverified_header.fetch("kid")
+      ]
+    end
+
+    def fetch_provider(issuer)
+      return nil if issuer.blank?
+
+      OpenIDConnect::Provider.where(available: true).find { |p| p.issuer == issuer }
+    end
+
+    def all_required_claims
+      claims = ["iss"] + @required_claims
+      claims << "aud" if @verify_audience
+
+      claims.uniq
+    end
+  end
+end

modules/openid_connect/spec/services/openid_connect/provider_token_parser_spec.rb

@@ -0,0 +1,147 @@
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+require "spec_helper"
+
+RSpec.describe OpenIDConnect::ProviderTokenParser do
+  subject(:parse) { described_class.new.parse(token) }
+
+  let(:private_key) { OpenSSL::PKey::RSA.generate(2048) }
+  let(:payload) { { "sub" => "M. Curie", "iss" => "International Space Station", "aud" => our_client_id } }
+  let(:token) { JWT.encode(payload, private_key, "RS256", { kid: "key-identifier" }) }
+
+  let!(:provider) { create(:oidc_provider) }
+  let(:known_issuer) { "International Space Station" }
+  let(:our_client_id) { "openproject.org" }
+
+  before do
+    allow(JSON::JWK::Set::Fetcher).to receive(:fetch).and_return(
+      instance_double(JSON::JWK, to_key: private_key.public_key)
+    )
+
+    provider.options["issuer"] = known_issuer
+    provider.options["client_id"] = our_client_id
+    provider.options["jwks_uri"] = "https://example.com/certs"
+    provider.save!
+  end
+
+  it "parses the token" do
+    parsed, = parse
+    expect(parsed).to eq payload
+  end
+
+  it "returns the provider configuration for the associated provider" do
+    _, p = parse
+    expect(p).to eq provider
+  end
+
+  it "correctly queries for the token's public key" do
+    parse
+
+    expect(JSON::JWK::Set::Fetcher).to have_received(:fetch).with("https://example.com/certs", kid: "key-identifier")
+  end
+
+  context "when the provider signing the token is not known" do
+    let(:known_issuer) { "Lunar Gateway" }
+
+    it "raises an error" do
+      expect { parse }.to raise_error(OpenIDConnect::ProviderTokenParser::Error, /issuer is unknown/)
+    end
+  end
+
+  context "when the provider signing the token is not available" do
+    before do
+      provider.update!(available: false)
+    end
+
+    it "raises an error" do
+      expect { parse }.to raise_error(OpenIDConnect::ProviderTokenParser::Error, /issuer is unknown/)
+    end
+  end
+
+  context "when the token is not a valid JWT" do
+    let(:token) { Base64.encode64("banana").strip }
+
+    it "raises an error" do
+      expect { parse }.to raise_error(JWT::DecodeError)
+    end
+  end
+
+  context "when the token is signed using an unsupported signature" do
+    let(:token) { JWT.encode(payload, "secret", "HS256", { kid: "key-identifier" }) }
+
+    it "raises an error" do
+      expect { parse }.to raise_error(OpenIDConnect::ProviderTokenParser::Error, /HS256 is not supported/)
+    end
+  end
+
+  context "when we are not the token's audience" do
+    before do
+      payload["aud"] = "Alice"
+    end
+
+    it "raises an error" do
+      expect { parse }.to raise_error(JWT::InvalidAudError)
+    end
+
+    context "and the audience shall not be verified" do
+      subject(:parse) { described_class.new(verify_audience: false).parse(token) }
+
+      it "parses the token" do
+        parsed, = parse
+        expect(parsed).to eq payload
+      end
+    end
+  end
+
+  context "when the token does not indicate a Key Identifier" do
+    let(:token) { JWT.encode(payload, private_key, "RS256") }
+
+    it "raises an error" do
+      expect { parse }.to raise_error(OpenIDConnect::ProviderTokenParser::Error, /Key Identifier .+ is missing/)
+    end
+  end
+
+  context "when requiring a specific claim" do
+    subject(:parse) { described_class.new(required_claims: ["sub"]).parse(token) }
+
+    it "parses the token" do
+      parsed, = parse
+      expect(parsed).to eq payload
+    end
+
+    context "and when the required claim is missing" do
+      before do
+        payload.delete("sub")
+      end
+
+      it "raises an error" do
+        expect { parse }.to raise_error(JWT::MissingRequiredClaim)
+      end
+    end
+  end
+end