Skip to content

validating jwt signature from alma#42

Draft
davezuckerman wants to merge 1 commit intomainfrom
AP-634-alma-jwt
Draft

validating jwt signature from alma#42
davezuckerman wants to merge 1 commit intomainfrom
AP-634-alma-jwt

Conversation

@davezuckerman
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown
Contributor

@jason-raitz jason-raitz left a comment

Choose a reason for hiding this comment

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

lgtm.

Copy link
Copy Markdown
Member

@anarchivist anarchivist left a comment

Choose a reason for hiding this comment

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

I think this needs some refactoring - we should look at using ruby-jwt's native methods here rather than rolling our own.

Comment on lines +20 to +47
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
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would it be possible for us to use JWT:JWK::Keyfinder to validate the signature? I think that will greatly simplify the code here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Similarly, there's a lot in this file that seems to duplicate what's in JWT::JWK's methods here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants