Closed Bug 1450565 Opened 6 years ago Closed 6 years ago

Option to prevent launchWebAuthFlow HEAD request

Categories

(WebExtensions :: Frontend, defect, P2)

defect

Tracking

(firefox-esr52 unaffected, firefox59 wontfix, firefox60+ fixed, firefox61 fixed)

VERIFIED FIXED
mozilla61
Tracking Status
firefox-esr52 --- unaffected
firefox59 --- wontfix
firefox60 + fixed
firefox61 --- fixed

People

(Reporter: freaktechnik, Assigned: mixedpuppy)

References

Details

Attachments

(1 file)

Currently launcWebAuthFlow tests if an actual login is even required by first sending a HEAD request to the provided url. If that is redirected to the redirect_url it concludes that it is not necessary to show the auth window.

However, GitHub has decided that HEAD requests to their OAuth endpoint are to be treated as denied by the user. I've raised that with their support, but it has also shown, that a HEAD request may not behave reliable for every OAuth implementation.

I propose to add an option to circumvent the HEAD request or to optionally make it a GET request.
GitHub decided that it's not really a fault on their end, since they don't officially support HEAD requests as part of their OAuth flow. To be "compliant" it'd be necessary to modify launchWebAuthFlow's behavior then.
What is the actual response to the HEAD request?  If it makes sense, we could just show the window in that case.
Flags: needinfo?(martin)
Also, what actually breaks here if anything?  Or is this just to avoid the head request?
It responds with a redirect to the redirect URL and an error code, that the user denied the OAuth request.

This means Firefox decides that there is no reason to show the OAuth dialog, since the HEAD request already lead to a redirect to the redirect URL, which is the correct behavior IMO. It means, however, that you can not actually ever grant a request while logged in to GitHub (I believe this doesn't happen when not logged in, since it can't "deny without being logged in).

Since GitHub says, that they only support their documented OAuth protocol, yes, the goal is to avoid the HEAD request so the protocol is "properly" followed.
Flags: needinfo?(martin)
Assignee: nobody → mixedpuppy
Priority: -- → P2
I'm thinking the most compatible way to deal with this is to use GET.  

More complicated but...

We could actually use a hidden browser to start the process.  If it redirects with a response, we return as we do with the head request.  If it does not, we open the window and swap browsers.  That would save us the extra request.
Comment on attachment 8968560 [details]
Bug 1450565 use GET for initial authorization request,

https://reviewboard.mozilla.org/r/237244/#review243056
Attachment #8968560 - Flags: review?(dtownsend) → review+
Pushed by mixedpuppy@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/e17c105c0be8
use GET for initial authorization request, r=mossop
Comment on attachment 8968560 [details]
Bug 1450565 use GET for initial authorization request,

Approval Request Comment
[Feature/Bug causing the regression]: identity api
[User impact if declined]: some services cannot be authorized, specifically github
[Is this code covered by automated tests?]: yes, existing tests still pass
[Has the fix been verified in Nightly?]: not yet
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: changing a head request to get request should have no real change on the api use.  change is also limited to this one api call in webextensions.
[String changes made/needed]:
Attachment #8968560 - Flags: approval-mozilla-beta?
https://hg.mozilla.org/mozilla-central/rev/e17c105c0be8
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Comment on attachment 8968560 [details]
Bug 1450565 use GET for initial authorization request,

fix webextensions identity api vs github, approved for 60.0b14
Attachment #8968560 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
I can confirm the issue is fixed in nightly.
(In reply to Thiago Arrais from comment #13)
> I can confirm the issue is fixed in nightly.

Thanks!
Flags: qe-verify-
OAuth works correctly again with the latest version of my extension (https://addons.mozilla.org/en-US/firefox/addon/advanced-github-notifier/) in Firefox 60+
Status: RESOLVED → VERIFIED
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.