pyronear / pyro-platform

Detection & monitoring platform of wildfires
https://platform.pyronear.org/
Apache License 2.0
10 stars 11 forks source link

User credentials storage and token refresh #88

Closed Akilditu closed 6 months ago

Akilditu commented 2 years ago

Hi Everyone,

For now this "patch solution" has been adopted (#85) to keep users's sessions alive once logged :

  1. We store user credentials in a session-scoped dash component once logged
  2. We use those username / pwd to refresh tokens when expired

The main purpose was to maintain minimal alert screen's broadcast "forever" but this also leads to a lack of security.

Some options would be to

frgfm commented 2 years ago

Perhaps an idea in the long term:

pechouc commented 2 years ago

Thanks for opening this issue and for the proposals above. Letting the token expire after 24 hours might indeed be a solution (which I prefer to having the user log in every hour to be honest).

But I was wondering whether we may have an alternative by providing two options to renew the token:

client.refresh_token(user_ID=user_ID, admin_key=key)

Which would yield a new token, specific to the user's scope, without requiring full credentials. This second option would only be used by the platform to renew the user's token every hour.

Why could this be a solution? The goal with the token expiration, if I am not mistaken, is to make sure that anyone outside the org who obtains by chance a token for our API cannot exploit it for an unlimited amount of time. Right now, such a person would need valid credentials to renew the token, making the intrusion (normally) limited to 1 hour. Here, the person would need either full credentials or the admin key, which preserves the restriction objective.

frgfm commented 2 years ago

From a security perspective, we cannot go with your 2nd proposal

As of now, device are the only type of access with non-expiring tokens

Also:

That being said, I prefer to change the default expiration of tokens for some access scope than setting them to unlimited. I was actually thinking about switching back to expiring token for device (setting it to a month and adding a mechanism to request a new token each time).

Something to keep in mind:

The potential consequences of someone capturing an unexpiring token with, say, admin scope are disastreous :

pechouc commented 1 year ago

From a security perspective, we cannot go with your 2nd proposal

Thanks a lot for your reply!

But to be honest, I am not sure to clearly see the arguments in favour of your conclusion.

You explain that having unexpiring tokens would lead to security issues, especially with the admin scope. Although I understand this very well, I was not proposing to have unexpiring tokens.

To put it very simply, my proposal would be:

In that case, if someone captures a token, this person does not have unlimited access to the database, it only has for 1 hour. To have unlimited access to the database, one would need either the credentials corresponding to the token captured or the admin credentials. Note that already in the current situation, someone with credentials has an unlimited access to (part of) the database.

frgfm commented 1 year ago

admin credentials or some kind of admin authentication allow to refresh any token, be it a user or admin scope.

About that part, the current token is generated with some content (access ID + the scope + expiration timestamp). But even an admin cannot guess the access ID of a user without having the login for instance.

However, if I understand your suggestion:

Is that correct? I must admit I don't really see the point if so, because then you'll need to store the credentials for the admin token somewhere :man_shrugging: But maybe I'm misunderstanding

MateoLostanlen commented 6 months ago

Fix with new platform version