Fixes inability to authenticate through the older cmdQuery mechanism #4066
Conversation
|
|
Evengard
force-pushed
the
fix-cmdQuery-auth
branch
2 times, most recently
from
d66a77f
to
52764d3
Compare
Evengard
force-pushed
the
fix-cmdQuery-auth
branch
from
52764d3
to
c66296b
Compare
| } | ||
|
|
||
| // TODO https://github.com/FerretDB/FerretDB/issues/3008 | ||
|
|
||
| // database name typically is either "$external" or "admin" | ||
|
|
||
| if cmd == "saslStart" && strings.HasSuffix(collection, ".$cmd") { | ||
| mechanism, err := common.GetRequiredParam[string](body, "mechanism") | ||
| if err == nil && mechanism == "PLAIN" { |
There was a problem hiding this comment.
Let's return the error for the case where err != nil. 🤗
Let's also handle the case where mechanism != "PLAIN". I think we can handle it similarly to how it's done in msg_saslstart.go
There was a problem hiding this comment.
The idea was to provide extreme backwards compatibility by just falling back to the "stub" behavior if something went wrong, as I have no ideas if that would break any other clients. But I don't mind returning explicit errors here...
There was a problem hiding this comment.
Thanks for your quick response 🤗. Let us check, @b1ron do you know which other driver uses cmd_query.go? My guess is, it doesn't work for those drivers? If that's the case having an appropriate error is sensible here for a client attempting to authenticate with other mechanisms.
There was a problem hiding this comment.
It depends on the specific driver and version. Looking at this compatibility matrix drivers that do not support MongoDB 6.0 will use legacy opcodes. I suspect @Evengard is using mongocsharpdriver version 2.15 or older.
OP_QUERY
Deprecated in MongoDB 5.0. Removed in MongoDB 5.1.
Fixing #3121 might be easier.
There was a problem hiding this comment.
Removed in MongoDB 5.1.
Narrator: It wasn't actually removed.
But yeah, to move forward, we have to know the driver version.
There was a problem hiding this comment.
I was able to force an OP_MSG using version 2.19.1, strange.
There was a problem hiding this comment.
The thing is I can force an OP_MSG authentication with StableAPI, it's the inserts which bugs then.
I can do a PR in attempts to ignore theese apiVersions parameters probably instead. But looking that the actual issue seemed abandoned wasn't sure it is the way to go.
There was a problem hiding this comment.
#4067 Went ahead with the StableAPI parameter ignoring... It works with 2.23.1 too.
There was a problem hiding this comment.
Thanks @Evengard, and apologies for my confusion. I was able to reproduce the error in both 2.19.1 and 2.23.1.
Unhandled exception. MongoDB.Driver.MongoCommandException: Command ping failed: [msg_ping.go:52 handler.(*Handler).MsgPing] [backend.go:67 postgresql.(*backend).Status] [registry.go:259 metadata.(*Registry).DatabaseList] [registry.go:132 metadata.(*Registry).getPool] [pool.go:128 pool.(*Pool).Get] [opendb.go:95 pool.openDB] [opendb.go:110 pool.checkConnection] failed to connect to `host=postgres user= database=ferretdb`: server error (FATAL: no PostgreSQL user name specified in startup packet (SQLSTATE 28000)).
#4067 fixes the issue I also tested it. The core team will review it.
There was a problem hiding this comment.
We should do FerretDB/dance#769 first
|
@Evengard this pull request has merge conflicts. |
Description
Apparently, the older opQuery authentication mechanism was just a stub which wasn't doing anything. Because of that, the DotNet MongoDB client wasn't able to authenticate, as it was using this older mechanism when not using StableAPI. I mimicked the mechanism from opMsg, and now DotNet works.
Closes #4065.
Readiness checklist
task all, and it passed.@FerretDB/core), Milestone (Next), Labels, Project and project's Sprint fields.