-
Notifications
You must be signed in to change notification settings - Fork 121
fix: Multiple digest auth headers not filtered by opted algorithm #1524
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
fix: Multiple digest auth headers not filtered by opted algorithm #1524
Conversation
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (41.66%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #1524 +/- ##
===========================================
- Coverage 38.66% 38.62% -0.05%
===========================================
Files 46 46
Lines 3525 3534 +9
Branches 1023 1026 +3
===========================================
+ Hits 1363 1365 +2
- Misses 2065 2071 +6
- Partials 97 98 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logic looks good. Suggested some optional refactoring.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add tests for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked the digest auth server we run for our integration suite, that version of passport doesn't seem to support multiple digest auth headers as part of www-authenticate.
I'll check a little further and find another way to verify this change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved. Tests to be added.
…s-not-filtered-by-opted-algorithm
Works to fix postmanlabs/postman-app-support#13756
Context
Since servers can return multiple values for
www-authenticateheaders to present clients with multiple options, we need to filter the right header based on theauth.algorithmsetting passed bypostman-runtime's consumer.