Skip to content

Update SSO guides to take into account new commands: tctl sso test, tctl sso configure.#12941

Closed
Tener wants to merge 42 commits into
masterfrom
tener/tctl-sso-docs
Closed

Update SSO guides to take into account new commands: tctl sso test, tctl sso configure.#12941
Tener wants to merge 42 commits into
masterfrom
tener/tctl-sso-docs

Conversation

@Tener
Copy link
Copy Markdown
Contributor

@Tener Tener commented May 26, 2022

Planned scope of this PR:

  • CLI reference for tctl sso test, tctl sso configure. Moved to PR: CLI ref for tctl sso commands. #13148
  • Update to Github SSO guide
  • Updates to Enterprise SSO page
  • Individual IdP guides
    • Okta guide
    • Azure Active Directory (AD) guide
    • Active Directory (ADFS) guide
    • Google Workspace guide
    • GitLab guide
    • OneLogin guide
    • OIDC guide

Parent: #9270

stevenGravy and others added 24 commits May 26, 2022 15:01
Co-authored-by: Paul Gottschling <paul.gottschling@goteleport.com>
Co-authored-by: Paul Gottschling <paul.gottschling@goteleport.com>
Co-authored-by: Paul Gottschling <paul.gottschling@goteleport.com>
Co-authored-by: Paul Gottschling <paul.gottschling@goteleport.com>
Co-authored-by: Paul Gottschling <paul.gottschling@goteleport.com>
Co-authored-by: Paul Gottschling <paul.gottschling@goteleport.com>
Co-authored-by: Paul Gottschling <paul.gottschling@goteleport.com>
Co-authored-by: Paul Gottschling <paul.gottschling@goteleport.com>
Co-authored-by: Paul Gottschling <paul.gottschling@goteleport.com>
Co-authored-by: Paul Gottschling <paul.gottschling@goteleport.com>
Co-authored-by: Paul Gottschling <paul.gottschling@goteleport.com>
Co-authored-by: Paul Gottschling <paul.gottschling@goteleport.com>
Co-authored-by: Paul Gottschling <paul.gottschling@goteleport.com>
Edit the SSO introduction:

- Add examples of all connectors so users don't need to find them
  in the examples directory
- Add subheadings to the "Configuring SSO" section for clarity
- Minor grammar/style edits

Specific SSO guides:

- Use the commercial-prereqs-tabs partial in Prerequisites sections,
  since this partial already exists, rather than ent-user-prereqs.mdx.
  This partial is more specific about requiring a running Teleport
  cluster, and links to the Getting Started guide for the Enterprise
  edition.

- Minor tweaks to the scoped information shown in the
  samlauthentication.mdx and oidcauthentication.mdx partials

- Restored the original wording of edition-prereqs-tabs.mdx, which
  is more specific about how to get started with a running Teleport
  cluster on different editions, and includes instructions for tsh.
Copy link
Copy Markdown
Contributor

@ptgott ptgott left a comment

Choose a reason for hiding this comment

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

I've left a partial review. I need to continue reviewing at the top of the new "Step 3/5. Test the connector" section.

Comment thread docs/pages/setup/admin/github-sso.mdx Outdated
Comment thread docs/pages/setup/admin/github-sso.mdx Outdated
Comment thread docs/pages/setup/admin/github-sso.mdx Outdated
Comment thread docs/pages/setup/admin/github-sso.mdx Outdated

The values of `client_id`, `client_secret`, and `redirect_url` come from the
GitHub OAuth App you created earlier.
The values of `client_id`, `client_secret` come from the GitHub OAuth App you created earlier. `redirect_url` is automatically configured by [`tctl sso configure github`](../reference/cli.mdx#tctl-sso-configure-github) based on the settings of Teleport cluster.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
The values of `client_id`, `client_secret` come from the GitHub OAuth App you created earlier. `redirect_url` is automatically configured by [`tctl sso configure github`](../reference/cli.mdx#tctl-sso-configure-github) based on the settings of Teleport cluster.
The values of `client_id` and `client_secret` come from the GitHub OAuth App you created earlier. `redirect_url` was automatically configured based on the settings of your Teleport cluster when you ran `tctl sso configure github`.

To connect the tctl command to the earlier step in this section. Also, I think we can remove the inline link to avoid breaking the reader's focus (https://goteleport.com/docs/contributing/documentation/style-guide/#expectations-1).

Copy link
Copy Markdown
Contributor Author

@Tener Tener Jun 2, 2022

Choose a reason for hiding this comment

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

@ptgott

Thank you for the improved phrasing.

I'm not sure if including the link constitutes the breaking of reader focus: we are not forcing them to follow the link, but rather making an option to do so if they are interested. This is similar to inline-link style in Wikipedia, for example:

== Architecture ==

Teleport is written in Go programming language, and runs on UNIX-compatible operating systems, including Linux, macOS, and several BSD/OS variants. Teleport consists of two executables: tsh (command line client) and teleport (server daemon).

Let me know what you think, though.

Comment thread docs/pages/setup/admin/github-sso.mdx Outdated
Comment thread docs/pages/setup/admin/github-sso.mdx Outdated
@Tener
Copy link
Copy Markdown
Contributor Author

Tener commented Jun 1, 2022

@ptgott thank you for the review. I discovered there is another SSO docs PR in flight, which is about to land soon: #12407. I'll need to merge my changes with those before they are review-worthy...

@Tener Tener force-pushed the tener/tctl-sso-docs branch from f410f8b to f295584 Compare June 2, 2022 12:10
@Tener
Copy link
Copy Markdown
Contributor Author

Tener commented Jun 2, 2022

@ptgott I have rebased this PR on top of another, so this is tagged as "Outdated" #12941 (comment), but this is definitely still relevant. Hoping to hear your thoughts on this.

@Tener Tener changed the title Update docs for tctl sso test, tctl sso configure. Update SSO guides to take into account new commands: tctl sso test, tctl sso configure. Jun 3, 2022
@ptgott
Copy link
Copy Markdown
Contributor

ptgott commented Jun 3, 2022

@Tener Thanks for working on this! Let me know when all guides have been added, and I'll carve out time to give this PR a complete review.

@ptgott
Copy link
Copy Markdown
Contributor

ptgott commented Jun 10, 2022

@Tener Also, would it be feasible to split this into multiple PRs, either one per guide or one per ≈250-500 lines of changes? This will make it a lot easier for me to review. We can also can get completed guides merged while others are waiting to be finished.

@Tener
Copy link
Copy Markdown
Contributor Author

Tener commented Jun 13, 2022

@Tener Also, would it be feasible to split this into multiple PRs, either one per guide or one per ≈250-500 lines of changes? This will make it a lot easier for me to review. We can also can get completed guides merged while others are waiting to be finished.

No problem, I can split it. I'm waiting for #12407 to be merged before I continue the work on this one.

@Tener
Copy link
Copy Markdown
Contributor Author

Tener commented Jul 8, 2022

Closing in favor of more granular PRs.

@Tener Tener closed this Jul 8, 2022
@Tener Tener deleted the tener/tctl-sso-docs branch July 22, 2022 08:59
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