Last Comment Bug 743225 - Unable to log in from a bug using BrowserID
: Unable to log in from a bug using BrowserID
Status: RESOLVED FIXED
blocker will fix
:
Product: bugzilla.mozilla.org
Classification: Other
Component: Extensions: Persona (show other bugs)
: Production
: All All
: -- normal (vote)
: ---
Assigned To: Byron Jones ‹:glob›
:
Mentors:
Depends on: 743735
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-06 06:41 PDT by David Illsley
Modified: 2012-04-14 14:46 PDT (History)
4 users (show)
See Also:
Due Date:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Screenshot of warning (216.88 KB, image/png)
2012-04-06 06:41 PDT, David Illsley
no flags Details
patch v1 (2.15 KB, patch)
2012-04-09 04:54 PDT, Byron Jones ‹:glob›
dkl: review-
Details | Diff | Review

Description David Illsley 2012-04-06 06:41:11 PDT
Created attachment 612873 [details]
Screenshot of warning

When I attempt to sign in via BrowserID from the top of [1], I successfully authenticate, and then land at [2] where there's a warning:

"It looks like you didn't come from the right page. One reason could be that you entered the URL in the address bar of your web browser directly, which should be safe. Another reason could be that you clicked on a URL which redirected you here without your consent.

Are you sure you want to commit these changes? "

(screenshot attached)

I don't believe I'd actually made any changes to bug 711552, but nevertheless, didn't want to select "Yes, Confirm Changes", and selecting "No, throw away these changes" resulted in me being bounced to the front page without being logged in.

I can sign in successfully using BrowserID from the main page.

I've reproduced this behaviour on Nightly/OSX and MobileSafari/iPad.

David

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=711552
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=711552?token=1333705928-a2c6f9e2d1fb12b734017f291073cf05&browserid_assertion=eyJhbGciOiJSUzI1NiJ9.eyJpc3MiOiJicm93c2VyaWQub3JnIiwiZXhwIjoxMzMzNzkyMzU5MjE5LCJpYXQiOjEzMzM3MDU5NTkyMTksInB1YmxpYy1rZXkiOnsiYWxnb3JpdGhtIjoiRFMiLCJ5IjoiYWNhOTZjYmQ5ZGQ5MmQ4N2E1MGQ1YjVkMjJjY2I3YWEwZjI4YTJmYWVmNzgzNDA2OGE3Mzk2ZmVjMjFmYzU1ZTE1NmZiNTY1ZWRjZDBiNGVjMzBjZGFiZDI1ZDcxOWMyNDdmYzBlZGI1YWQ5MmRmNTUwMmNjZjdiOGQ3NGNlYTgyNjAwNjU5NzYxMTcxMWM3NzZiZjcxNzExZDdkZjY0ZWU2OGQyNmQ0ZDU4MmVmMWIzMWQwYTM0ZTVlZjM3ZmM5NmFlZDUyYzJlZDU3MmQxYzg5Njk2MzcxN2UyNGE4MWY1YzM2MGQwN2VhYjA1NTY1M2ZkOTNjMmE1N2VhYTY4ZTc4MWY1OTkxNjJhZjdkNjY0MGEwNzVlYjA2MmNmZWE5MzBlMmJiNWMxZmFjMTcyZmRkYjM5MzgxNDllNjFhZDJjMmM0ODJlMmVkNjMxZGQ2Zjg0YjRhMjBmN2JjMzZiYzRmNmYzMGJhMTU1MDU4YjcxNzFlN2IxZjdkNmVlNTZhMGRjNDYyNjk0MmQ5ZmEzOWE4ZDlkN2VjNDg4ZDc5MzJmY2M0MWNjNDEyMDYwM2U5NmY2NDc5NGMzODJmNGFkOGZlMzQ3NzE3MjJjYWMyMjI5Y2MyOTQwYTkxMTZmZTM0MWJiYzBiYjEyMDkwMGM5ODcxYzM2NTUxZDJiZTMzOWEiLCJwIjoiZDZjNGU1MDQ1Njk3NzU2YzdhMzEyZDAyYzIyODljMjVkNDBmOTk1NDI2MWY3YjU4NzYyMTRiNmRmMTA5YzczOGI3NjIyNmIxOTliYjdlMzNmOGZjN2FjMWRjYzMxNmUxZTdjNzg5NzM5NTFiZmM2ZmYyZTAwY2M5ODdjZDc2ZmNmYjBiOGMwMDk2YjBiNDYwZmZmYWM5NjBjYTQxMzZjMjhmNGJmYjU4MGRlNDdjZjdlNzkzNGMzOTg1ZTNiM2Q5NDNiNzdmMDZlZjJhZjNhYzM0OTRmYzNjNmZjNDk4MTBhNjM4NTM4NjJhMDJiYjFjODI0YTAxYjdmYzY4OGU0MDI4NTI3YTU4YWQ1OGM5ZDUxMjkyMjY2MGRiNWQ1MDViYzI2M2FmMjkzYmM5M2JjZDZkODg1YTE1NzU3OWQ3ZjUyOTUyMjM2ZGQ5ZDA2YTRmYzNiYzIyNDdkMjFmMWE3MGY1ODQ4ZWIwMTc2NTEzNTM3Yzk4M2Y1YTM2NzM3ZjAxZjgyYjQ0NTQ2ZThlN2YwZmFiYzQ1N2UzZGUxZDljNWRiYTk2OTY1YjEwYTJhMDU4MGIwYWQwZjg4MTc5ZTEwMDY2MTA3ZmI3NDMxNGEwN2U2NzQ1ODYzYmM3OTdiNzAwMmViZWMwYjAwMGE5OGViNjk3NDE0NzA5YWMxN2I0MDEiLCJxIjoiYjFlMzcwZjY0NzJjODc1NGNjZDc1ZTk5NjY2ZWM4ZWYxZmQ3NDhiNzQ4YmJiYzA4NTAzZDgyY2U4MDU1YWIzYiIsImciOiI5YTgyNjlhYjJlM2I3MzNhNTI0MjE3OWQ4ZjhkZGIxN2ZmOTMyOTdkOWVhYjAwMzc2ZGIyMTFhMjJiMTljODU0ZGZhODAxNjZkZjIxMzJjYmM1MWZiMjI0YjA5MDRhYmIyMmRhMmM3Yjc4NTBmNzgyMTI0Y2I1NzViMTE2ZjQxZWE3YzRmYzc1YjFkNzc1MjUyMDRjZDdjMjNhMTU5OTkwMDRjMjNjZGViNzIzNTllZTc0ZTg4NmExZGRlNzg1NWFlMDVmZTg0NzQ0N2QwYTY4MDU5MDAyYzM4MTlhNzVkYzdkY2JiMzBlMzllZmFjMzZlMDdlMmM0MDRiN2NhOThiMjYzYjI1ZmEzMTRiYTkzYzA2MjU3MThiZDQ4OWNlYTZkMDRiYTRiMGI3ZjE1NmVlYjRjNTZjNDRiNTBlNGZiNWJjZTlkN2FlMGQ1NWIzNzkyMjVmZWIwMjE0YTA0YmVkNzJmMzNlMDY2NGQyOTBlN2M4NDBkZjNlMmFiYjVlNDgxODlmYTRlOTA2NDZmMTg2N2RiMjg5YzY1NjA0NzY3OTlmN2JlODQyMGE2ZGMwMWQwNzhkZTQzN2YyODBmZmYyZDdkZGYxMjQ4ZDU2ZTFhNTRiOTMzYTQxNjI5ZDZjMjUyOTgzYzU4Nzk1MTA1ODAyZDMwZDdiY2Q4MTljZjZlZiJ9LCJwcmluY2lwYWwiOnsiZW1haWwiOiJkYXZpZEBpbGxzbGV5Lm9yZyJ9fQ.DswhtgCUWvVbWyDqloPoLhcSKbDbYCVUl6y5-Yeiq0Otdr2NGFLaQJV8AXIIQ1s8UEjYbDBO0Ac2nTHp3oSGGbj2kgbgc5B9VtAjAoMXpcc_vikCN8YcazOoBgaymfv0qF5_q3zzToruffOs4Ayp_sUI5AMRAIKfPOU6hOEOhLU0cbDAcxKlv4SoaY5uRtnhiBBuSpaAu2XVM7ZPzPHjVImv4ndC4Fdylk_k0JMq-emkdYlLXyhAj-p5zhw7Vlm00w6m5Vkgc--Pw0hWyHEmu1cX4uNxN-qkTPk5XBHTtdgZe8gaVbIHvUayT77wz-Q_zJEYzHKIAyLT6Vc60wACLg~eyJhbGciOiJEUzI1NiJ9.eyJleHAiOjEzMzM3MDY4OTc4MjksImF1ZCI6Imh0dHBzOi8vYnVnemlsbGEubW96aWxsYS5vcmcifQ.dOGTsIFZd9NFLpPT4D5F04KgFc9HgEIaKsnSXHom1vF4rC4CeV7Rc8OqkSfE9MjrpstTIwDfKKzisY0gKxPE-w
Comment 1 Frédéric Buclin 2012-04-06 16:41:15 PDT
I see that the browserid_assertion token is 2900 characters long, which seems ridiculously long. Isn't this going to break Internet Explorer, based on bug 290977 and http://support.microsoft.com/default.aspx?scid=KB;en-us;q208427 ? Also, Apache has a 8K limit, and 35% of its capacity is already wasted by this token. This is going to make very long queries to fail again.

Confirming this bug as someone else is complaining on IRC about the same problem.
Comment 2 Gervase Markham [:gerv] 2012-04-07 09:14:19 PDT
This is probably my screw-up; note the double question mark in the URL in the screenshot.

As for the length of the assertion, you need to take that up with the BrowserID developers...

Gerv
Comment 3 Byron Jones ‹:glob› 2012-04-09 04:54:45 PDT
Created attachment 613269 [details] [diff] [review]
patch v1
Comment 4 David Lawrence [:dkl] 2012-04-09 08:49:31 PDT
Comment on attachment 613269 [details] [diff] [review]
patch v1

Review of attachment 613269 [details] [diff] [review]:
-----------------------------------------------------------------

::: extensions/BrowserID/template/en/default/hook/account/auth/login-additional_methods.html.tmpl
@@ +7,5 @@
>          if (assertion) {
>              // This code will be invoked once the user has successfully
>              // selected an email address they control to sign in with.
> +            var token = '[% issue_hash_token(['login']) FILTER js %]';
> +            var url = '[% login_target FILTER js %]';

Doesn't work for me as it is supposed to be 'target' and not 'login_target'. But in the case of this bug conditions, 'target' is only show_bug.cgi and doesn't include the query portion of the URL. So the url.indexOf will always be -1. What you need is:

var url = '[% cgi.url("-relative" => 1, "-query" => 1) FILTER js %]';

which then works for me.
Comment 5 Byron Jones ‹:glob› 2012-04-09 09:54:49 PDT
(In reply to David Lawrence [:dkl] from comment #4)
> Doesn't work for me as it is supposed to be 'target'

ah, evidently the non-small login form has never worked when accessing a secure bug.  will work on a revised patch.
Comment 6 Byron Jones ‹:glob› 2012-04-09 11:06:57 PDT
(In reply to Gervase Markham [:gerv] from comment #2)
> As for the length of the assertion, you need to take that up with the
> BrowserID developers...

i spoke with the browserID developers, and the assertion length can't be changed -- in fact they said they can be "arbitrarily large".  raised bug 743735 for switching to POSTing data instead of redirecting.
Comment 7 Byron Jones ‹:glob› 2012-04-10 00:22:18 PDT
the patch on bug 743735 will also address this issue

Note You need to log in before you can comment on or make changes to this bug.