[#6734] - URL-encode at-sign in HTTP Basic Auth credentials
- Closed
- 12 Jul 2015
- Medium
- Build: staging
- # 6734
- Diff
- jissereitsma:origin/github-auth
- Success The Travis CI build passed Details
User tests: Successful: Unsuccessful:
This is kind of bad ass, but needed. During numerous PBF sessions, we encountered issues when people were using the PatchTester component in combination with GitHub credentials that contained an at-sign (@), so either an username or password that contains the at-sign. When the component fetches the list of issues from GitHub, the username and password are appended to the URL in the form "http://USERNAME:PASSWORD@URL.com". However, because the at-sign serves as separator, the URL is determined from the first at-sign that is encountered. This results in authentication failure.
The solution of this patch is childish but simple: Replace the at-sign in the credentials with an URL-encoded version. Note that this fix is only needed for the at-sign - no other characters are effected. Also, Basic Authentication in GitHub is until now the only place where this issue was encountered. So this patch only fixes that area.
How to test the problem:
1) Install the PatchTester component.
2) Reset your GitHub username or password to contain an at-sign (@)
3) Configure PatchTester component with your credentials
4) Use the "Fetch Data" button to connect
Step 4 should give you an error in the browsers Error Console saying "Bad Credentials". (That the component is not properly showing this error is another unrelated issue.)
Next, apply the patch and test again. Step 4 should now succeed.
joomla-cms-bot
- | Labels |
Added:
?
|
||
| Labels |
Added:
?
|
||
While testing we ran into a of a kind of deadlock situation, as we were testing the testing software itself. But as we were in the same room with Jisse, the deadlock was solved quickly. And, yes, the issue is solved as well :-)
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6734.
@jissereitsma i have just send you a PR to fix the Travis issues 
jissereitsma#1
| Category | ⇒ | Libraries |
@zero-24 many thanks. The JoomlaDay Netherlands weekend was over in the wink of an eye, and I had no time to fix this yet. And during the PBF itself, I only had a lousy laptop without proper IDE and dozens of people asking me questions :) Many thanks for the PR, I checked it and submitted it.
Setting RTC - thanks
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6734.
| Status | Pending | ⇒ | Ready to Commit |
| Labels |
Added:
?
|
||
| Labels |
Added:
?
|
||
I think this is something we should do in the JUri class chain as presumably other services could hit the same issue. At least for our API connectors, it would create a global fix if any still accept HTTP authentication.
@jissereitsma Could you look into the proposal of Michael? So it can be fixed globally?
| Milestone |
Added: |
||
| Milestone |
Added: |
||
| Status | Ready to Commit | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2015-07-12 20:05:19 |
| Closed_By | ⇒ | Kubik-Rubik |
Thank you @jissereitsma! Merged with commit: ac3bd8c
I know, once we've fixed it globally, then we'll of course remove this specific case. At least this should solve the issue for the Patchtester for now.
Personally I am happy that this PR is merged in the first place - this fixes an awkward thing for people right away. But yes, the comments of @mbabker make sense, the location of this fix could be improved. I'll keep this issue open for myself so I'll look into this.
| Labels |
Removed:
?
|
||
Tested according to instructions. Thumbs up!
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6734.