Skip to content

Ignore authorization headers if not Bearer#325

Merged
mcollina merged 6 commits intofastify:masterfrom
cberescu:ignoreheader
Jan 7, 2024
Merged

Ignore authorization headers if not Bearer#325
mcollina merged 6 commits intofastify:masterfrom
cberescu:ignoreheader

Conversation

@cberescu
Copy link
Copy Markdown
Contributor

@cberescu cberescu commented Jan 5, 2024

Fixes: #318

Checklist

Screenshot 2024-01-05 155257

mcollina
mcollina approved these changes Jan 5, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina
Copy link
Copy Markdown
Member

mcollina commented Jan 5, 2024

I'm a bit conflicted if this is major or not. Anybody has an opinion?

climba03003
climba03003 reviewed Jan 6, 2024
View reviewed changes
Comment thread test/jwt.test.js Outdated
climba03003
climba03003 reviewed Jan 6, 2024
View reviewed changes
Comment thread test/jwt.test.js Outdated
@climba03003
Copy link
Copy Markdown
Member

climba03003 commented Jan 6, 2024

I'm a bit conflicted if this is major or not. Anybody has an opinion?

Although it stills throw Error, but changing the return from 400 to 401 status code is absolutely a major.

gurgunday
gurgunday reviewed Jan 6, 2024
View reviewed changes
Comment thread jwt.js Outdated
@cberescu @climba03003
Apply suggestions from code review
8a5d3c9 
Co-authored-by: KaKa <23028015+climba03003@users.noreply.github.com>
Signed-off-by: cberescu <ciprian@berescu.com>
gurgunday
gurgunday requested changes Jan 6, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@gurgunday gurgunday left a comment

Choose a reason for hiding this comment

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

#325 (comment)

@cberescu @gurgunday
Update jwt.js
e8d332a 
Co-authored-by: Gürgün Dayıoğlu <gurgun.dayioglu@icloud.com>
Signed-off-by: cberescu <ciprian@berescu.com>
@cberescu cberescu requested a review from gurgunday January 6, 2024 12:33
gurgunday
gurgunday approved these changes Jan 6, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@gurgunday gurgunday left a comment

Choose a reason for hiding this comment

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

looks good to me

gurgunday
gurgunday reviewed Jan 6, 2024
View reviewed changes
Comment thread jwt.js Outdated
@cberescu
Copy link
Copy Markdown
Contributor Author

cberescu commented Jan 6, 2024

@gurgunday not sure why one of the tests failed... any ideas ?

@gurgunday gurgunday requested a review from mcollina January 7, 2024 09:47
@gurgunday
Copy link
Copy Markdown
Member

gurgunday commented Jan 7, 2024

@gurgunday not sure why one of the tests failed... any ideas ?

It was a CI issue, fixed now

mcollina
mcollina approved these changes Jan 7, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina mcollina merged commit 3bab18e into fastify:master Jan 7, 2024
@willpoorman
Copy link
Copy Markdown

willpoorman commented Aug 19, 2024 •
edited
Loading

Hey guys. Appreciate the work guys have done with this package. Could you potentially update the changelog for the 8.0.0 to clarify what the exact breaking change is and what users need to adjust when upgrading? https://github.com/fastify/fastify-jwt/releases/tag/v8.0.0. I had to dig in to find this PR to read the comments to find out what was considered BREAKING about it: #325 (comment)

This was referenced Oct 9, 2024
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@climba03003 climba03003 climba03003 left review comments

@mcollina mcollina mcollina approved these changes

@gurgunday gurgunday gurgunday approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

If authorization is not of type Bearer to ignore it and check the cookie

6 participants

@cberescu @mcollina @climba03003 @gurgunday @willpoorman @lordnimrod