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

Implement authentication for database #72

Open
super-cooper opened this issue Feb 22, 2021 · 0 comments
Open

Implement authentication for database #72

super-cooper opened this issue Feb 22, 2021 · 0 comments
Assignees
Labels
low-priority Low priority issues for the back burner

Comments

@super-cooper
Copy link
Owner

Does this improvement fix a bug/issue? Please describe.
N/A

What do you dislike about the feature in its current state?
The initial implementation of the database does not require client authentication. This is not a huge deal since the database is on an isolated network, cut off from the internet, but this should be a bare-minimum security measure in case a container on that network is somehow compromised.

Describe the solution you'd like
I believe we should implement and require authentication using X.509 certificates. This should be fairly easy to do with OpenSSL and the MONGODB-X509 protocol.

What tradeoffs are made by implementing your improvement?
Getting the database up and running for testing and tinkering is more annoying/complicated, but I think this can be fixed by writing a script that generates testing certificates that don't expire and creates accounts in a fresh database, so one could just fire that once and forget about it.

Describe alternatives you've considered
Username and password would be easier to do, but then we would have to save the password on disk anyway for deployment via docker-compose. It's just much easier to use certificates to accomplish the same thing.

Additional context
Depends on #13.

@super-cooper super-cooper added the low-priority Low priority issues for the back burner label Feb 22, 2021
@super-cooper super-cooper self-assigned this Feb 22, 2021
super-cooper added a commit that referenced this issue May 5, 2021
The changes introduced in this patch do not change behavior, but do change deployment just slightly.

1. The [Dockerfile](./docker/Dockerfile) has been moved to the `docker` directory, since I have now added a [docker-compose.yaml](./docker/docker-compose.yaml).
2. The [docker-compose.yaml](./docker/docker-compose.yaml) exists to define the project which now includes a database container. The database is spawned using the most basic possible configuration. There is a plan to add authentication in a future patch (#72).
3. The databse is presently not used for anything. A test function is run once at startup to ensure that a connection to the database is able to be made.
4. There is a basic framework for where database functionality should be written, although this is malleable and is free to be changed radically in the future. 
5. I added a [.dockerignore](./.dockerignore) file which will prevent unnecessary files from being copied to the Docker daemon when the image is built. This prevents the database files from being passed as well.
6. Adjustments to the [Dockerfile](./docker/Dockerfile) were also made. These were mostly borrowed from #61. I was required to do this due to updates made to the `aiohttp` package which require `gcc` to be installed on the host system. I figured if I was doing that, I may as well pull in the other optimizations alongside it.
7. Three new command-line configuration flags were added to enable/disable the database and set its URI.
8. `Command` objects received a new flag which will tell if they require use of the database to function. Presently, none of them do; however, I wanted to add this flag so that our commands could be modular. If the database were having issues, I would prefer not to have to shut down memebot completely while the problems are resolved. This flags allows database-dependent commands to not be registered if the databse is not available. 

Resolves #13.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
low-priority Low priority issues for the back burner
Projects
Status: To do
Development

No branches or pull requests

1 participant