feat: add auth.format for formatting credentials#65
feat: add auth.format for formatting credentials#65Phillip9587 wants to merge 5 commits intojshttp:masterfrom
auth.format for formatting credentials#65Conversation
|
@blakeembrey I rebased and added your suggestions to disallow colons in username and control characters in both username and password. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #65 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 1 1
Lines 21 60 +39
Branches 6 22 +16
=========================================
+ Hits 21 60 +39 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| ); | ||
| } | ||
|
|
||
| if (credentials.name.length === 0 || credentials.pass.length === 0) { |
There was a problem hiding this comment.
I don't see anything requiring these to be non-empty, we should probably allow it.
There was a problem hiding this comment.
RFC 7613, Section 3.1 states:
A username MUST NOT be zero bytes in length. This rule is to be enforced after any normalization and mapping of code points.
RFC 7613, Section 4.1 states:
A password MUST NOT be zero bytes in length. This rule is to be enforced after any normalization and mapping of code points.
If you think we should not enforce it I'm fine with it.
There was a problem hiding this comment.
Thanks. I didn't see it in the specs for authentication, and the original basic auth specification said it was 0 or more characters. I think we should still remove it because that specification deviates further from allowed characters than just control characters.
There was a problem hiding this comment.
https://www.rfc-editor.org/rfc/rfc2617:
user-pass = userid ":" password userid = *<TEXT excluding ":"> password = *TEXT
There was a problem hiding this comment.
I saw that too but didn’t see any suggestion to disallow empty unless I overlooked it?
|
|
||
| if ( | ||
| typeof credentials.name !== 'string' || | ||
| typeof credentials.pass !== 'string' |
There was a problem hiding this comment.
I'm happy to get rid of all the safety checks in the new major, but can do it in a follow up PR instead.
There was a problem hiding this comment.
Why remove them?
2 participants
This pull request introduces a new feature to the
basic-authlibrary, adding a method to format credentials into a basic auth header string.