Using multiple 2FA authenticators, logging out, resetting password etc. proved to be handled wrong.
Example scenario:
The result is an error, because the `renderWrappedController` was called, despite the responses being a set of either array with Content or Form, or a redirect action.
The default action should be followed and not try to render if there is nothing to render
Because the logout (or changepassword, or resetpassword, etc.) has already been handled, the first response is the default authenticator's response. This _could_ be a form (in case of logout without valid token), a content set (reset password) or a form (change password).
This edge case only happens when there are multiple authenticators supporting the requested method that is _not_ login.
This will prevent long runnings builds (e.g. code coverage) from failing when the test database connection is gone (MySQL server has gone away) by the time the shutdown handler runs.
* New shortcode providers, update config, docs
* Use new ImageShortcodeProvider
* Move tests
* New shortcodes namespace
* Move file and image shortcode registrations from framework to assets
NEW: URL generation now handled by pluggable ResourceURLGenerator service.
NEW: Requirements::javascript() and Requirements::css() now support “vendor/package:resource” syntax.
These changes will make it easier to us to fully abstract:
- file access from module location
- file location from URL generation
API: ModulePath template global now takes any composer package name.
NEW: URL generation now handled by pluggable ResourceURLGenerator service.
NEW: Requirements::javascript() and Requirements::css() now support “vendor/package:resource” syntax.
These changes will make it easier to us to fully abstract:
- file access from module location
- file location from URL generation
BUG Fix up test regressions
FIX director references to request object
API Move all middlewares to common namespace
API Implement RequestHandlerMiddlewareAdapter
ENHANCEMENT Improve IP address parsing
Fix up PHPDoc / psr2 linting
BUG Fix property parsing in TrustedProxyMiddleware
BUG Fix Director::is_https()
NEW: Add HTMLMiddlewareAware trait to HTTPApplication, Director, and RequestHandler
NEW: Allow service specs to be passed to Director rules.
This refactor of the controller middlewares takes a service definition
approach rather than a static-method-and-config approach that Director
historically had.
The use of a trait for middleware means that the Middlewares array
property can be defined on RequestHandler, Director, and HTTPApplication
objects in the same way.
NEW: Pass HTTPRequest to session
NEW: Pass HTTPReuqest optionally to Director statics
The session handler now expects to operate on a specific
HTTPRequest object.
API: SS_TRUSTED_PROXY_HOST_HEADER replace with middleware config
API: SS_TRUSTED_PROXY_PROTOCOL_HEADER replace with middleware config
API: SS_TRUSTED_PROXY_IP_HEADER replace with middleware config
API: Front-End-Https = “on” header no longer supported
This middleware replaces the TRUSTED_PROXY setting and shifts its
configuration out of the env vars and bootstrap and into the Director
flow.
NEW: Add HTTPRequest::setIP()
API: Rely on HTTPRequestBuilder to set scheme and IP
These changes tidy up HTTPRequest making it a container for information
and removing special logic from it.
This makes it less feature-rich: it doesn’t contain trusted-proxy logic.
This will be able to provided by a middleware.
The new getScheme() method is designed to be closish to PSR-7’s
getUri()->getScheme() equivalent.
There are no more direct $_SERVER references in HTTPRequest.
HTTPRequest is provided as a service so that global references for
session, hostname, etc can be facilitated. It’s a bit of a hack and
should be avoided but we’re unlikely to scrub it completely from the
Silverstripe 4 code.
NEW: Allow application of HTTPMiddleware to Director.
Director can now use the same HTTPMiddleware objects as the app object.
They can be applied either globally or pre-rule.
Injector::get() looks up services by name. In yaml config it can make
things clearer to prefix service names by %$, which is how they must
be prefixed when referencing nested services within service definitions.
This change means that any other system referencing services will
support an optional prefix without needing to specifically code support
in themselves.
This wasn’t working because the database was being validated before
_config.php was loaed.
This is how the installer sets config so this is an important fix.
- Moved the Authenticators from statics to normal
- Moved MemberLoginForm methods to the getFormFields as they make more sense there
- Did some spring-cleaning on the LostPasswordHandler
- Removed the BuildResponse from ChangePasswordHandler after spring cleaning
- Move the success and message to a validationresult
- Fix tests for validationresult return
- We need to clear the session in Test logOut method
- Rename to MemberAuthenticator and CMSMemberAuthenticator for consistency.
- Unify all to getCurrentUser on Security
- ChangePasswordHandler removed from Security
- Update SapphireTest for CMS login/logout
- Get the Member ID correctly, if it's an object.
- Only enable "remember me" when it's allowed.
- Add flag to disable password logging
- Remove Subsites coupling, give it an extension hook to disable itself
- Change cascadeLogInTo to cascadeInTo for the logout method logic naming
- Docblocks
- Basicauth config
Repairing tests and regressions
Consistently use `Security::getCurrentUser()` and `Security::setCurrentUser()`
Fix for the logout handler to properly logout, some minor wording updates
Remove the login hashes for the member when logging out.
BasicAuth to use `HTTPRequest`
Move to canLogin in the authentication check. Protected isLockedOut
Enable login to be called with a different login service (CMSLogin), enabling CMS Log in. Seems the styling and/or output is still broken.
logOut could be managed from the Authenticator instead of the member
Authenticators is now a map of keys -> service names. The key is used
in things such as URL segments. The “default_authenticator” value has
been replaced with the key “default” in this map, although in time a
default authenticator may not be needed.
IX: Refactor login() to avoid code duplication on single/multiple handlers
IX: Refactor LoginHandler to be more amenable to extension
IX: Fixed permissionFailure hack
his LoginHandler is expected to be the starting point for other
custom authenticators so it should be easier to repurpose components
`of it.
IX: Fix database-is-ready checks in tests.
IX: Fixed MemberAuthenticatorTest to match the new API
IX: Update security URLs in MemberTest
Further down the line, I'm only returning the `Member` on the doLogin, so it's possible for the Handler or Extending Handler to move to a second step.
Also cleaned up some minor typos I ran in to. Nothing major.
This solution works and is manually tested for now. Supports multiple login forms that end up in the correct handler. I haven't gotten past the handler yet, as I've yet to refactor my Yubiauth implementation.
FIX: Corrections to the multi-login-form support.
Importantly, the system provide a URL-space for each handler, e.g.
“Security/login/default” and “Security/login/other”. This is much
cleaner than identifying the active authenticator by a get parameter,
and means that the tabbed interface is only needed on the very first view.
Note that you can test this without a module simply by loading the
default authenticator twice:
SilverStripe\Security\Security:
authenticators:
default: SilverStripe\Security\MemberAuthenticator\Authenticator
other: SilverStripe\Security\MemberAuthenticator\Authenticator
FIX: Refactor delegateToHandler / delegateToHandlers to have less
duplicated code.
The $class variable gets overwritten in the function.
This causes error messages to be less helpful. For example if you setup a has_many but forget the has_one on the other side the error will look something like
`[Emergency] Uncaught Exception: No has_one found on class 'SomeObject', the has_many relation from 'SilverStripe\View\ViewableData' to 'SomeObject' requires a has_one on 'SomeObject'`
fixing this gives a more useful error, like
`[Emergency] Uncaught Exception: No has_one found on class 'SomeObject', the has_many relation from 'Page' to 'SomeObject' requires a has_one on 'SomeObject'`
When modules are installed as the webroot,
manifest generation should behave the same way as when they're in a subfolder.
Which means accepting the module folder both with a _config/ folder
and a _config.php file present.
Note that our usage of `$asSingleton` in `get()` is fine. Quote from the PSR:
> Two successive calls to get with the same identifier SHOULD return the same value. However, depending on the implementor design and/or user configuration, different values might be returned, so user SHOULD NOT rely on getting the same value on 2 successive calls.
Regression introduced through https://github.com/silverstripe/silverstripe-framework/issues/6362.
Quote from the RFC:
```
Thus the order of action precedence becomes
action callback
action on the Form
action on the FormRequestHandler
action on any parent controller (if given)
```
* Test databases now include timestamp for easier debugging
* Use classname::class instead of string literal classnames
* Remove DataObject::get_one() from SapphireTest
* More fixes to ICU DB inconsitency for time formatting
* Correctly restore PHPUnits error handler
FIX for #4417: Ensuring ->removeValidation() is defined on instances of Validator. Setup new API for enabling/disabling validation. Documentation and better type handling.
Prevent html errors when FormField::create_tag('meta') is called from $MetaTags() so
```
<meta name="generator" content="SilverStripe - http://silverstripe.org"></meta>
```
becomes
```
<meta name="generator" content="SilverStripe - http://silverstripe.org" />
```
Add all void elements to list
* API Implement InheritedPermission calculator
* API Rename RootPermissions to DefaultPermissionChecker
API Refactor inherited permission fields into InheritedPermissionExtension
API Introduce PermissionChecker interface
* Stop relying on external constants
* Revise getTinyMCEPath method to throw exception when no path can be computed
* Throw exception on no gzip, better admin module check
Right now SapphireTest::objFromFixture() requires a class as the first
argument. This is fine when your fixture file uses classes as the keys,
but if populating a fixture via tables, objFromFixture() won’t work.
This patch lets you specify either the table name or the class name as
the key.
The benefit here is that you can build fixtures as raw inserts, which is
substantially quicker, and is likely to be a useful tool in building
more efficient test suites.
API: Add HTTPOutputHandler::setCLIFormatter
Fixes https://github.com/silverstripe/silverstripe-framework/issues/6835
This provides detailed errors (but not warnings or notices) in CLI calls
on live environments.
It does this by adding a 2nd argument to our output handler,
CliFormatter. This formatter will be used when Director::is_cli() is
true.
Partially reinstates the 3.x style PhpUnitWrapper which was removed in d1af214ef5.
While we no longer need the full wrapper, the part which creates a fake class is still useful.
The preferred alternative would be to remove any references to SapphireTest from non-dev files,
which mostly applies to SapphireTest::is_running_test(). This should be solved in a larger refactor
of SapphireTest into optional traits and more fine grained functionality.
Was missed from the removal of PHPUnitWrapper:
a16588aac3
Original reason for this: Don't fail dev/build without phpunit
When you install a SilverStripe project with "composer install --no-dev",
the PHPUnit dependency gets skipped. Which means the PHPUnit_Framework_TestListener
interface doesn't exist. The SilverStripe Classloader might still include
SapphireTestReporter which relies on this interface, which then breaks execution.
SS3 fixed this by NOT defining the class in the first place.
This has been removed in 2fdc96a0de (diff-82b3f89e8e5ae090c93e9c3a2ba8aa36L3),
as part of a PHPUnit version upgrade - but without an apparent fix to replace this.
API Remove Director::$test_servers / $dev_servers
API Remove MODULES_PATH / MODULES_DIR constants
ENHANCEMENT Injector backtick syntax now supports environment variables as well as constants
Fixes#6588
Resolves issue #6788
The AuthenticationMethed is passed in via hidden field as per usual, but due to changes, the fallback authenticator was always the MemberAuthenticator and the actual passed in authenticator was defaulting to an empty string.
This causes an issue when there are multiple authenticators and the default authenticator is _not_ in the allowed authenticators, but is still the default. It caused the getAuthenticator method to return the default MemberAuthenticator to be returned, despite it being disabled.
A second issue around multiple authenticators, was the template using a no-longer used method `getAuthenticatorName`. This method returned a null on the default MemberLoginForm (as nothing was set), causing a Warning.
Because the getAuthenticator and getAuthenticatorName are no longer in
use, I've opted to replace these with a translatable string
`getAuthenticatorName`, to
display the title of the form on the tabs, as per the tabset on
Security_MultiAuthenticatorLogin template.
- has the fields and actions extracted to a separate method, so it's more easily overridable
- Moved the global variable $_REQUEST to getting the info from the controller
- Updated string variables to `::class`
- Updated RequiredFields to be set in the YML, so it's overridable/updatable from either Config or code
The query SHOW TABLES LIKE 'some_thing' will match the table some\thing.
This causes issues when the namespace separator has changed.
The fix is to escape the _s in this LIKE statement, as done here.
