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

DOCSP-24594-Compass-auth-examples #708

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

Conversation

ltran-mdb2
Copy link
Collaborator

@ltran-mdb2 ltran-mdb2 commented Jan 10, 2025

Copy link

netlify bot commented Jan 10, 2025

Deploy Preview for docs-compass ready!

Name Link
🔨 Latest commit 4ae08c2
🔍 Latest deploy log https://app.netlify.com/sites/docs-compass/deploys/6797f5c02b55690008c9be92
😎 Deploy Preview https://deploy-preview-708--docs-compass.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Collaborator

@jeff-allen-mongo jeff-allen-mongo left a comment

Choose a reason for hiding this comment

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

@ltran-mdb2 thanks for these updates. I think this is a good start and definitely in line with my initial suggestions. I left a few thoughts for your consideration. Feel free to reach out if you want to discuss anything.

Jeff

@@ -78,6 +78,14 @@ Procedure
read preference option.
- Any specified tag sets.

The following screenshot shows the :guilabel:`Advanced`
Copy link
Collaborator

Choose a reason for hiding this comment

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

I might consider moving these new examples under a dedicated "Example" heading after "Procedure". I feel like this might make them slightly easier to find, since they'd be clearly visible in the right-hand page ToC.

I was also thinking that another potential way to organize this content could be to create a new page under "Connect" called "Connection Examples" (or similar), and make these examples into distinct pages. As in the ToC would look something like:

| - Connect
| - | - Connection Examples
| - | - | - Connect to Atlas Deployment 
| - | - | - Connect to LDAP Deployment
| - | - | - etc

This would be more work content-wise, but I'm wondering if it's worth having these examples on dedicated pages so they can be more easily found by users searching for phrases like "compass connect to Atlas" or "compass connect with LDAP".

I think it could also be fine to keep the structure largely as we have it now with the examples added to the existing pages, and consider a broader restructure in a future effort. Curious what you think.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added an "Example" section for each page because I think it makes sense to keep the examples on the same pages as the procedures.

tab configured with the read preference of :guilabel:`Secondary
Preferred`.

.. figure:: /images/authentication/secondary-preferred.png
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm curious for product / engineering's take here, but I'm wondering if in the examples we should be using the SRV Connection format. My understanding is that the SRV strings have the advantage of having TLS enabled by default, which might promote more secure practices. Not 100% sure about this though.

Copy link
Collaborator Author

@ltran-mdb2 ltran-mdb2 Jan 14, 2025

Choose a reason for hiding this comment

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

I changed the examples to use the srv connection format - @betsybutton what do you think?

Choose a reason for hiding this comment

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

Yes, +srv connection strings have TLS enabled by default, and we encourage users to use their connection strings when possible. Thanks for leaving this comment, Jeff!

@@ -266,10 +266,19 @@ Procedure

- (Optional) AWS Session Token

The following screenshot shows the :guilabel:`Authentication`
Copy link
Collaborator

Choose a reason for hiding this comment

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

The example here feels slightly out of place to me. Since it immediately follows the AWS content, it seems like it should be showing an AWS example. Moving the examples to their own section (or page) as mentioned in my other comment might help.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done!

tab configured for username and password authentication with
SCRAM-SHA-256.

.. figure:: /images/authentication/authentication-configuration.png
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a bit of a nit, but I'm wondering if we should change the "Name" field in these example screenshots to be slightly more descriptive, rather than reusing the hostname and port. My concern is that users might think that their connection "Name" has to match the username and port, but it can be anything that helps identify the cluster.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done!


.. note::

If you use the Standard Connection String Format to
Copy link
Collaborator

Choose a reason for hiding this comment

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

[Nit]

I don't think "Standard Connection String Format" needs to be title case here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed!

.. step:: (Optional) For advanced connection configuration options, click the :ref:`Advanced <advanced-connection-tab>` tab.

.. step:: Click Connect.


Copy link
Collaborator

Choose a reason for hiding this comment

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

I think also including an LDAP example on this page would be helpful.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done!

@@ -78,6 +78,14 @@ Procedure
read preference option.
- Any specified tag sets.

The following screenshot shows the :guilabel:`Advanced`
tab configured with the read preference of :guilabel:`Secondary
Preferred`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

It might help to add a bit of context here explaining why a user might want to use "secondary preferred".

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done!

@ltran-mdb2
Copy link
Collaborator Author

Thanks for your feedback @jeff-allen-mongo ! Could you take another look when you get a chance?

Copy link
Collaborator

@jeff-allen-mongo jeff-allen-mongo left a comment

Choose a reason for hiding this comment

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

LGTM! Just have a couple optional suggestions. Thanks @ltran-mdb2

Example
-------

The following screenshot shows the :guilabel:`Advanced` tab configured
Copy link
Collaborator

Choose a reason for hiding this comment

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

[Suggestion]

Maybe to tighten up the wording slightly:

The following example specifies a connection with a read preference of :guilabel:Secondary Preferred.

We could apply a similar wording to the other example lead-ins.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done!

@@ -273,3 +273,22 @@ Procedure
.. seealso::

To disconnect from your deployment, see :ref:`<disconnect-tab>`.

Example
Copy link
Collaborator

@jeff-allen-mongo jeff-allen-mongo Jan 14, 2025

Choose a reason for hiding this comment

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

[Suggestion]

Maybe we can make this "Examples" and have subheadings for each example? As in:

Examples
------------

Username and Password Authentication
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

(example 1)

LDAP Authentication
~~~~~~~~~~~~~~~~~~~~

(example 2)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done!

</core/security-scram/>`. Since the connection uses the ``mongodb+srv``
connection string scheme, the connection automatically enables TLS/SSL.

.. figure:: /images/authentication/new-atlas-connection.png
Copy link
Collaborator

Choose a reason for hiding this comment

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

I noticed the other figures specified a figwidth of 662px. Should we add that here as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for catching this! Done!

:figwidth: 662px
:alt: Screenshot of Compass configured to authenticate with username and password

LDAP Authentication
Copy link
Collaborator

@jeff-allen-mongo jeff-allen-mongo Jan 15, 2025

Choose a reason for hiding this comment

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

Sorry, I realized after I suggested that we add this example that LDAP is actually deprecated. I wonder if we should remove this or replace with a different auth example? Happy to defer to product for their opinion here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@betsybutton Do you have thoughts here?

Choose a reason for hiding this comment

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

Even though LDAP is deprecated, this is an area that has caused a lot of pain in the past, so I think it's worth including here. Some larger customers take much longer to upgrade to the latest MongoDB version, and we might as well provide the assistance.

Copy link

@betsybutton betsybutton left a comment

Choose a reason for hiding this comment

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

Thanks so much for taking this work on, Lauren! So far, these examples cover the following types of connections:

  1. SCRAM-SHA-256
  2. Secondary Preferred
  3. Username & password
  4. LDAP Authentication
  5. TLS/SSL

What would you think of adding examples for the following, which are other areas users have struggled a lot with? If it would be better to handle some of these in a followup ticket, that's also fine.

  1. Private endpoints
  2. OIDC
  3. x509
  4. Analytics nodes

For private endpoint and OIDC, it might be helpful to link to other docs that could help users diagnose these types of problems. For example, https://www.mongodb.com/docs/atlas/troubleshoot-private-endpoints/#std-label-pl-troubleshooting and https://www.mongodb.com/docs/atlas/security-oidc/

:figwidth: 662px
:alt: Screenshot of Compass configured to authenticate with username and password

LDAP Authentication

Choose a reason for hiding this comment

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

Even though LDAP is deprecated, this is an area that has caused a lot of pain in the past, so I think it's worth including here. Some larger customers take much longer to upgrade to the latest MongoDB version, and we might as well provide the assistance.

@@ -273,3 +273,28 @@ Procedure
.. seealso::

To disconnect from your deployment, see :ref:`<disconnect-tab>`.

Examples

Choose a reason for hiding this comment

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

[q] Is there a reason to list these examples all the way at the bottom, instead of including them in the sections pertinent to each connection method above? And if it's truly better to have these at the bottom, can we link to them from above? I'm worried they won't be very visible down here since they're hidden & not linked.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added a note in the username/password and ldap sections linking to the according examples. The reason they are at the bottom is to make a dedicated ToC item on this page with the examples (see this comment from Jeff)

Choose a reason for hiding this comment

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

Got it, thanks for adding the link!

Comment on lines 122 to 127
The following example specifies a connection with TLS/SSL enabled in the
:guilabel:`TLS / SSL` tab.

.. figure:: /images/authentication/tls-ssl-configuration.png
:figwidth: 662px
:alt: Screenshot of Compass configured to connect to an example cluster with TLS/SSL

Choose a reason for hiding this comment

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

There isn't really a reason to explicitly enable TLS/SSL when using an srv connection string, since it's enabled by default, especially when tlsCertificateKeyFile isn't specified. Maybe we could turn this into an x509 example, where we need to supply the client certificate and key?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done!

@ltran-mdb2
Copy link
Collaborator Author

ltran-mdb2 commented Jan 24, 2025

Thanks for your feedback @betsybutton ! Could you take another look at this PR when you get a chance?

I also implemented some feedback from Felicia on the general connection example (changing it to use the mongodb scheme) as well as adding notes on when TLS/SSL is required.

I opened DOCSP-46801 to address the other examples you mentioned!

Copy link

@betsybutton betsybutton left a comment

Choose a reason for hiding this comment

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

1 nit pick, otherwise LGTM!

Comment on lines +128 to +131
The following example specifies a connection with TLS/SSL enabled in the
:guilabel:`TLS / SSL` tab. This connection uses :manual:`X.509
</core/security-x.509/>` authentication which requires a client
certificate.

Choose a reason for hiding this comment

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

The screenshot doesn't showcase selecting a .pem file right now - could we update it to mirror what a successful x509 setup would look like?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done!

Copy link

@betsybutton betsybutton left a comment

Choose a reason for hiding this comment

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

LGTM, thank you Lauren!

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.

3 participants