Remove outbound email confirm setting

[Matthewrbowker]

Related, bin the email confirm setting, and add an outbound email null
route option, disabling all outbound mail since that is the most common
issue for development and the reason that flag still exists. Would simplify
the workflow significantly too.

On Thu, 31 Mar 2016 00:54 FastLizard4, [email protected] wrote:

Well, I'm thinking we may not necessarily, such as the instances when
someone just emails the list with a subject like "confirm" and a blank body

• in that case, we'd only have the email address to go on.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#259 (comment)

Originally posted by @stwalkerster in #259 (comment)

[stwalkerster]

We can probably nullroute by just skipping the call to mail() in EmailHelper with a configuration option. We'll need to pass the SiteConfiguration object into the EmailHelper constructor, and in ApplicationBase.

I'm wondering about the email confirmation approach though, since if we nullroute email and remove the email confirmation option (forcing confirmation), will developers be able to submit requests successfully to the system?

[FastLizard4]

Presumably, disabling email would also disable the email confirmation requirement, so it's not really removing the email confirmation setting but superseding it with one that disables all email-related functionality.

In any case, I wonder if this is still a good idea—there's several options for free, light-weight SMTP sinks/mocks that can be used for testing email, both software that can be run locally and cloud services. Indeed, with my Docker changes recently, if you use docker-compose to run ACC locally you'll get a functional mailsink right out of the box, and you can even just run the mailsink service on its own (docker compose up mailsink) to integrate it with an existing dev setup.

The email functionality is pretty central to the account request process, so I'd prefer to simplify having it locally rather than having an option to remove it entirely (though, to be clear, I'm not opposed to an option to disabling outbound mail either).

[Matthewrbowker]

I also agree with creating and updating a mailsink. If I understand correctly, here's what I propose:

• We create a configuration to disable outgoing mail. Because we now have the ability to manually confirm requests, this would be a null route. In INSTALLING.md, we mention this option in passing only. This works due to manual confirmation introduced in Ability for admins to manually confirm account requests #259
• We configure Docker to include the mailsink tool. We document setting that up under the docker setup.
• We include documentation to configure an example online mailsink. I'm familiar with mailtrap.io so I can write some documentation for that. This would also be its own section and given prominence.

[FastLizard4]

• We configure Docker to include the mailsink tool. We document setting that up under the docker setup.

This is already done, and no setup is required. Just run the docker compose up command, which spins up the application, database, RabbitMQ server, and mailsink with configurations to make them all talk to each other. It even includes a working configuration for the app; it all works out of the box. Though it could probably use more explicit documentation on how to do things like open the mailsink in your browser, accessing the RabbitMQ management interface to see that notifications are being delivered correctly, etc., but it is currently documented in-line in docker-compose.yml.

Having thought about it some more since my last comment, based on my professional experience over the past year I would actually endorse making Docker the primary way development on ACC happens since it makes it extremely easy to have platform-independent, batteries-included development environments that are consistent for everyone. It also reduces how much work we would have to do - a configuration to disable outgoing mail is essentially only useful for local development, and it's completely obviated if one is using Docker, which is becoming an increasingly standard part of software development toolchains anyway.

To put it another way, I think we should really think hard about whether it's worth the effort of doing things like adding a config to disable outbound email confirmation, or if we just make Docker the primary way to run the app for local dev. Of course, I'm not saying that we should not support running it in non-Docker environments (hence I'm still not strictly opposed to adding the config), but I think we should thoroughly consider whether it's worth adding a config that is basically just for local dev when there's an alternative already present, and one that IMO is better than asking devs to install and maintain PHP, Apache, MariaDB, etc. locally on their machines (plus no worrying about configuring any of those tools or how they might conflict with existing configs since they all live within Docker containers).

What do y'all think?

[stwalkerster]

The email functionality is pretty central to the account request process

Yes, it is, but I'd say that 95% of the tool development work I've done recently hasn't touched email at all. As weird as it may seem, the only bit* that touches email is new request submission and request closure. There's way more to the tool code than that.

making Docker the primary way development on ACC happens since it makes it extremely easy to have platform-independent, batteries-included development environments that are consistent for everyone.

While common development environments are great, the usual focus is to make sure that what you're running in dev matches what you're running in prod. I'm wondering if we should shift our focus towards deploying containers in production too, built in the same way that we build the containers for development (preferably using the same Dockerfile)?

Of course, I'm not saying that we should not support running it in non-Docker environments (hence I'm still not strictly opposed to adding the config)

We should definitely keep the core of the application standalone from Docker, but I'm thinking that we should make decisions about configuration which prioritise a) the configuration we need in Production, then b) the configuration which makes sense for Docker, then c) the catch-all configuration to make it safe to develop locally.

By c), I think I mean the ability to disconnect from the trickier things to configure (OAuth, RabbitMQ, email, maybe even the MediaWiki API itself). Historically, we've done this with settings like $dontUseWikiDb, and injecting in FakeFlobberService-type classes which mock out some external connectivity, though this pattern also died somewhat with newinternal. We already wrap around mail() with our own class that we can mock out for the unit tests.

I'm not really sure where I stand on adding this as a config option at the moment - I think I'd still like it.

asking devs to install and maintain PHP, Apache, MariaDB

My dev process doesn't involve Apache at all. I use PHP in the development server mode (run from PhpStorm), and MariaDB in a container (several containers actually, so I can jump between schema versions if there are multiple in-flight). I do have a Traefik front-end for TLS termination, but that's really only used for WebAuthn, which I'm not touching at the moment and I expect almost nobody will want to touch either.

I definitely see the benefits of me switching to PHP-in-a-container, and I'll probably look into shifting my dev process to that. I do like having real email working though when I need it.

tl;dr: ¯\_(ツ)_/¯

[FastLizard4]

Yes, it is, but I'd say that 95% of the tool development work I've done recently hasn't touched email at all. As weird as it may seem, the only bit* that touches email is new request submission and request closure. There's way more to the tool code than that.

True and fair. I guess I still feel like it's a little weird to add configs just for local dev (even though I know we already have one that just outright disables email confirmation without disabling email). Like I said, it's a very minor (and, the more I think about it, kinda dumb) objection and I wouldn't oppose adding the config. I think more what's in my head is along the lines of the overall developer experience (which, itself, is admittedly a consequence of me coming back to ACC and going "how tf do I set this up locally again?").

While common development environments are great, the usual focus is to make sure that what you're running in dev matches what you're running in prod. I'm wondering if we should shift our focus towards deploying containers in production too, built in the same way that we build the containers for development (preferably using the same Dockerfile)?

Honestly, I think this is a reasonable-to-good idea. Should be perfectly doable using multistage builds, though I'd probably go the route of creating a base build stage that is just the app and required dependencies (extensions, apt-get packages, etc.) for prod then a stage on top of that that adds XDebug and sets up the other stuff for local dev (so not strictly multistage, more using the earlier stage as a base image on which to add the local dev stuff).

We could even productionize the Docker Compose stuff too and support prod and dev profiles; really it's just a question of how far do we want to take it (e.g., can we/do we want to use Docker Compose or Swarm to manage things in production, do we want to orchestrate services aside from the app, etc.).

The biggest unknown here for me is how this would actually work from the Labs side of things.

We should definitely keep the core of the application standalone from Docker, but I'm thinking that we should make decisions about configuration which prioritise a) the configuration we need in Production, then b) the configuration which makes sense for Docker, then c) the catch-all configuration to make it safe to develop locally.

I guess the nice thing about shifting to prod Docker is that A and B become much closer, and if we prefer Docker as the local dev method as well then they all become pretty close. But otherwise, makes sense.

My dev process doesn't involve Apache at all. I use PHP in the development server mode (run from PhpStorm), and MariaDB in a container (several containers actually, so I can jump between schema versions if there are multiple in-flight). I do have a Traefik front-end for TLS termination, but that's really only used for WebAuthn, which I'm not touching at the moment and I expect almost nobody will want to touch either.

I definitely see the benefits of me switching to PHP-in-a-container, and I'll probably look into shifting my dev process to that. I do like having real email working though when I need it.

Makes sense. Certainly I don't want to make any decisions that close doors (e.g., actually prevents us from running on not-Docker); that's an anti-pattern in any case. More my point is that this is a considerable amount of effort to set up, and certainly one I didn't want to undertake, partially because I commonly do dev work on multiple machines (Windows desktop at home, macOS laptop on the go). By comparison, at least I personally found Docker much more straightforward, even if I did have to figure out a few things to fix (and then getting it running on my second machine was basically one-command).

[...]but that's really only used for WebAuthn, which I'm not touching at the moment and I expect almost nobody will want to touch either.

I don't know, I might if I have some extra time. YubiOTP is cool, but I miss being able to U2F. :D

---

tl;dr: I'm nitpicking things that probably don't matter; let's add the config to disable all email, but the thought of switching to containers in prod is interesting and something I'd like to consider.

[stwalkerster]

Sounds good to me.

As a tangent, at some point I'm wanting to completely overhaul the configuration system we use. My thinking was something along the lines of deserialising yaml into the SiteConfiguration class, but I've not quite figured out a) if that's truly the way I want to go and b) how to get there.

There's a lot of config options and general detritus in config.inc.php which is either unused or badly named or should be elsewhere. There's also a lot of other debugging options around, like the one to enable stack traces to be displayed (as well as being saved to ./errorlog/) and there's one to enable CSS breakpoint display too. Arguably most of the config that is actually used is to differentiate between environments, and as we only have one prod env and one staging env, plus all our dev envs, you could say almost all config is intended for dev.

Anyway, thoughts for another time.

---

To clarify, are we:

• removing the option to disable email confirmation (thus it's always enabled): AND
• adding a flag to skip sending all email?