osuAkatsuki / bancho.py

An osu! server for the generic public, optimized for maintainability in modern python
https://akatsuki.gg
MIT License
213 stars 128 forks source link

fix: ignore "0" disk-serial md5 on client hash checks #604

Closed minisbett closed 7 months ago

minisbett commented 7 months ago

Describe your changes

Some disk manufacturers do not have a disk serial set, defaulting to the number 0. This can cause lots of trouble over time as anyone with such a disk serial will not be able to verify unless manually approved by staff.

I think this check is safe to implement and does not disregard the actual purpose of this check.

I'm not sure this is the right place to implement it though.

Checklist

minisbett commented 7 months ago

it points out why an automatic restriction policy for this is generally flawed in my eyes

I mean, it doesn't automatically restrict. It just prevents verification (first login).

NiceAesth commented 7 months ago

would prefer an allowed serials table adding on to what @tsunyoku said

perhaps we can seed with some default values in migrations or something

minisbett commented 7 months ago

list config variable (with defaults) or a database table sounds better, just because if people find new ones then they don't have to PR every time.

hmm, haven't come across any other than this one so far but fair.

tsunyoku commented 7 months ago

I mean, it doesn't automatically restrict. It just prevents verification (first login).

fair, i forgot. still sucks for both parties though

minisbett commented 7 months ago

Would you rather use a database table or config? I feel like the config makes more sense, I wouldnt really put configuration values that are barely ever touched into the database.

tsunyoku commented 7 months ago

hmm, haven't come across any other than this one so far but fair.

it's not really a predictable thing so yeah, the issue is just that manufacturers are not forced into creating unique identifiers (if any at all) so there is many cases of these clashing identifiers that depend on what the manufacturer decided or what the OS decided. there's no real way to deterministically solve this issue, which is why i mentioned any kind of auto enacting policy is quite flawed

tsunyoku commented 7 months ago

Would you rather use a database table or config? I feel like the config makes more sense, I wouldnt really put configuration values that are barely ever touched into the database.

the advantage i see (probably doesn't matter in the future) is being able to modify without restarting. this wouldn't matter but bpy is stateful so it would mean yeeting sessions, multis etc. to change this. i guess it opens a can of worms about other config variables that exist, but this one in particular isn't sensitive and also not a simple toggle so i think it's justifiable.

cmyui commented 7 months ago

Tested and working