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

Est doc update #4956

Merged
merged 4 commits into from
Feb 10, 2025
Merged

Est doc update #4956

merged 4 commits into from
Feb 10, 2025

Conversation

fmarco76
Copy link
Member

@fmarco76 fmarco76 commented Feb 7, 2025

The EST doc has been modified so the admin section include only information to add users in the DB while the installation and configuration is in the installation section.

The CI has been modified to reflect the documentation so the installation is before EST install while the user are added after.

Additionally, realm configurations have been splitted in different
folders.
@fmarco76 fmarco76 requested review from edewata and ladycfu February 7, 2025 14:45
Copy link
Contributor

@edewata edewata left a comment

Choose a reason for hiding this comment

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

I have some comments & questions.

Is estauthz actually a default script or just an example? It's installed under /usr/share/pki/est/bin so it looks like a default script and most people probably won't change that. Since the script uses estclient group people probably will think that it is the official group name for EST and cannot be changed (so the group description shouldn't include a "test").

If it's an example it probably should be stored under /usr/share/pki/est/examples or something like that and we should describe how to install it in the doc.

If we want to keep the script as a default script under bin folder we probably should provide a way to customize the group name using a config file without having to create a new script (this can be done separately later).

Comment on lines 243 to 248
dn: cn=estclient,ou=groups,dc=est,dc=pki,dc=example,dc=com
objectClass: top
objectClass: groupOfUniqueNames
cn: estclient
uniqueMember: uid=est-test-user,ou=People,dc=est,dc=pki,dc=example,dc=com
EOF
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the estclient group should be added during installation right after importing create.ldif since that group is specified in the default estauthz script, so after installation we just need to add the uniqueMember attribute for each new user.

DIGEST=$(docker exec pki tomcat-digest Secret.123 | sed 's/.*://')

docker exec postgresql psql -U est -t -A -c "INSERT INTO users VALUES ('est-test-user', 'EST TEST USER', '$DIGEST');" est
docker exec postgresql psql -U est -t -A -c "INSERT INTO groups VALUES ('estclient', 'EST TEST USERS');" est
Copy link
Contributor

Choose a reason for hiding this comment

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

Same thing, the estclient group should be added during installation too.

The group description probably should be changed into something like EST Users or EST Clients since it's part of the installation (i.e. not a test group) and is unlikely to change later, but the est-test-user can remain a test user.

Comment on lines 279 to 284
dn: cn=estclient,ou=groups,dc=est,dc=pki,dc=example,dc=com
objectClass: top
objectClass: groupOfUniqueNames
cn: estclient
uniqueMember: uid=est-test-user,ou=People,dc=est,dc=pki,dc=example,dc=com
EOF
Copy link
Contributor

Choose a reason for hiding this comment

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

Same thing here, estclient group should be added during installation.

----
$ psql -U est -t -A -f /tmp/create.sql est
----
Then fill the tables with the commands:
----
$ psql -U est -t -A -c "INSERT INTO users VALUES ('est-test-user', 'EST TEST USER', '<tomcat_digest>');" est
$ psql -U est -t -A -c "INSERT INTO groups VALUES ('estclient', 'EST TEST USERS');" est
Copy link
Contributor

Choose a reason for hiding this comment

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

Same thing here, estclient should be added during installation.

Comment on lines 17 to 21
----
ldapadd -x -H ldap://<ds_server_hostname>:<ds_server_port> \
-D "cn=Directory Manager" -w Secret.123 \
-f /usr/share/pki/est/conf/realm/ds/create.ldif
----
Copy link
Contributor

Choose a reason for hiding this comment

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

Similarly, we should describe how to add the estclient group to match the estauthz script.

`/usr/share/pki/est/conf/realm/postgresql/create.sql` and then filled
with the user information. The tables can be created with the command:
----
$ psql -U est -t -A -f /tmp/create.sql est
Copy link
Contributor

Choose a reason for hiding this comment

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

Similarly, we should describe how to add the estclient group to match the estauthz script.

The /tmp/ prefix probably can be dropped since we here we don't describe how to copy the file there.

@fmarco76
Copy link
Member Author

fmarco76 commented Feb 7, 2025

Is estauthz actually a default script or just an example?

Initially it starts as an example but it is more a default so I would consider it as default and modify the other item with this consideration. In future we could have a better approach to avoid forcing group name.

Copy link
Contributor

@edewata edewata left a comment

Choose a reason for hiding this comment

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

Thanks for the update! Feel free to merge.

One more thing, since the estclient group is now the default group should we use a more descriptive/generic name like EST Clients or EST Users since it can be used by other EST clients? This is similar to Administrators or Certificate Manager Agents group in CA. We can update this separately later.

Additionally, update docs reference.
@fmarco76
Copy link
Member Author

One more thing, since the estclient group is now the default group should we use a more descriptive/generic name like EST Clients or EST Users since it can be used by other EST clients?

I have update the default group to EST Users in both the documentation and the CI tests. Thanks!

@fmarco76
Copy link
Member Author

@edewata Thanks!

@fmarco76 fmarco76 merged commit 25a7e7a into dogtagpki:master Feb 10, 2025
166 of 174 checks passed
@fmarco76 fmarco76 deleted the ESTDocUpdate branch February 10, 2025 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants