bnhf / openvpn-admin-plus

Docker-based web interface (with golang backend) for monitoring and admin of an OpenVPN TAP/TUN server setup with PiVPN or other OpenVPN server installations. This project has been renamed from pivpn-tap-web-ui, to reflect its new broader scope.
MIT License
141 stars 23 forks source link

Common name check when creating a new certificate #29

Closed karabelnikov closed 1 year ago

karabelnikov commented 1 year ago

Scott, after you added the banner "Certificate for the name "name" created", then this banner comes out even if the user has not been created in the case when such a user already exists!

Снимок экрана 2022-12-12 201715

Let me explain. We need a check to be made when creating a new user certificate with the same certificate name that already exists as either valid or invalid. The administrator forgot that a certificate for example with the name "director" already exists in our database, whether it is valid or not (revoked or expired). The page should give him an error with a red background that "Error! There is already a valid or invalid certificate for the name "director""

Снимок экрана 2022-12-12 201510

Thus, it will be clear that the certificate no not created due to the error of an already existing certificate with the same name. You must either revoke the current certificate and delete it, or delete the expired certificate.

bnhf commented 1 year ago

@karabelnikov

OK, I see where the problem is -- I think I'll work on this issue first, since it's a bug. There is a validation check, for existing names, so let me know if you think that's not working. Based on the code I'm looking at, we need to add an red alert banner when a duplicate name has been used -- as you're suggesting. And, I need to move the green success banner after the name validation check.

bnhf commented 1 year ago

@karabelnikov

Though the solution ended up being relatively simple, this was a challenging issue to deal with. I learned a few things about error handling in Go along the way. The approach I used does not allow for a variable like "name" to be added to the message, but I still think it will do the job nicely. New multi-arch beta building now, that incorporates your recent PRs too -- it'll be uploaded and available in 20 minutes or so. Let me know how you like it.

karabelnikov commented 1 year ago

Scott, maybe it's better to display the result of the banner like this, for example:

Success! Certificate for the name of the "director" has been created

If we indicate an error in one of the banners in order to attract the attention of the administrator, then in the other we must be label success. What do you say?

bnhf commented 1 year ago

@karabelnikov

OK, I believe we have fully addressed this issue. I added Success! to all of the banners that needed it. Also, I was able to figure out how to add "name" to the Error banner, so they are all consistent. I'm not going to build a new beta right away, I'll wait until there are few more changes.