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

When removing an account, the keychain entry for the password should be erased #5752

Closed
SamuAlfageme opened this issue May 8, 2017 · 7 comments
Assignees
Labels
ReadyToTest QA, please validate the fix/enhancement sev3-medium type:bug
Milestone

Comments

@SamuAlfageme
Copy link
Contributor

Similar discussion to #5342.

Actual behavior

  • When you logout of an account in the client, the keychain entry for the password is removed. (Certificate and key are preserved though.)
  • When the account is deleted, all the information on the keychain is preserved.

Expected behavior

Semantically, the keychain entry for the deleted account should be removed on the account's deletion (deletion should be seen as log-out)

Steps to reproduce

  1. Remove an account from the client
  2. Check Keychain Access app (OS X) for ownCloud entries relative to the account that was removed

Seen it on OS X, but I believe it affects the rest of the OSS -> will reproduce there.

@ckamm
Copy link
Contributor

ckamm commented May 22, 2017

PR: #5787

@ckamm ckamm added the ReadyToTest QA, please validate the fix/enhancement label May 22, 2017
@SamuAlfageme
Copy link
Contributor Author

SamuAlfageme commented May 31, 2017

  • OS X
  • Windows
  • Linux

@SamuAlfageme
Copy link
Contributor Author

👍

@SamuAlfageme
Copy link
Contributor Author

Note to self: verify if OAuth access tokens are being removed from the keychain when deleting the account. I'm getting way too many stale entries lately.

@SamuAlfageme
Copy link
Contributor Author

SamuAlfageme commented Oct 24, 2017

Some findings after checking out the keychain integration:

  • Non-SSL (plain http://) connections also create a <username>_clientCertificatePEM & <username>_clientKeyPEM empty entries in the keychain
  • These are not erased together with the account (like the password is)

That's why I was getting the so many entries left. @ckamm would it be worth/easy to erase those as well?

@ckamm
Copy link
Contributor

ckamm commented Oct 24, 2017

@SamuAlfageme I'll check it out. Probably simple enough to get into 2.4

ckamm added a commit that referenced this issue Oct 25, 2017
This doesn't do anything about deleting the client cert keychain
entries when the whole account is removed though.
@ckamm
Copy link
Contributor

ckamm commented Oct 25, 2017

@SamuAlfageme PR is ready, see #6118 -- wiping the client cert keychain entries when the account is deleted won't make it into 2.4 though. But at least we'll not be creating these entries when there's not client cert anymore.

ckamm added a commit that referenced this issue Nov 3, 2017
This doesn't do anything about deleting the client cert keychain
entries when the whole account is removed though.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ReadyToTest QA, please validate the fix/enhancement sev3-medium type:bug
Projects
None yet
Development

No branches or pull requests

3 participants