Skip to content

Confusing file permission message after authentication (Windows/Mac) #249

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

Closed
m-mohr opened this issue Oct 11, 2021 · 5 comments
Closed

Confusing file permission message after authentication (Windows/Mac) #249

m-mohr opened this issue Oct 11, 2021 · 5 comments
Labels

Comments

@m-mohr
Copy link
Member

m-mohr commented Oct 11, 2021

When authenticating with OIDC on Windows (e.g, connection.authenticate_oidc()), you get the following warning after successfully running through the Device code flow:

c:\dev\openeo-python-client\openeo\rest\auth\config.py:34: UserWarning: File C:\Users\m_mohr08\AppData\Roaming\openeo-python-client\refresh-tokens.json could be readable by others: mode 666 (expected: 600).
warnings.warn(message)

While this is true, it doesn't make a lot of sense to show this to Windows users. CHMOD is known by Linux users usually, but on other OS (primarily Windows, but I guess this may also be true for MacOS users) this is usually not known and very rarely used. So for the average Windows user, this is pretty confusing. Especially as these numbers are not used. You only get something like this on Windows:

image

@soxofaan
Copy link
Member

Indeed, permission management on windows is a known issue: #198
Unfortunately I don't have a lot of experience with programmatic file permissions management on windows.
We're just using standard file permission functionality built in in Python, which is linux/posix oriented. On Mac it works also fine last time I tried.

The goal of #198 is to automatically set the right file permission on windows too,
but in the mean time regarding the warning message: should we just drop the part mode 666 (expected 600) on windows?

@m-mohr
Copy link
Member Author

m-mohr commented Oct 11, 2021

I wouldn't try to set the permissions at all on Windows (non-servers?) until a better solution may be implemented in #198.
The file seems to be stored in a user directory (e.g. C:\Users\m_mohr08) and is somewhat protected through that and the error message for Windows users really just confuses mostly everyone that's not deep into Windows (Server) administration. You could mention it in the docs though.

soxofaan added a commit that referenced this issue Oct 11, 2021
@soxofaan
Copy link
Member

replaced the warning with INFO level logging (which is invisible by default)

I'm not sure what to describe in the docs. Are you familiar with best practices on windows regarding files that might contain sensitive data (e.g. tokens)?

@m-mohr
Copy link
Member Author

m-mohr commented Oct 11, 2021

I think the Windows best practice would be to not store it in a file (due to lack of good permission management), but instead, use the Windows Credentials Manager. But that won't help here, I guess. So, ideally, you should remove the file if it's a "shared" computer (similar to how cookies are erased on such computers after the Browser has been closed). But we don't know that, so maybe just give an option to "logout" and emphasize that this in the docs that this is important in case the computer is a "shared" / public one?

@soxofaan
Copy link
Member

a logout feature is an interesting idea -> #250

will close this ticket (about message that is not visible anymore) for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants