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

Protect proxy password by not storing it between sessions #9652

Merged
merged 26 commits into from
Mar 27, 2023

Conversation

JacobTrossing
Copy link
Contributor

@JacobTrossing JacobTrossing commented Mar 6, 2023

Fixes #8055 by storing the proxy password only during run time and not in the system preferences. Also adds a pop-up at the start of the program if the user has checked "Use proxy" and "Use authentication" to allow them to enter the password for the proxy. An image of this pop-up can be seen below.

image

We considered using encryption to add extra security but were unsure if it was needed. What are your opinions on this?

  • Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

JacobTrossing and others added 25 commits March 1, 2023 14:28
 Instead asks user for the password at the start of the program if they
 have enabled proxy with password.
Don't accept the pull request and start the testing when password changes
have been implemented.
 No ui changes in this commit, leaves password field empty on launch
…ered from JabRefGUI if proxy requires password
…hentication are enabled, before only auth was checked
(Feat): #1 Added test for storage of proxy preferences
(feat) #2 Adds pop-up asking for password at launch.
(Fix) #10 Removed dependency issue test functions
@koppor
Copy link
Member

koppor commented Mar 20, 2023

Summarizing the requirements of the issue:

  1. Ask for proxy password at first connect
  2. Ask the user whether they want to store the password permanently
  3. If no: The password is stored in-memory only
  4. If yes: Store the password using the Windows credential manager (or other credential managers). See Do not save password in Preferences #8055 (comment) for a discussion on the libraries.

This PR addresses the "If no" case only. It does not address all the other requirements.

Could you comment on your success investigating the libraries listed at #8055 (comment) - to handle requirements 2 and 4?

I am not sure whether requirement 1 is easy to implement.

@koppor koppor added the status: waiting-for-feedback The submitter or other users need to provide more information about the issue label Mar 23, 2023
@Siedlerchr Siedlerchr merged commit f4f3b3d into JabRef:main Mar 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
preferences status: waiting-for-feedback The submitter or other users need to provide more information about the issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Do not save password in Preferences
8 participants