Details
-
Type:
Bug
-
Status: To-Do (View Workflow)
-
Priority:
Major
-
Resolution: Unresolved
-
Affects Version/s: None
-
Fix Version/s: None
-
Labels:None
-
Story Points:0.5
-
Epic Link:
-
Sprint:Fall 4 2022 Oct 10
Description
When adding a secure data source, a user is asked to enter their password multiple times after submitting the data source originally.
The first 16 steps are the same as in IGBF-3195.
To reproduce:
1. Reset preferences by going to File > Preferences > Other Options > Reset Preferences to Defaults
2. Open IGB and go to Preferences > Data Sources > Add…
3. Set Name to: secure1
4. Type to: Quickload
5. URL to: http://igbquickload.org/secureQuickloadTestSites/secureSiteTest
6. Click Submit
7. A pop-up should appear.
8. It reads “https://igbquickload.org/rnaseq/ asks for authentication:” when it should read “http://igbquickload.org/secureQuickloadTestSites/secureSiteTest asks for authentication”
9. Enter User: “guest” and Password: “guest”
10. Make sure that Save Password is checked and click OK.
11. Again click Add..
12. Name: secure2
13. Type: Quickload
14. URL: http://igbquickload.org/secureQuickloadTestSites/secureSiteTest2
15. Click Submit
16. A pop-up appears showing the URL for the previous data source: https://igbquickload.org/secureQuickloadTestSites/secureSiteTest/ when it should have the …/secureSiteTest2 url
17. Enter User: “guest2” and Password: “guest2” .
18. Make sure that Save Password is checked and click OK.
19. Both secure1 and secure2 are now available data sources.
20. Make sure Auto Load Data is checked and close the window.
21. On the home page, click the A. thaliana genome icon.
22. IGB opens the pop-up again asking for the password for secure1, then another for secure2.
23. Entering the usernames and passwords again loads the data.
This problem was only found on IGB version 9.1.10
I tested on 9.1.8 with no issue.
Release testing protocol found at: Data Provider, Secure Data Providers. https://wiki.bioviz.org/confluence/display/ITD/Data+Provider#:~:text=Secured%20Data%20Providers
Attachments
Issue Links
- is blocked by
-
IGBF-3211 Update logic to send redirect url to IGB Data Source endpoint
-
- Closed
-
Activity
| Field | Original Value | New Value |
|---|---|---|
| Epic Link | IGBF-1765 [ 17855 ] |
| Status | To-Do [ 10305 ] | In Progress [ 3 ] |
| Assignee | Nowlan Freese [ nfreese ] |
| Status | In Progress [ 3 ] | To-Do [ 10305 ] |
| Status | To-Do [ 10305 ] | In Progress [ 3 ] |
There's a call to DataProviderManager.java getServerFromUrlStatic that is behaving oddly. It is being called when AddDataProvider.java checkValidAndSetUrl calls conn.getResponseCode() which then calls IGBAuthenticator.java getPasswordAuthentication which then calls DataProviderManager.java getServerFromUrlStatic. The getServerFromUrlStatic returns a DataProvider object, but it is unclear why we are checking if the data provider already exists, which because the secure site has not been added yet, it does not.
| Status | In Progress [ 3 ] | To-Do [ 10305 ] |
| Sprint | Fall 4 2022 Oct 10 [ 156 ] | Fall 4 2022 Oct 10, Fall 5 2022 Oct 24 [ 156, 157 ] |
| Rank | Ranked higher |
| Status | To-Do [ 10305 ] | In Progress [ 3 ] |
| Status | In Progress [ 3 ] | To-Do [ 10305 ] |
| Status | To-Do [ 10305 ] | In Progress [ 3 ] |
| Status | In Progress [ 3 ] | To-Do [ 10305 ] |
| Status | To-Do [ 10305 ] | In Progress [ 3 ] |
| Status | In Progress [ 3 ] | To-Do [ 10305 ] |
| Status | To-Do [ 10305 ] | In Progress [ 3 ] |
| Status | In Progress [ 3 ] | To-Do [ 10305 ] |
| Status | To-Do [ 10305 ] | In Progress [ 3 ] |
| Status | In Progress [ 3 ] | To-Do [ 10305 ] |
| Status | To-Do [ 10305 ] | In Progress [ 3 ] |
Several things to consider.
When we make the call to checkValidAndSetUrl the conn.getResponseCode() will trigger the Authenticator if the Quickload is a secure site. Authenticator is extended by IGBAuthenticator.java and makes a call to getServerFromUrlStatic in DataProviderManager.java. The getServerFromUrlStatic attempts to find the data provider based on the url. However, we haven't added the data provider as of the time when checkValidAndSetUrl has been called. This causes IGB to select a quickload at random and saves the password info to that quickload.
One of the potential workarounds is to add the data provider first, and then make a recursive call with the isEditPanel set to true. This in theory works, as the edit panel logic can then follow the redirect and modify the data provider, but it requires the recursive call be wrapped in a Timer as we run into a concurrency issue where the data provider is not added before the call to checkValidAndSetUrl is called, which then locks IGB. However, this solution fails when the user incorrectly enters the password to the secure quickload as IGBAuthenticator brings up the password panel while checkValidAndSetUrl again locking IGB. The best solution may be to lock everything until the user has entered the password correctly or cancelled out of the password panel, which may require changes to IGBAuthenticator.java in addition to DataProviderManager.java.
An additional option would be to modify checkValidAndSetUrl so that it does not attempt to connect to secure quickloads, and instead assumes that they do not redirect. While not the best solution, it would be a very minor edge case. However, I cannot find a way to determine if a URL is secure using HttpURLConnection. Any call trips the Authenticator and then begins the problems.
An additional option would be to revert the changes to redirect Quickloads until we have updated to Java 11 or greater. Java 11 introduced HttpClient which may help us to determine if the URL requires authentication without connecting, but I am not sure. There are also additional libraries that could be imported such as Apache HttpClient.
Finally, I'm not sure why IGB does not follow redirects by default. According to this post, HttpUrlConnection should follow redirects by default. Is it possible we can force IGB to follow redirects without using checkValidAndSetUrl?
I would like to work on this with Karthik on Tuesday. If we can't figure out a working solution, I would suggest reverting the redirect commits as I would like to prioritize the release.
| Assignee | Nowlan Freese [ nfreese ] | Karthik Raveendran [ karthik ] |
| Status | In Progress [ 3 ] | Needs 1st Level Review [ 10005 ] |
| Assignee | Karthik Raveendran [ karthik ] |
| Status | Needs 1st Level Review [ 10005 ] | First Level Review in Progress [ 10301 ] |
| Status | First Level Review in Progress [ 10301 ] | Needs 1st Level Review [ 10005 ] |
| Status | Needs 1st Level Review [ 10005 ] | First Level Review in Progress [ 10301 ] |
| Status | First Level Review in Progress [ 10301 ] | Ready for Pull Request [ 10304 ] |
| Status | Ready for Pull Request [ 10304 ] | Pull Request Submitted [ 10101 ] |
| Status | Pull Request Submitted [ 10101 ] | Reviewing Pull Request [ 10303 ] |
Would you please take a look at the above PR? It looks like there are four commits - three "merge" commits and the one reversion commit.
It would be brilliant if we could modify this so that the "merge" commits are not there.
attn: Nowlan Freese and Karthik Raveendran
I am working on the above now. My apologies for the delay - I think I can take care of it quickly.
Attn Nowlan Freese and Karthik Raveendran:
I cherry-picked Karthiks reversion commit and stuck it on a new branch aloraine-IGBF-3196, minus the merge commits.
The installers are built and are ready for download at https://bitbucket.org/aloraine/integrated-genome-browser/downloads/.
Would you take a look and let me know if this will work OK?
If yes, I would like to make a PR from aloraine-IGBF-3196 instead of the currently open PR so as to avoid the extraneous merge commits.
[~aloraine] - It looks good to me, I think it should be good to go ahead and merge to master.
At this time we are unable to modify the code such that both redirects and secure Quickloads work correctly. Additional refactoring will need to be done. See the comments above. Most likely we will need to refactor AddDataProvider.java.
OK thanks!
Made PR and am merging it now.
| Assignee | Ann Loraine [ aloraine ] |
Master branch installers are built. Notarizing the Mac installer now.
| Status | Reviewing Pull Request [ 10303 ] | Merged Needs Testing [ 10002 ] |
| Assignee | Ann Loraine [ aloraine ] |
Notarized installer master.dmg is uploaded to https://bitbucket.org/lorainelab/integrated-genome-browser/downloads/. Ready for testing.
| Status | Merged Needs Testing [ 10002 ] | Post-merge Testing In Progress [ 10003 ] |
| Status | Post-merge Testing In Progress [ 10003 ] | To-Do [ 10305 ] |
| Sprint | Fall 4 2022 Oct 10, Fall 5 2022 Oct 24 [ 156, 157 ] | Fall 4 2022 Oct 10 [ 156 ] |
The issue is due to checkValidAndSetUrl method introduced in
IGBF-3103where we looked to see if the http url was being redirected to https. This required connecting to the server to see if there was a redirect, but when the server is secure it appears to be initiating the IGB authentication, but it does not appear to save the password correctly. This is why the password is asked for again when loading the data.