743225 - Unable to log in from a bug using BrowserID

Status: RESOLVED FIXED

(bugzilla.mozilla.org Graveyard :: Extensions: Persona, defect)

Description

[David Illsley]

Attachment: 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]

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]]

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

[:glob ✱]

Attachment: patch v1

Comment 4

[David Lawrence [:dkl]]

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

[:glob ✱]

(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

[:glob ✱]

(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

[:glob ✱]

the patch on bug 743735 will also address this issue