Piwigo / Piwigo-Mobile

Piwigo iOS Mobile Application
MIT License
101 stars 30 forks source link

Do not store user password on device #314

Open rclement opened 5 years ago

rclement commented 5 years ago

Is your feature request related to a problem? Please describe. Currently, the iOS Piwigo app stores the user password in the device native keychain. Even though the native keychain should be safe, their is obviously room for improvement here in terms of account security. Every time the app is opened, the account credentials are sent server-side for login. One can easily observe this behavior using an HTTPS proxy such as mitmproxy.

Describe the solution you'd like The mobile app should only use the account password on first login, receiving both an access token and a refresh token from the identity server in exchange (for now, only a cookie named pwg_id is delivered from the server as authentication). Those tokens should be the ones stored in the device keychain instead of the password, and only those be used for all further requests with the Piwigo server.

Describe alternatives you've considered This is what OAuth2 / OpenID Connect is designed for (there are authorization grant designed for mobile apps). I understand that this app can only use what the Piwigo server can provide so I do not expect a resolution for this immediately, but I think this is worth mentioning for future enhancements.

Additional context Example of "bad request" when re-opening the app with an already logged-in account (captured using mitmproxy):

POST https://<account>.piwigo.com/ws.php?format=json&method=pwg.session.login
HTTP/1.1

Host: <account>.piwigo.com
Content-Type: application/x-www-form-urlencoded
Cookie: pwg_id=oh74s03kok5o...
Connection: keep-alive
Accept: */*
User-Agent: piwigo/2.3.5 (iPhone; iOS 12.3.1; Scale/2.00)
Accept-Language: en-FR;q=1, fr-FR;q=0.9
Content-Length: 69
Accept-Encoding: br, gzip, deflate

Body:
password: <plain-text-password>
username: <plain-text-username>
EddyLB commented 5 years ago

Related with issue #253.