pyronear / pyro-platform

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

[credentials] Logged credentials aren't used for API interactions #83

Closed frgfm closed 2 years ago

frgfm commented 2 years ago

There is a major problem regarding credentials:

The consequence is that the scopes are not respected: a user will see the devices of someone else. This needs to be fixed shortly!

pechouc commented 2 years ago

Once we instantiate the API client with the credentials used for login, we can use it to load alerts for the first time but we cannot store the client Python object to update the alerts afterwards. The simplest solution could be to (i) use the login credentials to create the API client and load the alerts for the first time with it, (ii) store the associated token in a dedicated Dash component and (iii) use this token to authenticate the user in the next calls to the API. How does this sound to you?

frgfm commented 2 years ago

@pechouc yes I agree! The routes of the alert API were designed for this :) I have no clue how to handle the cache in dash though :sweat_smile:

pechouc commented 2 years ago

Ok, I have started implementing this approach. Just a question however: how should we deal with the extinction of the token?

One possibility would be to store the credentials used for logging (the same way we store the user's initial token) and use them to re-authenticate each time we have a 401 error. But it feels weird to put the username and password somewhere of course.

Or maybe we can regularly refresh the token if the session keeps going? Adding a timer and updating the component that stores the token would be easy via Dash, but I am not sure of what would be the procedure to refresh the token.

frgfm commented 2 years ago

Just a question however: how should we deal with the extinction of the token?

Do you mean this? https://github.com/pyronear/pyro-platform/blob/master/app/main.py#L98-L100

One possibility would be to store the credentials used for logging (the same way we store the user's initial token) and use them to re-authenticate each time we have a 401 error. But it feels weird to put the username and password somewhere of course.

I'm not sure I fully get what you mean, but we don't need to store the creds unless we need to refresh. So I suggest caching the token, and rerequest the user to enter the creds when the token expires? Otherwise we need to encrypt this somehow

Or maybe we can regularly refresh the token if the session keeps going? Adding a timer and updating the component that stores the token would be easy via Dash, but I am not sure of what would be the procedure to refresh the token.

Either way, to refresh the token, you need the creds :sweat_smile:

pechouc commented 2 years ago

Yes, this is indeed what I mean.

And I don't think we want firemen to re-enter the credentials for the big minimal screen every hour, do we?

frgfm commented 2 years ago

And I don't think we want firemen to re-enter the credentials for the big minimal screen every hour, do we?

On the minimal screen, we can make a special token that doesn't expire

It needs to be tested, but (this is also kind of a problem) : the device_status & minimal screen can be accessed without logging in as of right now.

pechouc commented 2 years ago

Ok, well noted. I will open a quick PR where we can discuss this and what we implement before / after the Engie install.

Akilditu commented 2 years ago

This issue and #88 are kinda similar but I guess that #85 closes this one