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

See Auth added as a session variable #6982

Closed

Conversation

juggernautsei
Copy link
Member

Fixes #6981

Short description of what this resolves:

Adds the see auth as a session variable.

Changes proposed in this pull request:

@sjpadgett
Copy link
Sponsor Member

This is a very useful and requested option which I never knew wasn't actually implemented or somehow was pulled from core in the past. Anyways thanks @juggernautsei

@@ -1376,6 +1376,7 @@ public static function setUserSessionVariables($username, $hash, $userInfo, $aut
$_SESSION['authUserID'] = $userInfo['id']; // user id
$_SESSION['authProvider'] = $authGroup; // user group
$_SESSION['userauthorized'] = $userInfo['authorized']; // user authorized setting
$_SESSION['seeAuth'] = $userInfo['see_auth']; // user see_auth setting (see auth group)
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay so now we have a session flag where is it implemented? If we need the flag it is retrieved when the user is fetched and used in place. I don't get why we need another var in session if not used unless needed for modules.

I thought see auth wasn't working and this fixed!! Or am I missing something?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not implemented anywhere. That is why I added it. I used it to hide the provider list on the calendar so the provider only sees their own calendar and cannot view other providers calendars. That was the request.

Here is how I used it in the views/day/ajax_template.html

 // EOS FF
 // ==============================
 echo "</div>";
	if ($_SESSION['seeAuth'] != '2') {
 echo "   <select multiple size='5' name='pc_username[]' id='pc_username' class='view2 form-control'>\n";
 echo "    <option value='__PC_ALL__'>"  . xlt("All Users") . "</option>\n";
 foreach ($provinfo as $doc) {
	$username = $doc['username'];
	echo "    <option value='" . attr($username) . "'";
	foreach ($providers as $provider)
		if ($provider['username'] == $username) echo " selected";
	echo ">" . text($doc['lname']) . ", " . text($doc['fname']) . "</option>\n";
 }
 echo "   </select>\n";
	}
[-/php-]

Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay I guess I'm missing it because I can't find so I'm guessing you haven't implemented yet or my ide search engine is failing me?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sjpadgett, that code snippet was a mode I did for someone else. I did not submit that to the codebase because it may not have mass appeal. This is one of those if you build it they will use it.

@juggernautsei
Copy link
Member Author

Bump, will this make the patch?

@juggernautsei
Copy link
Member Author

Bump again...

@adunsulag
Copy link
Sponsor Member

@juggernautsei I'm fine with bringing in the session variable.

Your application in the calendar has a bug in it as it's using 'see_auth' instead of 'seeAuth'. In the instance of your use of it in the calendar application, it seems odd as the flag is supposed to be whether user's can see their own/others Authorizations in the Miscellaneous -> Authorizations screen. I can see how it makes it nice to piggy-back on the feature to use it for hiding / display calendar views for the provider but it seems like its doubling up on its intended use. I'd rather you back out the last commit and just leave it in the session like you had it originally. I'd be fine with bringing it in at that point.

@juggernautsei
Copy link
Member Author

I remember that I tested the feature first before creating the session variable. Then I did not go back to switch it out. No excuses. Just explaining what happened. @adunsulag good catch.
When people come to me, they don't want staff members seeing other staff members calendars at all. The fastest way was to remove the ability to see other staff members. As you know, seeing what other staff members schedule can cause hostility in some work places.
On another side not, I have never understood the Authorization page. There no explanation of purpose of the feature.

echo " </select>\n";

}
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi @juggernautsei , looks like you have a residual change here that is unintended?

@@ -355,9 +355,7 @@
foreach ($providers as $provider)
if ($provider['username'] == $username) echo " selected";
echo ">" . text($doc['lname']) . ", " . text($doc['fname']) . "</option>\n";
}
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to revert this change @juggernautsei

echo " </select>\n";

Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can also revert this so that only 1 file has changes for the PR

@juggernautsei
Copy link
Member Author

@juggernautsei I'm fine with bringing in the session variable.

Your application in the calendar has a bug in it as it's using 'see_auth' instead of 'seeAuth'. In the instance of your use of it in the calendar application, it seems odd as the flag is supposed to be whether user's can see their own/others Authorizations in the Miscellaneous -> Authorizations screen. I can see how it makes it nice to piggy-back on the feature to use it for hiding / display calendar views for the provider but it seems like its doubling up on its intended use. I'd rather you back out the last commit and just leave it in the session like you had it originally. I'd be fine with bringing it in at that point.

Can I pick your brain on how would you apply seeAuth on the calendar?

@adunsulag
Copy link
Sponsor Member

@juggernautsei What I'm saying is that seeAuth has another purpose in the system and if you mingle it with the calendar it could create problems for people who are using it for its intended purposes in the Authorization screen. If you want to use that flag, I'd introduce a variable in the calendar (seeOwnAppointments or something like that) and then fire off an event if one doesn't already exist in PHP land in the calendar controller code.

Let your module grab the event and set the seeOwnAppointments value using the seeAuth session value that this PR brings in the way you want it to be used. That way you can customize the way your installations utilize the seeAuth and not impact other installations that are using it a different way.

If no one really uses the Authorization screen then I think it'd be fine to re-purpose the seeAuth value, but we'd need to hear from enough vendors/admins that its not used before I'd feel comfortable re-purposing the flag.

Other options would be to have another flag for just this purpose in the user's table, or something in the user global settings, or even using the ACL system. Its more work and may be beyond meeting this customer's need so you may not want to do it right now.

@juggernautsei
Copy link
Member Author

@adunsulag again we don't know what we don't know. And this was a clear case of I did not find a use in the code for Mine only. did but I found it today because I was looking for sse_auth and they had named it seeAuth. There is were the conflict came in.
I will drop this PR and look to do something else. This was not for a module. This was someone out of the Philippians that wanted a way to had others calendar. Which for me is a regular request. In this use case they are trying to restrict those marked as clinicians to see only their own calendars. On a case by case basis. I will find another way to change the original behavior. Thanks for your input.

@sjpadgett
Copy link
Sponsor Member

Umm This would make for a good core addition or fix. I don't think it is working now at least like people think it's supposed to work. You might think about a new additional flag in the See Authorizations like 'Only Mine include Calendar' and set your new flag and original flag.
Just a thought

@juggernautsei
Copy link
Member Author

@sjpadgett I will keep this thought around. I am glad that @adunsulag pointed out the original intent of the flag in the users settings. I still don't know what the Misc Authentication is supposed to do but that does not matter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adding See Auth as a session variable
4 participants