Skip to content

Added algorithm option to jwt policy #999

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.

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

Merged
merged 2 commits into from
Jul 18, 2020

Conversation

Brady-Conn
Copy link
Contributor

@Brady-Conn Brady-Conn commented Jul 15, 2020

Hello,
Express-gateway is fantastic — it has become the backbone of our team’s api architecture. That being said, we have run into one small issue.
Problem: The JWT policy does not currently accept algorithms as an option to specify which algorithms can be used to verify tokens. Under the hood I noticed express-gateway uses passport-jwt, which uses jsonwebtoken. This is the same package we use. Both passport-jwt and jsonwebtoken support specifying algorithms. This is a good feature because otherwise jsonwebtoken will allow any algorithm to be used depending on what is decoded via the JWT header. Specifying algorithms allows enforcing of standards across our apis.
Solution: Expose the algorithms option in the JWT policy.
Here is the pr for the docs update: ExpressGateway/express-gateway.io#336

@codecov
Copy link

codecov bot commented Jul 15, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@c294fa7). Click here to learn what that means.
The diff coverage is n/a.

@Brady-Conn
Copy link
Contributor Author

Brady-Conn commented Jul 15, 2020

The tests failing in these pipelines, failed once(with the same timeout error) locally for me, but have since passed. Is there a way to re-try these pipelines?

@XVincentX
Copy link
Member

I can take care of this and make sure all tests pass. Can you rebase with latest master?

@Brady-Conn Brady-Conn force-pushed the jwt-algorithm-options branch from faf9712 to bccffbf Compare July 16, 2020 16:02
@Brady-Conn
Copy link
Contributor Author

Awesome, thank you. I just pushed up that rebase, let me know if there is anything else I can do.

@@ -38,6 +38,10 @@ module.exports = {
type: 'boolean',
default: true,
description: 'Value istructing the gateway whether verify the sub against the internal SOC'
},
algorithms: {
type: 'array',
Copy link
Member

Choose a reason for hiding this comment

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

I think we should probably specify the items to be a string and then enum the possible choice. JWT has a fixed set of algorithms

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes sense, I just pushed up a change to reflect that. I wasn't 100% sure about the syntax so let me know if I need to modify it at all.

@Brady-Conn Brady-Conn force-pushed the jwt-algorithm-options branch from 8a957db to 954df8f Compare July 17, 2020 20:12
@Brady-Conn Brady-Conn force-pushed the jwt-algorithm-options branch from 954df8f to 7ae36e8 Compare July 17, 2020 20:16
@XVincentX XVincentX merged commit f77ab36 into ExpressGateway:master Jul 18, 2020
gatherchou pushed a commit to yilu-tech/express-gateway that referenced this pull request Jul 29, 2021
…ptions

Added algorithm option to jwt policy
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