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

Add ability to access authenticated resources using pluggable internet accounts framework #2279

Merged
merged 158 commits into from Oct 6, 2021

Conversation

peterkxie
Copy link
Contributor

This feature adds internet accounts to JBrowse 2 for use with authentication. This initially comes with support for 4 methods

  1. OAuth
    • Dropbox
    • Google Drive
  2. External Tokens (enter a token from an external resource to use in an Auth Header)
  3. HTTPBasic

peterkxie and others added 30 commits June 4, 2021 16:13
…lugin, will make new branch with just plugin logic tomorrow to mvoe away from hardcoding endpoints
…pp with plugin and allowing for custom headers with auth token
…nCreate creates an internetaccountmodel for each internet account in the config
…oken from storage if it exists instead of always opening flow, add model for existingtoken flow i.e GDC
…google and others, continue working on externaltoken model
…getBlob so accessToken can be passed to webworker, next need to replace existing openlocations with the rootmodel one
…cmethodtypes to pass the token, eventually can move specific code out of addtrackwidget and add here
…ddtrackwidget, this should also allow for hand editing
…piration to try refresh token or log in, add in more refreshtoken logic, once things are smoothed over can move on to external token model and done with Oauth
@cmdcolin
Copy link
Collaborator

cmdcolin commented Oct 1, 2021

random thing to note: the file selector for the "Open sequence" dialog will not be able to access authenticated resources on jbrowse desktop from it's start screen. no pluginmanager exists at the startscreen as is. if we needed to, we could maybe make a blank rootmodel or something of this sort to assist

@cmdcolin
Copy link
Collaborator

cmdcolin commented Oct 3, 2021

I found a possible race-condition but I found a failure while opening a Google Drive file for a BAM track

Re-entering the data in the same way resulted in a working track, and refreshing the page fixed the broken track

Screenshot from 2021-10-03 17-02-53

@cmdcolin
Copy link
Collaborator

cmdcolin commented Oct 3, 2021

Currently all tracks appear to fail to load on desktop when using the preloadedSessions e.g. checkbox of hg38 with this error

Screenshot from 2021-10-03 17-20-51

It is looking for e.g.

    const selectedId = location.internetAccountId

Which is undefined, that then it runs basically undefined.split('-')

@peterkxie
Copy link
Contributor Author

I tried using a google drive share link on https://jbrowse.org/code/jb2/604_oauth_new_implementation/ but it results in 'Error 400: redirect_uri_mismatch' ?

Is the client id authorized for jbrowse.org?

I had the redirect uri for the main branch but had removed the redirect uri for this specific branch so that's why there was a mismatched

@peterkxie
Copy link
Contributor Author

Currently all tracks appear to fail to load on desktop when using the preloadedSessions e.g. checkbox of hg38 with this error

Screenshot from 2021-10-03 17-20-51

It is looking for e.g.

    const selectedId = location.internetAccountId

Which is undefined, that then it runs basically undefined.split('-')

Thanks for catching that, I fixed it on the web rootmodel but forgot to sync over the changes to the desktop rootmodel too

@peterkxie
Copy link
Contributor Author

While pairing with @garrettjstevens , realized that OAuth Authentication was not working on production jbrowse-desktop since the window.location is a file:// format which does not work well with OAuth redirect uri's. Changed desktop logic to use a dummy redirect_uri since we just need the information. Google drive does work on production desktop with these new cahnges

Dropbox seemed to stop working on desktop and web, it seems to have been broken even before these changes. We suspect it may have something to do with dropbox servers, since it was working for me on Friday without any changes to the code, and since there has been issues with various internet services being down today, we are going to double check tomorrow to see if it's a dropbox server issue or an actual issue with the code before merging

@cmdcolin
Copy link
Collaborator

cmdcolin commented Oct 5, 2021

Can we avoid the change to the data adapter constructor prototype? This change would, for example, break the quantseq plugin which uses getSubAdapter

@peterkxie
Copy link
Contributor Author

will be moving pluginManager to the third param and making it optional

@peterkxie
Copy link
Contributor Author

@cmdcolin when you have some time could you check out the changes in the latest commit (moving pm to optional and the third parameter) to make sure I did it correctly + it won't break older plugins now

@peterkxie peterkxie merged commit 24b45e2 into main Oct 6, 2021
@cmdcolin cmdcolin deleted the 604_oauth_new_implementation branch October 7, 2021 00:13
@cmdcolin cmdcolin changed the title Add Internet Accounts and Authentication Capabilities to JBrowse 2 Add ability to access authenticated resources using pluggable internet accounts framework Oct 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
JBrowse team board
  
Awaiting triage
Development

Successfully merging this pull request may close these issues.

Accessing authenticated resources
5 participants