SecretURL prompting for a password

[Gary Beldon]

If a View contains resumé information displayed in a box (Employment History, Education History, etc.), and is accessed exclusively via a SecretURL, then it asks for a password. Only the first time though, subsequent access doesn't ask for a password even if you don't enter one the first time.

Mahara 1.2.4
Ubuntu Linux 8.04
MySQL

[François Marier (fmarier)]

The password prompt might be coming from some AJAX request...

[Alan McNatty (alanmc)]

One to be reviewed with a wider audience.

Basically view/view.php allows access by checking viewid and taking into consideration user and mnet tokens (consider also get_view_from_token) as follows:

if (!can_view_view($viewid, null, $usertoken, $mnettoken)) {
    throw new AccessDeniedException(get_string('accessdenied', 'error'));
}

However when we try and render the actual resume object (a ArtefactTypeResumeComposite which extends ArtefactTypeResume) we find it's own render_self is a little bit broken (it is the only type that checks access again for example). A simple fix is to remove this check and hence the exception is not raised but it suggests that the class may need general review.

diff --git a/htdocs/artefact/resume/lib.php b/htdocs/artefact/resume/lib.php
index ec07f41..89057c7 100644
--- a/htdocs/artefact/resume/lib.php
+++ b/htdocs/artefact/resume/lib.php
@@ -539,9 +539,10 @@ abstract class ArtefactTypeResumeComposite extends ArtefactTypeResume {
             ORDER BY ar.displayorder';

         if (!empty($options['viewid'])) {
- if (!can_view_view($options['viewid'])) {
- throw new AccessDeniedException();
- }
+ //if (!can_view_view($options['viewid'])) {
+ //AJM Should be ... if (!can_view_view($options['viewid'], null, $options['usertoken'], $options['mnettoken'])) {
+ // throw new AccessDeniedException();
+ //}
             require_once('view.php');
             $v = new View($options['viewid']);
             $owner = $v->get('owner');

Note: it was useful to disable exception handling to find this bug.

diff --git a/htdocs/lib/errors.php b/htdocs/lib/errors.php
index c042804..91a9ab6 100644
--- a/htdocs/lib/errors.php
+++ b/htdocs/lib/errors.php
@@ -68,8 +68,7 @@ define('DEVMODE_UNPACKEDJS', 8);
 // Tell PHP about our error settings
 error_reporting(E_ALL);
 set_error_handler('error');
-set_exception_handler('exception');
-
+//set_exception_handler('exception');

[Alan McNatty (alanmc)]

Fix pushed to http://gitorious.org/mahara/alanmcs-mahara/commit/a5205c6bd0b1e2e9ec286fcb872aabfe63900de2

[Richard Mansfield (richard-mansfield)]

Looks good. I'm sure that the point of Nigel's patch (the render resume without javascript one) was to remove the use of composite.json.php except when editing your own resume; it's no longer used in views.

This means that in addition to the clean up you made above, we can remove the view parameter altogether (and anything that depends on it) from that file, and just assume the owner is always the logged in user ($USER), and also get rid of the define('PUBLIC',1) stuff.

Should be more secure, so I'll do that once I've pushed it. Hopefully I'm right about that and it won't break a bunch of stuff...