Skip to content
This repository has been archived by the owner on Aug 24, 2022. It is now read-only.

PMM-9320: fix username/password incorrect escape #372

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
9 changes: 7 additions & 2 deletions actions/mongodb_explain_action.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ import (
"github.com/pkg/errors"
"go.mongodb.org/mongo-driver/bson"
"go.mongodb.org/mongo-driver/mongo"
"go.mongodb.org/mongo-driver/mongo/options"

"github.com/percona/pmm-agent/mongo_fix"
"github.com/percona/pmm-agent/utils/templates"
)

Expand Down Expand Up @@ -68,7 +68,12 @@ func (a *mongodbExplainAction) Run(ctx context.Context) ([]byte, error) {
return nil, errors.WithStack(err)
}

client, err := mongo.Connect(ctx, options.Client().ApplyURI(dsn))
opts, err := mongo_fix.ClientOptionsForDSN(dsn)
if err != nil {
return nil, errors.WithStack(err)
}

client, err := mongo.Connect(ctx, opts)
if err != nil {
return nil, errors.WithStack(err)
}
Expand Down
9 changes: 7 additions & 2 deletions actions/mongodb_query_admincommand_action.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ import (
"github.com/pkg/errors"
"go.mongodb.org/mongo-driver/bson"
"go.mongodb.org/mongo-driver/mongo"
"go.mongodb.org/mongo-driver/mongo/options"

"github.com/percona/pmm-agent/mongo_fix"
"github.com/percona/pmm-agent/utils/templates"
)

Expand Down Expand Up @@ -77,7 +77,12 @@ func (a *mongodbQueryAdmincommandAction) Run(ctx context.Context) ([]byte, error
return nil, errors.WithStack(err)
}

client, err := mongo.Connect(ctx, options.Client().ApplyURI(dsn))
opts, err := mongo_fix.ClientOptionsForDSN(dsn)
if err != nil {
return nil, errors.WithStack(err)
}

client, err := mongo.Connect(ctx, opts)
if err != nil {
return nil, errors.WithStack(err)
}
Expand Down
11 changes: 8 additions & 3 deletions agents/mongodb/internal/profiler/collector/collector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@ import (
"go.mongodb.org/mongo-driver/bson"
"go.mongodb.org/mongo-driver/bson/primitive"
"go.mongodb.org/mongo-driver/mongo"
"go.mongodb.org/mongo-driver/mongo/options"
"go.mongodb.org/mongo-driver/mongo/readpref"

"github.com/percona/pmm-agent/mongo_fix"
)

const (
Expand Down Expand Up @@ -202,8 +203,12 @@ func createSession(dsn string, agentID string) (*mongo.Client, error) {
ctx, cancel := context.WithTimeout(context.Background(), MgoTimeoutDialInfo)
defer cancel()

opts := options.Client().
ApplyURI(dsn).
opts, err := mongo_fix.ClientOptionsForDSN(dsn)
if err != nil {
return nil, err
}

opts = opts.
SetDirect(true).
SetReadPreference(readpref.Nearest()).
SetSocketTimeout(MgoTimeoutSessionSocket).
Expand Down
11 changes: 8 additions & 3 deletions agents/mongodb/internal/profiler/profiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ import (

"github.com/sirupsen/logrus"
"go.mongodb.org/mongo-driver/mongo"
"go.mongodb.org/mongo-driver/mongo/options"
"go.mongodb.org/mongo-driver/mongo/readpref"

"github.com/percona/pmm-agent/agents/mongodb/internal/profiler/aggregator"
"github.com/percona/pmm-agent/agents/mongodb/internal/profiler/sender"
"github.com/percona/pmm-agent/mongo_fix"
)

// New creates new Profiler
Expand Down Expand Up @@ -192,8 +192,13 @@ func signalReady(ready *sync.Cond) {
func createSession(dsn string, agentID string) (*mongo.Client, error) {
ctx, cancel := context.WithTimeout(context.Background(), MgoTimeoutDialInfo)
defer cancel()
opts := options.Client().
ApplyURI(dsn).

opts, err := mongo_fix.ClientOptionsForDSN(dsn)
if err != nil {
return nil, err
}

opts = opts.
SetDirect(true).
SetReadPreference(readpref.Nearest()).
SetSocketTimeout(MgoTimeoutSessionSocket).
Expand Down
11 changes: 9 additions & 2 deletions connectionchecker/connection_checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ import (
"github.com/sirupsen/logrus"
"go.mongodb.org/mongo-driver/bson"
"go.mongodb.org/mongo-driver/mongo"
"go.mongodb.org/mongo-driver/mongo/options"

"github.com/percona/pmm-agent/config"
"github.com/percona/pmm-agent/mongo_fix"
"github.com/percona/pmm-agent/tlshelpers"
"github.com/percona/pmm-agent/utils/templates"
)
Expand Down Expand Up @@ -162,7 +162,14 @@ func (cc *ConnectionChecker) checkMongoDBConnection(ctx context.Context, dsn str
return &res
}

client, err := mongo.Connect(ctx, options.Client().ApplyURI(dsn))
opts, err := mongo_fix.ClientOptionsForDSN(dsn)
if err != nil {
cc.l.Debugf("failed to parse DSN: %s", err)
res.Error = err.Error()
return &res
}

client, err := mongo.Connect(ctx, opts)
if err != nil {
cc.l.Debugf("checkMongoDBConnection: failed to Connect: %s", err)
res.Error = err.Error()
Expand Down
42 changes: 42 additions & 0 deletions mongo_fix/mongo_fix.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// pmm-agent
// Copyright 2019 Percona LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package mongo_fix
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

called mongo_fix not to clash with mongo driver

ritbl marked this conversation as resolved.
Show resolved Hide resolved

import (
"net/url"

"github.com/pkg/errors"
"go.mongodb.org/mongo-driver/mongo/options"
)

// ClientOptionsForDSN applies URI to Client.
func ClientOptionsForDSN(dsn string) (*options.ClientOptions, error) {
clientOptions := options.Client().ApplyURI(dsn)

// Workaround for PMM-9320
parsedDsn, err := url.Parse(dsn)
if err != nil {
return nil, errors.Wrap(err, "cannot parse DSN")
}

username := parsedDsn.User.Username()
password, _ := parsedDsn.User.Password()
if username != "" || password != "" {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to set auth only when creds are provided, otherwise auth fill fail

clientOptions = clientOptions.SetAuth(options.Credential{Username: username, Password: password})
}

return clientOptions, nil
}
47 changes: 47 additions & 0 deletions mongo_fix/mongo_fix_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package mongo_fix

import (
"github.com/stretchr/testify/assert"
"net/url"
"testing"
)

func TestClientOptionsForDSN(t *testing.T) {
tests := []struct {
name string
dsn string
expectedUser string
expectedPassword string
}{
{
name: "Escape username",
dsn: (&url.URL{
Scheme: "mongo",
Host: "localhost",
Path: "/db",
User: url.UserPassword("user+", "pass"),
}).String(),
expectedUser: "user+",
expectedPassword: "pass",
},
{
name: "Escape password",
dsn: (&url.URL{
Scheme: "mongo",
Host: "localhost",
Path: "/db",
User: url.UserPassword("user", "pass+"),
}).String(),
expectedUser: "user",
expectedPassword: "pass+",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := ClientOptionsForDSN(tt.dsn)
assert.Nil(t, err)
assert.Equal(t, got.Auth.Username, tt.expectedUser)
assert.Equal(t, got.Auth.Password, tt.expectedPassword)
})
}
}
10 changes: 8 additions & 2 deletions utils/tests/mongodb.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ import (
"github.com/stretchr/testify/require"
"go.mongodb.org/mongo-driver/bson/primitive"
"go.mongodb.org/mongo-driver/mongo"
"go.mongodb.org/mongo-driver/mongo/options"

"github.com/percona/pmm-agent/mongo_fix"
)

// GetTestMongoDBDSN returns DNS for MongoDB test database.
Expand Down Expand Up @@ -104,7 +105,12 @@ func GetTestMongoDBReplicatedWithSSLDSN(tb testing.TB, pathToRoot string) (strin
func OpenTestMongoDB(tb testing.TB, dsn string) *mongo.Client {
tb.Helper()

client, err := mongo.Connect(context.Background(), options.Client().ApplyURI(dsn))
opts, err := mongo_fix.ClientOptionsForDSN(dsn)
if err != nil {
require.NoError(tb, err)
}

client, err := mongo.Connect(context.Background(), opts)
require.NoError(tb, err)

require.NoError(tb, client.Ping(context.Background(), nil))
Expand Down