Multiple valid passwords per user in `CredentialProvider`

[ramnes]

Hey there, thanks for great library!

Currently, CredentialProvider.GetCredential returns a single password per username. In some use cases, a user may have multiple valid credentials (e.g., API tokens, rotating secrets) that should all be accepted during authentication.

Would you be open to replace GetCredential by a new GetCredentials method that returns a slice of valid credentials?

Another approach if we don't want to break existing implementations of the interface would be to add a new MultiCredentialProvider interface that embeds CredentialProvider and adds that new method.

Then in the authentication flow, we could check if the provider implements MultiCredentialProvider via type assertion:

  if mcp, ok := provider.(MultiCredentialProvider); ok {
      passwords, found, err := mcp.GetCredentials(username)
      // ...
  } else {
      password, found, err := provider.GetCredential(username)
      // ...
  }

Happy to submit a PR if any of the two approaches sounds reasonable.

[lance6716]

I only have some primitive ideas for now, maybe we should discuss first.

1. MultiCredentialProvider is good at API compatibility. But I'm not sure the abstraction is correct. Should it be "multiple credentials of one user" or "multiple providers of single credential of one user"? But the latter one ([]CredentialProvider) can't be directly used in current code 🤔
2. Maybe it's more extendible that let implementation of CredentialProvider to verify the password, which is to say, change the method to

CheckCredential(username, plaintextPassword string) (ok bool, err error)

, or change existing Credential to have a verify function to compare multiple credential sources internally.

type Credential struct {
	PasswordVerifier func (string) (bool, error)
	AuthPluginName   string
}

[ramnes]

If we're ready to break API compatibility by updating Credential or CredentialProvider, I would just replace GetCredential with GetCredentials, what do you think? That's probably the simplest approach here, and I already have a PR ready for it actually. :)

On the PasswordVerifier idea, I'm not sure to understand what you have in mind. What would be an example implementation and why would we keep AuthPluginName?

PS - MultiCredentialProvider was meant for "multiple credentials of one user". Maybe something like CredentialsProvider (note the s at the end of Credentials) would be easier to read if we want to go that route.

[lance6716]

After one day I changed my idea 😂 Previously I think there should be multiple providers like PasswordProvider, APITokenProvider, .... Each one uses GetCredential to return only one credential. But I remember GitHub personal access token and there can be multiple token under one GitHub user, so it's reasonable to allow one provider returns multiple credentials.

And I prefer the MultiCredentialProvider way for compatibility. 👍 If you found it's hard to make it work, the other way also LGTM.

[ramnes]

I just opened two other issues (#1070 and #1071) that also discuss authentication and could also affect CredentialProvider. I'd be interested in your opinion on these before opening a PR for this one!

[ramnes]

Playing a little more with this, I think the only way to allow for multiple passwords is actually to keep GetCredential as-is, but instead modify Credential so that it holds multiple passwords with a Passwords []string field.

My understanding of the MySQL protocol is that it's impossible to handle authentication against multiple auth plugins for a single user on the wire, even if the database implementation would support it.

[lance6716]

it's impossible to handle authentication against multiple auth plugins for a single user on the wire

Yes. But for this issue, I think that plugin can internally implement checking one given password with multiple targets

[ramnes]

Yep, I think that’s exactly what I implemented in #1072. Can you confirm it's the approach you have in mind for handling multiple passwords per user?