Auth: Force lowercase login/email for users

[eleijonmarck]

why/what

We are lowercasing the login, email parameters of user methods and queries due to enforcement of case insensitive login and emails for our users.
The change introduced makes users whom do not have lowercased login or email not able to login, this is why we opted for a migration as well. This is also part of the work to move to lowercased emails and logins.

• Adds a migration that lowercases login or emails where we do not find a lowercased entry in the database for those users
• Adds lowercasing of the parameters login and email for function calls of getting users
• Removes LOWER() from query parameters, that was not using the indexes of the databases

Tasks

Edit tasklist title

Beta Give feedback Tasklist Tasks, more options

• Rename
• Copy markdown
• Delete tasklist

Delete tasklist

Delete tasklist block?

Are you sure? All relationships in this tasklist will be removed.

Cancel Delete

1. test cases for lowercasing logins

   test cases for lowercasing logins
   Options

   • Convert to issue
   • Toggle completion
   • Rename
   • Remove
2. test cases for lowercasing emails

   test cases for lowercasing emails
   Options

   • Convert to issue
   • Toggle completion
   • Rename
   • Remove
3. test with terraform provider and state change for the users

   test with terraform provider and state change for the users
   Options

   • Convert to issue
   • Toggle completion
   • Rename
   • Remove

Add item to Tasks

Issue
https://github.com/grafana/identity-access-team/issues/657

Notes

We were initially querying the db with LOWER(login) which did not make use of our created indexes, causing the db to scan all rows.

Without the LOWER() operation, we now see that it is using the index correctly.

EXPLAIN QUERY PLAN SELECT `id`, `uid`, `version`, `email`, `name`, `login`, `password`, `salt`, `rands`, `company`, `email_verified`, `theme`, `help_flags1`, `is_disabled`, `is_admin`, `is_service_account`, `org_id`, `created`, `updated`, `last_seen_at` FROM `user` WHERE (`user`.is_service_account = 0) AND (login=?) LIMIT 1;

• using indx SEARCH user USING INDEX UQE_user_login (login=?)

---

A follow up issue is to enforce lowercasing as part of the columns.

Release notice breaking change

We are migrating email and login automatically to lowercasing for our user table.

This is a breaking change that introduces lowercasing input fields in methods for GetByEmail, GetByLogin. The users are now using lowercase as part of their login and emails, regardless if they login with Grafanauser@Grafana.com.

Troubleshooting

If you are experiencing any issues with your users not being logged in with the correct user. Please refer to the CLI we built to deal with conflicting users.

Use our CLI introduced in Grafana 9.3 to resolve user conflicts.

You might experience constraints for duplicate entries (since you might have two user entries, one lowercased and one with mixed or capitalised cases).
Query 1 ERROR at Line 1: : UNIQUE constraint failed: user.email

There might be cases where you as an admin, have to go and look at the users table to see

[IevaVasiljeva]

This is a breaking change for users who might have mixed case login details, right? I think we need to add a breaking change notice and link to some steps on how to lowercase DB entries?

[Jguer]

We never forced migration to lower case and as such this will break more users than the previous patch. I think this needs a migration patch to set all user logins to lower case other wise weird lookups on auth will start to happen and new users might be created

[eleijonmarck]

We never forced migration to lower case and as such this will break more users than the previous patch. I think this needs a migration patch to set all user logins to lower case other wise weird lookups on auth will start to happen and new users might be created

let me work out a bunch of scenarios for different cases and make sure that we secure the desired state after a migration

[Jguer]

missing the cases:
"2 users - same login, one already lowercase"
"2 users - same login, none lowercase"

[eleijonmarck]

Yea I will expand on the test cases. Thanks! In general it looks good?

[IevaVasiljeva]

Code LGTM, I added a few suggestions for optimising the migration a bit.

I'm not sure what the expected behaviour should be when it comes to several non-lowercased conflicting entries - I guess sticking with the more recent one makes sense, but then we should clearly document how the conflict resolution tool can be used to pick the older user instead. I'm mostly worried about the cloud scenario, where users can neither access the DB nor use the CLI, right?

Also breaking change notice should be updated to explain that we're migrating emails and logins automatically (so it will only break for users with conflicting entries).

[IevaVasiljeva]

question: how is the preference of the more recent user enforced? Is it just based on the more recently created (so the more recent entry in the DB table)?

[eleijonmarck]

currently it will be the one that loops through for the user. ideally we would want to choose the one that has the latest updated

[grafana-delivery-bot]

This PR must be merged before a backport PR will be created.

[eleijonmarck]

Terraform considerations

We need to consider that terraform provider will break due to the migration.

1. Create a script that created UPPERCASE user logins for Grafana terraform apply

// Create 100 Grafana users with uppercase login and email
resource "grafana_user" "user_upper" {
  count = 100
  login    = upper(format("user%03d", count.index + 1))
  email    = upper(format("user%03d@example.com", count.index + 1))
  name     = upper(format("User %03d", count.index + 1))
  password = "InitialPassword123!"
  is_admin = false
}

2. remove the migration from the migration_log
3. terraform apply

This will produce an error for the terrraform

  # grafana_user.user_upper[97] will be updated in-place
  ~ resource "grafana_user" "user_upper" {
      ~ email    = "user098@example.com" -> "USER098@EXAMPLE.COM"
        id       = "138"
      ~ login    = "user098" -> "USER098"
        name     = "USER 098"
        # (3 unchanged attributes hidden)
    }

  # grafana_user.user_upper[98] will be updated in-place
  ~ resource "grafana_user" "user_upper" {
      ~ email    = "user099@example.com" -> "USER099@EXAMPLE.COM"
        id       = "132"
      ~ login    = "user099" -> "USER099"
        name     = "USER 099"
        # (3 unchanged attributes hidden)
    }

  # grafana_user.user_upper[99] will be updated in-place
  ~ resource "grafana_user" "user_upper" {
      ~ email    = "user100@example.com" -> "USER100@EXAMPLE.COM"
        id       = "149"
      ~ login    = "user100" -> "USER100"
        name     = "USER 100"
        # (3 unchanged attributes hidden)
    }

[IevaVasiljeva]

I've tested this, and the migrations as well as lower casing logic upon logins and user email/login changes work as I'd expect it to.

I still think the migration can be optimised a bit, but I don't see it as a blocking change. Similarly, the Terraform fix can be merged later.

[grafana-delivery-bot]

The backport to v11.0.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-86359-to-v11.0.x origin/v11.0.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x e394e16073dd73fef589d2b42afbb9e0a9eed606

When the conflicts are resolved, stage and commit the changes:

git add . && git cherry-pick --continue

If you have the GitHub CLI installed:

# Push the branch to GitHub:
git push --set-upstream origin backport-86359-to-v11.0.x
# Create the PR body template
PR_BODY=$(gh pr view 86359 --json body --template 'Backport e394e16073dd73fef589d2b42afbb9e0a9eed606 from #86359{{ "\n\n---\n\n" }}{{ index . "body" }}')
# Create the PR on GitHub
echo "${PR_BODY}" | gh pr create --title "[v11.0.x] Auth: Force lowercase login/email for users" --body-file - --label "type/bug" --label "area/backend" --label "add to changelog" --label "breaking change" --label "area/backend/db/migration" --label "backport" --base v11.0.x --milestone 11.0.x --web

Or, if you don't have the GitHub CLI installed (we recommend you install it!):

# Push the branch to GitHub:
git push --set-upstream origin backport-86359-to-v11.0.x

# Create a pull request where the `base` branch is `v11.0.x` and the `compare`/`head` branch is `backport-86359-to-v11.0.x`.

# Remove the local backport branch
git switch main
git branch -D backport-86359-to-v11.0.x