fix: changes regex for basic auth to be case-insensitive and adds tests for mixed case prefixes - #1788
Conversation
nbulaj
left a comment
There was a problem hiding this comment.
I checked OAuth RFC which refers HTTP Authentication RFC and it tells about case-insensitive auth token as well. So LGTM 👍
There was a problem hiding this comment.
Pull request overview
This pull request updates the Basic authentication header parsing to be case-insensitive, aligning with RFC 7617 specifications. The regex pattern is modified from /^Basic (.*)/m to /^Basic (.*)/im and comprehensive test coverage is added.
Changes:
- Modified regex pattern in
from_basicmethod to use case-insensitive matching with theiflag - Added test cases for lowercase 'basic' and mixed case 'BaSiC' authentication prefixes
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| lib/doorkeeper/oauth/client/credentials.rb | Updated regex pattern to add case-insensitive flag (i) for matching the "Basic" authentication scheme |
| spec/lib/oauth/client/credentials_spec.rb | Added two new test cases to verify case-insensitive matching works with lowercase and mixed case variations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Hello @caiubi .Sorry for the delay. Can you please also add a changelog entry and squash commits then ? 🙏 Thanks! |
…ts for mixed case prefixes
a24053e to
584b26e
Compare
|
It's done! 😄 |
Summary
The RFC for
basicauthentication specifies that the "scheme" should be matched case-insensitively, so I changed the regex and added some specs.Other Information
N/A