silverstripe / silverstripe-admin

Silverstripe Admin Component
BSD 3-Clause "New" or "Revised" License
25 stars 92 forks source link

Changing the admin URL causes a 500 error on the default route #689

Open DorsetDigital opened 6 years ago

DorsetDigital commented 6 years ago

Tested on Silverstripe 4.1.1 and 4.2.1

When changing the admin route by adding the following YML (as per the docs):

---
Name: myadmin
After:
- '#adminroutes'
---
SilverStripe\Control\Director:
  rules:
    'admin': ''
    'cms': 'SilverStripe\Admin\AdminRootController'

The new URL becomes active and works correctly. The old URL (eg. example.com/admin) now throws a 500 error (below). I would expect it to return a 404.

Uncaught InvalidArgumentException: Missing spec class
GET /admin
Line 403 in /app/vendor/silverstripe/framework/src/Core/Injector/Injector.php

The full stack trace is attached. admin_error_trace.txt

robbieaverill commented 6 years ago

@DorsetDigital did you also set the AdminRootController.url_base config prop to cms? I realise it's documented as a fallback option, but I'm just curious

DorsetDigital commented 6 years ago

I've just tested that now, and it doesn't seem to change the result.

DorsetDigital commented 6 years ago

As an aside, I noticed that might be necessary for things like the subsites add-on.. I'll update the docs to reflect that as well.

kinglozzer commented 6 years ago

Does 'admin': null work? I guess we need to make director ignore routes that are mapped to false-y values

DorsetDigital commented 6 years ago

Changing to null doesn't work I'm afraid. In fact, you get an additional warning for your trouble:

[Warning] array_merge(): Argument #1 is not an array
GET /admin

Line 337 in /app/vendor/silverstripe/framework/src/Control/Director.php
shot131 commented 5 years ago

https://github.com/silverstripe/silverstripe-admin/blob/8f760ffbbc645ed136aa5f30fadf36afe38fbfee/client/src/boot/apollo/getGraphqlFragments.js#L26 admin url is hardcoded here. If i change default admin url, then /admin/graphql/types return 404 error.

robbieaverill commented 5 years ago

@shot131 thanks for highlighting that. Would you mind raising that as a new issue on https://github.com/silverstripe/silverstripe-graphql?

shot131 commented 5 years ago

@robbieaverill i think this issue does not touch https://github.com/silverstripe/silverstripe-graphql. This routing is announces in https://github.com/silverstripe/silverstripe-admin/blob/1/_config/routes.yml#L8

robbieaverill commented 5 years ago

Sorry, I only skim read your message. You're right, the two issues (JavaScript and YAML config) are both in this module, so fine to leave it here =) Thanks

itspers commented 5 years ago

Hi! Did anybody found working solution for graqhQL? I had working config for a months with

'admin': 'SilverStripe\CMS\Controllers\RootURLController'
'backend': 'SilverStripe\Admin\AdminRootController'

Before 4.3 it was working fine, but starting from 4.3 Files page tries to reach 'admin/graphql', fails and crash :(

kinglozzer commented 5 years ago

A workaround for now is to remove any 'admin': '' or 'admin': 'RootURLController' overrides from your config.yml and instead remove the default admin route in _config.php:

$rules = Config::inst()->get(Director::class, 'rules');
unset($rules['admin']);
Config::modify()->remove(Director::class, 'rules');
Config::modify()->set(Director::class, 'rules', $rules);

The problem is that when a request to admin/graphql/types is made, Director looks through the routes, sees 'admin': '' (which matches) and then stops. I think to fix this properly, we need to do at least one of the following:

  1. Make sure that the admin/graphql route is inserted in the correct place, so that Director checks that rule before any 'admin': '' rules. (This might already be possible with Before:/After:, which would make this a docs issue - I haven’t checked);
  2. Make Director ignore/skip over any routes which map to '' or null, or;
  3. Add a “remove”/“unset”/“replace” directive to our YAML so you can just remove the default admin route in YAML (there’s an issue open somewhere for this enhancement)
michalkleiner commented 5 years ago

~There's already a ticket for unsetting routes via yaml. Let me find it.~

Related ticket that could benefit from the solution here - https://github.com/silverstripe/silverstripe-cms/issues/2349

michalkleiner commented 5 years ago

The suggested solution was to go with null values.