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 in documentation for Okta access list synchronization. #38355

Merged
merged 10 commits into from
Feb 29, 2024

Conversation

mdwn
Copy link
Contributor

@mdwn mdwn commented Feb 16, 2024

Documentation has been added for Okta access list synchronization.

Copy link

🤖 Vercel preview here: https://docs-97v8bdqan-goteleport.vercel.app/docs/ver/preview

@tcsc tcsc force-pushed the tcsc/okta-docs-update branch from 24d209c to f6a4dce Compare February 19, 2024 10:32
Base automatically changed from tcsc/okta-docs-update to master February 19, 2024 11:40
Documentation has been added for Okta access list synchronization.
@mdwn mdwn force-pushed the mike.wilson/okta-access-list-sync-docs branch from 46b43e1 to a913983 Compare February 21, 2024 18:58
@mdwn mdwn marked this pull request as ready for review February 21, 2024 18:58
Copy link

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

Copy link

🤖 Vercel preview here: https://docs-50sqt1080-goteleport.vercel.app/docs/ver/preview

docs/pages/application-access/okta/hosted-guide.mdx Outdated Show resolved Hide resolved
docs/pages/application-access/okta/hosted-guide.mdx Outdated Show resolved Hide resolved
docs/pages/application-access/okta/hosted-guide.mdx Outdated Show resolved Hide resolved
docs/pages/application-access/okta/hosted-guide.mdx Outdated Show resolved Hide resolved
docs/pages/application-access/okta/hosted-guide.mdx Outdated Show resolved Hide resolved
docs/pages/application-access/okta/sync-scim.mdx Outdated Show resolved Hide resolved
docs/pages/application-access/okta/sync-scim.mdx Outdated Show resolved Hide resolved
docs/pages/application-access/okta/sync-scim.mdx Outdated Show resolved Hide resolved
docs/pages/application-access/okta/sync-scim.mdx Outdated Show resolved Hide resolved
Comment on lines 165 to 167
When first synchronizing an Access List, the owners of each Access List will be assigned default owners
that are configured when setting up the Okta integration, and the initial review date will be
set 6 months from the current date. These fields are modifiable, as well as the owner and
Copy link
Contributor

Choose a reason for hiding this comment

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

i'm not sure i undrestood the owners of each Access List will be assigned default owners that are configured when setting up the Okta integration, did you mean like default grants?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll reword this, but basically on initial import, the "owners" of the access list are the configured defaults.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just reworded this. Let me know what you think.

Copy link

🤖 Vercel preview here: https://docs-6promfpwl-goteleport.vercel.app/docs/ver/preview

Copy link

🤖 Vercel preview here: https://docs-1x17z16ly-goteleport.vercel.app/docs/ver/preview

@zmb3
Copy link
Collaborator

zmb3 commented Feb 22, 2024

I wouldn't put this in the change log since docs go live immediately after merging the PR, they aren't "shipped" with the release.

docs/img/enterprise/plugins/okta/okta-access-list.png Outdated Show resolved Hide resolved
docs/pages/application-access/okta/hosted-guide.mdx Outdated Show resolved Hide resolved
docs/pages/application-access/okta/hosted-guide.mdx Outdated Show resolved Hide resolved
![Import Okta Application Assignments](../../../img/enterprise/plugins/okta/okta-access-list-import-applications.png)

In addition to importing Okta User Groups, you can also import direct application
assignments within Okta as Access Lists as well. This behaves in exactly the same way
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here you did the reverse of above. Access Lists is now capitalized but application assignments is not.

Copy link
Contributor

Choose a reason for hiding this comment

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

@zmb3 should all mention of access lists be capitalized to Access Lists? I think i remember all product name should be capitalized, but I wasn't sure (we seem to cap access lists most of the time in this PR)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The intent was to capitalize all mentions of access list, so I'll go over this with a fine toothed comb and make sure they're all capitalized properly.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is that correct though? Access list is not a product name. It's just a noun like user or role, isn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IIRC the prior art is to use Access List instead of access list.

docs/pages/application-access/okta/hosted-guide.mdx Outdated Show resolved Hide resolved

<Admonition type="warning">
If the hosted integration is still active, removing Okta sourced Access Lists
could revoke Okta access from users in your organization. Please exercise caution
Copy link
Collaborator

Choose a reason for hiding this comment

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

Feels like the UI should help prevent this. Does it give you a warning?

Copy link
Contributor

@kimlisa kimlisa Feb 23, 2024

Choose a reason for hiding this comment

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

no warning, but that's good feedback, i'll add a confirm delete dialog when i add okta badge on the access list listing

docs/pages/application-access/okta/hosted-guide.mdx Outdated Show resolved Hide resolved
docs/pages/application-access/okta/hosted-guide.mdx Outdated Show resolved Hide resolved
![Import Okta Application Assignments](../../../img/enterprise/plugins/okta/okta-access-list-import-applications.png)

In addition to importing Okta User Groups, you can also import direct application
assignments within Okta as Access Lists as well. This behaves in exactly the same way
Copy link
Contributor

Choose a reason for hiding this comment

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

@zmb3 should all mention of access lists be capitalized to Access Lists? I think i remember all product name should be capitalized, but I wasn't sure (we seem to cap access lists most of the time in this PR)


<Admonition type="warning">
If the hosted integration is still active, removing Okta sourced Access Lists
could revoke Okta access from users in your organization. Please exercise caution
Copy link
Contributor

@kimlisa kimlisa Feb 23, 2024

Choose a reason for hiding this comment

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

no warning, but that's good feedback, i'll add a confirm delete dialog when i add okta badge on the access list listing

docs/pages/application-access/okta/hosted-guide.mdx Outdated Show resolved Hide resolved
@mdwn mdwn added the no-changelog Indicates that a PR does not require a changelog entry label Feb 23, 2024
@mdwn
Copy link
Contributor Author

mdwn commented Feb 28, 2024

Ping @zmb3 @kimlisa @r0mant @ptgott, I'd appreciate another look at this.

docs/pages/application-access/okta/sync-scim.mdx Outdated Show resolved Hide resolved
The owners of an Okta synchronized Access List will be preserved between runs.

<Admonition type="warning">
Removing members from an Okta synchronzied Access List will remove the user from the Okta group
Copy link
Contributor

Choose a reason for hiding this comment

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

this also sounds like it would be good to add a confirmation dialog when deleting members from okta lists right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hrm, I'm not sure. I get it on one hand, but on the other it could be frustrating if you had to work with a lot of users.

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 love the "Regex supported" text on the search bar.

  • I'm not sure I've seen similar language in other products
  • "Regex" is a very developer focused term
  • It's not consistent with other search bars in the app
  • We support glob and regex syntax, but the UI only mentions regex

Was this reviewed by design?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it was, at least to my knowledge (cc @kimlisa).

Copy link
Contributor

Choose a reason for hiding this comment

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

Was this reviewed by design?

yes, this was the original design, and deviated through iteration.

We support glob and regex syntax, but the UI only mentions regex

that's an oversight, i've created an issue to track list of small improvments

docs/pages/application-access/okta/hosted-guide.mdx Outdated Show resolved Hide resolved
docs/pages/application-access/okta/hosted-guide.mdx Outdated Show resolved Hide resolved
docs/pages/application-access/okta/sync-scim.mdx Outdated Show resolved Hide resolved
docs/pages/application-access/okta/sync-scim.mdx Outdated Show resolved Hide resolved
docs/pages/application-access/okta/sync-scim.mdx Outdated Show resolved Hide resolved
docs/pages/application-access/okta/sync-scim.mdx Outdated Show resolved Hide resolved
docs/pages/application-access/okta/sync-scim.mdx Outdated Show resolved Hide resolved
docs/pages/includes/config-reference/okta-service.yaml Outdated Show resolved Hide resolved
Co-authored-by: Zac Bergquist <[email protected]>
Co-authored-by: Lisa Kim <[email protected]>
Copy link

🤖 Vercel preview here: https://docs-kewgbaa1d-goteleport.vercel.app/docs/ver/preview

as user groups.

<Admonition type="note">
Only Okta Applications with assignments will be imported as an Access List. If an Okta
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 this would prompt a user to ask a question "what happens if an application assignment is removed, will it remove access list?"

docs/pages/includes/config-reference/okta-service.yaml Outdated Show resolved Hide resolved
@public-teleport-github-review-bot public-teleport-github-review-bot bot removed the request for review from xinding33 February 29, 2024 18:42
Copy link

🤖 Vercel preview here: https://docs-q6lz88e0w-goteleport.vercel.app/docs/ver/preview

Copy link

🤖 Vercel preview here: https://docs-r0uk53mih-goteleport.vercel.app/docs/ver/preview

Copy link

🤖 Vercel preview here: https://docs-q7hkhccbw-goteleport.vercel.app/docs/ver/preview

@mdwn mdwn added this pull request to the merge queue Feb 29, 2024
Copy link

🤖 Vercel preview here: https://docs-eys1gbyum-goteleport.vercel.app/docs/ver/preview

Merged via the queue into master with commit 20cfeb2 Feb 29, 2024
35 checks passed
@mdwn mdwn deleted the mike.wilson/okta-access-list-sync-docs branch February 29, 2024 20:16
@public-teleport-github-review-bot

@mdwn See the table below for backport results.

Branch Result
branch/v15 Create PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/branch/v15 documentation no-changelog Indicates that a PR does not require a changelog entry size/md
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants