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

Update sso-overview.md to describe disabling password logins for admins #6100

Merged
merged 3 commits into from
Sep 24, 2024

Conversation

asmclean
Copy link
Contributor

@asmclean asmclean commented Sep 19, 2024

What are you changing in this pull request and why?

Add information about the the new checkbox to control whether or not administrators can login via password once SSO is configured.

Checklist

  • I have reviewed the Content style guide so my content adheres to these guidelines.
  • The topic I'm writing about is for specific dbt version(s) and I have versioned it according to the version a whole page and/or version a block of content guidelines.
  • I have added checklist item(s) to this list for anything anything that needs to happen before this PR is merged, such as "needs technical review" or "change base branch."

@asmclean asmclean requested a review from a team as a code owner September 19, 2024 17:40
Copy link

vercel bot commented Sep 19, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
docs-getdbt-com ✅ Ready (Inspect) Visit Preview Sep 24, 2024 11:53pm

@github-actions github-actions bot added the content Improvements or additions to content label Sep 19, 2024
@github-actions github-actions bot added the size: x-small This change will take under 3 hours to fix. label Sep 19, 2024
@nehahystad
Copy link

nehahystad commented Sep 23, 2024

One bit of feedback after taking a closer look at the docs is it might be helpful to add a screenshot of the new checkbox for enforcing SSO
Screenshot 2024-09-23 at 1 32 36 PM

And add to the security best practices section:

  • Once the SSO configuration has been tested, un-check the setting for allowing admins to login with password. This ensures SSO login is enforced for all users, regardless of role.

@asmclean
Copy link
Contributor Author

asmclean commented Sep 24, 2024

One bit of feedback after taking a closer look at the docs is it might be helpful to add a screenshot of the new checkbox for enforcing SSO

I can add this if we're sure want it, but we're not using it for any other SSO settings, and our docs instruct using them sparingly, so I'm a little hesitant. @dbt-labs/product-docs, what do you think?

And add to the security best practices section:

* Once the SSO configuration has been tested, un-check the setting for allowing admins to login with password. This ensures SSO login is enforced for all users, regardless of role.

@nehahystad, If we're considering this best practice, it contradicts our existing item in the list of "Identity Provider is down — Account admins will continue to be able to log in with a password which would allow them to work with your Identity Provider to troubleshoot the problem." Should that one be removed/replaced with your suggested line?

@nehahystad
Copy link

Sounds good on the screenshot — we can follow the best practice here!

Good call out on the security best practices. I think we should remove the one about Account admins being able to login and replace it with the recommendation to enforce SSO for all users. Unless you know of cases where the account admin being able to login really helped them troubleshoot an issue with SSO?

Copy link
Contributor

@matthewshaver matthewshaver left a comment

Choose a reason for hiding this comment

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

Thank you @asmclean I think a screenshot would be a good idea. We try not to go overboard on them, but we do have actionable feedback from clients that they would be helpful regarding setup instructions or account settings feature locations. I can take it up as a follow-up item and see where we can enhance the SSO overview page as a whole.

@matthewshaver matthewshaver enabled auto-merge (squash) September 24, 2024 23:48
@matthewshaver matthewshaver merged commit e2197cc into current Sep 24, 2024
10 checks passed
@matthewshaver matthewshaver deleted the asmclean-patch-1 branch September 24, 2024 23:53
@asmclean
Copy link
Contributor Author

Thank you @asmclean I think a screenshot would be a good idea. We try not to go overboard on them, but we do have actionable feedback from clients that they would be helpful regarding setup instructions or account settings feature locations. I can take it up as a follow-up item and see where we can enhance the SSO overview page as a whole.

Sounds good; thanks for the feedback, @matthewshaver. I did not yet make a change to the best practices section that @nehahystad and I were discussing, so hopefully that can be included in your follow-up as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content Improvements or additions to content size: x-small This change will take under 3 hours to fix.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants