Skip to content

feat(EdDSA): Accept EdDSA as algorithm header#446

Merged
anakinj merged 1 commit intojwt:masterfrom
Pierre-Michard:master
Sep 29, 2021
Merged

feat(EdDSA): Accept EdDSA as algorithm header#446
anakinj merged 1 commit intojwt:masterfrom
Pierre-Michard:master

Conversation

@Pierre-Michard
Copy link
Copy Markdown
Contributor

Some implementations of EdDSA algorithm use 'EdDSA' as header instead of ED25519. This MR aims to make this library compatible with such clients.

@sourcelevel-bot
Copy link
Copy Markdown

Hello, @Pierre-Michard! This is your first Pull Request that will be reviewed by SourceLevel, an automatic Code Review service. It will leave comments on this diff with potential issues and style violations found in the code as you push new commits. You can also see all the issues found on this Pull Request on its review page. Please check our documentation for more information.

Comment thread lib/jwt/algos/eddsa.rb
algorithm, msg, key = to_sign.values
raise EncodeError, "Key given is a #{key.class} but has to be an RbNaCl::Signatures::Ed25519::SigningKey" if key.class != RbNaCl::Signatures::Ed25519::SigningKey
raise IncorrectAlgorithm, "payload algorithm is #{algorithm} but #{key.primitive} signing key was provided" if algorithm.downcase.to_sym != key.primitive
if key.class != RbNaCl::Signatures::Ed25519::SigningKey
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

JWT::Algos::Eddsa#sign calls 'key.class' 2 times

Read more about it here.

@sourcelevel-bot
Copy link
Copy Markdown

SourceLevel has finished reviewing this Pull Request and has found:

  • 2 fixed issues! 🎉

See more details about this review.

@anakinj
Copy link
Copy Markdown
Member

anakinj commented Sep 29, 2021

Looks great! Think this addresses a part of #334 also

@anakinj anakinj merged commit ffad2de into jwt:master Sep 29, 2021
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.

2 participants