Skip to content

Commit faafd30

Browse files
heiytorheiytor
committed
feat(api): user's external ID
External IDs represents the user's identifier in an external system. It is always empty when User.Origin is UserOriginManual.
1 parent 18c7e5c commit faafd30

File tree

9 files changed

+681
-10
lines changed

9 files changed

+681
-10
lines changed

‎api/store/mocks/store.go‎

Lines changed: 404 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎api/store/mongo/fixtures/users.json‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
{
22
"users": {
33
"507f1f77bcf86cd799439011": {
4+
"external_id": "01JEVPB55XVSK890Z2MYKMWXXY",
45
"status": "confirmed",
56
"created_at": "2023-01-01T12:00:00.000Z",
67
"last_login": "2023-01-01T12:00:00.000Z",
@@ -12,6 +13,7 @@
1213
"username": "john_doe"
1314
},
1415
"608f32a2c7351f001f6475e0": {
16+
"external_id": "",
1517
"status": "confirmed",
1618
"created_at": "2023-01-02T12:00:00.000Z",
1719
"last_login": "2023-01-02T12:00:00.000Z",
@@ -23,6 +25,7 @@
2325
"username": "jane_smith"
2426
},
2527
"709f45b5e812c1002f3a67e7": {
28+
"external_id": "",
2629
"status": "confirmed",
2730
"created_at": "2023-01-03T12:00:00.000Z",
2831
"last_login": "2023-01-03T12:00:00.000Z",
@@ -34,6 +37,7 @@
3437
"username": "bob_johnson"
3538
},
3639
"80fdcea1d7299c002f3a67e8": {
40+
"external_id": "",
3741
"status": "not-confirmed",
3842
"created_at": "2023-01-04T12:00:00.000Z",
3943
"last_login": null,
@@ -45,6 +49,7 @@
4549
"username": "alex_rodriguez"
4650
},
4751
"6509e169ae6144b2f56bf288": {
52+
"external_id": "",
4853
"status": "confirmed",
4954
"created_at": "2023-01-05T12:00:00.000Z",
5055
"last_login": "2023-01-05T12:00:00.000Z",

‎api/store/mongo/migrations/main.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ func GenerateMigrations() []migrate.Migration {
9595
migration83,
9696
migration84,
9797
migration85,
98+
migration86,
9899
}
99100
}
100101

‎api/store/mongo/migrations/migration_86.go‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
package migrations
2+
3+
import (
4+
"context"
5+
6+
"github.com/sirupsen/logrus"
7+
migrate "github.com/xakep666/mongo-migrate"
8+
"go.mongodb.org/mongo-driver/bson"
9+
"go.mongodb.org/mongo-driver/mongo"
10+
)
11+
12+
var migration86 = migrate.Migration{
13+
Version: 86,
14+
Description: "Adding an external ID attribute to users collection",
15+
Up: migrate.MigrationFunc(func(ctx context.Context, db *mongo.Database) error {
16+
logrus.WithFields(logrus.Fields{
17+
"component": "migration",
18+
"version": 86,
19+
"action": "Up",
20+
}).Info("Applying migration")
21+
22+
filter := bson.M{
23+
"external_id": bson.M{"$exists": false},
24+
}
25+
26+
update := bson.M{
27+
"$set": bson.M{
28+
"external_id": "",
29+
},
30+
}
31+
32+
_, err := db.
33+
Collection("users").
34+
UpdateMany(ctx, filter, update)
35+
36+
return err
37+
}),
38+
Down: migrate.MigrationFunc(func(ctx context.Context, db *mongo.Database) error {
39+
logrus.WithFields(logrus.Fields{
40+
"component": "migration",
41+
"version": 86,
42+
"action": "Down",
43+
}).Info("Reverting migration")
44+
45+
filter := bson.M{
46+
"external_id": bson.M{"$exists": true},
47+
}
48+
49+
update := bson.M{
50+
"$unset": bson.M{
51+
"external_id": "",
52+
},
53+
}
54+
55+
_, err := db.
56+
Collection("users").
57+
UpdateMany(ctx, filter, update)
58+
59+
return err
60+
}),
61+
}

‎api/store/mongo/migrations/migration_86_test.go‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
package migrations
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/shellhub-io/shellhub/pkg/envs"
8+
envmock "github.com/shellhub-io/shellhub/pkg/envs/mocks"
9+
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
11+
migrate "github.com/xakep666/mongo-migrate"
12+
"go.mongodb.org/mongo-driver/bson"
13+
)
14+
15+
func TestMigration86Up(t *testing.T) {
16+
ctx := context.Background()
17+
18+
mock := &envmock.Backend{}
19+
envs.DefaultBackend = mock
20+
21+
cases := []struct {
22+
description string
23+
setup func() error
24+
test func() error
25+
}{
26+
{
27+
description: "Success to apply up on migration 86",
28+
setup: func() error {
29+
_, err := c.
30+
Database("test").
31+
Collection("users").
32+
InsertOne(ctx, map[string]interface{}{
33+
"name": "john doe",
34+
})
35+
36+
return err
37+
},
38+
},
39+
}
40+
41+
for _, tc := range cases {
42+
tc := tc
43+
t.Run(tc.description, func(tt *testing.T) {
44+
tt.Cleanup(func() {
45+
assert.NoError(tt, srv.Reset())
46+
})
47+
48+
assert.NoError(tt, tc.setup())
49+
50+
migrates := migrate.NewMigrate(c.Database("test"), GenerateMigrations()[85])
51+
require.NoError(tt, migrates.Up(context.Background(), migrate.AllAvailable))
52+
53+
query := c.
54+
Database("test").
55+
Collection("users").
56+
FindOne(context.TODO(), bson.M{"name": "john doe"})
57+
58+
user := make(map[string]interface{})
59+
require.NoError(tt, query.Decode(&user))
60+
61+
_, ok := user["external_id"]
62+
require.Equal(tt, true, ok)
63+
})
64+
}
65+
}
66+
67+
func TestMigration86Down(t *testing.T) {
68+
ctx := context.Background()
69+
70+
mock := &envmock.Backend{}
71+
envs.DefaultBackend = mock
72+
73+
cases := []struct {
74+
description string
75+
setup func() error
76+
test func() error
77+
}{
78+
{
79+
description: "Success to apply up on migration 86",
80+
setup: func() error {
81+
_, err := c.
82+
Database("test").
83+
Collection("users").
84+
InsertOne(ctx, map[string]interface{}{
85+
"name": "john doe",
86+
"external_id": "unique_string",
87+
})
88+
89+
return err
90+
},
91+
},
92+
}
93+
94+
for _, tc := range cases {
95+
tc := tc
96+
t.Run(tc.description, func(t *testing.T) {
97+
t.Cleanup(func() {
98+
assert.NoError(t, srv.Reset())
99+
})
100+
101+
assert.NoError(t, tc.setup())
102+
103+
migrates := migrate.NewMigrate(c.Database("test"), GenerateMigrations()[85])
104+
require.NoError(t, migrates.Up(context.Background(), migrate.AllAvailable))
105+
require.NoError(t, migrates.Down(context.Background(), migrate.AllAvailable))
106+
107+
query := c.
108+
Database("test").
109+
Collection("users").
110+
FindOne(context.TODO(), bson.M{"name": "john doe"})
111+
112+
user := make(map[string]interface{})
113+
require.NoError(t, query.Decode(&user))
114+
115+
_, ok := user["external_id"]
116+
require.Equal(t, false, ok)
117+
})
118+
}
119+
}

‎api/store/mongo/user.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,16 @@ func (s *Store) UserGetByEmail(ctx context.Context, email string) (*models.User,
125125
return user, nil
126126
}
127127

128+
func (s *Store) UserGetByExternalID(ctx context.Context, id string) (*models.User, error) {
129+
user := new(models.User)
130+
131+
if err := s.db.Collection("users").FindOne(ctx, bson.M{"external_id": id}).Decode(&user); err != nil {
132+
return nil, FromMongoError(err)
133+
}
134+
135+
return user, nil
136+
}
137+
128138
func (s *Store) UserGetByID(ctx context.Context, id string, ns bool) (*models.User, int, error) {
129139
user := new(models.User)
130140
objID, err := primitive.ObjectIDFromHex(id)

‎api/store/mongo/user_test.go‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ func TestUserList(t *testing.T) {
4040
users: []models.User{
4141
{
4242
ID: "507f1f77bcf86cd799439011",
43+
ExternalID: "01JEVPB55XVSK890Z2MYKMWXXY",
4344
CreatedAt: time.Date(2023, 1, 1, 12, 0, 0, 0, time.UTC),
4445
LastLogin: time.Date(2023, 1, 1, 12, 0, 0, 0, time.UTC),
4546
EmailMarketing: true,
@@ -289,6 +290,7 @@ func TestStore_UserCreateInvited(t *testing.T) {
289290
tt,
290291
map[string]interface{}{
291292
"_id": objID,
293+
"external_id": nil,
292294
"created_at": primitive.NewDateTimeFromTime(now),
293295
"last_login": primitive.NewDateTimeFromTime(time.Time{}),
294296
"origin": nil,
@@ -337,6 +339,7 @@ func TestUserGetByUsername(t *testing.T) {
337339
expected: Expected{
338340
user: &models.User{
339341
ID: "507f1f77bcf86cd799439011",
342+
ExternalID: "01JEVPB55XVSK890Z2MYKMWXXY",
340343
CreatedAt: time.Date(2023, 1, 1, 12, 0, 0, 0, time.UTC),
341344
LastLogin: time.Date(2023, 1, 1, 12, 0, 0, 0, time.UTC),
342345
EmailMarketing: true,
@@ -399,6 +402,7 @@ func TestUserGetByEmail(t *testing.T) {
399402
expected: Expected{
400403
user: &models.User{
401404
ID: "507f1f77bcf86cd799439011",
405+
ExternalID: "01JEVPB55XVSK890Z2MYKMWXXY",
402406
CreatedAt: time.Date(2023, 1, 1, 12, 0, 0, 0, time.UTC),
403407
LastLogin: time.Date(2023, 1, 1, 12, 0, 0, 0, time.UTC),
404408
EmailMarketing: true,
@@ -465,6 +469,7 @@ func TestUserGetByID(t *testing.T) {
465469
expected: Expected{
466470
user: &models.User{
467471
ID: "507f1f77bcf86cd799439011",
472+
ExternalID: "01JEVPB55XVSK890Z2MYKMWXXY",
468473
CreatedAt: time.Date(2023, 1, 1, 12, 0, 0, 0, time.UTC),
469474
LastLogin: time.Date(2023, 1, 1, 12, 0, 0, 0, time.UTC),
470475
EmailMarketing: true,
@@ -491,6 +496,7 @@ func TestUserGetByID(t *testing.T) {
491496
expected: Expected{
492497
user: &models.User{
493498
ID: "507f1f77bcf86cd799439011",
499+
ExternalID: "01JEVPB55XVSK890Z2MYKMWXXY",
494500
CreatedAt: time.Date(2023, 1, 1, 12, 0, 0, 0, time.UTC),
495501
LastLogin: time.Date(2023, 1, 1, 12, 0, 0, 0, time.UTC),
496502
EmailMarketing: true,
@@ -526,6 +532,74 @@ func TestUserGetByID(t *testing.T) {
526532
}
527533
}
528534

535+
func TestUserGetByExternalID(t *testing.T) {
536+
type Expected struct {
537+
user *models.User
538+
ns int
539+
err error
540+
}
541+
542+
cases := []struct {
543+
description string
544+
externalID string
545+
ns bool
546+
fixtures []string
547+
expected Expected
548+
}{
549+
{
550+
description: "fails when user is not found",
551+
externalID: "invalid",
552+
fixtures: []string{fixtureUsers},
553+
expected: Expected{
554+
user: nil,
555+
ns: 0,
556+
err: store.ErrNoDocuments,
557+
},
558+
},
559+
{
560+
description: "succeeds when user is found with ns equal false",
561+
externalID: "01JEVPB55XVSK890Z2MYKMWXXY",
562+
ns: false,
563+
fixtures: []string{fixtureUsers},
564+
expected: Expected{
565+
user: &models.User{
566+
ID: "507f1f77bcf86cd799439011",
567+
ExternalID: "01JEVPB55XVSK890Z2MYKMWXXY",
568+
CreatedAt: time.Date(2023, 1, 1, 12, 0, 0, 0, time.UTC),
569+
LastLogin: time.Date(2023, 1, 1, 12, 0, 0, 0, time.UTC),
570+
EmailMarketing: true,
571+
Status: models.UserStatusConfirmed,
572+
UserData: models.UserData{
573+
Name: "john doe",
574+
Username: "john_doe",
575+
Email: "[email protected]",
576+
},
577+
MaxNamespaces: 0,
578+
Password: models.UserPassword{
579+
Hash: "fcf730b6d95236ecd3c9fc2d92d7b6b2bb061514961aec041d6c7a7192f592e4",
580+
},
581+
},
582+
ns: 0,
583+
err: nil,
584+
},
585+
},
586+
}
587+
588+
for _, tc := range cases {
589+
t.Run(tc.description, func(t *testing.T) {
590+
ctx := context.Background()
591+
592+
assert.NoError(t, srv.Apply(tc.fixtures...))
593+
t.Cleanup(func() {
594+
assert.NoError(t, srv.Reset())
595+
})
596+
597+
user, err := s.UserGetByExternalID(ctx, tc.externalID)
598+
assert.Equal(t, tc.expected, Expected{user: user, err: err})
599+
})
600+
}
601+
}
602+
529603
func TestUserConflicts(t *testing.T) {
530604
type Expected struct {
531605
conflicts []string

‎api/store/user.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,10 @@ type UserStore interface {
2121
// It returns the inserted ID or an error, if any.
2222
UserCreateInvited(ctx context.Context, email string) (insertedID string, err error)
2323

24-
UserGetByUsername(ctx context.Context, username string) (*models.User, error)
25-
UserGetByEmail(ctx context.Context, email string) (*models.User, error)
2624
UserGetByID(ctx context.Context, id string, ns bool) (*models.User, int, error)
25+
UserGetByExternalID(ctx context.Context, id string) (*models.User, error)
26+
UserGetByEmail(ctx context.Context, email string) (*models.User, error)
27+
UserGetByUsername(ctx context.Context, username string) (*models.User, error)
2728

2829
// UserConflicts reports whether the target contains conflicting attributes with the database. Pass zero values for
2930
// attributes you do not wish to match on. For example, the following call checks for conflicts based on email only:

‎pkg/models/user.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,10 @@ type User struct {
4343
ID string `json:"id,omitempty" bson:"_id,omitempty"`
4444
// Origin specifies the the user's signup method.
4545
Origin UserOrigin `json:"-" bson:"origin"`
46-
Status UserStatus `json:"status" bson:"status"`
46+
// ExternalID represents the user's identifier in an external system. It is always empty when [User.Origin]
47+
// is [UserOriginLocal].
48+
ExternalID string `json:"-" bson:"external_id"`
49+
Status UserStatus `json:"status" bson:"status"`
4750
// MaxNamespaces represents the count of namespaces that the user can owns.
4851
MaxNamespaces int `json:"max_namespaces" bson:"max_namespaces"`
4952
CreatedAt time.Time `json:"created_at" bson:"created_at"`

0 commit comments

Comments
 (0)