Inconsistent connect fallback behaviour for different authentication methods

[niksajakovljevic]

When trying to establish a connection using MD5 auth method, if authentication fails due to wrong password pgconn will skip checking fallback configs. However if authentication method used when connecting is SCRAM-SHA-256 then even after failing with wrong password it will continue checking fallback configs. Digging deeper into the code the problem seems to lay in the following code lines in pgconn.go:148:

for _, fc := range fallbackConfigs {
		pgConn, err = connect(ctx, config, fc)
		if err == nil {
			break
		} else if err, ok := err.(*PgError); ok {
			return nil, &connectError{config: config, msg: "server error", err: err}
		}
	}

Namely, connect function call will return PgError when MD5 auth fails, but will return connectError that wraps PgError for SCRAM-SHA-256. So basically condition else if err, ok := err.(*PgError); ok will not be hit. I assume that one of the workarounds is to inspect the root cause error of connectError and return if it's PgError but not sure how that plays with other fallback scenarios.

[jackc]

Can you try it with this code?

	for _, fc := range fallbackConfigs {
		pgConn, err = connect(ctx, config, fc)
		if err == nil {
			break
		}
		var pgErr *PgError
		if errors.As(err, &pgErr) {
			if _, ok := err.(*connectError); ok {
				return nil, err
			} else {
				return nil, &connectError{config: config, msg: "server error", err: err}
			}
		}
	}

I think that will resolve it, but I don't have a convenient way to test SCRAM at the moment.

[niksajakovljevic]

Sure I can give that a run. Judging from the code change I'm confident that it should work. However this change might not play nicely with ValidateConnect function b/c the code comments is saying that on error it should try next fallback config and your proposed change will not allow that...