-
Notifications
You must be signed in to change notification settings - Fork 101
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: remove user credential from keyring #2627
base: develop
Are you sure you want to change the base?
fix: remove user credential from keyring #2627
Conversation
mslib/mscolab/file_manager.py
Outdated
@@ -39,6 +39,8 @@ | |||
from mslib.utils.verify_waypoint_data import verify_waypoint_data | |||
from mslib.mscolab.models import db, Operation, Permission, User, Change, Message | |||
from mslib.mscolab.conf import mscolab_settings | |||
from mslib.utils.auth import del_password_from_keyring | |||
from mslib.utils.config import config_loader |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this won't work,
the config_loader belongs to.the MSUI and this is the application which is installed on the client side.
The server MSCOLAB is on some other location and of course has no idea about the passwords the user has in his machines keyring.
You need to lookup where in the MSUI the Delete Account is used and implement it nearby.
![delete_account](https://private-user-images.githubusercontent.com/278728/410135313-36c048d2-ac90-4e02-b25a-af790d5c8a19.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MzkyNTM0NzEsIm5iZiI6MTczOTI1MzE3MSwicGF0aCI6Ii8yNzg3MjgvNDEwMTM1MzEzLTM2YzA0OGQyLWFjOTAtNGUwMi1iMjVhLWFmNzkwZDVjOGExOS5wbmc_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjUwMjExJTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI1MDIxMVQwNTUyNTFaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT0xZjEzNTczODgzYWI2Zjc5MGM1MDFlNWM2MThhMTNiYTkwYWUwNzU2OGNmYTkzODJmM2ZmMDY3MGU1OTNiODQ1JlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCJ9.f7xNdzQGTOcbm-NxUz0ZQNLZUjVXAUEMg9y4wPZz6VE)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is one of three from the issue.
You need to extend the existing test to verify that the keyring is cleared, see
https://github.com/Open-MSS/MSS/blob/develop/tests/_test_msui/test_mscolab.py#L873
There can be another key/value pair in the keyring when the server has the login protected.
https://mss.readthedocs.io/en/stable/mscolab.html#protecting-login
The auth_username is configured in the users configuration and can be retrieved from there https://github.com/Open-MSS/MSS/blob/develop/mslib/utils/config.py#L208
you need a second delete for this, e.g. keyring del f"MSCOLAB_AUTH_{url}" MSCOLAB_auth_user_name
Purpose of PR?: User password is removed from keyring when account is deleted from MSColab.
Fixes #2624