search

ausocean / cloud

GNU General Public License v3.0
1 stars 1 forks source link

Bench: Dissallow insertion of account into save request #155

Closed ao-david closed 42 minutes ago

ao-david commented 1 week ago

Since we started showing the account in the broadcast field, we have also been reading this value to make sure that a save will correctly retain the associated account.

Whilst the field is readonly, a user can edit their request and send a different account value as to what they have access to. This will allow the user to stream to any channel that has already generated a token, even if they don't have access to that channel.

Since we are only changing the account value through the generate token action, we should in all other cases read the account not from the form, but from the datastore to make sure that no malicious requests can be made.

scruzin commented 6 days ago

Would this be easier to accomplish if the broadcast config were elevated to a first-class datastore entity, and not merely a variable value?

ao-david commented 6 days ago

I think as well, it would be worth keeping track of which users have access to which YouTube credentials

scruzin commented 6 days ago

We have our User entity, which is an association between a user and a site.

If we're okay with the restriction that a given user only requires a single YouTube account for a given site, we could add a YouTubeID to the User entity. A user could have different YT accounts for different sites.

ao-david commented 5 days ago

I think the trouble will be if a different user is trying to edit broadcast settings for a broadcast they have access to, but not necessarily signed in as the youtube channel.

ie if an ausocean user is editing a broadcast that is streaming to the network blue channel, how will we be able to authenticate that this is an authenticated (ish) user?

Since we should only ever be changing the channel when generating a token (clicking generate token), we should only allow the account field to be changed in this case. In all other cases, we should just keep using the old account the broadcast has been set to.

At some point we should also allow a user/program to remove a channel and set it back to streaming to our default channel (probably NB not ausocean)

scruzin commented 4 days ago

YT account editing could be factored out of the broadcast config form, and handled by a separate form, which only the YT account owner is permitted to change.

The User entity should include the user's YT account, if any.

On Wed, 26 June 2024, 10:00 am David (AusOcean), @.***> wrote:

I think the trouble will be if a different user is trying to edit broadcast settings for a broadcast they have access to, but not necessarily signed in as the youtube channel.

ie if an ausocean user is editing a broadcast that is streaming to the network blue channel, how will we be able to authenticate that this is an authenticated (ish) user?

Since we should only ever be changing the channel when generating a token (clicking generate token), we should only allow the account field to be changed in this case. In all other cases, we should just keep using the old account the broadcast has been set to.

At some point we should also allow a user/program to remove a channel and set it back to streaming to our default channel (probably NB not ausocean)

— Reply to this email directly, view it on GitHub https://github.com/ausocean/cloud/issues/155#issuecomment-2190285700, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE3LX6CUDUTIIYFCTYROCY3ZJIDR3AVCNFSM6AAAAABJVWUB76VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOJQGI4DKNZQGA . You are receiving this because you commented.Message ID: @.***>

saxon-milton commented 2 days ago

I think the root of this problem is really the data flow of broadcast page/back-end handler. I don't think we should need to get the account from the page data. Shouldn't we only be saving config fields that are writable ? I don't think it needs to be the case that anything in the form, even read only things, get written to the BroadcastConfig value and saved.

saxon-milton commented 2 days ago

I think the root of this problem is really the data flow of broadcast page/back-end handler. I don't think we should need to get the account from the page data. Shouldn't we only be saving config fields that are writable ? I don't think it needs to be the case that anything in the form, even read only things, get written to the BroadcastConfig value and saved.

As with Alan's suggestion a couple of weeks ago, and which has been implemented for the other pages, now that the complexity of the data flow has increased, perhaps we should have multi handlers for the broadcast page, i.e. get and set maybe?