Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minimize potential forum-breaking issues #85

Open
5 of 7 tasks
davwheat opened this issue Jun 7, 2021 · 13 comments
Open
5 of 7 tasks

Minimize potential forum-breaking issues #85

davwheat opened this issue Jun 7, 2021 · 13 comments

Comments

@davwheat
Copy link
Member

davwheat commented Jun 7, 2021

Feature Request

Is your feature request related to a problem? Please describe.
If there is a backend/boot/JS error, we should still allow admins to access the admin page.

This could be done with a simple No-JS admin page accessible via a URL such as example.forum/admin?safe=true.

Describe the solution you'd like

Solution 1

If this safe=true is present, we should not load any extensions while displaying this page. We can then show the user a page that allows them to make changes to the active extensions (enabling/disabling).

Solution 2

Alternative with using just Blade and DB password as authorization to enable and disable extensions without any booting of Flarum.

Justify why this feature belongs in Flarum's core, rather than in a third-party extension
This is a core part of Flarum that would improve accessibility to more users.


Agreed Upon Tasks

@SychO9
Copy link
Member

SychO9 commented Aug 25, 2021

This would amazing to have,

I don't think the problem it would be solving is about Flarum not booting, but rather just extensions not booting.

Solution 1 would avoid us creating another interface, but would need to separate the core js bundle from extensions to only load that, but code splitting is already doing something similar I believe with the bundles, so not sure if we want to mess with that here as well.

Solution 2 might be better, the blade template would be minimal, it would only need to allow disabling/enabling extensions, but it might also require adding a login form in case the admin is logged out for whatever reason.

@askvortsov1
Copy link
Sponsor Member

I think there's a bunch of separate things we need here:

  • A safe mode (possibly via URL param) that loads Flarum without booting any extensions
  • A no-js login page for admin, as currently that shows a "not authenticated" error page
  • A blade admin page that shows a table of extensions and a bunch of switches, so extensions can be disabled if their JS is broken
  • Better JS error messages so that if JS is broken:
    • We show which file (and preferably, which extension) things went wrong in
    • We show more info about the error

@SychO9
Copy link
Member

SychO9 commented Aug 25, 2021

A safe mode (possibly via URL param) that loads Flarum without booting any extensions

We wouldn't load the JS though right? since currently extension JS code is contained in the same bundle, unless that changes.

@askvortsov1
Copy link
Sponsor Member

A safe mode (possibly via URL param) that loads Flarum without booting any extensions

We wouldn't load the JS though right? since currently extension JS code is contained in the same bundle, unless that changes.

We could omit the initializer step. On the other hand, we should probably eventually split each extension's code into its own file instead of a single mega-file, so that could be another vector.

@SychO9
Copy link
Member

SychO9 commented Aug 25, 2021

We could omit the initializer step.

Oh! I forgot we had that! nice.

@davwheat
Copy link
Member Author

davwheat commented Aug 25, 2021

On the other hand, we should probably eventually split each extension's code into its own file instead of a single mega-file, so that could be another vector.

I'm against this idea. Multiple script files would mean multiple HTTP requests, which would add extra overhead compared to the single file workflow we have at the moment.

While HTTP/2 removes a fair amount of overhead compared to HTTP/1, we still have ~50ms TTFB on our main JS payload, and parallel connections on slower connections become painful.

Separating into different frontends is a good idea still, because of the overall bundle improvement, and the fact that we won't have 30 different FEs, most likely :P

@SychO9
Copy link
Member

SychO9 commented Aug 27, 2021

A safe mode (possibly via URL param) that loads Flarum without booting any extensions

Looking at the code, URL parameter wouldn't work, extensions are booted before we start handling the coming request.

The safe mode would be useful when an extension breaks the backend, so perhaps we can utilise the configurable offline mode to not boot extensions and allow admin access solving https://github.com/flarum/core/issues/1732 in the process.

Though we can effectively start working on the other points, I'll try to tackle the blade admin page.

@askvortsov1
Copy link
Sponsor Member

Looking at the code, URL parameter wouldn't work, extensions are booted before we start handling the coming request.

You're right. We could do some PHP globals hackery, but it would be preferable to avoid that.

For offline mode, it feels like a major use case could be adjusting / configuring extension settings, so I would be hesitant to lump disabling extensions into that. We could have another key in config.php for that though?

@SychO9
Copy link
Member

SychO9 commented Aug 27, 2021

For offline mode, it feels like a major use case could be adjusting / configuring extension settings, so I would be hesitant to lump disabling extensions into that. We could have another key in config.php for that though?

That's a good point, an additional configurable safe mode option makes sense then!

@SychO9 SychO9 pinned this issue Aug 27, 2021
@askvortsov1
Copy link
Sponsor Member

askvortsov1 commented Aug 30, 2021

I've been thinking more about the safe more option, and if we decide to have one, I'm leaning towards going with something similar to Discourse's approach. I don't think we should allow disabling extensions on the backend via safe mode because that opens up a whole slew of potential security issues. Furthermore, it couples per-request state to global application state, which is something we'd like to avoid.

That being said, I started working on a safe mode PR, but stopped when I realized I didn't understand the use case. What do we want this to do? Do we want to make it easier to debug frontend issues? Offer a way for admins to rescue a "bricked" forum by disabling all extensions? Allow admins to edit extension settings in the admin panel without those reflecting on the forum side?

@askvortsov1 askvortsov1 changed the title Add a no-JS Admin page Minimize potential forum-breaking issues Aug 30, 2021
@SychO9
Copy link
Member

SychO9 commented Aug 30, 2021

I'm leaning towards going with something similar to Discourse's approach.

How does discourse handle it ?

Do we want to make it easier to debug frontend issues? Offer a way for admins to rescue a "bricked" forum by disabling all extensions? Allow admins to edit extension settings in the admin panel without those reflecting on the forum side?

No, yes, no.

What do we want this to do?

If we take the admin NoJS view as an example, which solves the issue of an extension breaking the admin frontend and locking out the admin from fixing it by disabling said extension. The same can happen if an extension breaks the backend and prevents Flarum from booting, the admin NoJS view is only solving half of the problem, having the possibility to safely boot to disable said extensions and therefor require less SSH access is needed.

I don't know what exactly you're referring to when mentioning security issues, but if my assumptions are correct, I think the smart thing to do is to have safe mode as sort of an offline mode level 2, where it is an offline mode (which locks users from accessing the forum apart from the admin) but it also doesn't boot any extensions.

Furthermore, it couples per-request state to global application state, which is something we'd like to avoid.

Isn't that why we're avoiding the URL safe mode option, or is it also an issue with other approaches ?

@askvortsov1
Copy link
Sponsor Member

Makes sense, I think I was overthinking this when I wrote it. I'll open a PR for this.

How does discourse handle it ?

Discourse's approach is that any user can trigger a "no-js safe mode" where the backend works with all plugins, but plugin JS isn't booted. Apparently, the goal is to help diagnose broken JS issues.

I believe that when we convert our JS system to be more extender based, a lot more errors will happen on boot, meaning that diagnosing where stuff goes wrong will become a lot easier. I think that might be a more effective system than just disabling JS.

askvortsov1 referenced this issue in flarum/framework Sep 17, 2021
Part of https://github.com/flarum/core/issues/2911

If the `boot_extensions` key is set to false, core will not boot any extensions in the backend, or run any initializers in the frontend. This allows admins to recover their forum without SSH access by disabling extension boot, going to the admin panel, and turning off offending extensions.

**Questions/Concerns for Reviewers**:

- Should we somehow signal to admins that this mode is enabled, possibly via alert? Otherwise, it might be confusing why extensions are indicated as enabled in the admin dashboard. This is challenging, as the alerts we currently have are in `content.blade.php`, and that view does not have access to payload/config.
- Should this require maintenance mode? If we disable all extensions (e.g. tags), some restricted discussions / content will immediately become public. This could lead to leaks via web scraping.
@SychO9
Copy link
Member

SychO9 commented Oct 14, 2021

I've added a new entry:

  • Safe Less compilation failures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants