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

Add Certificate List Function V2 #669

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

psgoundar
Copy link

Added a Certificate List function with requested Changed.
Function will List issues Certificates with CN and Validity Dates.

@randshell
Copy link
Contributor

Fixes #563

Copy link
Contributor

@randshell randshell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some issues with the PR. Other than what I wrote in the comments, can you remove the openvpn-install submodule?

If you can, merge the commits with the changes in one after.

Thanks for contributing!

@@ -1309,9 +1338,12 @@ function manageMenu() {
revokeClient
;;
3)
removeOpenVPN
listcerts
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use the same casing as the other functions and remove the added space

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed Casing

echo " 4) Exit"
until [[ $MENU_OPTION =~ ^[1-4]$ ]]; do
read -rp "Select an option [1-4]: " MENU_OPTION
echo " 3) List Current Issued Certificates"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use the same casing as the other options

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed Casing

Comment on lines 1305 to 1312
if [ "${STATUS}" == "V" ]; then
printf " Valid :: %s :: %s\\n" "$NAME" "$EXPD"
elif [ "${STATUS}" == "R" ]; then
#printf " Revoked :: %s :: %s\\n" "$NAME" "$EXPD"
continue
else
printf " Unknown :: %s :: %s\\n" "$NAME" "$EXPD"
fi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use \t instead of multiple spaces?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to Tabs

@randshell randshell mentioned this pull request Jun 29, 2020
Copy link
Author

@psgoundar psgoundar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes Complete

Comment on lines 1305 to 1312
if [ "${STATUS}" == "V" ]; then
printf " Valid :: %s :: %s\\n" "$NAME" "$EXPD"
elif [ "${STATUS}" == "R" ]; then
#printf " Revoked :: %s :: %s\\n" "$NAME" "$EXPD"
continue
else
printf " Unknown :: %s :: %s\\n" "$NAME" "$EXPD"
fi
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to Tabs

echo " 4) Exit"
until [[ $MENU_OPTION =~ ^[1-4]$ ]]; do
read -rp "Select an option [1-4]: " MENU_OPTION
echo " 3) List Current Issued Certificates"
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed Casing

@@ -1309,9 +1338,12 @@ function manageMenu() {
revokeClient
;;
3)
removeOpenVPN
listcerts
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed Casing

openvpn-install.sh Outdated Show resolved Hide resolved
openvpn-install.sh Outdated Show resolved Hide resolved
while read -r line || [ -n "$line" ]; do
STATUS=$(echo "$line" | awk '{print $1}')
NAME=$(echo "$line" | awk '{print $5}' | awk -FCN= '{print $2}')
EXPD=$(echo "$line" | awk '{if (length($2) == 15) print $2; else print "20"$2}' | cut -b 1-8 | date +"%b %d %Y" -f -)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The expiration date is wrong for revoked clients, we should use the column provided instead of calculating it.

psgoundar and others added 2 commits July 4, 2020 21:09
Co-authored-by: randomshell <randshell@protonmail.com>
Co-authored-by: randomshell <randshell@protonmail.com>
Copy link
Author

@psgoundar psgoundar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I"m fine with the proposed changes. Thanks for doing that.

openvpn-install.sh Outdated Show resolved Hide resolved
Co-authored-by: randomshell <randshell@protonmail.com>
Copy link
Contributor

@randshell randshell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants