ui: logging in with capitalized username prevent access to Databases and Statements pages

[sheaffej]

Edit 2021-03-01 (@knz) The issue description is misguided. See comment belows by @aaron-crl and myself.

---

Describe the problem

When logging into the Admin UI and specifying a valid user with the admin role with the username capitalized, the Admin UI mostly works OK except the Databases and Statements pages do not work properly. Logging in using the same user with the username lower-cased does provides proper access to these pages.

To Reproduce

1. Create a secure CRDB cluster

2. Create a new user and assign the admin role to the user. You can create this user in any combination of lower-, upper- or mixed-case. The user will be created in the database with a username that is lower-case.

   create user j4 password 'j4';
   grant admin to j4;

3. Login to the Admin UI using the user created above, but specifying the username capitalized --> J4 Notice the UI displays the logged in user with the capitalized name

4. Attempt to access the Statements page, and observe the error message.

5. Attempt to access the Databases page, and observe the page does not populate.

6. Logout. Then login using the same username, but specify the username in lower-case. Notice the UI displays the logged in user with the lower case name.

7. Access the Statements and Database pages, and observe that they work properly.

Expected behavior Admin UI should convert the username specified during login into lower-case, matching the username expected by the database.

Environment: Build Tag: v20.1.1 Build Time: 2020/05/19 14:40:33 Distribution: CCL Platform: darwin amd64 (x86_64-apple-darwin14) Go Version: go1.13.9 C Compiler: 4.2.1 Compatible Clang 3.8.0 (tags/RELEASE_380/final) Build SHA-1: 6123c0c73ff0eea223cfd25e1e557648413126f8 Build Type: release

Jira issue: CRDB-4017

Epic CRDB-32130

[tharun208]

@asubiotto, I like to work on this, before starting the implementation, can we change it to lower-case while storing it in the state or throw a validation error when the capital letter is entered.

[asubiotto]

Good question. Intuitively, I think we should throw a validation error, but cc @aaron-crl who might have a more informed opinion of this.

[aaron-crl]

This is a correctness bug for auth too; if we don't accept upper-case characters in usernames in the database we should not accept and use them in the UI.

I'd be opposed to silently normalizing too as this may result in user surprise when using a password manager where they've stored the uppercase username and it works on one interface but not another.

On an aside it sounds like some but not all of our UI code is normalizing usernames before they are sent to the database. We should review these endpoints to ensure they are properly sanitizing input.

Possibly related: https://github.com/cockroachdb/cockroach/pull/55398

cc @knz @bdarnell

[knz]

To start, I don't think this is a project we're willing to farm out to contributors outside of our team. It needs to touch multiple areas across the product and also align with our (internal) CockroachCloud authentication strategy.

Now to the specific issue, in the description at top:

Admin UI should convert the username specified during login into lower-case, matching the username expected by the database.

This behavior would be incorrect. If the username inside the database contains capitals, then it would become impossible for that user to log in into the UI.

What we need here is twofold:

1. stop normalizing usernames in the web UI altogether
2. when a user enters a username in the login form and no user with that name is found in the database, but there is another username with a different case, give a hint in the web form that the form is case-sensitive as a reminder (But do not actually show the different-case username in the tooltip, this would be a confidentiality breach)

In general I am not keen to looking at this issue in isolation. There should be a wider discussion.

[bdarnell]

Last time this came up, I thought we decided that usernames should be case-insensitive (possibly for compatibility with postgres), and that all entry points should be normalizing to lowercase (see the MakeSQLUsernameFromUserInput function). The CREATE USER statement already does this; I'm not sure if it's possible to create a user with a non-lowercase name unless you modify the system.users table manually.

[knz]

Ok so I think the "next action" here is to normalize the input in the login form in the UI, so that the UI does not retain a username that's not normalized for subsequent lookups in views.

How does this sound?

@dhartunian can you help us triage this?

[mgoddard]

My experience just now seems related to this issue, around case sensitivity (or insensitivity) for objects in the DB. When I run CREATE ROLE IF NOT EXISTS "rw-role-94e1ffe8-d4e3-46a3-8968-0c034537b6a9-Nm9bM"; I get a role that looks like this: root@localhost:5432/defaultdb> show roles; username | options | member_of -----------------------------------------------------+---------+------------ admin | | {} root | | {admin} rw-role-94e1ffe8-d4e3-46a3-8968-0c034537b6a9-nm9bm | NOLOGIN | {} (3 rows)

Later, when I attempt to run this, it fails since this role doesn't exist (in this context, the uppercase characters are preserved): ALTER TABLE public.user_mapping OWNER TO "rw-role-94e1ffe8-d4e3-46a3-8968-0c034537b6a9-Nm9bM";

Is there any way to get around this apart from lower-casing any of these role names?

[knz]

this is a different issue. Let's file it separately.

[knz]

@mgoddard can you double check this? I just ran the following without error:

 CREATE ROLE IF NOT EXISTS "rw-role-94e1ffe8-d4e3-46a3-8968-0c034537b6a9-Nm9bM";
 grant create on database defaultdb to "rw-role-94e1ffe8-d4e3-46a3-8968-0c034537b6a9-Nm9bM";
 create table user_mapping(x int);
 ALTER TABLE public.user_mapping OWNER TO "rw-role-94e1ffe8-d4e3-46a3-8968-0c034537b6a9-Nm9bM";

[mgoddard]

$ cockroach sql --certs-dir ../certs  --host=localhost:5432
#
# Welcome to the CockroachDB SQL shell.
# All statements must be terminated by a semicolon.
# To exit, type: \q.
#
# Server version: CockroachDB CCL v21.1.11 (x86_64-unknown-linux-gnu, built 2021/10/18 14:39:35, go1.15.14) (same version as client)
# Cluster ID: 84c1cece-112f-456e-9334-52193e60f809
#
# Enter \? for a brief introduction.
#
root@localhost:5432/defaultdb> CREATE ROLE IF NOT EXISTS "rw-role-94e1ffe8-d4e3-46a3-8968-0c034537b6a9-Nm9bM";
CREATE ROLE

Time: 11ms total (execution 11ms / network 0ms)

root@localhost:5432/defaultdb> grant create on database defaultdb to "rw-role-94e1ffe8-d4e3-46a3-8968-0c034537b6a9-Nm9bM";
GRANT

Time: 42ms total (execution 41ms / network 0ms)

root@localhost:5432/defaultdb> create table user_mapping(x int);
CREATE TABLE

Time: 10ms total (execution 10ms / network 0ms)

root@localhost:5432/defaultdb> ALTER TABLE public.user_mapping OWNER TO "rw-role-94e1ffe8-d4e3-46a3-8968-0c034537b6a9-Nm9bM";
ERROR: role/user "rw-role-94e1ffe8-d4e3-46a3-8968-0c034537b6a9-Nm9bM" does not exist
SQLSTATE: 42704
root@localhost:5432/defaultdb>

[knz]

@rafiss can you look at the failure reported by @mgoddard in the last comment?

This error does not occur in the master branch but does occur in 21.2 and 21.1. I think this might be a missing backport. Is this commit d2fc2a33eb7 perhaps the one that would help?

[rafiss]

The PR that fixes this bug is https://github.com/cockroachdb/cockroach/pull/70439/

specifically this part of the diff

It's quite a large change which is why it wasn't backported. ~Given that there's a workaround, I think instead we should list it as a known limitation for v21.2 and earlier. I'll file a docs issue.~ Edit: on second thought, let me try a smaller fix

However, as @knz mentioned above, this is different than the original issue reported here. The DB Console still needs to normalize usernames into lower case to address this issue.

[github-actions[bot]]

We have marked this issue as stale because it has been inactive for 18 months. If this issue is still relevant, removing the stale label or adding a comment will keep it active. Otherwise, we'll close it in 10 days to keep the issue queue tidy. Thank you for your contribution to CockroachDB!