-
Notifications
You must be signed in to change notification settings - Fork 544
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
Update mongodb.json #1
Conversation
--Update the regex validation for MongoDb version so valid numeric version (semver) is only acceptable (3.10 is ok but 3. is not) --Update the regex for password so it can accept also special character, and make it stricter for password --Update the password description to state more information about what password should it be
"description": "Only use alphanumeric chars.", | ||
"validRegex": "/^([a-zA-Z0-9])+$/" | ||
"description": "Must contain at least 1 lowercase alphabetical character, at least 1 uppercase alphabetical character, at least 1 numeric character, contain at least one special character,the string must be eight characters or longer", | ||
"validRegex": "/^(?=.*[a-z])(?=.*[A-Z])(?=.*[0-9])(?=.*[!@#\$%\^&\*])(?=.{8,})" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two problems with this:
-
This too rigid. You shouldn't dictate how the password should look like. Users might be free to choose any password. It was a warning, it would have been okay, but since this is forcing the users, it's not good.
-
You need to make sure that there is no backslash
\
or double quote"
present in the password. This JSON will be stringified and values will be replaced. Presence of these characters breaks the JSON. That's why I left it as simple as alphanumeric chars. Previously this was possible as one-click-apps in CaptainDuckDuck were completely written in javascript. Now they are simplified as a simple JSON. This will increase maintainability, but it has drawbacks like customized post processing of entered values.
Keep in mind that this is just an initialization password anyways. You can change it to a more complex one at any point by connecting to db.
@@ -28,7 +28,7 @@ | |||
"label": "MongoDB Version", | |||
"defaultValue": "4", | |||
"description": "Checkout their docker page for the valid tags https://hub.docker.com/r/library/mongo/tags/", | |||
"validRegex": "/^([a-zA-Z0-9])+$/" | |||
"validRegex": "/^(\d+\.)?(\d+\.)?(\*|\d+)$" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good change. But again, this is too restrictive. It assumes a very specific format for docker tags. For example, it doesn't match 3.6.9-stretch
which is currently a valid tag. In general, it should not be restrictive. There is no guideline in Docker that prevents a tag from being 3.0.
. This regex does not need to prevent that. It simply passes the inline validation and the deploy will fail. It's not end of the world for a typo. Better that blocking the users.
Also, missing the slash in the end.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But the problem is the regexp, it does not allow me to input dots and dash.
My application is a machine that prevents the alien invasion that needs mongodb 3.4.10, but caprover does not allow me create that specific version because of regex issue. Here is the screenshoot,
If this is not fix immediately i guess your end of the world for typo is inevitable. :)
R.e. "slash at the end" -- To be honest, i was not able to test the regex
from actual caprover environment (dont know how to do it, sorry), i just
test the expression from online tools so i thought it should work too.
Ok, i should change the regexp to accept special chars like dots, dash,
because 3.4.10 or 3-stretch are currently not working for tags
My itention about password being strict is so it should represent it's
purpose as a password. UI should help the human, and if the user can use
"a" as a password, then why bother putting textbox there, why not just
pre-create the password for it. I know it can be replace on db anytime,
then that is my point, why create now and intentionally change afterwards.
BTW long live caprover, i love the new touch.
…On Sun, Jan 20, 2019, 3:27 PM Kasra Bigdeli ***@***.*** wrote:
***@***.**** commented on this pull request.
------------------------------
In public/v1/apps/mongodb.json
<#1 (comment)>:
> @@ -28,7 +28,7 @@
"label": "MongoDB Version",
"defaultValue": "4",
"description": "Checkout their docker page for the valid tags https://hub.docker.com/r/library/mongo/tags/",
- "validRegex": "/^([a-zA-Z0-9])+$/"
+ "validRegex": "/^(\d+\.)?(\d+\.)?(\*|\d+)$"
Good change. But again, this is too restrictive. It assumes a very
specific format for docker tags. For example, it doesn't match
3.6.9-stretch which is currently a valid tag. In general, it should not
be restrictive. There is no guideline in Docker that prevents a tag from
being 3.0.. This regex does not need to prevent that. It simply passes
the inline validation and the deploy will fail. It's not end of the world
for a typo. Better that blocking the users.
Also, missing the slash in the end.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAK0cXtBZ7wNG31aqx6rCX6Z44sjC-Y9ks5vFBpTgaJpZM4aJfOW>
.
|
I merged this to fix the mongodb version issue as it's a bug. But the password will be reverted. |
Just so you know, you don't need to update your CapRover to have the MongoDB fix. CapRover automatically fetches the one click apps from this repo. |
--Update the regex validation for MongoDb version so valid numeric version (semver) is only acceptable (3.10 is ok but 3. is not)
--Update the regex for password so it can accept also special character, and make it stricter for password
--Update the password description to state more information about what password should it be