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

When setting User Groups to 'Defer to the User', setting can lead to user being told they have no permissions #3266

Closed
kim-fitness opened this issue Feb 17, 2020 · 5 comments
Labels
bug Undesired behaviour resolved A fixed issue
Milestone

Comments

@kim-fitness
Copy link
Contributor

Describe the bug
“Defer to the Users Setting” in user group setting leads to user cannot login to his specified page

To Reproduce
Steps to reproduce the behavior:

  1. Set the Login Options to a non default option in user setting page for user A (cacti/user_admin.php).
  2. Give the user A permission to the specified page from user setting page->Permissions tab and remove other permissions.
  3. Log out and log in, the user A will be redirected to the specified page.
  4. Log in as admin and put the user A to a user group.
  5. Set the Login Options as 'Defer to the Users Setting' to the user group and give the group permission to the specified page that set in step Few miscellaneous fixes, typos and minor things #2.
  6. Log in as the user A, the A will login failed as no permission to the page.

Expected behavior
When setting “Defer to the Users Setting” in user group setting, the group member should be login to his specified page as default.

Screenshots
If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

  • OS: [e.g. iOS]
  • Browser [e.g. chrome, safari]
  • Version [e.g. 22]

Smartphone (please complete the following information):

  • Device: [e.g. iPhone6]
  • OS: [e.g. iOS8.1]
  • Browser [e.g. stock browser, safari]
  • Version [e.g. 22]

Additional context
Add any other context about the problem here.

TheWitness added a commit that referenced this issue Feb 22, 2020
“Defer to the Users Setting” in user group setting leads to user cannot login to his specified page.
@TheWitness TheWitness added bug Undesired behaviour resolved A fixed issue labels Feb 22, 2020
@TheWitness TheWitness added this to the 1.2.10 milestone Feb 22, 2020
@TheWitness
Copy link
Member

The original fix merged from @kim-fitness was incorrect.

@ddb4github
Copy link
Contributor

The original fix merged from @kim-fitness was incorrect.

From original code:

		if (db_table_exists('user_auth_group')) {
			$group_options = db_fetch_cell_prepared('SELECT MAX(login_opts)
				FROM user_auth_group AS uag
				INNER JOIN user_auth_group_members AS uagm
				ON uag.id=uagm.group_id
				WHERE user_id=?',
				array($_SESSION['sess_user_id']));

			if ($group_options > 0) {
				$user['login_opts'] = $group_options;
			}
		}
  • SQL is conflict with if->judgement below.
  • Buildin login options are similar between User and UserGroup.
  • User's login options is extensible by global array and hook login_options_navigate. So User's login options can be 4 or more
    I am not sure if UserGroup has new design during moving ugroup plugin into Cacti 1.0 core.

So fixing can be:

  • Change UserGroup's Defer to the Users Setting to -1 as 0f884a0 , and keep if->judgement. UserGroup's login options can be extensible in future.
    • But end-user have to manually update login_opts for all user group.
    • upgrade/1.2.10.php can also do it.
  • Or keep UserGroup's Defer to the Users Setting as 4 with fe9ac8d fixing, add few code to check empty($group_options).

@TheWitness
Copy link
Member

So, the defer option is option 4. So, that's why the new code block specifically excludes option4. But I'm okay changing to

if (!empty($group_options)) {

TheWitness added a commit that referenced this issue Feb 23, 2020
Updating check to !empty() per discussion with Jing.
@TheWitness
Copy link
Member

Okay, I've made the update.

@kim-fitness
Copy link
Contributor Author

kim-fitness commented Feb 24, 2020

Okay, I've made the update.

The fix fe9ac8d is incorrect.

For the case that a user in two different user groups, and the two groups have different login opts (group #1 is 4, and group #2 is 1), then following sql will return 1 other than empty. (If we thought login-opts are sorted by 4 > 3 > 2 > 1, then the previous fix is incorrect.)

'SELECT MAX(login_opts) FROM user_auth_group AS uag INNER JOIN user_auth_group_members AS uagm ON uag.id=uagm.group_id WHERE user_id = ? AND login_opts != 4'

I will make a new PR for this issue, will keep the “Defer to the Users Setting” as 4.

@netniV netniV linked a pull request Mar 1, 2020 that will close this issue
@netniV netniV changed the title “Defer to the Users Setting” in user group setting leads to user cannot login to his specified page When setting User Groups to 'Defer to the User', setting can lead to user being told they have no permissions Mar 1, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Jun 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Undesired behaviour resolved A fixed issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants