DRIVERS-2415 Implement OIDC SASL mechanism

[blink1073]

Please complete the following before merging:

• Update changelog.
• Make sure there are generated JSON files from the YAML test files.
• Test changes in at least one language driver.
• Test these changes against all server versions and topologies (including standalone, replica set, sharded clusters, and serverless).

[addaleax]

It’s a surprise that we expect PRINCIPAL_NAME as an auth mechanism property instead of passing it in the username. Is there a specific reason for this? As I understand it, it fulfills the same role as the username, and in particular also matches the role of the username for similar auth mechanisms like Kerberos or LDAP.

[blink1073]

I thought it might be misleading to use the username field, since the server is actually creating a derived username for this session.

[addaleax]

I guess the question then is about the semantics of the username field, whether it is “This is the identity I am attempting to authenticate as” or “This is the username string that the database uses and reports”. I would maybe still lean towards interpreting it as the former, although I see your point about this differing from the DB username.

[spencerjackson]

@sgolemon do you have any feelings on this? I'm vaguely reminded of how during LDAP authentication we map the presented authentication name into a name which the LDAP server will recognize through the UserToDNMapping.

I'm also leaning heavily toward the username in mongodb://[username:password@]host1 is "This is the identity I am attempting to authenticate as" as Anna said

[sgolemon]

I do have feelings! My assumption was that the principal name hint would and should be the username, and not a custom property as that seems like "least surprise" from an end-developer point of view. It's true that the derived principal name may differ (depending on IDP and MongoDB configurations), but from the application point of view the synthesized principal name is an implementation detail and all we get by placing the hint into mechanism properties and prohibiting the username field is a little confusion about why it's not in the usual spot.

[blink1073]

From my understanding of the server technical design and implementation, the principal name is by default the "sub" field of the jwt token, which if the IdP uses pairwise identifiers is obfuscated by a salt.

[blink1073]

So in that case it feels even less like a user name.

[addaleax]

@blink1073 On today's initiative sync we briefly touched on this, and the sentiment was to use the username field like we would for LDAP and Kerberos. Unless there are concerns with it that haven't been brought up so far, this shouldn't be an auth mechanism property.

[blink1073]

Okay fair, I will update the spec and connection string tests.

[blink1073]

Done

[addaleax]

Two things:

• In the devtools products, we won’t really be able to make use of this feature since we’ll need to maintain a cache across MongoClient instances in different processes. I could imagine that other products, if there are any, that use browser flows, would run into similar issues at some point. Consequently, I don’t know if adding global mutable state is worth it here – definitely not a blocker though!
• The cache key should probably include some form of cluster identifier, since the principal name may refer to different things for different clusters. Right now, on our side we’re planning to use the client_id provided in the initial SASL response for this.

[blink1073]

If we rely on a value from server step 1, we cannot skip client step 1 on subsequent calls.

[blink1073]

After thinking about this more, we'll forgo skipping client step 1 to be more conservative.

[DmitryLukyanov]

The cache key should probably include some form of cluster identifier, since the principal name may refer to different things for different clusters. Right now, on our side we’re planning to use the client_id provided in the initial SASL response for this.

@addaleax , if we need to determine what cluster provided a cached value, is it easier to add a host address to a cache key instead tracking a client_id? It would be good to build a cache key based on initial client configuration and not on something dynamic. Also it will allow skipping step 1.

[addaleax]

Right, that’s a good point – I don’t mind using the host address as a key here if that’s easy enough. I’m not sure how common it would be to use the same client id for multiple clusters anyway.

[blink1073]

Updated

[DmitryLukyanov]

I think we need to clear cache (inside azure) too in case of failure?

[blink1073]

Done

[addaleax]

Just re-affirming my LGTM

[DmitryLukyanov]

LGTM