Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Able to assign group admin to unexisting group #5868

Closed
PVince81 opened this issue Nov 14, 2013 · 10 comments
Closed

Able to assign group admin to unexisting group #5868

PVince81 opened this issue Nov 14, 2013 · 10 comments

Comments

@PVince81
Copy link
Contributor

Steps to reproduce

  1. Go to the users page
  2. In the controls bar behind the password field, click the "Groups" dropdown
  3. Click "+ add group"
  4. Type in "unexist"
  5. Close the dropdown, do NOT click "Create".
  6. In the user list, assign a user to the group "unexist"
  7. Refresh the page

Expected result

At step 6, the non-existing group should not happen yet in the "Group admin" dropdown of the users list.

Actual result

Possible to select the not existing group.
After refresh the dropdown appears empty because it tries to select a value that it not in the dropdown.
The database table oc_group_admin has an entry with the non-existing group.

Versions

OC 6 beta 4

This is not a showstopper, as it doesn't break much.
It is still possible to create group admins.

Is there a reason why we're not using foreign keys to prevent such cases, maybe database compatibility ? @DeepDiver1975

@PVince81
Copy link
Contributor Author

Setting to OC7 as it's not a showstopper.

@DeepDiver1975
Copy link
Member

Is there a reason why we're not using foreign keys to prevent such cases, maybe database compatibility ?

My evil me wants to answer: some devs have no clue about proper database design 👿

But at the end it comes down to the broad support of databases - e.g. MySQL engine MyISAM has no clue about foreign keys

@tigran-a
Copy link

tigran-a commented Dec 9, 2013

BTW, wouldn't it be better to add integer autoincrement ID field to the users and groups tables ?
(since these fields are used in other tables, that would be more "academic" I guess, otherwise the space is wasted for storing multiple copies of arbitrary strings). However it might require some code rewriting...

@craigpg craigpg modified the milestones: ownCloud 8, ownCloud 7 Sep 2, 2014
@DeepDiver1975 DeepDiver1975 modified the milestones: 8.1-next, ownCloud 8 Jan 9, 2015
@DeepDiver1975
Copy link
Member

@MorrisJobke please have a look once you work on user management for 8.1 - THX

@PVince81
Copy link
Contributor Author

PVince81 commented Feb 4, 2015

or @LukasReschke.

I haven't retried in 8.0 yet

@DeepDiver1975 DeepDiver1975 modified the milestones: 8.1-current, 8.2-next Apr 4, 2015
@PVince81
Copy link
Contributor Author

Still happening on master (8.2 pre-alpha)

@ghost
Copy link

ghost commented Sep 9, 2015

moving to 9.0

@ghost ghost modified the milestones: 9.0-next, 8.2-current Sep 9, 2015
@rullzer
Copy link
Contributor

rullzer commented Dec 22, 2015

I can't reprodce this anymore. The user page does not even shot the bug anymore and the subadmin stuff should not also properly handle this.

@PVince81 please confirm and close ;)

@PVince81
Copy link
Contributor Author

Works fine now on master 1b36987 / 9.0 pre-alpha

@lock
Copy link

lock bot commented Aug 7, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants