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

draft SSO docs #8409

Merged
merged 6 commits into from
Nov 3, 2020
Merged

draft SSO docs #8409

merged 6 commits into from
Nov 3, 2020

Conversation

taroface
Copy link
Contributor

@taroface taroface commented Sep 15, 2020

Fixes #7812.

Note: Admin UI screenshot is pending new login page & SSO button design.

@taroface taroface requested a review from dhartunian September 15, 2020 21:08
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@taroface
Copy link
Contributor Author

@dhartunian Friendly ping for review!

@ericharmeling
Copy link
Contributor

@taroface

I don't think this closes #7984

Copy link
Contributor

@dhartunian dhartunian left a comment

Choose a reason for hiding this comment

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

Apologies for the immense delay.

Reviewed 3 of 3 files at r1.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @taroface)


v20.2/sso.md, line 20 at r1 (raw file):

- An external OAuth 2.0 identity provider
- A callback URL to redirect the user to the CockroachDB cluster

In theory the URL is "available" if the user can access the Admin UI since it's part of that server. It's a pre-requisite but wondering if it can be omitted.


v20.2/sso.md, line 30 at r1 (raw file):

1. The user successfully authenticates with the provider, completing the OAuth flow.
1. The user is redirected to the CockroachDB cluster via a callback URL.
1. CockroachDB matches the information passed in the callback query to a registered SQL user.

Before this step, there's an exchange of a "code" for an "id_token". Not visible to user but wondering if it's worth pointing out. The callback query does not actually contain any user data, just the code.


v20.2/sso.md, line 43 at r1 (raw file):

| `server.oidc_authentication.client_id` | '32789079457-g3hdfw8cbw85obi5cb525hsceaqf69unn.apps.googleusercontent.com'
| `server.oidc_authentication.client_secret` | '6Q_jmRMgrPNOc_mN91boe-9EP'
| `server.oidc_authentication.redirect_url` | 'https://localhost:8080/oidc/callback'

/oidc/v1/callback


v20.2/sso.md, line 51 at r1 (raw file):

- `enabled` is a Boolean that enables or disables SSO.
- `client_id` and `client_secret` are generated by the external identity provider.
- `redirect_url` specifies the callback URL that redirects the user to CockroachDB after a successful authentication. This can be the address of a node in the cluster or the address of a load balancer that routes traffic to the nodes. The path **must** be appended with `/oidc/callback`.

Technically the path can be anything if it's on their proxy, the key is that it routes to the /oidc/v1/callback URL on the CRDB nodes. Probably not worth complicating this section though.

Either way, it's now version prefixed with oidc/v1.


v20.2/sso.md, line 53 at r1 (raw file):

- `redirect_url` specifies the callback URL that redirects the user to CockroachDB after a successful authentication. This can be the address of a node in the cluster or the address of a load balancer that routes traffic to the nodes. The path **must** be appended with `/oidc/callback`.
- `provider_url` specifies the OAuth issuer identifier. Ensure that the URL does not have a terminating `/`. For more information, see the [OIDC specification](https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfig). Note that CockroachDB appends the required `/.well-known/openid-configuration` by default. You do not need to include it.
- `scopes` is a space-delimited list of the [OAuth scopes](https://openid.net/specs/openid-connect-core-1_0.html#ScopeClaims) being requested for an Access Token. The `openid` scope is added by default and should not be included.

The openid scope is now required to be included, there's a validation that will check for its presence. Default value is just "openid" now


v20.2/sso.md, line 71 at r1 (raw file):

	<img src="{{ 'images/v20.2/google-oidc-client.png' | relative_url }}" alt="Google OAuth 2.0 client details" style="border:1px solid #eee;max-width:100%" />

1. Add the URL for the Admin UI to the list of **Authorized Javascript origins**. On a local cluster, this will be `https://localhost:8080`. For details on how to determine this URL on a production cluster, see [Admin UI access](admin-ui-overview.html#admin-ui-access). 

I don't think this is necessary. Only the authorized redirect URIs are needed. I tested locally with removing the "localhost" from the JS origins and things still worked.


v20.2/sso.md, line 105 at r1 (raw file):

	{% include copy-clipboard.html %}
	~~~ sql
	> SET CLUSTER SETTING server.oidc_authentication.redirect_url = 'https://localhost:8080/oidc/callback';

oidc/v1/callback


v20.2/sso.md, line 114 at r1 (raw file):

	{% include copy-clipboard.html %}
	~~~ sql
	> SET CLUSTER SETTING server.oidc_authentication.scopes = 'email';

openid email

Copy link
Contributor Author

@taroface taroface left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @dhartunian)


v20.2/sso.md, line 20 at r1 (raw file):

Previously, dhartunian (David Hartunian) wrote…

In theory the URL is "available" if the user can access the Admin UI since it's part of that server. It's a pre-requisite but wondering if it can be omitted.

OK - I think it makes sense to remove it.


v20.2/sso.md, line 30 at r1 (raw file):

Previously, dhartunian (David Hartunian) wrote…

Before this step, there's an exchange of a "code" for an "id_token". Not visible to user but wondering if it's worth pointing out. The callback query does not actually contain any user data, just the code.

I revised these steps a bit. Can you let me know if they're now correct?


v20.2/sso.md, line 43 at r1 (raw file):

Previously, dhartunian (David Hartunian) wrote…

/oidc/v1/callback

Done.


v20.2/sso.md, line 51 at r1 (raw file):

Previously, dhartunian (David Hartunian) wrote…

Technically the path can be anything if it's on their proxy, the key is that it routes to the /oidc/v1/callback URL on the CRDB nodes. Probably not worth complicating this section though.

Either way, it's now version prefixed with oidc/v1.

Done.


v20.2/sso.md, line 53 at r1 (raw file):

Previously, dhartunian (David Hartunian) wrote…

The openid scope is now required to be included, there's a validation that will check for its presence. Default value is just "openid" now

Done.


v20.2/sso.md, line 71 at r1 (raw file):

Previously, dhartunian (David Hartunian) wrote…

I don't think this is necessary. Only the authorized redirect URIs are needed. I tested locally with removing the "localhost" from the JS origins and things still worked.

Done.


v20.2/sso.md, line 105 at r1 (raw file):

Previously, dhartunian (David Hartunian) wrote…

oidc/v1/callback

Done.


v20.2/sso.md, line 114 at r1 (raw file):

Previously, dhartunian (David Hartunian) wrote…

openid email

Done.

v20.2/sso.md Outdated
toc: true
---

Single sign-on (SSO) allows a user to access the Admin UI in a secure cluster via an external identity provider. When SSO is configured and enabled, the [Admin UI login page](admin-ui-overview.html#admin-ui-access) will display an OAuth login button in addition to the password access option.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we specify which user we are referring to? CockroachDB user, SQL user, or Google user? And is the same user being referred to throughout the doc?

| `server.oidc_authentication.claim_json_key` | 'email'
| `server.oidc_authentication.principal_regex` | '^([^@]+)@[^@]+$'

- `enabled` is a Boolean that enables or disables SSO.
Copy link
Contributor

Choose a reason for hiding this comment

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

We could add another column named "Description" and include these bullet points in the table.

Copy link
Contributor Author

@taroface taroface left a comment

Choose a reason for hiding this comment

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

@dhartunian TFTR, and no worries! I had two follow-up questions for you (one of them below, the other in your review comments).

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @Amruta-Ranade and @dhartunian)


v20.2/sso.md, line 7 at r3 (raw file):

Previously, Amruta-Ranade (Amruta Ranade) wrote…

Can we specify which user we are referring to? CockroachDB user, SQL user, or Google user? And is the same user being referred to throughout the doc?

I think this is just a CockroachDB user, since SSO can be used with any external identity provider. @dhartunian does that sound right?


v20.2/sso.md, line 49 at r3 (raw file):

Previously, Amruta-Ranade (Amruta Ranade) wrote…

We could add another column named "Description" and include these bullet points in the table.

I tried this, but it made the table way too big and flowed off the page :(

Copy link
Contributor

@dhartunian dhartunian left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 2 files at r2, 2 of 2 files at r5.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @Amruta-Ranade)


v20.2/sso.md, line 7 at r3 (raw file):

Previously, taroface (Ryan Kuo) wrote…

I think this is just a CockroachDB user, since SSO can be used with any external identity provider. @dhartunian does that sound right?

The SSO process logs you in as a specific SQL user but in this case they would not be using to actually execute any SQL so I think saying "CockroachDB User" makes sense.

@taroface taroface merged commit 8fa3db2 into master Nov 3, 2020
@taroface taroface deleted the admin-ui-sso branch November 3, 2020 20:56
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.

SSO in Admin UI
5 participants