Feature: Add global configuration editable by admin for schools/entreprises

PadawanNico21 commented 8 months ago

Yes, this is a big PR...

Why I Implemented this feature?

Because it has been requested by my school, and I think it can be a good idea to add my job to the project. But I don't know if it's in the philosophy of the project.

Features

Added Endpoints

GET /api/config Returns the current configuration.
PUT /api/config Sets the current configuration.
POST /api/login The first time this endpoint is called, it creates the user with the provided credentials. Then it will verify that the credentials are correct.

UI Changes

Add login dialog.
Add Update configuration button.
Add Logout button.

Shortcuts

Add F2 Button to open the login UI.

Hash URL

Add #admin hash URL to open the login UI.

Technical Choices

HTTP server: express
HTTP body parser: body-parser
Hashing algorithm: argon2id
Database: SQLite with the driver better-sqlite3
Session management: just a variable because I don't want to add any external databases.
Token generation: built-in crypto module from Node.js.

Important note

Everything that I implemented is only effective on the new build onlineServer.

Screenshots

Login Dialog

New Buttons in settings

victrme commented 8 months ago

Hi @PadawanNico21, I have some questions 😅

I'm really curious to know what your school requested. Was it "please contribute to open source" ? I understand if your school just want you to write stuff for the sake of it, but creating an issue to talk about it would have been much more productive !

In the future, please avoid creating massive PR like this to Open Source projects without asking first. PRs like this have a lot of changes that the maintainers have to review or at least consider, and it's work that could have been avoided on their end. I hope you can understand.

Bonjourr not saving user configs to a database is by design, and because of this reason alone I cannot accept your PR. Also the server would have been added to our already existing API and not use express and commonjs.

Having said all that, your PR is very detailed and easy to understand, and your code is readable enough for me to know what's going on just by looking at it !

PadawanNico21 commented 8 months ago

I'm really curious to know what your school requested.

My school wanted this feature for their use in this project.

In the future, please avoid creating massive PR like this to Open Source projects without asking first.

Initially, I implemented the change for my school. Then, I considered whether this feature would be interesting for the project, so I submitted a PR.

Having said all that, your PR is very detailed and easy to understand, and your code is readable enough for me to know what's going on just by looking at it !

Thank you!

victrme commented 8 months ago

Wow, your school is actually using Bonjourr 😮 I'm wondering: what are you using to deploy your API ?

Initially, I implemented the change for my school. Then, I considered whether this feature would be interesting for the project, so I submitted a PR.

Understandable!

PadawanNico21 commented 8 months ago

I'm wondering: what are you using to deploy your API ?

Currently, I don’t know. Maybe docker (on premise)

victrme commented 7 months ago

Right, Docker makes sense in this case. Good luck on your deploy !

victrme / Bonjourr