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.

Sign up for GitHub

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

Jump to bottom

FI-3155 Fix JWT encoding method #5

Merged
merged 4 commits into from
Sep 27, 2024
Merged

FI-3155 Fix JWT encoding method #5

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
21c22ef
Update dependencies
alisawallace Sep 20, 2024
2101ebd
Update use of urlsafe_encode/decode64 to just encode/decode64 for JWTs
alisawallace Sep 20, 2024
b57d0f1
Require generate_client_certs_test in registration group
alisawallace Sep 23, 2024
2559a76
Rubocop auto-corrections
alisawallace Sep 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
248 changes: 119 additions & 129 deletions Gemfile.lock
View file
Open in desktop

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions lib/udap_security_test_kit/dynamic_client_registration_group.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
require_relative 'generate_client_certs_test'
require_relative 'registration_failure_invalid_contents_test'
require_relative 'registration_failure_invalid_jwt_signature_test'
require_relative 'registration_success_test'
Expand Down
2 changes: 1 addition & 1 deletion lib/udap_security_test_kit/signed_metadata_contents_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class SignedMetadataContentsTest < Inferno::Test
assert token_header.key?('x5c'), 'JWT header does not contain `x5c` field'
assert token_header.key?('alg'), 'JWT header does not contain `alg` field'

leaf_cert_der = Base64.urlsafe_decode64(token_header['x5c'].first)
leaf_cert_der = Base64.decode64(token_header['x5c'].first)
leaf_cert = OpenSSL::X509::Certificate.new(leaf_cert_der)
signature_validation_result = UDAPSecurityTestKit::UDAPJWTValidator.validate_signature(
signed_metadata_jwt,
Expand Down
2 changes: 1 addition & 1 deletion lib/udap_security_test_kit/udap_jwt_builder.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def self.encode_jwt_with_x5c_header(payload, private_key_pem_string, alg, x5c_ce

x5c_certs_encoded = x5c_certs_pem_string.map do |cert|
cert_pem = OpenSSL::X509::Certificate.new(cert)
Base64.urlsafe_encode64(cert_pem.to_der)
Base64.encode64(cert_pem.to_der)
end

JWT.encode payload, private_key, alg, { x5c: x5c_certs_encoded }
Expand Down
2 changes: 1 addition & 1 deletion lib/udap_security_test_kit/udap_jwt_validator.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def self.validate_signature(signed_metadata_jwt, algorithm, cert)

def self.validate_trust_chain(x5c_header_encoded, trust_anchor_certs)
cert_chain = x5c_header_encoded.map do |cert|
cert_der = Base64.urlsafe_decode64(cert)
cert_der = Base64.decode64(cert)
OpenSSL::X509::Certificate.new(cert_der)
end
crl_uris = cert_chain.map(&:crl_uris).compact.flatten
Expand Down
8 changes: 4 additions & 4 deletions spec/udap_security_test_kit/udap_jwt_builder_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
require_relative '../../lib/udap_security_test_kit/udap_jwt_builder'
require_relative '../../lib/udap_security_test_kit/default_cert_file_loader'

RSpec.describe UDAPSecurityTestKit::UDAPJWTBuilder do # rubocop:disable RSpec/FilePath,RSpec/SpecFilePathFormat
RSpec.describe UDAPSecurityTestKit::UDAPJWTBuilder do # rubocop:disable RSpec/SpecFilePathFormat
let(:jwt_alg) { 'RS256' }
let(:rsa_private_string) do
UDAPSecurityTestKit::DefaultCertFileLoader.load_test_client_private_key_file
Expand Down Expand Up @@ -78,9 +78,9 @@ def validate_cert_array(contents, expected_length)
expect(jwt_header['x5c'].is_a?(Array)).to be true

# verify enclosed certificate
cert = OpenSSL::X509::Certificate.new(Base64.urlsafe_decode64(jwt_header['x5c'].first))
cert = OpenSSL::X509::Certificate.new(Base64.decode64(jwt_header['x5c'].first))

jwt_client_cert = OpenSSL::X509::Certificate.new(Base64.urlsafe_decode64(jwt_header['x5c'].first))
jwt_client_cert = OpenSSL::X509::Certificate.new(Base64.decode64(jwt_header['x5c'].first))
expect(jwt_client_cert.check_private_key(rsa_private)).to be true

ca_cert = OpenSSL::X509::Certificate.new(ca_cert_string)
Expand Down Expand Up @@ -113,7 +113,7 @@ def validate_cert_array(contents, expected_length)
expect(jwt_header['x5c'].is_a?(Array)).to be true

# fails when using public key from attached certificate
jwt_client_cert = OpenSSL::X509::Certificate.new(Base64.urlsafe_decode64(jwt_header['x5c'].first))
jwt_client_cert = OpenSSL::X509::Certificate.new(Base64.decode64(jwt_header['x5c'].first))

expect do
JWT.decode encoded_jwt, jwt_client_cert.public_key, true, { algorithm: jwt_alg }
Expand Down
4 changes: 2 additions & 2 deletions spec/udap_security_test_kit/udap_jwt_validator_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
require_relative '../../lib/udap_security_test_kit/default_cert_file_loader'
require 'pry'

RSpec.describe UDAPSecurityTestKit::UDAPJWTValidator do # rubocop:disable RSpec/FilePath,RSpec/SpecFilePathFormat
RSpec.describe UDAPSecurityTestKit::UDAPJWTValidator do # rubocop:disable RSpec/SpecFilePathFormat
let(:inferno_client_cert) do
UDAPSecurityTestKit::DefaultCertFileLoader.load_test_client_cert_pem_file
end
Expand Down Expand Up @@ -83,7 +83,7 @@
)
_token_body, token_header = JWT.decode(test_jwt, nil, false)

cert = OpenSSL::X509::Certificate.new(Base64.urlsafe_decode64(token_header['x5c'].first))
cert = OpenSSL::X509::Certificate.new(Base64.decode64(token_header['x5c'].first))

validation_result = described_class.validate_signature(
test_jwt,
Expand Down
2 changes: 1 addition & 1 deletion spec/udap_security_test_kit/udap_x509_certificate_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
require_relative '../../lib/udap_security_test_kit/udap_x509_certificate'
require_relative '../../lib/udap_security_test_kit/default_cert_file_loader'

RSpec.describe UDAPSecurityTestKit::UDAPX509Certificate do # rubocop:disable RSpec/SpecFilePathFormat,RSpec/FilePath
RSpec.describe UDAPSecurityTestKit::UDAPX509Certificate do # rubocop:disable RSpec/SpecFilePathFormat
let(:ca_cert_string) do
UDAPSecurityTestKit::DefaultCertFileLoader.load_default_ca_pem_file
end
Expand Down
4 changes: 2 additions & 2 deletions udap_security_test_kit.gemspec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ Gem::Specification.new do |spec|
spec.description = 'UDAP Security IG Test Kit'
spec.homepage = 'https://github.com/inferno-framework/udap-security-test-kit'
spec.license = 'Apache-2.0'
spec.add_runtime_dependency 'inferno_core', '>= 0.4.2'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, why is this changing? I see spec.add_runtime_dependency everywhere else.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this was a Rubocop auto-correction. When I revert it back to the original my editor gives me a blue squiggly line and says:

Gemspec/AddRuntimeDependency: Use add_dependency instead of add_runtime_dependency.RuboCopGemspec/AddRuntimeDependency

I saw you approved the PR but from this comment can't tell if you changed your mind, so @Jammjammjamm just confirm if I'm good to go or if you want me to revert to spec.add_runtime_dependency!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha, it's good to go.

spec.add_runtime_dependency 'jwt', '~> 2.3'
spec.add_dependency 'inferno_core', '>= 0.4.2'
spec.add_dependency 'jwt', '~> 2.3'
spec.required_ruby_version = Gem::Requirement.new('>= 3.1.2')
spec.metadata['homepage_uri'] = spec.homepage
spec.metadata['source_code_uri'] = 'https://github.com/inferno-framework/udap-security-test-kit'
Expand Down
Loading