diff --git a/admin/css/screen.css b/admin/css/screen.css index 03be662ec..d94b647bc 100644 --- a/admin/css/screen.css +++ b/admin/css/screen.css @@ -192,7 +192,7 @@ form.small .field input.text, form.small .field textarea, form.small .field sele .field .chzn-container-single .chzn-single div b { background-position: 4px 0px; } .field .chzn-choices { -webkit-border-radius: 3px; -moz-border-radius: 3px; -ms-border-radius: 3px; -o-border-radius: 3px; border-radius: 3px; } .field input.month, .field input.day, .field input.year { width: 56px; } -.field input.time { width: 64px; } +.field input.time { width: 88px; } .field.remove-splitter { border-bottom: none; box-shadow: none; } /** ---------------------------------------------------- Buttons ---------------------------------------------------- */ diff --git a/admin/javascript/LeftAndMain.js b/admin/javascript/LeftAndMain.js index 471e7548e..9c8bc9cb7 100644 --- a/admin/javascript/LeftAndMain.js +++ b/admin/javascript/LeftAndMain.js @@ -1051,6 +1051,7 @@ jQuery.noConflict(); }(jQuery)); var statusMessage = function(text, type) { + text = $('
').text(text).html(); // Escape HTML entities in text jQuery.noticeAdd({text: text, type: type}); }; diff --git a/admin/scss/_forms.scss b/admin/scss/_forms.scss index 7f3638c3a..85a32a76c 100644 --- a/admin/scss/_forms.scss +++ b/admin/scss/_forms.scss @@ -265,7 +265,7 @@ form.small .field, .field.small { } input.time { - width: ($grid-x * 8); // smaller time field, since input is restricted + width: ($grid-x * 11); // smaller time field, since input is restricted } /* Hides borders in settings/access. Activated from JS */ diff --git a/control/RequestHandler.php b/control/RequestHandler.php index 76796e10d..618d94ba9 100644 --- a/control/RequestHandler.php +++ b/control/RequestHandler.php @@ -88,7 +88,7 @@ class RequestHandler extends ViewableData { * * * Form getters count as URL actions as well, and should be included in allowed_actions. - * Form actions on the other handed (first argument to {@link FormAction()} shoudl NOT be included, + * Form actions on the other handed (first argument to {@link FormAction()} should NOT be included, * these are handled separately through {@link Form->httpSubmission}. You can control access on form actions * either by conditionally removing {@link FormAction} in the form construction, * or by defining $allowed_actions in your {@link Form} class. @@ -322,21 +322,30 @@ class RequestHandler extends ViewableData { // If we get here an the action is 'index', then it hasn't been specified, which means that // it should be allowed. if($action == 'index' || empty($action)) return true; - - if($allowedActions === null || !$this->config()->get('allowed_actions', - Config::UNINHERITED | Config::EXCLUDE_EXTRA_SOURCES)) { - - // If no allowed_actions are provided, then we should only let through actions that aren't handled by + + // Get actions defined for this specific class + $relevantActions = null; + if($allowedActions !== null && method_exists($this, $action)) { + $r = new ReflectionClass(get_class($this)); + $m = $r->getMethod($actionOrigCasing); + $relevantActions = Object::uninherited_static( + $m->getDeclaringClass()->getName(), + 'allowed_actions' + ); + } + + // If no allowed_actions are provided on in the whole inheritance chain, + // or they aren't provided on the specific class... + if($allowedActions === null || $relevantActions === null) { + // ... only let through actions that aren't handled by // magic methods we test this by calling the unmagic method_exists. if(method_exists($this, $action)) { - // Disallow any methods which aren't defined on RequestHandler or subclasses - // (e.g. ViewableData->getSecurityID()) $r = new ReflectionClass(get_class($this)); if($r->hasMethod($actionOrigCasing)) { $m = $r->getMethod($actionOrigCasing); return ($m && is_subclass_of($m->getDeclaringClass()->getName(), 'RequestHandler')); } else { - throw new Exception("method_exists() true but ReflectionClass can't find method - PHP is b0kred"); + throw new Exception("method_exists() true but ReflectionClass can't find method"); } } else if(!$this->hasMethod($action)){ // Return true so that a template can handle this action diff --git a/dev/install/install.php5 b/dev/install/install.php5 index c2d5226df..08aff9a80 100644 --- a/dev/install/install.php5 +++ b/dev/install/install.php5 @@ -1289,6 +1289,8 @@ ErrorDocument 500 /assets/error-500.html RedirectMatch 403 /silverstripe-cache(/|$) + RedirectMatch 403 /vendor(/|$) + RedirectMatch 403 /composer\.(json|lock) diff --git a/docs/en/changelogs/3.0.4.md b/docs/en/changelogs/3.0.4.md index fd9f55e1f..ced2806cc 100644 --- a/docs/en/changelogs/3.0.4.md +++ b/docs/en/changelogs/3.0.4.md @@ -1,12 +1,242 @@ -# 3.0.4 +# 3.0.4 (2013-02-19) ## Overview + * Security: Undefined or empty `$allowed_actions` overrides parent definitions + * Security: Information leakage through web access on YAML configuration files + * Security: Information leakage through web access on composer files + * Security: Require ADMIN permissions for `?showtemplate=1` + * Security: Reflected XSS in custom date/time formats in admin/security + * Security: Stored XSS in the "New Group" dialog + * Security: Reflected XSS in CMS status messages + * API: More restrictive `$allowed_actions` checks for `Controller` when used with `Extension` * Changed `dev/tests/setdb` and `dev/tests/startsession` from session to cookie storage. +## Details + +### Security: Undefined or empty `$allowed_actions` overrides parent definitions + +Severity: Important + +Description: `Controller` (and subclasses) failed to enforce `$allowed_action` restrictions +on parent classes if a child class didn't have it explicitly defined, or it is set to an empty array. +Since this is the default configuration on `Page_Controller`, most SilverStripe installations +will be affected. + +Impact: Depends on the used controller code. For any method with public visibility, +the flaw can expose the return value of the method (unless it fails due to wrong arguments). +It can also lead to unauthorized or unintended execution of logic, e.g. modifying the +state of a database record. + +Fix: Apply 3.0.4 update. In addition, we strongly recommend to define `$allowed_actions` +on all controller classes to ensure the intentions are clearly communicated. +Read more about `$allowed_actions` in our "[controller](/topics/controller/#access-control)" +docs. + +Reporter: Zann St Pierre + +### Security: Information exposure through web access on YAML configuration files + +Severity: Moderate + +Description: YAML files are used to configure the SilverStripe application +since its 3.0 release. These files can contain sensitive values such as database +and API credentials. By default, the installer still stores database credentials +in `_config.php` files which are safe from web access. So this only concerns +configuration values added in your own project, or a third party module. + +Resolution: Update your `.htaccess` file (for Apache), or your `web.config` file (for IIS) +with the new files from the project root, and reapply any customizations you've made. +Follow the [general upgrade instructions](/installation/upgrading). +The [nginx installation instructions](/installation/nginx) +have also been updated to reflect those changes. + +### Security: Information exposure through web access on composer files + +Severity: Low + +Description: [Composer](http://getcomposer.org) is a dependency management +tool which can optionally be used to install SilverStripe. The `composer.json` +and `composer.lock` files are required for its operation, so they are included +in the standard release since 3.0.2. These files contain information on the installed +versions of core and thirdparty modules, which could be used to target specific +versions of SilverStripe. + +Resolution: Update your `.htaccess` file (for Apache), or your `web.config` file (for IIS) +with the new files from the project root, and reapply any customizations you've made. +Follow the [general upgrade instructions](/installation/upgrading). +The [nginx installation instructions](/installation/nginx) +have also been updated to reflect those changes. + + +### Security: Require ADMIN permissions for `?showtemplate=1` + +Severity: Low + +Description: Avoids information leakage of compiled template data, +which might expose some of the internal template logic. + +### Security: Reflected XSS in custom date/time formats in admin/security + +Severity: Low + +Prerequisite: An attacker must have access to the admin interface. + +Description: When creating a new user on the security page +(Security->New User) within the admin interface, the user input +is not properly validated and not encoded. A reflected XSS is +possible within the `DateFormat_custom` and `TimeFormat_custom` fields. + +Credits: Andreas Hunkeler (Compass Security AG, http://www.csnc.ch) + +### Security: Stored XSS in the "New Group" dialog + +Severity: Low + +Prerequisite: An attacker must have access to the admin interface. + +Description: There is a stored XSS vulnerability on the "group" tab on the +security page in the admin interface +(Security -> Groups -> New Group). It's possible to store a +XSS within the group name. Everywhere where these group names +are used, the XSS is executed. E.g. "New User" or "New Group". + +Credits: Andreas Hunkeler (Compass Security AG, http://www.csnc.ch) + +### Security: XSS in CMS status messages + +Severity: Low + +Prerequisite: An attacker must have access to the admin interface. + +Description: Any data returned to CMS status messages (Growl-style popovers on top right) +was not escaped, allowing XSS e.g. when publishing a page with +a specifically crafted "Title" field. + +Credits: Andreas Hunkeler (Compass Security AG, http://www.csnc.ch) + +### API: More restrictive `$allowed_actions` checks for `Controller` when used with `Extension` + +Controllers which are extended with `$allowed_actions` (through an `Extension`) +now deny access to methods defined on the controller, unless this class also has them in its own +`$allowed_actions` definition. + ## Upgrading * If you are using `dev/tests/setdb` and `dev/tests/startsession`, you'll need to configure a secure token in order to encrypt the cookie value: Simply run `sake dev/generatesecuretoken` and add the resulting code to your `mysite/_config.php`. - Note that this functionality now requires the PHP `mcrypt` extension. \ No newline at end of file + Note that this functionality now requires the PHP `mcrypt` extension. + +## Changelog + +### API Changes + + * 2013-02-15 [2352317](https://github.com/silverstripe/silverstripe-installer/commit/2352317) Filter composer files in IIS and Apache rules (fixes #8011) (Ingo Schommer) + * 2013-02-12 [d969e29](https://github.com/silverstripe/sapphire/commit/d969e29) Require ADMIN for ?showtemplate=1 (Ingo Schommer) + * 2013-02-12 [45c68d6](https://github.com/silverstripe/sapphire/commit/45c68d6) Require ADMIN for ?showtemplate=1 (Ingo Schommer) + * 2013-01-23 [c69381c](https://github.com/silverstripe/sapphire/commit/c69381c) Remove Content-Length setting from HTTPResponse (fixes #8010) (Ingo Schommer) + * 2012-12-11 [10d447e](https://github.com/silverstripe/silverstripe-installer/commit/10d447e) Removed 'make getallmodules', use composer instead (Ingo Schommer) + * 2012-12-10 [efa9ff9](https://github.com/silverstripe/sapphire/commit/efa9ff9) Queries added by DataList::addInnerJoin() and DataList::leftJoin() come after the base joins, not before. (stojg) + * 2012-12-06 [c6b1d4a](https://github.com/silverstripe/sapphire/commit/c6b1d4a) Storing alternative DB name in cookie rather than session (Ingo Schommer) + * 2012-11-29 [f49f1ff](https://github.com/silverstripe/sapphire/commit/f49f1ff) Rename Transliterator to SS_Transliterator to remove conflict with Intl extension (Simon Welsh) + * 2012-11-19 [96e56a8](https://github.com/silverstripe/silverstripe-installer/commit/96e56a8) Removed 'new-project' command (Ingo Schommer) + * 2012-11-19 [8b68644](https://github.com/silverstripe/silverstripe-installer/commit/8b68644) Moved build tools to new silverstripe-buildtools module (Ingo Schommer) + * 2012-11-09 [22095da](https://github.com/silverstripe/sapphire/commit/22095da) Hash autologin tokens before storing in the database. (Mateusz Uzdowski) + * 2011-03-16 [d8bfc0b](https://github.com/silverstripe/sapphire/commit/d8bfc0b) Added Security::set_login_url() so that you can define an alternative log-in page if you have made one yourself. (Sam Minnee) + * 2011-03-16 [f546979](https://github.com/silverstripe/sapphire/commit/f546979) Add a PermissionFailureException that can be thrown to trigger a log-in. (Sam Minnee) + +### Features and Enhancements + + * 2013-02-05 [f0621cd](https://github.com/silverstripe/sapphire/commit/f0621cd) Added ability to query size of Varchar (Daniel Hensby) + * 2013-02-01 [119d8aa](https://github.com/silverstripe/silverstripe-cms/commit/119d8aa) Do not display SilverStripeNavigator_CMSLink when in a LeftAndMain extension not just CMSMain extensions (UndefinedOffset) + * 2013-01-11 [5b450f7](https://github.com/silverstripe/sapphire/commit/5b450f7) Added replaceExistingFile setting for UploadField. (Sam Minnee) + * 2013-01-11 [cc7318f](https://github.com/silverstripe/sapphire/commit/cc7318f) Added canAttachExisting config option for UploadField. (Sam Minnee) + * 2013-01-10 [5e6f5f9](https://github.com/silverstripe/sapphire/commit/5e6f5f9) Allow configuration of send_all_emails_to, ccs_all_emails_to, and bcc_all_emails_to via the config system. (Sam Minnee) + * 2013-01-09 [67c5db3](https://github.com/silverstripe/sapphire/commit/67c5db3) Global default config for UploadField (Ingo Schommer) + * 2013-01-09 [2dfd427](https://github.com/silverstripe/sapphire/commit/2dfd427) Restrict upload abilities in UploadField (Ingo Schommer) + * 2013-01-07 [abbee41](https://github.com/silverstripe/sapphire/commit/abbee41) Add ReadonlyField::setIncludeHiddenField() (Sam Minnee) + * 2012-12-07 [e8fbfc0](https://github.com/silverstripe/sapphire/commit/e8fbfc0) FixtureFactory separated out from YamlFixture (Ingo Schommer) + * 2012-11-15 [e07ae20](https://github.com/silverstripe/silverstripe-installer/commit/e07ae20) Added "phing phpunit" target (Ingo Schommer) + * 2012-11-09 [32f829d](https://github.com/silverstripe/sapphire/commit/32f829d) Support for Behat tests, and initial set of tests (Ingo Schommer) + * 2012-11-09 [9841d5b](https://github.com/silverstripe/silverstripe-cms/commit/9841d5b) Added Behat tests (Ingo Schommer) + * 2012-10-24 [ea2dc9d](https://github.com/silverstripe/sapphire/commit/ea2dc9d) Add ability to change URL for SS logo in CMS Menu (Loz Calver) + * 2012-10-16 [c4dde90](https://github.com/silverstripe/sapphire/commit/c4dde90) Allow hashes to be passed as ArrayList items; the will be turned into ArrayData objects. (Sam Minnee) + * 2011-09-29 [2916f20](https://github.com/silverstripe/sapphire/commit/2916f20) Improve HTTP caching logic to automatically disable caching for requests that use the session. (Hamish Friedlander) + * 2011-03-02 [c3a3ff4](https://github.com/silverstripe/sapphire/commit/c3a3ff4) Added Email::send_all_emails_from() setting. (Sam Minnee) + +### Bugfixes + + * 2013-02-17 [ede3813](https://github.com/silverstripe/sapphire/commit/ede3813) Secure composer files from web access (fixes #8011) (Ingo Schommer) + * 2013-02-17 [e21bd49](https://github.com/silverstripe/sapphire/commit/e21bd49) TimeField respects user choice (fixes #8260) (Ingo Schommer) + * 2013-02-07 [79eacb2](https://github.com/silverstripe/sapphire/commit/79eacb2) Group->canEdit() correct non-admin checks (fixes #8250) (Ingo Schommer) + * 2013-02-04 [857d8bb](https://github.com/silverstripe/sapphire/commit/857d8bb) Don't escape values on TreeDropdownField readonly views (Ingo Schommer) + * 2013-02-04 [97fbfd3](https://github.com/silverstripe/silverstripe-cms/commit/97fbfd3) Respect escaping rules on readonly fields in CMS history view (Ingo Schommer) + * 2013-02-04 [e56a78b](https://github.com/silverstripe/silverstripe-cms/commit/e56a78b) updateCMSFields not accepting var by reference (Michael Andrewartha) + * 2013-02-04 [866bb07](https://github.com/silverstripe/sapphire/commit/866bb07) validate doesn't take var by reference (Michael Andrewartha) + * 2013-02-04 [1960df8](https://github.com/silverstripe/sapphire/commit/1960df8) Strict error warnings on DataExtension (Michael Andrewartha) + * 2013-01-31 [1bb1090](https://github.com/silverstripe/sapphire/commit/1bb1090) Node updates in IE without non-object error (Ingo Schommer) + * 2013-01-30 [c9f728f](https://github.com/silverstripe/sapphire/commit/c9f728f) Only check the remember token if a user exists (Simon Welsh) + * 2013-01-24 [f574979](https://github.com/silverstripe/sapphire/commit/f574979) Exception handling and email notification mechanism now correctly considers the stacktrace as provided by the exceptionHandler function, instead of attempting to perform a debug_backtrace further down the reporting chain (which ends up generating an unnecessarily nested stacktrace). Debug was cleaned up so that errorHandler and exceptionHandler both act consistently. As a result, the LogErrorEmailFormatter class could be simplified. (Damian Mooyman) + * 2013-01-23 [45eb0f9](https://github.com/silverstripe/sapphire/commit/45eb0f9) PHPUnit latest not working with composer installed builds (Hamish Friedlander) + * 2013-01-21 [5d37d55](https://github.com/silverstripe/sapphire/commit/5d37d55) Form session message clearing regression (Ingo Schommer) + * 2013-01-18 [0c9b216](https://github.com/silverstripe/sapphire/commit/0c9b216) Escape the -f argument passed to mail() (Sam Minnee) + * 2013-01-17 [e74ec57](https://github.com/silverstripe/sapphire/commit/e74ec57) Permission checkbox display on members (fixes #8193) (Ingo Schommer) + * 2013-01-15 [a70df3e](https://github.com/silverstripe/sapphire/commit/a70df3e) PaginatedList deprecated method was calling non-existent method (Jeremy Thomerson) + * 2013-01-15 [014f541](https://github.com/silverstripe/sapphire/commit/014f541) Regression in Form->clearMessage() (fixes #8186) (Ingo Schommer) + * 2013-01-15 [64d3a3d](https://github.com/silverstripe/sapphire/commit/64d3a3d) Don't double unescape URLs in history.js (fixes #8170) (Ingo Schommer) + * 2013-01-15 [420c639](https://github.com/silverstripe/sapphire/commit/420c639) Properly show link for showing and hiding class spec in model admin (jean) + * 2013-01-15 [f06ba70](https://github.com/silverstripe/sapphire/commit/f06ba70) Undefined `$allowed_actions` overrides parent definitions, stricter handling of $allowed_actions on Extension (Ingo Schommer) + * 2013-01-11 [e020c7b](https://github.com/silverstripe/sapphire/commit/e020c7b) doSave() and doDelete() should use translated singular name (uniun) + * 2013-01-11 [f8758ba](https://github.com/silverstripe/sapphire/commit/f8758ba) Fixed margins so that margin is displayed between preview images and their title. (Sam Minnee) + * 2013-01-11 [2fdd9a3](https://github.com/silverstripe/sapphire/commit/2fdd9a3) Allow images attached to UploadFields to be unlinked without File::canEdit() or File::canDelete() permission. (Sam Minnee) + * 2013-01-11 [f4efaee](https://github.com/silverstripe/sapphire/commit/f4efaee) Fix DataObject::get_one() when the classname is passed with improper casing. (Sam Minnee) + * 2013-01-06 [30096ee](https://github.com/silverstripe/sapphire/commit/30096ee) Keep Member.PasswordEncryption setting on empty passwords (Ingo Schommer) + * 2013-01-04 [f8bbc0a](https://github.com/silverstripe/sapphire/commit/f8bbc0a) Escape HTML in DropdownField and ListboxField (Ingo Schommer) + * 2013-01-04 [604ede3](https://github.com/silverstripe/sapphire/commit/604ede3) Escape HTML in CMS status messages (Ingo Schommer) + * 2013-01-04 [7bb0bbf](https://github.com/silverstripe/sapphire/commit/7bb0bbf) Fixed XSS in admin/security and "My Profile" forms (Ingo Schommer) + * 2012-12-21 [f0f83b2](https://github.com/silverstripe/sapphire/commit/f0f83b2) Graceful handling of sprintf with too few params in i18n::_t() (Ingo Schommer) + * 2012-12-19 [22efd38](https://github.com/silverstripe/sapphire/commit/22efd38) Calling DataObject::relField() on a object with an empty relation list (Stig Lindqvist) + * 2012-12-18 [6aba24b](https://github.com/silverstripe/sapphire/commit/6aba24b) removeRequiredField() should use array_splice() instead of unset() (uniun) + * 2012-12-18 [d5a1c3d](https://github.com/silverstripe/sapphire/commit/d5a1c3d) SS has problems handling + in URLs. Filter them out. (Mateusz Uzdowski) + * 2012-12-14 [55b611d](https://github.com/silverstripe/sapphire/commit/55b611d) Hardcoded project name in include_by_locale() (uniun) + * 2012-12-13 [d42c004](https://github.com/silverstripe/silverstripe-cms/commit/d42c004) Fixed pagination functionality on root assets folder (Niklas Forsdahl) + * 2012-12-12 [639cc02](https://github.com/silverstripe/sapphire/commit/639cc02) Fix insert media form inserting images from other UploadFields (fixes #8051) (Loz Calver) + * 2012-12-11 [f431b35](https://github.com/silverstripe/sapphire/commit/f431b35) Confirmed Password Field now copies attributes to child fields. (Justin Martin) + * 2012-12-07 [4f63f91](https://github.com/silverstripe/sapphire/commit/4f63f91) Fixed issue with convertServiceProperty (Marcus Nyeholt) + * 2012-12-06 [1a4eaaa](https://github.com/silverstripe/sapphire/commit/1a4eaaa) Ensure has length before using string index access. (Simon Elvery) + * 2012-12-05 [205ee42](https://github.com/silverstripe/sapphire/commit/205ee42) Make sure a message is set on ValidationException objects. (Simon Elvery) + * 2012-12-05 [c0751df](https://github.com/silverstripe/silverstripe-cms/commit/c0751df) Remove handwritten SQL and use the ORM. (Mateusz Uzdowski) + * 2012-12-04 [0be51a9](https://github.com/silverstripe/sapphire/commit/0be51a9) Fix ModelAdmin search (fixes #8052) (Ingo Schommer) + * 2012-12-04 [1a4b245](https://github.com/silverstripe/sapphire/commit/1a4b245) Fix rewriteHashlinks in TabSet (Marcus Nyeholt) + * 2012-12-04 [3478813](https://github.com/silverstripe/sapphire/commit/3478813) Rewrite hashlinks failing on empty a tags (Marcus Nyeholt) + * 2012-11-26 [40a1a35](https://github.com/silverstripe/silverstripe-cms/commit/40a1a35) Namespaces for CmsFormsContext and CmsUiContext are wrong (Kirk Mayo) + * 2012-11-23 [4310718](https://github.com/silverstripe/sapphire/commit/4310718) Insert Media, inserts all previous media (fixes #7545) (UndefinedOffset) + * 2012-11-23 [453d04e](https://github.com/silverstripe/sapphire/commit/453d04e) Reset DataObject caches in SapphireTest->resetDBSchema() (Ingo Schommer) + * 2012-11-23 [a3cd7dd](https://github.com/silverstripe/sapphire/commit/a3cd7dd) Force SapphireTest schema reset for extension changes (Ingo Schommer) + * 2012-11-21 [41aec54](https://github.com/silverstripe/silverstripe-cms/commit/41aec54) Consistently use FormResponse in CMS JavaScript (fixes #8036) (Ingo Schommer) + * 2012-11-20 [8f89aa9](https://github.com/silverstripe/sapphire/commit/8f89aa9) only call filemtime if file exists (Sander van Dragt) + * 2012-11-16 [76c63fe](https://github.com/silverstripe/sapphire/commit/76c63fe) Fixed issue with SQLQuery::lastRow crashing on empty set. Added test cases for lastRow and firstRow. (Damian Mooyman) + * 2012-11-15 [c6fcb08](https://github.com/silverstripe/sapphire/commit/c6fcb08) Video embed from Add Media Feature no longer works (open #8033) (stojg) + * 2012-11-14 [91e48b8](https://github.com/silverstripe/silverstripe-cms/commit/91e48b8) Provide fallback text for translations. (Simon Elvery) + * 2012-11-12 [2657a27](https://github.com/silverstripe/sapphire/commit/2657a27) Adjust the handler to jQuery UI 1.9 API change. (Mateusz Uzdowski) + * 2012-11-12 [b6017a7](https://github.com/silverstripe/sapphire/commit/b6017a7) ArrayList now discards keys of the array passed in and keeps the numerically indexed array sequential. This fixes FirstLast and EvenOdd in templates, and makes ArrayList more consistent, as several methods already discarded the keys. (Andrew O'Neil) + * 2012-11-12 [d58b23d](https://github.com/silverstripe/silverstripe-cms/commit/d58b23d) AssetAdmin filter array indices (fixes #8014) (Kirk Mayo) + * 2012-11-09 [434759c](https://github.com/silverstripe/sapphire/commit/434759c) Correct redirection URL on deletion in GridFieldDetailForm (Ingo Schommer) + * 2012-11-08 [6882635](https://github.com/silverstripe/sapphire/commit/6882635) Fixing non-object on file upload (Sean Harvey) + * 2012-11-08 [6a69a2f](https://github.com/silverstripe/silverstripe-cms/commit/6a69a2f) Ensure required lang and css are loaded when using SiteTreeURLSegmentField (Simon Elvery) + * 2012-02-09 [c048a01](https://github.com/silverstripe/sapphire/commit/c048a01) Avoid infinite redirection when logging out and when showing a custom login page after displaying the draft version of a page. (jean) + * 2011-12-12 [1e1df8c](https://github.com/silverstripe/sapphire/commit/1e1df8c) Improved detection of empty HTMLText fields. (Sam Minnee) + * 2011-09-30 [f41a7d8](https://github.com/silverstripe/sapphire/commit/f41a7d8) Fix issue with not being able to log out on Chrome when caching enabled because of Chrome bug (Hamish Friedlander) + * 2011-09-01 [9a2ba48](https://github.com/silverstripe/sapphire/commit/9a2ba48) Made CSRF-error wording friendlier. (Sam Minnee) + * 2011-08-31 [729bcc9](https://github.com/silverstripe/sapphire/commit/729bcc9) Don't clear form messages unless forTemplate() is actually called. BUGFIX: Clear session-stored form data as well as form error message. (Sam Minnee) + * 2011-08-18 [5f9348b](https://github.com/silverstripe/sapphire/commit/5f9348b) Ensure that Security views respect redirections triggered by Page_Controller::init() (Sam Minnee) + * 2011-07-07 [b114aa2](https://github.com/silverstripe/sapphire/commit/b114aa2) Added X-Forwarded-Protocol and User-Agent to Vary header. (Sam Minnee) + * 2011-05-26 [55f3ec1](https://github.com/silverstripe/sapphire/commit/55f3ec1) Added error message fields to default search form (Jean-Fabien) + * 2011-05-23 [7026a48](https://github.com/silverstripe/sapphire/commit/7026a48) for date manipulation use the SS_Datetime::now, otherwise it does not respect the mock date. (Mateusz Uzdowski) + * 2011-05-21 [b7a1db7](https://github.com/silverstripe/sapphire/commit/b7a1db7) Set up the test mailer before loading the fixture, in case fixture-creation causes emails to be generated. (Sam Minnee) + * 2011-04-29 [47e037e](https://github.com/silverstripe/sapphire/commit/47e037e) Removed notice-level error after forms w/ required fields are made readonly. (Sam Minnee) + * 2011-04-20 [33a1fc7](https://github.com/silverstripe/sapphire/commit/33a1fc7) Fixed operation of inlined images in Mailer, when no inlined images actually attached. (Carlos Barberis) + * 2011-04-18 [f8206d1](https://github.com/silverstripe/sapphire/commit/f8206d1) Prevent notice-level error in Session code when non-array is turned into an array. (Sam Minnee) + * 2011-03-15 [6fcbad1](https://github.com/silverstripe/sapphire/commit/6fcbad1) Updated SilverStripe error handler so that log_errors still works. (Sam Minnee) + * 2011-03-11 [82988d4](https://github.com/silverstripe/sapphire/commit/82988d4) Better error message when 401 response is corrupted. (Sam Minnee) \ No newline at end of file diff --git a/docs/en/changelogs/index.md b/docs/en/changelogs/index.md index 78a80e545..918ee7ad8 100644 --- a/docs/en/changelogs/index.md +++ b/docs/en/changelogs/index.md @@ -11,6 +11,8 @@ For information on how to upgrade to newer versions consult the [upgrading](/ins * [3.1.0](3.1.0) - Unreleased + * [3.0.4](3.0.4) - 19 February 2013 + * [3.0.3](3.0.3) - 26 November 2012 * [3.0.2](3.0.2) - 17 September 2012 * [3.0.1](3.0.1) - 31 July 2012 * [3.0.0](3.0.0) - 28 June 2012 diff --git a/docs/en/howto/csv-import.md b/docs/en/howto/csv-import.md index 5c8253397..b53ce3c02 100644 --- a/docs/en/howto/csv-import.md +++ b/docs/en/howto/csv-import.md @@ -68,6 +68,9 @@ You can have more customized logic and interface feedback through a custom contr :::php renderWith('MyTemplate'); } diff --git a/docs/en/reference/grid-field.md b/docs/en/reference/grid-field.md index 6cfa8e87f..262b58071 100644 --- a/docs/en/reference/grid-field.md +++ b/docs/en/reference/grid-field.md @@ -41,6 +41,8 @@ Here is an example where we display a basic gridfield with the default settings: :::php class GridController extends Page_Controller { + + static $allowed_actions = array('index'); public function index(SS_HTTPRequest $request) { $this->Content = $this->AllPages(); diff --git a/docs/en/reference/templates.md b/docs/en/reference/templates.md index 1fe556dde..81b3ca064 100644 --- a/docs/en/reference/templates.md +++ b/docs/en/reference/templates.md @@ -568,6 +568,8 @@ default if it exists and there is no action in the url parameters. :::php class MyPage_Controller extends Page_Controller { + + static $allowed_actions = array('index'); public function init(){ parent::init(); diff --git a/docs/en/topics/controller.md b/docs/en/topics/controller.md index fca452add..1f3c74f01 100644 --- a/docs/en/topics/controller.md +++ b/docs/en/topics/controller.md @@ -3,7 +3,7 @@ Base controller class. You will extend this to take granular control over the actions and url handling of aspects of your SilverStripe site. -## Example +## Usage The following example is for a simple `[api:Controller]` class. If you're using the cms module and looking at Page_Controller instances you won't need to setup @@ -15,11 +15,14 @@ your own routes since the cms module handles these routes. cheesefries ) +
+ SilverStripe automatically adds a URL routing entry based on the controller's class name, + so a `MyController` class is accessible through `http://yourdomain.com/MyController`. +
+ +## Access Control + +### Through $allowed_actions + +All public methods on a controller are accessible by their name through the `$Action` +part of the URL routing, so a `MyController->mymethod()` is accessible at +`http://yourdomain.com/MyController/mymethod`. This is not always desireable, +since methods can return internal information, or change state in a way +that's not intended to be used through a URL endpoint. + +SilverStripe strongly recommends securing your controllers +through defining a `$allowed_actions` array on the class, +which allows whitelisting of methods, as well as a concise +way to perform checks against permission codes or custom logic. + + :::php + class MyController extends Controller { + public static $allowed_actions = array( + // someaction can be accessed by anyone, any time + 'someaction', + // So can otheraction + 'otheraction' => true, + // restrictedaction can only be people with ADMIN privilege + 'restrictedaction' => 'ADMIN', + // complexaction can only be accessed if $this->canComplexAction() returns true + 'complexaction' '->canComplexAction' + ); + } + +There's a couple of rules guiding these checks: + + * Each controller is only responsible for access control on the methods it defines + * If a method on a parent class is overwritten, access control for it has to be redefined as well + * An action named "index" is whitelisted by default + * A wildcard (`*`) can be used to define access control for all methods (incl. methods on parent classes) + * Specific method entries in `$allowed_actions` overrule any `*` settings + * Methods returning forms also count as actions which need to be defined + * Form action methods (targets of `FormAction`) should NOT be included in `$allowed_actions`, + they're handled separately through the form routing (see the ["forms" topic](/topics/forms)) + * `$allowed_actions` can be defined on `Extension` classes applying to the controller. + + +If the permission check fails, SilverStripe will return a "403 Forbidden" HTTP status. + +### Through the action + +Each method responding to a URL can also implement custom permission checks, +e.g. to handle responses conditionally on the passed request data. + + :::php + class MyController extends Controller { + public static $allowed_actions = array('myaction'); + public function myaction($request) { + if(!$request->getVar('apikey')) { + return $this->httpError(403, 'No API key provided'); + } + + return 'valid'; + } + } + +Unless you transform the response later in the request processing, +it'll look pretty ugly to the user. Alternatively, you can use +`ErrorPage::response_for()` to return a more specialized layout. + +Note: This is recommended as an addition for `$allowed_actions`, in order to handle +more complex checks, rather than a replacement. + +### Through the init() method + +After checking for allowed_actions, each controller invokes its `init()` method, +which is typically used to set up common state in the controller, and +include JavaScript and CSS files in the output which are used for any action. +If an `init()` method returns a `SS_HTTPResponse` with either a 3xx or 4xx HTTP +status code, it'll abort execution. This behaviour can be used to implement +permission checks. + + :::php + class MyController extends Controller { + public static $allowed_actions = array(); + public function init() { + parent::init(); + if(!Permission::check('ADMIN')) return $this->httpError(403); + } + } ## URL Handling @@ -60,7 +153,7 @@ through `/fastfood/drivethrough/` to use the same order function. :::php class FastFood_Controller extends Controller { - + static $allowed_actions = array('drivethrough'); public static $url_handlers = array( 'drivethrough/$Action/$ID/$Name' => 'order' ); diff --git a/docs/en/topics/form-validation.md b/docs/en/topics/form-validation.md index ccd7906f9..a99a3b421 100644 --- a/docs/en/topics/form-validation.md +++ b/docs/en/topics/form-validation.md @@ -49,6 +49,7 @@ Example: Validate postcodes based on the selected country (on the controller). :::php class MyController extends Controller { + static $allowed_actions = array('Form'); public function Form() { return Form::create($this, 'Form', new FieldList( diff --git a/docs/en/topics/forms.md b/docs/en/topics/forms.md index 5844912e8..5ac3189f3 100644 --- a/docs/en/topics/forms.md +++ b/docs/en/topics/forms.md @@ -25,9 +25,7 @@ Forms start at the controller. Here is an simple example on how to set up a form :::php class Page_Controller extends ContentController { - public static $allowed_actions = array( - 'HelloForm', - ); + public static $allowed_actions = array('HelloForm'); // Template method public function HelloForm() { @@ -41,11 +39,24 @@ Forms start at the controller. Here is an simple example on how to set up a form return $form; } - public function doSayHello(array $data, Form $form) { + public function doSayHello($data, Form $form) { // Do something with $data return $this->render(); } } + +The name of the form ("HelloForm") is passed into the `Form` +constructor as a second argument. It needs to match the method name. + +Since forms need a URL, the `HelloForm()` method needs to be handled +like any other controller action. In order to whitelist its access through +URLs, we add it to the `$allowed_actions` array. +Form actions ("doSayHello") on the other hand should NOT be included here, +these are handled separately through `Form->httpSubmission()`. +You can control access on form actions either by conditionally removing +a `FormAction` from the form construction, +or by defining `$allowed_actions` in your own `Form` class +(more information in the ["controllers" topic](/topics/controllers)). **Page.ss** diff --git a/docs/en/topics/security.md b/docs/en/topics/security.md index fff3e792d..efd48b67f 100644 --- a/docs/en/topics/security.md +++ b/docs/en/topics/security.md @@ -79,6 +79,7 @@ Example: :::php class MyController extends Controller { + static $allowed_actions = array('myurlaction'); public function myurlaction($RAW_urlParams) { $SQL_urlParams = Convert::raw2sql($RAW_urlParams); // works recursively on an array $objs = Player::get()->where("Name = '{$SQL_data[OtherID]}'"); @@ -93,7 +94,6 @@ This means if you've got a chain of functions passing data through, escaping sho :::php class MyController extends Controller { /** - * @param array $RAW_data All names in an indexed array (not SQL-safe) */ public function saveAllNames($RAW_data) { @@ -220,6 +220,7 @@ PHP: :::php class MyController extends Controller { + static $allowed_actions = array('search'); public function search($request) { $htmlTitle = '

Your results for:' . Convert::raw2xml($request->getVar('Query')) . '

'; return $this->customise(array( @@ -249,6 +250,7 @@ PHP: :::php class MyController extends Controller { + static $allowed_actions = array('search'); public function search($request) { $rssRelativeLink = "/rss?Query=" . urlencode($_REQUEST['query']) . "&sortOrder=asc"; $rssLink = Controller::join_links($this->Link(), $rssRelativeLink); diff --git a/docs/en/tutorials/3-forms.md b/docs/en/tutorials/3-forms.md index 8f8435660..635014f1c 100644 --- a/docs/en/tutorials/3-forms.md +++ b/docs/en/tutorials/3-forms.md @@ -21,6 +21,8 @@ The poll we will be creating on our homepage will ask the user for their name an :::php class HomePage_Controller extends Page_Controller { + static $allowed_actions = array('BrowserPollForm'); + // ... public function BrowserPollForm() { diff --git a/forms/MemberDatetimeOptionsetField.php b/forms/MemberDatetimeOptionsetField.php index 59d6e9855..fd0eada55 100644 --- a/forms/MemberDatetimeOptionsetField.php +++ b/forms/MemberDatetimeOptionsetField.php @@ -37,21 +37,27 @@ class MemberDatetimeOptionsetField extends OptionsetField { $value = ($this->value && !array_key_exists($this->value, $this->source)) ? $this->value : null; $checked = ($value) ? " checked=\"checked\"" : ''; $options .= "
  • " - . sprintf("", - $itemID, $this->name, $checked) - . sprintf('', - $itemID, _t('MemberDatetimeOptionsetField.Custom', 'Custom')) + . sprintf( + "", + $itemID, $this->name, + $checked + ) + . sprintf( + '', + $itemID, _t('MemberDatetimeOptionsetField.Custom', 'Custom') + ) . sprintf( "\n", - $this->name, - $value - ) - . sprintf("", - $this->Link() . '/validate'); + $this->name, Convert::raw2xml($value) + ) + . sprintf( + "", + $this->Link() . '/validate' + ); $options .= ($value) ? sprintf( '(%s: "%s")', _t('MemberDatetimeOptionsetField.Preview', 'Preview'), - Zend_Date::now()->toString($value) + Convert::raw2xml(Zend_Date::now()->toString($value)) ) : ''; $id = $this->id(); diff --git a/forms/TimeField.php b/forms/TimeField.php index 33c0fa079..47a2af262 100644 --- a/forms/TimeField.php +++ b/forms/TimeField.php @@ -27,7 +27,7 @@ class TimeField extends TextField { * @var array */ static $default_config = array( - 'timeformat' => 'HH:mm:ss', + 'timeformat' => null, 'use_strtotime' => true, 'datavalueformat' => 'HH:mm:ss' ); diff --git a/lang/nl.yml b/lang/nl.yml index c93eeb4da..fbb6dcd4f 100644 --- a/lang/nl.yml +++ b/lang/nl.yml @@ -19,7 +19,7 @@ nl: DROPAREA: 'Hierheen slepen' EDITALL: 'Alle bewerken' EDITANDORGANIZE: 'Bewerk en beheer' - EDITINFO: 'Edit files' + EDITINFO: 'Bewerk alle bestanden' FILES: Bestanden FROMCOMPUTER: 'Selecteer bestand op computer' FROMCOMPUTERINFO: 'Upload from your computer' @@ -175,16 +175,16 @@ nl: XlsType: 'Excel document' ZipType: 'ZIP bestand' FileIFrameField: - ATTACH: 'Attach {type}' - ATTACHONCESAVED: '{type}s can be attached once you have saved the record for the first time.' - ATTACHONCESAVED2: 'Files can be attached once you have saved the record for the first time.' - DELETE: 'Delete {type}' + ATTACH: 'Toevoegen {type}' + ATTACHONCESAVED: '{type}en kunnen worden toegevoegd na het voor het eerst opslaan.' + ATTACHONCESAVED2: 'Bestanden kunnen worden toegevoegd na het voor het eerst opslaan.' + DELETE: 'Verwijderen {type}' DISALLOWEDFILETYPE: 'Dit type bestand mag niet worden opgeslagen' FILE: Bestand FROMCOMPUTER: 'Vanaf computer' FROMFILESTORE: 'Vanaf de website''s bestandsopslag' NOSOURCE: 'Selecteer een bron bestand om toe te voegen' - REPLACE: 'Replace {type}' + REPLACE: 'Vervang {type}' FileIFrameField_iframe.ss: TITLE: 'Afbeelding uploaden' Filesystem: @@ -232,7 +232,7 @@ nl: DeletePermissionsFailure: 'Onvoldoende rechten om te verwijderen' GridFieldDetailForm: CancelBtn: Annuleren - Create: Create + Create: Creëren Delete: Verwijder DeletePermissionsFailure: 'No delete permissions' Deleted: '%s %s verwijderd' @@ -334,7 +334,7 @@ nl: VersionUnknown: onbekend LeftAndMain_Menu.ss: Hello: Hi - LOGOUT: 'Log out' + LOGOUT: Uitloggen LoginAttempt: Email: 'Email adres ' IP: 'IP Adres' @@ -410,7 +410,7 @@ nl: MemberImportForm: Help1: '

    Importeer leden in CSV formaat (Kommagescheiden bestandsformaat). Toon geavanceerd gebruik

    ' Help2: '

    Advanced usage

    • Allowed columns: %s
    • Existing users are matched by their unique Code property, and updated with any new values from the imported file.
    • Groups can be assigned by the Groups column. Groups are identified by their Code property, multiple groups can be separated by comma. Existing group memberships are not cleared.
    ' - ResultCreated: 'Created {count} members' + ResultCreated: '{count} leden aangemaakt' ResultDeleted: '%d leden verwijderd' ResultNone: 'Geen wijzingen' ResultUpdated: 'Updated {count} members' @@ -551,13 +551,13 @@ nl: ATTACHFILE: 'Voeg een bestand toe' ATTACHFILES: 'Voeg bestanden toe' AttachFile: 'Voeg bestanden toe' - DELETE: 'Delete from files' + DELETE: 'Volledig verwijderen' DELETEINFO: 'Verwijder dit bestand uit bestandsopslag van de website.' DOEDIT: Bewaar DROPFILE: 'Bestand hiernaar toe slepen' DROPFILES: 'Sleep hier je bestanden' Dimensions: Dimensions - EDIT: Edit + EDIT: Bewerken EDITINFO: 'Bewerk dit bestand' FIELDNOTSET: 'Bestandsinformatie niet gevonden' FROMCOMPUTER: 'Vanaf computer' diff --git a/lang/uk.yml b/lang/uk.yml index 894373877..767d0641b 100644 --- a/lang/uk.yml +++ b/lang/uk.yml @@ -77,7 +77,7 @@ uk: CHANGEPASSWORDTEXT2: 'Тепер Ви можете використовувати наступні дані для входу:' EMAIL: Email HELLO: Привіт - PASSWORD: Password + PASSWORD: Пароль CheckboxField: - 'False' - 'True' @@ -240,7 +240,7 @@ uk: Saved: 'Saved %s %s' GridFieldItemEditView.ss: null Group: - AddRole: 'Add a role for this group' + AddRole: 'Додати роль до цієї групи' Code: 'Код групи' DefaultGroupTitleAdministrators: Адміністратори DefaultGroupTitleContentAuthors: 'Content Authors' @@ -250,7 +250,7 @@ uk: NoRoles: 'Ролі не знайдені' PLURALNAME: Groups Parent: 'Батьківська група' - RolesAddEditLink: 'Manage roles' + RolesAddEditLink: 'Керувати ролями' SINGULARNAME: Group Sort: 'Порядок сортування' has_many_Permissions: Права @@ -501,7 +501,7 @@ uk: EDITPERMISSIONS_HELP: 'Ability to edit Permissions and IP Addresses for a group. Requires the "Access to ''Security'' section" permission.' GROUPNAME: 'Назва групи' IMPORTGROUPS: 'Імпортувати групи' - IMPORTUSERS: 'Import users' + IMPORTUSERS: 'Імпортувати користувачів' MEMBERS: Члени MENUTITLE: Security MemberListCaution: 'Caution: Removing members from this list will remove them from all groups and the database' diff --git a/security/Security.php b/security/Security.php index 3eafba25f..0a7d43b35 100644 --- a/security/Security.php +++ b/security/Security.php @@ -829,17 +829,8 @@ class Security extends Controller { * @see set_password_encryption_algorithm() */ public static function encrypt_password($password, $salt = null, $algorithm = null, $member = null) { - if( - // if the password is empty, don't encrypt - strlen(trim($password)) == 0 - // if no algorithm is provided and no default is set, don't encrypt - || (!$algorithm) - ) { - $algorithm = 'none'; - } else { - // Fall back to the default encryption algorithm - if(!$algorithm) $algorithm = self::$encryptionAlgorithm; - } + // Fall back to the default encryption algorithm + if(!$algorithm) $algorithm = self::$encryptionAlgorithm; $e = PasswordEncryptor::create_for_algorithm($algorithm); diff --git a/templates/forms/DropdownField.ss b/templates/forms/DropdownField.ss index f7df474e4..00d654e82 100644 --- a/templates/forms/DropdownField.ss +++ b/templates/forms/DropdownField.ss @@ -1,5 +1,5 @@ diff --git a/tests/control/ControllerTest.php b/tests/control/ControllerTest.php index f02fe50c4..a47bbba5a 100644 --- a/tests/control/ControllerTest.php +++ b/tests/control/ControllerTest.php @@ -31,54 +31,82 @@ class ControllerTest extends FunctionalTest { } public function testUndefinedActions() { - $response = Director::test('ControllerTest_UnsecuredController/undefinedaction'); + $response = Director::test('ControllerTest_AccessUnsecuredSubController/undefinedaction'); $this->assertEquals(404, $response->getStatusCode(), 'Undefined actions return a not found response.'); } public function testAllowedActions() { $adminUser = $this->objFromFixture('Member', 'admin'); - $response = $this->get("ControllerTest_SecuredController/methodaction"); - $this->assertEquals(200, $response->getStatusCode()); - - $response = $this->get("ControllerTest_SecuredController/stringaction"); - $this->assertEquals(404, $response->getStatusCode()); + $response = $this->get("ControllerTest_AccessBaseController/unsecuredaction"); + $this->assertEquals(200, $response->getStatusCode(), + 'Access granted on action without $allowed_actions on defining controller' + ); - $response = $this->get("ControllerTest_SecuredController/adminonly"); - $this->assertEquals(403, $response->getStatusCode()); - - $response = $this->get('ControllerTest_UnsecuredController/stringaction'); - $this->assertEquals(200, $response->getStatusCode(), - "test that a controller without a specified allowed_actions allows actions through" + $response = $this->get("ControllerTest_AccessBaseController/onlysecuredinsubclassaction"); + $this->assertEquals(200, $response->getStatusCode(), + 'Access granted on action without $allowed_actions on defining controller, ' . + 'even when action is secured in subclasses' ); - $response = $this->get("ControllerTest_FullSecuredController/index"); + $response = $this->get("ControllerTest_AccessSecuredController/onlysecuredinsubclassaction"); + $this->assertEquals(403, $response->getStatusCode(), + 'Access denied on action with $allowed_actions on defining controller, ' . + 'even if action is unsecured on parent class' + ); + + $response = $this->get("ControllerTest_AccessSecuredController/adminonly"); + $this->assertEquals(403, $response->getStatusCode(), + 'Access denied on action with $allowed_actions on defining controller, ' . + 'when action is not defined on any parent classes' + ); + + $response = $this->get("ControllerTest_AccessSecuredController/aDmiNOnlY"); + $this->assertEquals(403, $response->getStatusCode(), + 'Access denied on action with $allowed_actions on defining controller, ' . + 'regardless of capitalization' + ); + + // TODO Change this API + $response = $this->get('ControllerTest_AccessUnsecuredSubController/unsecuredaction'); + $this->assertEquals(200, $response->getStatusCode(), + "Controller without a specified allowed_actions allows its own actions through" + ); + + $response = $this->get('ControllerTest_AccessUnsecuredSubController/adminonly'); + $this->assertEquals(403, $response->getStatusCode(), + "Controller without a specified allowed_actions still disallows actions defined on parents" + ); + + $response = $this->get("ControllerTest_AccessAsteriskSecuredController/index"); $this->assertEquals(403, $response->getStatusCode(), "Actions can be globally disallowed by using asterisk (*) for index method" ); - $response = $this->get("ControllerTest_FullSecuredController/adminonly"); + $response = $this->get("ControllerTest_AccessAsteriskSecuredController/onlysecuredinsubclassaction"); $this->assertEquals(404, $response->getStatusCode(), - "Actions can be globally disallowed by using asterisk (*) instead of a method name" + "Actions can be globally disallowed by using asterisk (*) instead of a method name, " . + "in which case they'll be marked as 'not found'" ); - $response = $this->get("ControllerTest_FullSecuredController/unsecuredaction"); + $response = $this->get("ControllerTest_AccessAsteriskSecuredController/unsecuredaction"); $this->assertEquals(200, $response->getStatusCode(), "Actions can be overridden to be allowed if globally disallowed by using asterisk (*)" ); $this->session()->inst_set('loggedInAs', $adminUser->ID); - $response = $this->get("ControllerTest_SecuredController/adminonly"); + $response = $this->get("ControllerTest_AccessSecuredController/adminonly"); $this->assertEquals( 200, $response->getStatusCode(), "Permission codes are respected when set in \$allowed_actions" ); - $response = $this->get("ControllerTest_FullSecuredController/adminonly"); + $response = $this->get("ControllerTest_AccessUnsecuredSubController/adminonly"); $this->assertEquals(200, $response->getStatusCode(), "Actions can be globally disallowed by using asterisk (*) instead of a method name" ); + $this->session()->inst_set('loggedInAs', null); } @@ -227,48 +255,111 @@ class ControllerTest_Controller extends Controller implements TestOnly { } /** - * Controller with an $allowed_actions value + * Allowed actions flattened: + * - unsecuredaction: * + * - onlysecuredinsubclassaction: * + * - unsecuredinparentclassaction: * + * - unsecuredinsubclassaction: * */ -class ControllerTest_SecuredController extends Controller implements TestOnly { - static $allowed_actions = array( - "methodaction", - "adminonly" => "ADMIN", - ); - - public $Content = "default content"; - - public function methodaction() { - return array( - "Content" => "methodaction content" - ); - } - - public function stringaction() { - return "stringaction was called."; +class ControllerTest_AccessBaseController extends Controller implements TestOnly { + // Accessible by all + public function unsecuredaction() { + return 'unsecuredaction was called.'; } + // Accessible by all + public function onlysecuredinsubclassaction() { + return 'onlysecuredinsubclass was called.'; + } + + // Accessible by all + public function unsecuredinparentclassaction() { + return 'unsecuredinparentclassaction was called.'; + } + + // Accessible by all + public function unsecuredinsubclassaction() { + return 'unsecuredinsubclass was called.'; + } +} + +/** + * Allowed actions flattened: + * - unsecuredaction: * + * - onlysecuredinsubclassaction: ADMIN (parent: *) + * - unsecuredinparentclassaction: * + * - unsecuredinsubclassaction: (none) (parent: *) + * - adminonly: ADMIN + */ +class ControllerTest_AccessSecuredController extends ControllerTest_AccessBaseController implements TestOnly { + + static $allowed_actions = array( + "onlysecuredinsubclassaction" => 'ADMIN', + "adminonly" => "ADMIN", + ); + + // Accessible by ADMIN only + public function onlysecuredinsubclassaction() { + return 'onlysecuredinsubclass was called.'; + } + + // Not accessible, since overloaded but not mentioned in $allowed_actions + public function unsecuredinsubclassaction() { + return 'unsecuredinsubclass was called.'; + } + + // Accessible by all, since only defined in parent class + //public function unsecuredinparentclassaction() { + + // Accessible by ADMIN only public function adminonly() { return "You must be an admin!"; } } -class ControllerTest_FullSecuredController extends Controller implements TestOnly { +/** + * Allowed actions flattened: + * - unsecuredaction: * + * - onlysecuredinsubclassaction: ADMIN (parent: *) + * - unsecuredinparentclassaction: ADMIN (parent: *) + * - unsecuredinsubclassaction: (none) (parent: *) + * - adminonly: ADMIN (parent: ADMIN) + */ +class ControllerTest_AccessAsteriskSecuredController extends ControllerTest_AccessBaseController implements TestOnly { static $allowed_actions = array( "*" => "ADMIN", 'unsecuredaction' => true, ); + // Accessible by ADMIN only public function adminonly() { return "You must be an admin!"; } + // Accessible by all public function unsecuredaction() { return "Allowed for everybody"; } } -class ControllerTest_UnsecuredController extends ControllerTest_SecuredController implements TestOnly {} +/** + * Allowed actions flattened: + * - unsecuredaction: * + * - onlysecuredinsubclassaction: ADMIN (parent: *) + * - unsecuredinparentclassaction: * + * - unsecuredinsubclassaction: (none) (parent: *) + * - adminonly: ADMIN + */ +class ControllerTest_AccessUnsecuredSubController extends ControllerTest_AccessSecuredController implements TestOnly { + + // No $allowed_actions defined here + + // Accessible by ADMIN only, defined in parent class + public function onlysecuredinsubclassaction() { + return 'onlysecuredinsubclass was called.'; + } +} class ControllerTest_HasAction extends Controller { diff --git a/tests/control/RequestHandlingTest.php b/tests/control/RequestHandlingTest.php index b2db41446..a17b25655 100644 --- a/tests/control/RequestHandlingTest.php +++ b/tests/control/RequestHandlingTest.php @@ -162,7 +162,7 @@ class RequestHandlingTest extends FunctionalTest { public function testMethodsOnParentClassesOfRequestHandlerDeclined() { $response = Director::test('testGoodBase1/getIterator'); - $this->assertEquals(403, $response->getStatusCode()); + $this->assertEquals(404, $response->getStatusCode()); } public function testFormActionsCanBypassAllowedActions() { @@ -295,6 +295,17 @@ Config::inst()->update('Director', 'rules', array( * Controller for the test */ class RequestHandlingTest_Controller extends Controller implements TestOnly { + + static $allowed_actions = array( + 'method', + 'legacymethod', + 'virtualfile', + 'TestForm', + 'throwexception', + 'throwresponseexception', + 'throwhttperror', + ); + static $url_handlers = array( // The double-slash is need here to ensure that '$Action//$ID/$OtherID' => "handleAction", diff --git a/tests/security/MemberTest.php b/tests/security/MemberTest.php index c61f77cc7..f6a4412a5 100644 --- a/tests/security/MemberTest.php +++ b/tests/security/MemberTest.php @@ -114,6 +114,23 @@ class MemberTest extends FunctionalTest { Security::set_password_encryption_algorithm($origAlgo); } + + public function testKeepsEncryptionOnEmptyPasswords() { + $member = new Member(); + $member->Password = 'mypassword'; + $member->PasswordEncryption = 'sha1_v2.4'; + $member->write(); + + $member->Password = ''; + $member->write(); + + $this->assertEquals( + $member->PasswordEncryption, + 'sha1_v2.4' + ); + $result = $member->checkPassword(''); + $this->assertTrue($result->valid()); + } public function testSetPassword() { $member = $this->objFromFixture('Member', 'test'); diff --git a/view/SSViewer.php b/view/SSViewer.php index cac477c55..177f48753 100644 --- a/view/SSViewer.php +++ b/view/SSViewer.php @@ -833,7 +833,7 @@ class SSViewer { * @return string - The result of executing the template */ protected function includeGeneratedTemplate($cacheFile, $item, $overlay, $underlay) { - if(isset($_GET['showtemplate']) && $_GET['showtemplate']) { + if(isset($_GET['showtemplate']) && $_GET['showtemplate'] && Permission::check('ADMIN')) { $lines = file($cacheFile); echo "

    Template: $cacheFile

    "; echo "
    ";