SimplePortal
Development => Bugs => Topic started by: andy on November 30, 2015, 05:41:40 AM
-
Found this by accident today. Maybe its not worth changing but I will mention it.
During testing changes for member groups I set block permissions for the new group (ID=26). I deleted the new group after some problems with the group permission profile and then created a new group again, with the same name.
It had the same ID number.
SP blocks still had the same permission settings for the old group which I had just deleted. Really I think it should have cleared them if the group was deleted.
Lucky for me though as I didn’t need to set them all again.
Andy
-
1. This would appear to be a SMF bug, if it was assigned the same ID, as every time I used to delete a group, then make new, it would use a new ID number.
2. You are correct, though, that if group is deleted, SP should also delete the settings for that group ID.
-
I second Burke on this one, he is totally right on both of those.
-
I had assumed SMF would use the next ID for a new group. I need to test again to see if that is true.
Even if it uses the same ID after one is deleted is hard to call that a bug (on the SMF side).
As I set newly registered members to be put in a specific group Im certain about the deleted group number (ID 26). I expected the new group to have an incremented ID number. It is not actually necessary like a topic as when a group is deleted it cannot be retrieved.
I havent confirmed if this happens with board IDs.
-
Bedtime here for me but I can confirm....
New boards have incremented IDs
New groups do not!
Well, Im assuming the naming of it makes no difference. Just done it again and same ID number was used after one deleted.
You could call it an SMF bug and a SP bug.
-
Looks official then now - a simple portal bug...
http://www.simplemachines.org/community/index.php?topic=541648.0
-
Never liked the re-use of IDs... :-\
-
Never liked the re-use of IDs... :-\
I hope you've fixed this for Elkarte then.
We should probably sanitize SP's data when a membergroup is deleted, and check to be sure that we sanitize other data upon deletion of other elements when appropriate.
-
Never liked the re-use of IDs... :-\
I hope you've fixed this for Elkarte then.
Nope, because it's the first time I noticed. LOL O:-)
-
You could throw this one into the bug tracker, too - http://simpleportal.net/index.php?project=1
-
Could add a "maintenance" section to the SP admin, that runs a "optimize database" function that will clean up ununsed data. Also could change the functions for creating new blocks to delete the permissions when deleted
-
I noticed several things do not function uniformly or consistently in SMF.
If topics and board IDs are automatically incremented even if deleted then so should member groups. Over there they say its a saving for SMALLINT but its ridiculous for member groups - they will never reach the limit.
I just looked over there and surprised by the extra comments on SMF. Same old story - no need to change.
-
I noticed, even Bruno started to be disappointing since he joined the team, a bit sad.
-
I noticed several things do not function uniformly or consistently in SMF.
Typical SMF. Then they want to focus the blame elsewhere.
I just looked over there and surprised by the extra comments on SMF. Same old story - no need to change.
Again, typical, and this time, Old Kindred showed his true SMF/Coding ignorance, again.
-
If topics and board IDs are automatically incremented even if deleted then so should member groups. Over there they say its a saving for SMALLINT but its ridiculous for member groups - they will never reach the limit.
Take this scenario for example, imagine that you created a new group called "maintenance administrators" where they have various admin_forum permissions. This group is ID#20. You create various PHP scripts that look for "group 20" that are both external and internal.
Now say you deleted the group on accident when managing membergroups or account information. This group no longer exists, and any external scripts will no longer be able to find group #20. If you recreate a new group, it might or might not be #20 depending if there is a #21 existing already. If there is no group #21 or further, it will create #20 so you "fallback" your mistake. Without the fallback the only way to append that situation is to make a new group and then manually change its ID in the database.
It's designed purposely to fallback a mistake, which is about 90% of what would happen if you delete a membergroup intended to have high level privileges. Of course this can be bad because if you delete the group and then end up making a completely different group later down the road and then it ends up having your previous group's admin permissions without you knowing... this is a security risk. SMF should be automatically cleaning out data that is currently not associated. I think that is the real issue here-- the data is not being cleaned out, and not the increment process.
On the other hand, you should be aware of the permissions of every group as you're given quite a lot of tools through SMF to check them.
-
No offences meant, but deleting a membergroup by mistake, is a very, very slim chance, compared to choosing to delete one that is no longer needed.
Then what happens, when a new group takes that ID, made for a "banned" group, and the admin was not aware of this and missed on making needed changes that had been set for the old group. Then we have people in a banned group, having admin abilities. Unacceptable and a downright security issue. Especially, since those at SM.org always says, do not use the ban system, which I do agree with, as it tends to strain the servers, but making a banned group instead is a good way.
I'm sorry, I'm one that says the number should not be re-used, and that's for security reasons. If one accidentally deletes a membergroup, then they can restore it via a backup. Don't have a backup? Too bad, it's only the MOST recommended thing to do. Look in 85% of topics at SM.org. ;)
-
Agree with Burke on my part :/
-
Take this scenario for example, imagine that you created a new group called "maintenance administrators" where they have various admin_forum permissions. This group is ID#20. You create various PHP scripts that look for "group 20" that are both external and internal.
Let me stop right here.
If your script is meant to be executed automatically (e.g. by cronjob), there is absolutely no reason to check for permissions because in any case it would not be run "under" a certain user ID, and as such it will be a "guest", so it will not have any particular permission (and write a bot that simulate a member belonging to a certain group it's quite the effort just to run such a script).
If your script is meant to be executed based on the "current member" but adding a "maintenace admin" group in order to let the users do their work, then this is bad design. Sorry.
And finally: when checking permissions you should never, ever base your tests on the group ID, but always on the allowedTo functions.
So, nope, the example scenario doesn't justify the reuse.
This is very simply a design choice of those who coded the thing first, it dates back to YaBB (not YaBB-SE, really YaBB, the one in perl) where groups were saved in a file as an array. My perl is quite rusty, but I think the array didn't keep track of ids to simulate an autoincrement and so the last one was scraped when the group was removed without keeping any trace. Adding a new group, then, was replacing the previously existing group id.
Probably the behaviour was kept at first to maintain backward compatibility, then for laziness.
BTW, a nice article: http://blog.8thlight.com/uncle-bob/2014/04/03/Code-Hoarders.html :P
Ninja edit: fixed your usual typo with closing code tags everywhere, lol
-
Apart from fixing your closing tags... What about the fix to force it to increment the membergroup ID? Would it be easy or does it require massive amounts of work? I never dealt with these particular things before so I have no idea :D
-
Remove the MAX-query and drop the id from insert query.
I guess that's all there is to do (maybe retrieve the inserted id afterwards if needed for the redirect).
I doubt there is to touch anything else, those already inserted are already fine, any new one will be "good".
-
Remove the MAX-query and drop the id from insert query.
I guess that's all there is to do (maybe retrieve the inserted id afterwards if needed for the redirect).
I doubt there is to touch anything else, those already inserted are already fine, any new one will be "good".
Well, fancy doing what we need to fix the problem for the next SP version?
-
Likely add a function to clear out all data when a membergroup is deleted to follow that stupid smf bug. Code-wise... Ask emanuele ;D
-
The major issue is the fact this never would be an issue, if not for the SMF bug, that people like Kindred think:
I dont call it a bug or seexthe need to change it...
They need to learn over there, that the Project Manager needs to keep his mouth shut on stuff he knows squat about.
-
(http://i3.kym-cdn.com/photos/images/original/000/540/658/5e8.jpg) (http://knowyourmeme.com/photos/540658-beating-a-dead-horse)
:angel:
It's a quote from team boards so I censored the author and the link (NDA bla bla, but this is too funny and I'm not leaking actual info), but you got the meaning ;D
-
I don't see how this is a SimePortal bug. It's the fault of SMF for re-using the IDs for member groups and I can't think of any good reason why you would ever want to do that. You can't expect every mod out there to make sure this behavior isn't messing things up, especially when you don't even provide any useful hooks for member group related functions in SMF 2.0.
-
Because they are using a smallint or something and they assume people will create tens of thousands of groups in their forums :P Btw, have a look at the tracker when you have time, please.
-
I said over there that the way SMF handled IDs wasn’t uniform... but all they say is it was 'by design'.
They need to learn over there, that the Project Manager needs to keep his mouth shut on stuff he knows squat about.
I just had another run in with a not so Kindred spirit over there. The comments he makes just fans the flames.
Said there were dozens of responsive themes but later said that was a little exaggerated. More like 10 times exaggerated.
-
Don't get me started about Kindred and his lack of Project Manager skills.
Seems he's only that, due to his own "donations" to the SMF project and organization.
He sure is not PM due to his knowledge of SMF. XD
-
Thank you for the post....I think I’d like to speak but I’am gonna do some more reading first.....I have to say that idea looks intriguing....