Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(auth): Support 2FA via browser window auth #654

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

bmulholland
Copy link
Collaborator

@bmulholland bmulholland commented Oct 4, 2023

I'm working to remove the usage of electron remote, and Auth windows are the last one. They open the auth in a new window. Since Github has started enforcing 2FA for almost everyone, the electron-window auth approach isn't functional for most. It therefore doesn't make sense to spend work maintaining the existing broken auth; instead, we can switch to a new one.

This change moves auth to open in the default browser. When Auth is done, it will redirect to gitify://oauth-callback, which the app will register and monitor, finishing the auth process when it's done.

At this point, I've got an initial approach started, but there's still lots to do. Help is welcomed.

There are several pieces left:

  • Fix the start of the auth process
    • Allow the gitify://oauth-callback redirect in the Github app settings -- do we have access to that?
    • Fix the initialization of it -- all the types etc are broken, and the browser probably doesn't even open
  • Catch and handle the completion of the auth
    • Communicate the auth from the main process to the App context, which started the Auth
    • Error handling
  • Testing
    • Figure out automated testing approach, if possible
    • Manually test
  • Documentation

Fixes #561
Fixes #650
Fixes #429
Fixes #485
Closes #395

@bmulholland bmulholland force-pushed the 10-04-chore_Replace_remote_for_setting_auto-open_pref branch from 369befe to 4a5ffd9 Compare October 4, 2023 09:23
@bmulholland bmulholland force-pushed the 10-04-fix_auth_Support_2FA_via_browser_window_auth branch from 0d27be5 to a3768db Compare October 4, 2023 09:23
@bmulholland
Copy link
Collaborator Author

@afonsojramos do you have access to the Github App to add the redirect URI?

@afonsojramos
Copy link
Member

@bmulholland can't we keep using the existing gitify.io/callback redirect URI? Why do we need a new one?

@bmulholland
Copy link
Collaborator Author

@afonsojramos We need a way to get the information in the URL at the end of authentication to the app. Currently, this is done by controlling the browser itself, waiting until there's a redirect, and intercepting that event. If we open auth in an external browser, there isn't a way to monitor the event and intercept the page when auth is completed.

Using a custom protocol is the usual way to get that data from the browser to an app, in lieu of intercepting events like that. So this is the standard approach.

If we wanted to avoid adding a new redirect URI to the Github App, if we control gitify.io, we could perhaps add a redirect from that callback URI to our custom protocol, which would complete this flow. However, that wouldn't fix #429, and is also a genuine security hole: anyone with access to gitify.io could probably MITM the callback and get access to quite a lot of data.

@bmulholland bmulholland force-pushed the 10-04-chore_Replace_remote_for_setting_auto-open_pref branch from 4a5ffd9 to f0ec1cc Compare October 10, 2023 08:42
@bmulholland bmulholland force-pushed the 10-04-chore_Replace_remote_for_setting_auto-open_pref branch from f0ec1cc to 0fc4a21 Compare October 13, 2023 07:23
@bmulholland bmulholland force-pushed the 10-04-fix_auth_Support_2FA_via_browser_window_auth branch from a3768db to 29fd59c Compare October 13, 2023 07:24
Base automatically changed from 10-04-chore_Replace_remote_for_setting_auto-open_pref to main October 18, 2023 13:30
@bmulholland bmulholland mentioned this pull request Mar 22, 2024
1 task
@setchy setchy added the refactor Refactoring of existing feature label Mar 27, 2024
@setchy setchy mentioned this pull request Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Refactoring of existing feature
Projects
None yet
3 participants