I've noticed that there are still isguest() references in moodle 1.7 in /user/edit.php. Is this something that I ought to put in Tracker? I had a search but didn't find a reference. My understanding is that these is*() functions are to be wiped out!
If so, I'll gladly add it to tracker.
Greg.
Yes, they need to go.
We now have two types of guest ... the guest account itself and the state that one is in when "just visiting" a course, so keeping that function could be confusing.
The way we test whether someone is really logged in as the guest user is ($USER->username == 'guest'). We probably need a new function for that. isguestuser() ?
Yes please, file a bug!
We now have two types of guest ... the guest account itself and the state that one is in when "just visiting" a course, so keeping that function could be confusing.
The way we test whether someone is really logged in as the guest user is ($USER->username == 'guest'). We probably need a new function for that. isguestuser() ?
Yes please, file a bug!
isguestuser() gets my vote 
That's OK, but wouldn't isnotloggedin() be better?
Or maybe do there reverse, an isloggedin() function that does $USER->username != 'guest', the other code would look like if (!isloggedin()) { ... }. That looks quite readable to me.
I suppose my point is that the guest usename is an implementation detail. Whether the person has logged in is what we are really interested in.
P.S. should this thread me moved to general developer forum, so more people see it?
Or maybe do there reverse, an isloggedin() function that does $USER->username != 'guest', the other code would look like if (!isloggedin()) { ... }. That looks quite readable to me.
I suppose my point is that the guest usename is an implementation detail. Whether the person has logged in is what we are really interested in.
P.S. should this thread me moved to general developer forum, so more people see it?
P.S. should this thread me moved to general developer forum, so more people see it?
You mean, like ME? Heck I started it and I just now realized it's active
...Looks like it's carried on quite well without me 
. I guess I really HAVE been busy the last couple weeks.
You've got my vote to move it to GDF...
You mean, like ME? Heck I started it and I just now realized it's active
...Looks like it's carried on quite well without me . I guess I really HAVE been busy the last couple weeks. You've got my vote to move it to GDF...
Does MDL-7903 relate to this problem/confusion ?
I agree guest accounts and roles combined with not-logged-in users might be very confusing in 1.7. I guess the right way is to tweak the functions, clean up the code and add development docs.
I was working on guest enrolment just yesterday and I think I understand it now enough to propose a cleanup solution, I will file a bug and some stuff for review. I can also do the cleanup in main code base after we finish the ever growing accessibility list.
BTW we already have isloggedin() which is used for something else and $USER->loggedin which does something similar but not the same
I was working on guest enrolment just yesterday and I think I understand it now enough to propose a cleanup solution, I will file a bug and some stuff for review. I can also do the cleanup in main code base after we finish the ever growing accessibility list.
BTW we already have isloggedin() which is used for something else and $USER->loggedin which does something similar but not the same
In that case -100000 for introducing an isguestuser() function. There should only be one function, and it should do the right thing for all cases. Two similar but different functions would just lead to bugs.
Things related to guest, not-logged-in users and associated default roles are a bit confusing now 
There are several types of guests with legacy guest capability:
1/ real guest user where $USER->username == 'guest'
2/ user enrolled in course with Guest role or with guest role assigned at system level (it does not matter if he/she is enrolled in course with another role)
3/ user not enrolled in the course in case this course allows guest logins - default guest role which is obtained from get_guest_role() - which is not configurable, first role with moodle/legacy:guest
4/ not logged-in user in site course when login is not forced - from default not logged-in role defined by $CFG->notloggedinroleid
5/ not logged-in user in scripts that do not require login - from default not logged-in role defined by $CFG->notloggedinroleid
The 'moodle/legacy:guest' should not be used in module code at all, it usually indicates a missing capability there. It is negative capability, it usually excludes somebody from doing something, while the rest of capabilities allow you to do something. The general usage pattern is "if guest then prevent user to do something". This is the reason why you have to do:
has_capability('moodle/legacy:guest', $context, NULL, false)
please notice the last 'false' which says 'ignore admin' or else it would return true for admins too, because they have all capabilities including legacy:guest
isguest() and has_capability('moodle/legacy:guest'... are both obsoleted and should not be used anymore, instead define a capability that grants the needed capability and use it. Sounds easy, isn't it? But there is one placethat still relies on guest user concept - course settings and enrollment. Guest access options still present there, IMO the guess access into course should be solved by a special enrollment plugin. It could be solved by moving enrollment settings from course table into a new one and storing course specific enrollment settings in it. It would solve other problems that we have now, such as the PayPal currency and cost, shared password for guest access and enrollment, etc.
Not logged in user is allowed into site course by require_course_login(), require_login() is used to protect places that accept only logged-in users.
I am proposing:
1/ add new standard function isguestuser() - use it where we deal with real guest user instead of $user->username == 'guest'
function isguestuser($user=NULL) {
global $USER;
if ($user === NULL) {
$user = $USER;
} else if (is_numeric($user)) {
$user = get_record('user', 'id', $user);
}
if (empty($user->id)) {
return false; // not logged in
}
return $user->username == 'guest';
}
2/ review the use of 'moodle/legacy:guest' - replace some with isguestuser(), add needed capabilities or keep as is
3/ remove all isguest() - use isguestuser(), add missing capabilities and in some places has_capability('moodle/legacy:guest', $context, NULL, false)
4/ add admin setting to choose guest role returned by get_guest_role()
5/ in 2.0 - add default logged-in and default not-logged-in roles into new installs; current default settings with Guest role are IMHO very confusing; this needs changes in access.php
6/ in 2.0 - rewrite course guest access
other not much related cleanup - problems I noticed while looking at guest code
7/ replace all empty($USER->id) with isloggedin()
8/ fix require_login() and require_course_login() to accept both $course and $courseid as parameters
9/ review the use of require_course_login() and require_login()
10/ fix all login redirections by defining get_login_url() - some are missing the https replacement
I hope all info here is correct - tried my best
There are several types of guests with legacy guest capability:
1/ real guest user where $USER->username == 'guest'
2/ user enrolled in course with Guest role or with guest role assigned at system level (it does not matter if he/she is enrolled in course with another role)
3/ user not enrolled in the course in case this course allows guest logins - default guest role which is obtained from get_guest_role() - which is not configurable, first role with moodle/legacy:guest
4/ not logged-in user in site course when login is not forced - from default not logged-in role defined by $CFG->notloggedinroleid
5/ not logged-in user in scripts that do not require login - from default not logged-in role defined by $CFG->notloggedinroleid
The 'moodle/legacy:guest' should not be used in module code at all, it usually indicates a missing capability there. It is negative capability, it usually excludes somebody from doing something, while the rest of capabilities allow you to do something. The general usage pattern is "if guest then prevent user to do something". This is the reason why you have to do:
has_capability('moodle/legacy:guest', $context, NULL, false)
please notice the last 'false' which says 'ignore admin' or else it would return true for admins too, because they have all capabilities including legacy:guest
isguest() and has_capability('moodle/legacy:guest'... are both obsoleted and should not be used anymore, instead define a capability that grants the needed capability and use it. Sounds easy, isn't it? But there is one placethat still relies on guest user concept - course settings and enrollment. Guest access options still present there, IMO the guess access into course should be solved by a special enrollment plugin. It could be solved by moving enrollment settings from course table into a new one and storing course specific enrollment settings in it. It would solve other problems that we have now, such as the PayPal currency and cost, shared password for guest access and enrollment, etc.
Not logged in user is allowed into site course by require_course_login(), require_login() is used to protect places that accept only logged-in users.
I am proposing:
1/ add new standard function isguestuser() - use it where we deal with real guest user instead of $user->username == 'guest'
function isguestuser($user=NULL) {
global $USER;
if ($user === NULL) {
$user = $USER;
} else if (is_numeric($user)) {
$user = get_record('user', 'id', $user);
}
if (empty($user->id)) {
return false; // not logged in
}
return $user->username == 'guest';
}
2/ review the use of 'moodle/legacy:guest' - replace some with isguestuser(), add needed capabilities or keep as is
3/ remove all isguest() - use isguestuser(), add missing capabilities and in some places has_capability('moodle/legacy:guest', $context, NULL, false)
4/ add admin setting to choose guest role returned by get_guest_role()
5/ in 2.0 - add default logged-in and default not-logged-in roles into new installs; current default settings with Guest role are IMHO very confusing; this needs changes in access.php
6/ in 2.0 - rewrite course guest access
other not much related cleanup - problems I noticed while looking at guest code
7/ replace all empty($USER->id) with isloggedin()
8/ fix require_login() and require_course_login() to accept both $course and $courseid as parameters
9/ review the use of require_course_login() and require_login()
10/ fix all login redirections by defining get_login_url() - some are missing the https replacement
I hope all info here is correct - tried my best
I still don't understand the difference between isguestuser() == true, and isloggedin() == false.
But perhaps I am starting to. Is any of the following, is right:
Within one PHP session: (That is, one end user in a browser visiting a particular Moodle site).
Initially we are in state not-authenticated user.
Then, at some point, we may be asked to log in. At which point, if we log in successfully we end up as a real user, or if we log in as guest, we end up as a guest user.
Finally, if we click the logout button, or if our PHP session times out, we end up in state timed out user which is just like non-authenticated user, except that when we go to the login page, we see a bit of red text saying our session has timed out.
Then, when we try to do something within Moodle, some or all of the following checks are performed:
Those last two checks are actually rolled up in to a single operation has_capability, but I want to separate them because, as I understand it, most of this stuff about the various types of guest access happens at the search for roles stage (2). The data being searched normally comes from the role assignments table, but in guest access situations, an extra role is manually added to the list. Is that right?
If the above is right, perhaps we should tidy it up and add it to moodledocs.
But perhaps I am starting to. Is any of the following, is right:
Within one PHP session: (That is, one end user in a browser visiting a particular Moodle site).
Initially we are in state not-authenticated user.
Then, at some point, we may be asked to log in. At which point, if we log in successfully we end up as a real user, or if we log in as guest, we end up as a guest user.
Finally, if we click the logout button, or if our PHP session times out, we end up in state timed out user which is just like non-authenticated user, except that when we go to the login page, we see a bit of red text saying our session has timed out.
Then, when we try to do something within Moodle, some or all of the following checks are performed:
- a require(_course)_login check,
- then a search for the roles that user has,
- then a calculation of whether that combination of roles has the necessary capability.
Those last two checks are actually rolled up in to a single operation has_capability, but I want to separate them because, as I understand it, most of this stuff about the various types of guest access happens at the search for roles stage (2). The data being searched normally comes from the role assignments table, but in guest access situations, an extra role is manually added to the list. Is that right?
If the above is right, perhaps we should tidy it up and add it to moodledocs.
isloggedin() returns true only if user is not logged in yet. You can log in manually, by guest autologin or in case of some auth plugins you are logged-in automatically.
isguestuser() replicates the original function of isguest() which is now doing something more. Original isguest() was deprecated and extended to return true for some other types of guest access in order to improve backwards compatibility with legacy code.
require_login() also checks enrollments and assigns temporary guest role to real users not enrolled in course if the course allows guest access without password. Another function of require_login is to block access to hidden activities.
isguestuser() replicates the original function of isguest() which is now doing something more. Original isguest() was deprecated and extended to return true for some other types of guest access in order to improve backwards compatibility with legacy code.
require_login() also checks enrollments and assigns temporary guest role to real users not enrolled in course if the course allows guest access without password. Another function of require_login is to block access to hidden activities.
So, to clarify isguestuser() covers the bases where you are either:
- not-authenticated user
- logged in as guest user
I'm not sure, but in 1.7 not need use functions like isguest() nowhere, excepting only in link to logged-in user in header of pages and, may be, user-related system routines. In all other cases have need to check only context-based capabilities. Isn't it?
Preexisting functions is may be temporary saved to provide compatibility, but i can't see needs in adding new functions.
Preexisting functions is may be temporary saved to provide compatibility, but i can't see needs in adding new functions.
Unfortunately some core code still relies on guest user concept, the main reason are enrollments, In theory it should be possible to remove it completely, but AFAIK not now.
The main benefit of isguestuser() is that it helps a lot with code cleanup
The main benefit of isguestuser() is that it helps a lot with code cleanup