DominicMaas / TimelineExtension

Windows Timeline & Project Rome Web Extension
MIT License
483 stars 27 forks source link

New authentication flow for unsupported browsers. #43

Closed DominicMaas closed 6 years ago

DominicMaas commented 6 years ago

Issue

Some browsers like Edge and Firefox for mobile do not support chrome.identity.launchWebAuthFlow or browser.identity.launchWebAuthFlow. Required for #33 & #37.

Proposal

  1. Instead of calling browser.identity.launchWebAuthFlow we launch a new tab where the user can sign in.
  2. The redirect URL will be something I own (e.g https://gridentertainment.net/auth/fakeurl or even the github.io site for this repo - that might be better).
  3. The background script will listen to new tabs or redirects, when it sees something matching the redirect URL, grab the token from the URL and close the tab.
  4. Perform the usual authentication flow for grabbing the access and refresh tokens.

Changes Needed

Potential Issues

Are there any other issues that anyone sees with this system? I'm fine with implementing it, just wondering if I've missed any major security holes.

da2x commented 6 years ago

I’d suggest using externally_connectable to call the extension from the login endpoint, but this method is also unsupported in both Edge and Firefox for Android; so that won’t help one bit.

da2x commented 6 years ago

This will work pretty much the same way as externally_connectable for our purposes. This should also work in every browser.

manifest:

"content_scripts": [
  {
    "matches": ["https://timelineextension.github.io/callback"],
    "js": ["fallback_auth.js"]
} ]

fallback_auth.js:

chrome.runtime.sendMessage({type: "Authentication", payload: document.location.hash});
window.close();
DominicMaas commented 6 years ago

That's even better! I'll look at implementing it later today.

da2x commented 6 years ago

Okay, closing the window right away was stupid; only do it in response to promise or callback.

var authMsg = {
  type: "Authentication",
  payload: document.location.hash
};

// COMPAT: WebExtensions expect promise and Chromium extensions use callback
if (browser) {
   browser.runtime.sendMessage(authMsg).then(window.close, window.close);
} else {
   chrome.runtime.sendMessage(authMsg, window.close);
}

Firefox supports chrome.runtime.sendMessage(msg, callback) but Edge documentation doesn’t really indicate whether it supports callbacks in the chrome. compatibility namespace or only through promises in the browser. namespace. (Everyone supports chrome.runtime.sendMessage(msg) without any callbacks or promises.) Good documentation; who needs it anyway.

DominicMaas commented 6 years ago

I've added in detection for Edge and Firefox Mobile (hopefully I've set it up correctly), added the content script, and setup a new client id and secret for fallback authentication. Just have to setup the github.io page and auth backend to accept a new type of client id.

DominicMaas commented 6 years ago

Hmmm, pushed out an update to Firefox for mobile, tapping login still does nothing. I'm assuming that window.open() either does not work on mobile correctly, or does not work on the background script?

da2x commented 6 years ago

On startup:

D: Selected Browser: Firefox
D: [Auth Setup] Using fallback system (Firefox Mobile).
E: Unauthorized, no auth token set. background.js:1:8589
E: Unauthorized, no auth token set.

After tapping the login button:

W: Scripts may not close windows that were not opened by script.
da2x commented 6 years ago

window.open is indeed not allowed from background processes. This is the API to use instead:

chrome.tabs.create({ url: authUrl });

However, we don’t need to require extra permissions to use tabs as we only want to open a new tab. Do this instead:

<a href="${authURL}" target="_blank">This is cheating</a>

Can set the href and target to the event target on click. Or just create a new temporary link element, set the attributes, and click that onclick.

DominicMaas commented 6 years ago

We're already using the tabs permission, so the first point does not really matter. Unless we can remove the tabs permission (currently used to get the current active tab for sending to another device - is there another way of doing this?).

da2x commented 6 years ago

Ah, in that case then just use tabs.create.

DominicMaas commented 6 years ago

I had to move the window closing logic into the background script (it seems as window.close does not work unless the user initiates an event with it, or the script created the url. Also tested it in Opera and it works. I'll push out 1.0.3 so we can test if it works on Firefox Mobile.

DominicMaas commented 6 years ago

Published and it appears to be working for Firefox Mobile (successful login).