cockroach user command #838

sploiselle · 2016-11-09T16:48:52Z

Includes documentation for cockroach user, which creates password auth.

Supporting documentation also included.

Closes #870
Closes #741
Closes #474
Closes #336

This change is

asubiotto · 2016-11-09T18:18:01Z

Review status: 0 of 9 files reviewed at latest revision, 3 unresolved discussions.

create-users.md, line 57 at r1 (raw file):

`--insecure` | Set this only if the cluster is insecure and running on multiple machines. <br><br>If the cluster is insecure and local, leave this out. If the cluster is secure, leave this out and set the `--ca-cert`, `--cert`, and `--key` flags. <br><br>**Env Variable:** `COCKROACH_INSECURE`
`--key` | Path to the [client key](create-security-certificates.html) protecting the client certificate.  This flag is required if the user does not have a password. <br/><br/>**Env Variable:** `COCKROACH_KEY`
`--password` | The password for the user.<br/><br/>If not passed in on a secure cluster, CockroachDB will prompt you to enter and confirm the user's password.<br/><br/>If you want to [provide the password through standard input](#provide-password-via-standard-input), use '`-`' <br/>(i.e. `--password=-`).<br/><br/>[Find more detail about how CockroachDB handles passwords](#user-authentication).<br/><br/>**Env Variable:** `COCKROACH_PASSWORD`

It might make it clearer if we specify that this is the user being created. What do you think?

create-users.md, line 65 at r1 (raw file):

## User Authentication

On secure clusters, users must authenticate their access to the cluster's databases and tables. CockroachDB offers two types of authentication, which you can choose between on a per-user basis based on the password you enter through `cockroach user set`:

This is slightly incorrect. The authentication method for a client is not chosen based on a user's password. Rather, if a user has a blank password, they must use certificate authentication, but if a user does have a password they can choose to use either certificate or password authentication.

create-users.md, line 94 at r1 (raw file):

After issuing this command, you must enter a value for the password, which determines the [authentication methods available to the user](#user-authentication).

{{site.data.alerts.callout_success}}If you want allow password authentication for the user, you can simply include the <code>--password</code> flag to bypass entering it at the command prompt.{{site.data.alerts.end}}

Missing a "to" between "want" and "allow".

Comments from Reviewable

sploiselle · 2016-11-10T20:54:39Z

Review status: 0 of 15 files reviewed at latest revision, 3 unresolved discussions.

create-users.md, line 57 at r1 (raw file):

Previously, asubiotto (Alfonso Subiotto Marqués) wrote…

It might make it clearer if we specify that this is the user being created. What do you think?

Clarified.

create-users.md, line 65 at r1 (raw file):

Previously, asubiotto (Alfonso Subiotto Marqués) wrote…

This is slightly incorrect. The authentication method for a client is not chosen based on a user's password. Rather, if a user has a blank password, they must use certificate authentication, but if a user does have a password they can choose to use either certificate or password authentication.

Originally I'd tried to document the user experience instead of the code itself; let me know if this is better.

create-users.md, line 94 at r1 (raw file):

Previously, asubiotto (Alfonso Subiotto Marqués) wrote…

Missing a "to" between "want" and "allow".

Done.

Comments from Reviewable

bdarnell · 2016-11-11T15:03:08Z

Some belated comments here for @asubiotto; the docs brought something to my attention that hadn't been clear before.

Review status: 0 of 15 files reviewed at latest revision, 5 unresolved discussions.

create-users.md, line 57 at r2 (raw file):

`--insecure` | Set this only if the cluster is insecure and running on multiple machines. <br><br>If the cluster is insecure and local, leave this out. If the cluster is secure, leave this out and set the `--ca-cert`, `--cert`, and `--key` flags. <br><br>**Env Variable:** `COCKROACH_INSECURE`
`--key` | Path to the [client key](create-security-certificates.html) protecting the client certificate.  This flag is required if the user does not have a password. <br/><br/>**Env Variable:** `COCKROACH_KEY`
`--password` | The password for the user.<br/><br/>If not passed in on a secure cluster, CockroachDB will prompt you to enter and confirm the user's password.<br/><br/>If you want to [provide the password through standard input](#provide-password-via-standard-input), use '`-`' <br/>(i.e. `--password=-`).<br/><br/>[Find more detail about how CockroachDB handles passwords](#user-authentication).<br/><br/>**Env Variable:** `COCKROACH_PASSWORD`

I mentioned this in another PR, but I'd like to see us use a separate flag --password-file for the -, since - traditionally means stdin only in a filename context, and we don't want to have special password values with magical behavior.

create-users.md, line 94 at r2 (raw file):

After issuing this command, you must enter a value for the password, which determines the [authentication methods available to the user](#user-authentication).

{{site.data.alerts.callout_success}}If you want allow password authentication for the user, you can simply include the <code>--password</code> flag to bypass entering it at the command prompt.{{site.data.alerts.end}}

Passing --password without a value disallows password auth, right? I think this is backwards. We want to encourage cert auth; passwords are provided only for backwards compatibility with legacy deployments. It's weird that we prompt for the password by default, then. I think we should change it so that by default, the user is created with no password, and if you pass --password with no value you get the interactive prompt.

Comments from Reviewable

asubiotto · 2016-11-11T16:44:49Z

Review status: 0 of 15 files reviewed at latest revision, 5 unresolved discussions.

create-users.md, line 57 at r2 (raw file):

Previously, bdarnell (Ben Darnell) wrote…

I mentioned this in another PR, but I'd like to see us use a separate flag --password-file for the -, since - traditionally means stdin only in a filename context, and we don't want to have special password values with magical behavior.

I agree. For now I think switching to a boolean flag to enable/disable prompting and removing the "-" functionality is enough. I'll work on the file functionality as soon as I can.

create-users.md, line 94 at r2 (raw file):

Previously, bdarnell (Ben Darnell) wrote…

Passing --password without a value disallows password auth, right? I think this is backwards. We want to encourage cert auth; passwords are provided only for backwards compatibility with legacy deployments. It's weird that we prompt for the password by default, then. I think we should change it so that by default, the user is created with no password, and if you pass --password with no value you get the interactive prompt.

Right, that makes sense. The only thing is that passing password without a value and not passing anything seem to be equivalent unless I'm missing something. We could switch it to a boolean flag which would be kind of nice so we don't save the passed in password to the cli history.

Comments from Reviewable

asubiotto · 2016-11-18T20:49:29Z

Review status: 0 of 18 files reviewed at latest revision, 8 unresolved discussions.

create-and-manage-users.md, line 3 at r3 (raw file):

---
title: Create & Manage Users
summary: A secure CockroachDB cluster uses TLS for encrypted inter-node and client-node communication and requires CA, node, and client certificates and keys.

Update to include passwords?

create-and-manage-users.md, line 53 at r3 (raw file):

Flag | Description
-----|------------
`--ca-cert` | The path to the [CA certificate](create-security-certificates.html). This flag is required when creating a user for a secure cluster.<br><br>**Env Variable:** `COCKROACH_CA_CERT`

This flag is required to initiate any database connection with the secure cluster. Not sure how to phrase that in a user-friendly way but I think it's a bit confusing if we state this but omit that it's necessary for "ls", "get", and "rm".

create-and-manage-users.md, line 70 at r3 (raw file):

- [Client certificate and key authentication](#secure-clusters-with-client-certificates), which is available to all users. To ensure the highest level of security, we recommend only using client certificate and key authentication.
- [Password authentication authentication](#secure-clusters-with-passwords), which is available only to users with passwords. To create passwords, use the `WITH PASSWORD` clause of `CREATE USER`. <br/><br/>You can use this password to authenticate users without supplying their client certificate and key; however, we recommend instead using client certificate and key authentication whenever possible.

Accidental extra "authentication" in the title. Also maybe after "CREATE USER", do you think it would make sense to mention the password flag from the set user command as well.

Comments from Reviewable

sploiselle · 2016-11-21T16:21:00Z

Review status: 0 of 18 files reviewed at latest revision, 8 unresolved discussions.

create-and-manage-users.md, line 57 at r2 (raw file):

Previously, asubiotto (Alfonso Subiotto Marqués) wrote…

> I agree. For now I think switching to a boolean flag to enable/disable prompting and removing the "-" functionality is enough. I'll work on the file functionality as soon as I can.

Done.

create-and-manage-users.md, line 94 at r2 (raw file):

Previously, asubiotto (Alfonso Subiotto Marqués) wrote…

> Right, that makes sense. The only thing is that passing password without a value and not passing anything seem to be equivalent unless I'm missing something. We could switch it to a boolean flag which would be kind of nice so we don't save the passed in password to the cli history.

Done.

create-and-manage-users.md, line 3 at r3 (raw file):

Previously, asubiotto (Alfonso Subiotto Marqués) wrote…

> Update to include passwords?

Changed entirely to just mirror first sentence.

create-and-manage-users.md, line 53 at r3 (raw file):

Previously, asubiotto (Alfonso Subiotto Marqués) wrote…

> This flag is required to initiate any database connection with the secure cluster. Not sure how to phrase that in a user-friendly way but I think it's a bit confusing if we state this but omit that it's necessary for "ls", "get", and "rm".

Done.

create-and-manage-users.md, line 70 at r3 (raw file):

Previously, asubiotto (Alfonso Subiotto Marqués) wrote…

> Accidental extra "authentication" in the title. Also maybe after "CREATE USER", do you think it would make sense to mention the password flag from the set user command as well.

Fixed.

Comments from Reviewable

cockroach-teamcity · 2016-11-21T16:32:28Z

http://cockroach-docs-review.s3-website-us-east-1.amazonaws.com/4ddacd8897f5aedcb6634cf2e39bf85ce930b55b/

sploiselle · 2016-11-21T16:44:03Z

@jseldess Can I get a copy edit from you?

jseldess · 2016-11-21T19:59:21Z

Looks great. @sploiselle. Just a few questions/comments. Also, can you add the CREATE USER statement to sql-statements.md?

Review status: 0 of 18 files reviewed at latest revision, 12 unresolved discussions.

create-and-manage-users.md, line 22 at r4 (raw file):

Subcommand | Usage 
-----------|------
`get` | Retrieve a table containing a user and their hashed password

nit: Periods at the end of each usage statement.

create-and-manage-users.md, line 63 at r4 (raw file):

`--pretty` | Format tables using ASCII. When not specified, table rows are printed as tab-separated values (TSV). <br/><br/>**Default**: `true`
`--url` | Connect to the cluster on the provided URL, e.g., `postgresql://myuser@localhost:26257/mydb`. If left blank, the connection flags are used (`host`, `port`, `user`, `database`, `insecure`, `certs`). <br/><br/>**Env Variable:** `COCKROACH_URL`
`-u`, `--user` | The username you want to actively engage with.<br/><br/>**Env Variable:** `COCKROACH_USER` <br/>**Default**: `root`

I think this could be clarified a bit. Based on a brief conversation with Alfonso, a user other than root can be granted privileges to create users. Such a user would pass her username in this flag when creating users.

With this in mind, I think we should also state somewhere that running cockroach user set requires specific privileges.

create-user.md, line 20 at r4 (raw file):

## Required Privileges

Only the `root` user can create users.

Is this true? Based on a brief conversation with Alfonso, it sounded like a user could be granted privileges to create users for others...

grant.md, line 47 at r4 (raw file):

`table_name` | A comma-separated list of table names. Alternately, to grant privileges to all tables, use `*`. 
`database_name` | A comma-separated list of database names.<br><br>Privileges granted on databases will be inherited by any new tables created in the databases.
`user_name` | A comma-separated list of grantees.

Maybe link to the user docs here.

Comments from Reviewable

asubiotto · 2016-11-21T20:14:30Z

Review status: 0 of 18 files reviewed at latest revision, 14 unresolved discussions.

create-and-manage-users.md, line 49 at r3 (raw file):

## Flags

The `cert` command and subcommands support the following flags, as well as [logging flags](cockroach-commands.html#logging-flags).

s/cert/user?

create-and-manage-users.md, line 57 at r3 (raw file):

`-d`, `--database` | The name of the database to connect to. <br/><br/>**Env Variable:** `COCKROACH_DATABASE`
`--host` | Database server host to connect to.<br/><br/>**Env Variable:** `COCKROACH_HOST`
`--insecure` | Set this only if the cluster is insecure and running on multiple machines. <br><br>If the cluster is insecure and local, leave this out. If the cluster is secure, leave this out and set the `--ca-cert`, `--cert`, and `--key` flags. <br><br>**Env Variable:** `COCKROACH_INSECURE`

This should be "set the --ca-cert and --cert and --key, or --password, depending on the authentication method you want to use." Or something like that. Or maybe remove the flags you have to set if not specifying --insecure. I think it's outside the scope of the description.

create-user.md, line 20 at r4 (raw file):

Previously, jseldess wrote…

> Is this true? Based on a brief conversation with Alfonso, it sounded like a user could be granted privileges to create users for others...

It used to be true. I forgot to update sean on this.

Comments from Reviewable

sploiselle · 2016-11-21T22:06:22Z

@jseldess Added that (another good catch) and your other edits.

Review status: 0 of 19 files reviewed at latest revision, 14 unresolved discussions.

create-and-manage-users.md, line 49 at r3 (raw file):

Previously, asubiotto (Alfonso Subiotto Marqués) wrote…

> s/cert/user?

Done.

create-and-manage-users.md, line 57 at r3 (raw file):

Previously, asubiotto (Alfonso Subiotto Marqués) wrote…

> This should be "set the `--ca-cert` and `--cert` and `--key`, or `--password`, depending on the authentication method you want to use." Or something like that. Or maybe remove the flags you have to set if not specifying `--insecure`. I think it's outside the scope of the description.

The command requires those flags even if you are setting the `--password` flag. Edited other statements to reflect actual function.

create-and-manage-users.md, line 22 at r4 (raw file):

Previously, jseldess wrote…

> nit: Periods at the end of each usage statement.

Done.

create-and-manage-users.md, line 63 at r4 (raw file):

Previously, jseldess wrote…

> I think this could be clarified a bit. Based on a brief conversation with Alfonso, a user other than root can be granted privileges to create users. Such a user would pass her username in this flag when creating users. > > With this in mind, I think we should also state somewhere that running `cockroach user set` requires specific privileges.

Good catch on this, Jesse. This flag is actually unusable. Sent [an issue](https://github.com/cockroachdb/cockroach/issues/10921) to Alfonso.

create-user.md, line 20 at r4 (raw file):

Previously, asubiotto (Alfonso Subiotto Marqués) wrote…

> It used to be true. I forgot to update sean on this.

Just talked to Alfonso. Still true.

grant.md, line 47 at r4 (raw file):

Previously, jseldess wrote…

> Maybe link to the user docs here.

Great catch! Thank you!

Comments from Reviewable

sploiselle · 2016-11-21T22:11:21Z

Note to update the forum when this merges.

jseldess · 2016-11-21T22:15:32Z

, with one more question.

Reviewed 13 of 18 files at r3, 1 of 2 files at r4, 6 of 6 files at r5.
Review status: all files reviewed at latest revision, 12 unresolved discussions.

create-and-manage-users.md, line 55 at r5 (raw file):

`--ca-cert` | The path to the [CA certificate](create-security-certificates.html). This flag is required if the cluster is secure.<br><br>**Env Variable:** `COCKROACH_CA_CERT`
`--cert` | The path to the [client certificate](create-security-certificates.html) of the user *issuing the command* (not the user you're creating). This flag is required if the cluster is secure. <br><br>**Env Variable:** `COCKROACH_CERT`
`-d`, `--database` | The name of the database to connect to. <br/><br/>**Env Variable:** `COCKROACH_DATABASE`

Does this flag have any particular purpose for cockroach user? It shows up in cockroach zone as well, though it's not useful there either. In the docs there, we just say "Not currently implemented."

Comments from Reviewable

sploiselle · 2016-11-21T22:39:32Z

Review status: 18 of 19 files reviewed at latest revision, 12 unresolved discussions.

create-and-manage-users.md, line 55 at r5 (raw file):

Previously, jseldess wrote…

> Does this flag have any particular purpose for `cockroach user`? It shows up in `cockroach zone` as well, though it's not useful there either. In the docs there, we just say "Not currently implemented."

Got that. Thanks, Jesse.

Comments from Reviewable

sploiselle · 2016-11-22T18:32:17Z

Merge held until a beta is released with the feature as it's documented in this PR.

sploiselle · 2016-11-28T21:02:12Z

cockroachdb/cockroach#10921 was resolved somewhere in the source; users besides root can now create users.

asubiotto · 2016-12-01T15:19:05Z

Just update instances where we refer to who can create users.

Review status: 18 of 19 files reviewed at latest revision, 12 unresolved discussions.

Comments from Reviewable

cockroach-teamcity · 2016-12-01T15:31:57Z

http://cockroach-docs-review.s3-website-us-east-1.amazonaws.com/b84321dc60bbcef5bcb2a9cffd9fdfcec874cc60/

jseldess · 2016-12-01T15:33:24Z

@sploiselle, can you wrap this up and merge this morning, so these docs are in please for the release?

sploiselle added the C-product-change label Nov 9, 2016

sploiselle assigned asubiotto Nov 9, 2016

sploiselle force-pushed the password-auth branch 2 times, most recently from dc51094 to abe2c58 Compare November 10, 2016 20:43

sploiselle force-pushed the password-auth branch from abe2c58 to 4c18232 Compare November 11, 2016 16:49

asubiotto mentioned this pull request Nov 14, 2016

cli: switch password flag from string to bool cockroachdb/cockroach#10680

Merged

sploiselle force-pushed the password-auth branch 2 times, most recently from 3f8c7b2 to fe2c211 Compare November 17, 2016 16:47

sploiselle force-pushed the password-auth branch from fe2c211 to 4ddacd8 Compare November 21, 2016 16:31

sploiselle assigned jseldess and unassigned asubiotto Nov 21, 2016

sploiselle force-pushed the password-auth branch 2 times, most recently from c5cef78 to d00c428 Compare November 21, 2016 22:04

sploiselle force-pushed the password-auth branch from d00c428 to a325635 Compare November 21, 2016 22:38

cockroach user command & related updates

b84321d

sploiselle force-pushed the password-auth branch from a325635 to b84321d Compare December 1, 2016 15:30

sploiselle merged commit 09d28bb into gh-pages Dec 1, 2016

tamird deleted the password-auth branch February 3, 2017 19:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cockroach user command #838

cockroach user command #838

sploiselle commented Nov 9, 2016 •

edited

Loading

asubiotto commented Nov 9, 2016

sploiselle commented Nov 10, 2016

bdarnell commented Nov 11, 2016

asubiotto commented Nov 11, 2016

asubiotto commented Nov 18, 2016

sploiselle commented Nov 21, 2016

cockroach-teamcity commented Nov 21, 2016

sploiselle commented Nov 21, 2016

jseldess commented Nov 21, 2016

asubiotto commented Nov 21, 2016

sploiselle commented Nov 21, 2016

sploiselle commented Nov 21, 2016

jseldess commented Nov 21, 2016

sploiselle commented Nov 21, 2016

sploiselle commented Nov 22, 2016

sploiselle commented Nov 28, 2016 •

edited

Loading

asubiotto commented Dec 1, 2016

cockroach-teamcity commented Dec 1, 2016

jseldess commented Dec 1, 2016

cockroach user command #838

cockroach user command #838

Conversation

sploiselle commented Nov 9, 2016 • edited Loading

asubiotto commented Nov 9, 2016

sploiselle commented Nov 10, 2016

bdarnell commented Nov 11, 2016

asubiotto commented Nov 11, 2016

asubiotto commented Nov 18, 2016

sploiselle commented Nov 21, 2016

cockroach-teamcity commented Nov 21, 2016

sploiselle commented Nov 21, 2016

jseldess commented Nov 21, 2016

asubiotto commented Nov 21, 2016

sploiselle commented Nov 21, 2016

sploiselle commented Nov 21, 2016

jseldess commented Nov 21, 2016

sploiselle commented Nov 21, 2016

sploiselle commented Nov 22, 2016

sploiselle commented Nov 28, 2016 • edited Loading

asubiotto commented Dec 1, 2016

cockroach-teamcity commented Dec 1, 2016

jseldess commented Dec 1, 2016

sploiselle commented Nov 9, 2016 •

edited

Loading

sploiselle commented Nov 28, 2016 •

edited

Loading