From 69be2a8071e979f5b9815fe12bcd9a122ce43a13 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Wed, 1 Mar 2023 20:12:04 +0100 Subject: [PATCH 01/16] add generating token (only frame for now, actual token is only dummy) --- go.mod | 1 + go.sum | 2 + management/server/personal_access_token.go | 70 +++++++++++++++++++ .../server/personal_access_token_test.go | 41 +++++++++++ management/server/user.go | 7 +- 5 files changed, 119 insertions(+), 2 deletions(-) create mode 100644 management/server/personal_access_token.go create mode 100644 management/server/personal_access_token_test.go diff --git a/go.mod b/go.mod index 95a849db9..e34633892 100644 --- a/go.mod +++ b/go.mod @@ -28,6 +28,7 @@ require ( ) require ( + codeberg.org/ac/base62 v0.0.0-20210305150220-e793b546833a fyne.io/fyne/v2 v2.1.4 github.com/c-robinson/iplib v1.0.3 github.com/coreos/go-iptables v0.6.0 diff --git a/go.sum b/go.sum index 811f3a604..e64b2bb38 100644 --- a/go.sum +++ b/go.sum @@ -30,6 +30,8 @@ cloud.google.com/go/storage v1.5.0/go.mod h1:tpKbwo567HUNpVclU5sGELwQWBDZ8gh0Zeo cloud.google.com/go/storage v1.6.0/go.mod h1:N7U0C8pVQ/+NIKOBQyamJIeKQKkZ+mxpohlUTyfDhBk= cloud.google.com/go/storage v1.8.0/go.mod h1:Wv1Oy7z6Yz3DshWRJFhqM/UCfaWIRTdp0RXyy7KQOVs= cloud.google.com/go/storage v1.10.0/go.mod h1:FLPqc6j+Ki4BU591ie1oL6qBQGu2Bl/tZ9ullr3+Kg0= +codeberg.org/ac/base62 v0.0.0-20210305150220-e793b546833a h1:U6cY/g6VSiy59vuvnBU6J/eSir0qVg4BeTnCDLaX+20= +codeberg.org/ac/base62 v0.0.0-20210305150220-e793b546833a/go.mod h1:ykEpkLT4JtH3I4Rb4gwkDsNLfgUg803qRDeIX88t3e8= dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU= fyne.io/fyne/v2 v2.1.4 h1:bt1+28++kAzRzPB0GM2EuSV4cnl8rXNX4cjfd8G06Rc= fyne.io/fyne/v2 v2.1.4/go.mod h1:p+E/Dh+wPW8JwR2DVcsZ9iXgR9ZKde80+Y+40Is54AQ= diff --git a/management/server/personal_access_token.go b/management/server/personal_access_token.go new file mode 100644 index 000000000..16ab232e4 --- /dev/null +++ b/management/server/personal_access_token.go @@ -0,0 +1,70 @@ +package server + +import ( + "crypto/sha256" + "fmt" + "hash/crc32" + "math/rand" + "time" + + "codeberg.org/ac/base62" +) + +type PersonalAccessToken struct { + Description string + HashedToken [32]byte + ExpirationDate time.Time + // scope could be added in future + CreatedBy User // should we add that? + CreatedAt time.Time + LastUsed time.Time +} + +const ( + // IEEE is by far and away the most common CRC-32 polynomial. + // Used by ethernet (IEEE 802.3), v.42, fddi, gzip, zip, png, ... + IEEE = 0xedb88320 + // Castagnoli polynomial, used in iSCSI. + // Has better error detection characteristics than IEEE. + // https://dx.doi.org/10.1109/26.231911 + Castagnoli = 0x82f63b78 + // Koopman polynomial. + // Also has better error detection characteristics than IEEE. + // https://dx.doi.org/10.1109/DSN.2002.1028931 + Koopman = 0xeb31d82e +) + +func CreateNewPAT(description string, expirationInDays int, createdBy User) (*PersonalAccessToken, string) { + hashedToken, plainToken := generateNewToken() + currentTime := time.Now().UTC() + return &PersonalAccessToken{ + Description: description, + HashedToken: hashedToken, + ExpirationDate: currentTime.AddDate(0, 0, expirationInDays), + CreatedBy: createdBy, + CreatedAt: currentTime, + LastUsed: currentTime, // using creation time as nil not possible + }, plainToken +} + +func generateNewToken() ([32]byte, string) { + token := randStringRunes(30) + + crc32q := crc32.MakeTable(IEEE) + checksum := crc32.Checksum([]byte(token), crc32q) + encodedChecksum := base62.Encode(checksum) + paddedChecksum := fmt.Sprintf("%06s", encodedChecksum) + plainToken := "nbp_" + token + paddedChecksum + hashedToken := sha256.Sum256([]byte(plainToken)) + return hashedToken, plainToken +} + +var letterRunes = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789") + +func randStringRunes(n int) string { + b := make([]rune, n) + for i := range b { + b[i] = letterRunes[rand.Intn(len(letterRunes))] + } + return string(b) +} diff --git a/management/server/personal_access_token_test.go b/management/server/personal_access_token_test.go new file mode 100644 index 000000000..375533068 --- /dev/null +++ b/management/server/personal_access_token_test.go @@ -0,0 +1,41 @@ +package server + +import ( + "crypto/sha256" + "hash/crc32" + "strings" + "testing" + + "codeberg.org/ac/base62" + "github.com/stretchr/testify/assert" +) + +func TestPAT_GenerateToken_Hashing(t *testing.T) { + hashedToken, plainToken := generateNewToken() + + assert.Equal(t, hashedToken, sha256.Sum256([]byte(plainToken))) +} + +func TestPAT_GenerateToken_Prefix(t *testing.T) { + _, plainToken := generateNewToken() + fourLetterPrefix := plainToken[:4] // should be 3 + assert.Equal(t, "nbp_", fourLetterPrefix) +} + +func TestPAT_GenerateToken_Checksum(t *testing.T) { + _, plainToken := generateNewToken() + tokenWithoutPrefix := strings.Split(plainToken, "_")[1] + if len(tokenWithoutPrefix) != 36 { + t.Fatal("Token has wrong length") + } + token := tokenWithoutPrefix[:len(tokenWithoutPrefix)-6] + tokenCheckSum := tokenWithoutPrefix[len(tokenWithoutPrefix)-6:] + + crc32q := crc32.MakeTable(IEEE) + expectedChecksum := crc32.Checksum([]byte(token), crc32q) + actualChecksum, err := base62.Decode(tokenCheckSum) + if err != nil { + t.Fatal(err) + } + assert.Equal(t, expectedChecksum, actualChecksum) +} diff --git a/management/server/user.go b/management/server/user.go index 767d39df2..8492e667f 100644 --- a/management/server/user.go +++ b/management/server/user.go @@ -2,12 +2,14 @@ package server import ( "fmt" + "strings" + + log "github.com/sirupsen/logrus" + "github.com/netbirdio/netbird/management/server/activity" "github.com/netbirdio/netbird/management/server/idp" "github.com/netbirdio/netbird/management/server/jwtclaims" "github.com/netbirdio/netbird/management/server/status" - log "github.com/sirupsen/logrus" - "strings" ) const ( @@ -44,6 +46,7 @@ type User struct { Role UserRole // AutoGroups is a list of Group IDs to auto-assign to peers registered by this user AutoGroups []string + PATs []PersonalAccessToken } // IsAdmin returns true if user is an admin, false otherwise From e0fc779f58db71e5ad82f28f173732716d69c6df Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Thu, 2 Mar 2023 16:19:31 +0100 Subject: [PATCH 02/16] add id to the PAT --- management/server/personal_access_token.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/management/server/personal_access_token.go b/management/server/personal_access_token.go index 16ab232e4..07f621aa9 100644 --- a/management/server/personal_access_token.go +++ b/management/server/personal_access_token.go @@ -8,9 +8,11 @@ import ( "time" "codeberg.org/ac/base62" + "github.com/rs/xid" ) type PersonalAccessToken struct { + ID string Description string HashedToken [32]byte ExpirationDate time.Time @@ -38,6 +40,7 @@ func CreateNewPAT(description string, expirationInDays int, createdBy User) (*Pe hashedToken, plainToken := generateNewToken() currentTime := time.Now().UTC() return &PersonalAccessToken{ + ID: xid.New().String(), Description: description, HashedToken: hashedToken, ExpirationDate: currentTime.AddDate(0, 0, expirationInDays), From b3f339c753b4b41dd58b61ce26ab38e43da3fea5 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Fri, 3 Mar 2023 14:51:33 +0100 Subject: [PATCH 03/16] improved code for token checksum calc --- management/server/personal_access_token.go | 17 +---------------- management/server/personal_access_token_test.go | 3 +-- 2 files changed, 2 insertions(+), 18 deletions(-) diff --git a/management/server/personal_access_token.go b/management/server/personal_access_token.go index 07f621aa9..622ab01b7 100644 --- a/management/server/personal_access_token.go +++ b/management/server/personal_access_token.go @@ -22,20 +22,6 @@ type PersonalAccessToken struct { LastUsed time.Time } -const ( - // IEEE is by far and away the most common CRC-32 polynomial. - // Used by ethernet (IEEE 802.3), v.42, fddi, gzip, zip, png, ... - IEEE = 0xedb88320 - // Castagnoli polynomial, used in iSCSI. - // Has better error detection characteristics than IEEE. - // https://dx.doi.org/10.1109/26.231911 - Castagnoli = 0x82f63b78 - // Koopman polynomial. - // Also has better error detection characteristics than IEEE. - // https://dx.doi.org/10.1109/DSN.2002.1028931 - Koopman = 0xeb31d82e -) - func CreateNewPAT(description string, expirationInDays int, createdBy User) (*PersonalAccessToken, string) { hashedToken, plainToken := generateNewToken() currentTime := time.Now().UTC() @@ -53,8 +39,7 @@ func CreateNewPAT(description string, expirationInDays int, createdBy User) (*Pe func generateNewToken() ([32]byte, string) { token := randStringRunes(30) - crc32q := crc32.MakeTable(IEEE) - checksum := crc32.Checksum([]byte(token), crc32q) + checksum := crc32.ChecksumIEEE([]byte(token)) encodedChecksum := base62.Encode(checksum) paddedChecksum := fmt.Sprintf("%06s", encodedChecksum) plainToken := "nbp_" + token + paddedChecksum diff --git a/management/server/personal_access_token_test.go b/management/server/personal_access_token_test.go index 375533068..623045c16 100644 --- a/management/server/personal_access_token_test.go +++ b/management/server/personal_access_token_test.go @@ -31,8 +31,7 @@ func TestPAT_GenerateToken_Checksum(t *testing.T) { token := tokenWithoutPrefix[:len(tokenWithoutPrefix)-6] tokenCheckSum := tokenWithoutPrefix[len(tokenWithoutPrefix)-6:] - crc32q := crc32.MakeTable(IEEE) - expectedChecksum := crc32.Checksum([]byte(token), crc32q) + expectedChecksum := crc32.ChecksumIEEE([]byte(token)) actualChecksum, err := base62.Decode(tokenCheckSum) if err != nil { t.Fatal(err) From 2f2d45de9e11f1bab7dae02eec97ec05b46076af Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Fri, 3 Mar 2023 16:37:39 +0100 Subject: [PATCH 04/16] updated PAT struct to only use user id instead of user --- management/server/personal_access_token.go | 4 ++-- management/server/personal_access_token_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/management/server/personal_access_token.go b/management/server/personal_access_token.go index 622ab01b7..c00aa8de3 100644 --- a/management/server/personal_access_token.go +++ b/management/server/personal_access_token.go @@ -17,12 +17,12 @@ type PersonalAccessToken struct { HashedToken [32]byte ExpirationDate time.Time // scope could be added in future - CreatedBy User // should we add that? + CreatedBy string CreatedAt time.Time LastUsed time.Time } -func CreateNewPAT(description string, expirationInDays int, createdBy User) (*PersonalAccessToken, string) { +func CreateNewPAT(description string, expirationInDays int, createdBy string) (*PersonalAccessToken, string) { hashedToken, plainToken := generateNewToken() currentTime := time.Now().UTC() return &PersonalAccessToken{ diff --git a/management/server/personal_access_token_test.go b/management/server/personal_access_token_test.go index 623045c16..e1e4cb520 100644 --- a/management/server/personal_access_token_test.go +++ b/management/server/personal_access_token_test.go @@ -18,7 +18,7 @@ func TestPAT_GenerateToken_Hashing(t *testing.T) { func TestPAT_GenerateToken_Prefix(t *testing.T) { _, plainToken := generateNewToken() - fourLetterPrefix := plainToken[:4] // should be 3 + fourLetterPrefix := plainToken[:4] assert.Equal(t, "nbp_", fourLetterPrefix) } From 95d87384ab941b69a29e733da30c07766f7dae73 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Mon, 6 Mar 2023 13:49:07 +0100 Subject: [PATCH 05/16] fixed some namings --- management/server/personal_access_token.go | 2 +- management/server/personal_access_token_test.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/management/server/personal_access_token.go b/management/server/personal_access_token.go index c00aa8de3..288489311 100644 --- a/management/server/personal_access_token.go +++ b/management/server/personal_access_token.go @@ -32,7 +32,7 @@ func CreateNewPAT(description string, expirationInDays int, createdBy string) (* ExpirationDate: currentTime.AddDate(0, 0, expirationInDays), CreatedBy: createdBy, CreatedAt: currentTime, - LastUsed: currentTime, // using creation time as nil not possible + LastUsed: currentTime, }, plainToken } diff --git a/management/server/personal_access_token_test.go b/management/server/personal_access_token_test.go index e1e4cb520..59d5fc116 100644 --- a/management/server/personal_access_token_test.go +++ b/management/server/personal_access_token_test.go @@ -18,8 +18,8 @@ func TestPAT_GenerateToken_Hashing(t *testing.T) { func TestPAT_GenerateToken_Prefix(t *testing.T) { _, plainToken := generateNewToken() - fourLetterPrefix := plainToken[:4] - assert.Equal(t, "nbp_", fourLetterPrefix) + fourCharPrefix := plainToken[:4] + assert.Equal(t, "nbp_", fourCharPrefix) } func TestPAT_GenerateToken_Checksum(t *testing.T) { @@ -28,10 +28,10 @@ func TestPAT_GenerateToken_Checksum(t *testing.T) { if len(tokenWithoutPrefix) != 36 { t.Fatal("Token has wrong length") } - token := tokenWithoutPrefix[:len(tokenWithoutPrefix)-6] + secret := tokenWithoutPrefix[:len(tokenWithoutPrefix)-6] tokenCheckSum := tokenWithoutPrefix[len(tokenWithoutPrefix)-6:] - expectedChecksum := crc32.ChecksumIEEE([]byte(token)) + expectedChecksum := crc32.ChecksumIEEE([]byte(secret)) actualChecksum, err := base62.Decode(tokenCheckSum) if err != nil { t.Fatal(err) From bcac5f7b327c382fce382e49ae82af4f9753198d Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Mon, 6 Mar 2023 13:51:32 +0100 Subject: [PATCH 06/16] fixed some namings --- management/server/personal_access_token.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/management/server/personal_access_token.go b/management/server/personal_access_token.go index 288489311..a4b382c9c 100644 --- a/management/server/personal_access_token.go +++ b/management/server/personal_access_token.go @@ -37,12 +37,12 @@ func CreateNewPAT(description string, expirationInDays int, createdBy string) (* } func generateNewToken() ([32]byte, string) { - token := randStringRunes(30) + secret := randStringRunes(30) - checksum := crc32.ChecksumIEEE([]byte(token)) + checksum := crc32.ChecksumIEEE([]byte(secret)) encodedChecksum := base62.Encode(checksum) paddedChecksum := fmt.Sprintf("%06s", encodedChecksum) - plainToken := "nbp_" + token + paddedChecksum + plainToken := "nbp_" + secret + paddedChecksum hashedToken := sha256.Sum256([]byte(plainToken)) return hashedToken, plainToken } From cb8abacadd198bc6e2d498b2ed3aa7576db40934 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Mon, 6 Mar 2023 14:01:18 +0100 Subject: [PATCH 07/16] extend User Copy function --- management/server/user.go | 1 + 1 file changed, 1 insertion(+) diff --git a/management/server/user.go b/management/server/user.go index 8492e667f..925beb2b9 100644 --- a/management/server/user.go +++ b/management/server/user.go @@ -98,6 +98,7 @@ func (u *User) Copy() *User { Id: u.Id, Role: u.Role, AutoGroups: autoGroups, + PATs: u.PATs, } } From ed470d7dbeec1393b3ce4f67b387dd514d2b336a Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Mon, 6 Mar 2023 14:46:04 +0100 Subject: [PATCH 08/16] add comments for exported functions --- management/server/personal_access_token.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/management/server/personal_access_token.go b/management/server/personal_access_token.go index a4b382c9c..291312794 100644 --- a/management/server/personal_access_token.go +++ b/management/server/personal_access_token.go @@ -11,6 +11,7 @@ import ( "github.com/rs/xid" ) +// PersonalAccessToken holds all information about a PAT including a hashed version of it for verification type PersonalAccessToken struct { ID string Description string @@ -22,6 +23,8 @@ type PersonalAccessToken struct { LastUsed time.Time } +// CreateNewPAT will generate a new PersonalAccessToken that can be assigned to a User. +// Additionally, it will return the token in plain text once, to give to the user and only save a hashed version func CreateNewPAT(description string, expirationInDays int, createdBy string) (*PersonalAccessToken, string) { hashedToken, plainToken := generateNewToken() currentTime := time.Now().UTC() From 83e7e30218521553fb0cc8978f1abdb6db253baf Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Wed, 8 Mar 2023 11:30:09 +0100 Subject: [PATCH 09/16] store hashedToken as string --- management/server/personal_access_token.go | 6 +++--- management/server/personal_access_token_test.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/management/server/personal_access_token.go b/management/server/personal_access_token.go index 291312794..e4be53a2c 100644 --- a/management/server/personal_access_token.go +++ b/management/server/personal_access_token.go @@ -15,7 +15,7 @@ import ( type PersonalAccessToken struct { ID string Description string - HashedToken [32]byte + HashedToken string ExpirationDate time.Time // scope could be added in future CreatedBy string @@ -39,7 +39,7 @@ func CreateNewPAT(description string, expirationInDays int, createdBy string) (* }, plainToken } -func generateNewToken() ([32]byte, string) { +func generateNewToken() (string, string) { secret := randStringRunes(30) checksum := crc32.ChecksumIEEE([]byte(secret)) @@ -47,7 +47,7 @@ func generateNewToken() ([32]byte, string) { paddedChecksum := fmt.Sprintf("%06s", encodedChecksum) plainToken := "nbp_" + secret + paddedChecksum hashedToken := sha256.Sum256([]byte(plainToken)) - return hashedToken, plainToken + return string(hashedToken[:]), plainToken } var letterRunes = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789") diff --git a/management/server/personal_access_token_test.go b/management/server/personal_access_token_test.go index 59d5fc116..712de1f72 100644 --- a/management/server/personal_access_token_test.go +++ b/management/server/personal_access_token_test.go @@ -12,8 +12,8 @@ import ( func TestPAT_GenerateToken_Hashing(t *testing.T) { hashedToken, plainToken := generateNewToken() - - assert.Equal(t, hashedToken, sha256.Sum256([]byte(plainToken))) + expectedToken := sha256.Sum256([]byte(plainToken)) + assert.Equal(t, hashedToken, string(expectedToken[:])) } func TestPAT_GenerateToken_Prefix(t *testing.T) { From 2b1965c9416c04a22e6cc7e7d5d72236d48e2be1 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Wed, 8 Mar 2023 11:36:03 +0100 Subject: [PATCH 10/16] switch secret generation to use lib --- management/server/personal_access_token.go | 30 ++++++++----------- .../server/personal_access_token_test.go | 6 ++-- 2 files changed, 16 insertions(+), 20 deletions(-) diff --git a/management/server/personal_access_token.go b/management/server/personal_access_token.go index e4be53a2c..d15bad079 100644 --- a/management/server/personal_access_token.go +++ b/management/server/personal_access_token.go @@ -4,10 +4,10 @@ import ( "crypto/sha256" "fmt" "hash/crc32" - "math/rand" "time" "codeberg.org/ac/base62" + b "github.com/hashicorp/go-secure-stdlib/base62" "github.com/rs/xid" ) @@ -25,8 +25,11 @@ type PersonalAccessToken struct { // CreateNewPAT will generate a new PersonalAccessToken that can be assigned to a User. // Additionally, it will return the token in plain text once, to give to the user and only save a hashed version -func CreateNewPAT(description string, expirationInDays int, createdBy string) (*PersonalAccessToken, string) { - hashedToken, plainToken := generateNewToken() +func CreateNewPAT(description string, expirationInDays int, createdBy string) (*PersonalAccessToken, string, error) { + hashedToken, plainToken, err := generateNewToken() + if err != nil { + return nil, "", err + } currentTime := time.Now().UTC() return &PersonalAccessToken{ ID: xid.New().String(), @@ -36,26 +39,19 @@ func CreateNewPAT(description string, expirationInDays int, createdBy string) (* CreatedBy: createdBy, CreatedAt: currentTime, LastUsed: currentTime, - }, plainToken + }, plainToken, nil } -func generateNewToken() (string, string) { - secret := randStringRunes(30) +func generateNewToken() (string, string, error) { + secret, err := b.Random(30) + if err != nil { + return "", "", err + } checksum := crc32.ChecksumIEEE([]byte(secret)) encodedChecksum := base62.Encode(checksum) paddedChecksum := fmt.Sprintf("%06s", encodedChecksum) plainToken := "nbp_" + secret + paddedChecksum hashedToken := sha256.Sum256([]byte(plainToken)) - return string(hashedToken[:]), plainToken -} - -var letterRunes = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789") - -func randStringRunes(n int) string { - b := make([]rune, n) - for i := range b { - b[i] = letterRunes[rand.Intn(len(letterRunes))] - } - return string(b) + return string(hashedToken[:]), plainToken, nil } diff --git a/management/server/personal_access_token_test.go b/management/server/personal_access_token_test.go index 712de1f72..cba321749 100644 --- a/management/server/personal_access_token_test.go +++ b/management/server/personal_access_token_test.go @@ -11,19 +11,19 @@ import ( ) func TestPAT_GenerateToken_Hashing(t *testing.T) { - hashedToken, plainToken := generateNewToken() + hashedToken, plainToken, _ := generateNewToken() expectedToken := sha256.Sum256([]byte(plainToken)) assert.Equal(t, hashedToken, string(expectedToken[:])) } func TestPAT_GenerateToken_Prefix(t *testing.T) { - _, plainToken := generateNewToken() + _, plainToken, _ := generateNewToken() fourCharPrefix := plainToken[:4] assert.Equal(t, "nbp_", fourCharPrefix) } func TestPAT_GenerateToken_Checksum(t *testing.T) { - _, plainToken := generateNewToken() + _, plainToken, _ := generateNewToken() tokenWithoutPrefix := strings.Split(plainToken, "_")[1] if len(tokenWithoutPrefix) != 36 { t.Fatal("Token has wrong length") From b4bb5c6bb886f6f19376be34bc86077e3b7225a7 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Wed, 8 Mar 2023 11:54:10 +0100 Subject: [PATCH 11/16] use const and do array copy --- go.mod | 2 ++ go.sum | 4 ++++ management/server/personal_access_token.go | 9 +++++++-- management/server/personal_access_token_test.go | 2 +- management/server/user.go | 4 +++- 5 files changed, 17 insertions(+), 4 deletions(-) diff --git a/go.mod b/go.mod index e34633892..e2e52fd53 100644 --- a/go.mod +++ b/go.mod @@ -38,6 +38,7 @@ require ( github.com/gliderlabs/ssh v0.3.4 github.com/godbus/dbus/v5 v5.1.0 github.com/google/nftables v0.0.0-20220808154552-2eca00135732 + github.com/hashicorp/go-secure-stdlib/base62 v0.1.2 github.com/hashicorp/go-version v1.6.0 github.com/libp2p/go-netroute v0.2.0 github.com/magiconair/properties v1.8.5 @@ -83,6 +84,7 @@ require ( github.com/goki/freetype v0.0.0-20181231101311-fa8a33aabaff // indirect github.com/google/go-cmp v0.5.9 // indirect github.com/google/gopacket v1.1.19 // indirect + github.com/hashicorp/go-uuid v1.0.2 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/josharian/native v0.0.0-20200817173448-b6b71def0850 // indirect github.com/matttproud/golang_protobuf_extensions v1.0.1 // indirect diff --git a/go.sum b/go.sum index e64b2bb38..0ca706447 100644 --- a/go.sum +++ b/go.sum @@ -277,6 +277,10 @@ github.com/gorilla/mux v1.8.0 h1:i40aqfkR1h2SlN9hojwV5ZA91wcXFOvkdNIeFDP5koI= github.com/gorilla/mux v1.8.0/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB71So= github.com/gorilla/websocket v1.4.2/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE= github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFbcEtjT1g+wF4CSlocrBnw= +github.com/hashicorp/go-secure-stdlib/base62 v0.1.2 h1:ET4pqyjiGmY09R5y+rSd70J2w45CtbWDNvGqWp/R3Ng= +github.com/hashicorp/go-secure-stdlib/base62 v0.1.2/go.mod h1:EdWO6czbmthiwZ3/PUsDV+UD1D5IRU4ActiaWGwt0Yw= +github.com/hashicorp/go-uuid v1.0.2 h1:cfejS+Tpcp13yd5nYHWDI6qVCny6wyX2Mt5SGur2IGE= +github.com/hashicorp/go-uuid v1.0.2/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro= github.com/hashicorp/go-version v1.6.0 h1:feTTfFNnjP967rlCxM/I9g701jU+RN74YKx2mOkIeek= github.com/hashicorp/go-version v1.6.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA= github.com/hashicorp/golang-lru v0.5.0/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8= diff --git a/management/server/personal_access_token.go b/management/server/personal_access_token.go index d15bad079..2dff75ec2 100644 --- a/management/server/personal_access_token.go +++ b/management/server/personal_access_token.go @@ -11,6 +11,11 @@ import ( "github.com/rs/xid" ) +const ( + PATPrefix = "nbp_" + secretLength = 30 +) + // PersonalAccessToken holds all information about a PAT including a hashed version of it for verification type PersonalAccessToken struct { ID string @@ -43,7 +48,7 @@ func CreateNewPAT(description string, expirationInDays int, createdBy string) (* } func generateNewToken() (string, string, error) { - secret, err := b.Random(30) + secret, err := b.Random(secretLength) if err != nil { return "", "", err } @@ -51,7 +56,7 @@ func generateNewToken() (string, string, error) { checksum := crc32.ChecksumIEEE([]byte(secret)) encodedChecksum := base62.Encode(checksum) paddedChecksum := fmt.Sprintf("%06s", encodedChecksum) - plainToken := "nbp_" + secret + paddedChecksum + plainToken := PATPrefix + secret + paddedChecksum hashedToken := sha256.Sum256([]byte(plainToken)) return string(hashedToken[:]), plainToken, nil } diff --git a/management/server/personal_access_token_test.go b/management/server/personal_access_token_test.go index cba321749..a4e02f750 100644 --- a/management/server/personal_access_token_test.go +++ b/management/server/personal_access_token_test.go @@ -19,7 +19,7 @@ func TestPAT_GenerateToken_Hashing(t *testing.T) { func TestPAT_GenerateToken_Prefix(t *testing.T) { _, plainToken, _ := generateNewToken() fourCharPrefix := plainToken[:4] - assert.Equal(t, "nbp_", fourCharPrefix) + assert.Equal(t, PATPrefix, fourCharPrefix) } func TestPAT_GenerateToken_Checksum(t *testing.T) { diff --git a/management/server/user.go b/management/server/user.go index 925beb2b9..369df5238 100644 --- a/management/server/user.go +++ b/management/server/user.go @@ -94,11 +94,13 @@ func (u *User) toUserInfo(userData *idp.UserData) (*UserInfo, error) { func (u *User) Copy() *User { autoGroups := make([]string, 0) autoGroups = append(autoGroups, u.AutoGroups...) + pats := make([]PersonalAccessToken, 0) + pats = append(pats, u.PATs...) return &User{ Id: u.Id, Role: u.Role, AutoGroups: autoGroups, - PATs: u.PATs, + PATs: pats, } } From c4d9b766344903f66fca4eeff81560e990f1b65e Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Wed, 8 Mar 2023 12:09:22 +0100 Subject: [PATCH 12/16] add comment for exported const --- management/server/personal_access_token.go | 1 + 1 file changed, 1 insertion(+) diff --git a/management/server/personal_access_token.go b/management/server/personal_access_token.go index 2dff75ec2..e7ee05dad 100644 --- a/management/server/personal_access_token.go +++ b/management/server/personal_access_token.go @@ -12,6 +12,7 @@ import ( ) const ( + // PATPrefix is the globally used, 4 char prefix for personal access tokens PATPrefix = "nbp_" secretLength = 30 ) From 62de0829618a5669064fc890c5c1ab48b9544619 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Wed, 8 Mar 2023 12:21:44 +0100 Subject: [PATCH 13/16] fix account test --- management/server/account_test.go | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/management/server/account_test.go b/management/server/account_test.go index 1d672e1b7..4cac2a6bb 100644 --- a/management/server/account_test.go +++ b/management/server/account_test.go @@ -2,19 +2,21 @@ package server import ( "fmt" - nbdns "github.com/netbirdio/netbird/dns" - "github.com/netbirdio/netbird/management/server/activity" - "github.com/netbirdio/netbird/route" "net" "reflect" "sync" "testing" "time" - "github.com/netbirdio/netbird/management/server/jwtclaims" + nbdns "github.com/netbirdio/netbird/dns" + "github.com/netbirdio/netbird/management/server/activity" + "github.com/netbirdio/netbird/route" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.zx2c4.com/wireguard/wgctrl/wgtypes" + + "github.com/netbirdio/netbird/management/server/jwtclaims" ) func verifyCanAddPeerToAccount(t *testing.T, manager AccountManager, account *Account, userID string) { @@ -1229,6 +1231,17 @@ func TestAccount_Copy(t *testing.T) { Id: "user1", Role: UserRoleAdmin, AutoGroups: []string{"group1"}, + PATs: []PersonalAccessToken{ + { + ID: "pat1", + Description: "First PAT", + HashedToken: "SoMeHaShEdToKeN", + ExpirationDate: time.Now().AddDate(0, 0, 7), + CreatedBy: "user1", + CreatedAt: time.Now(), + LastUsed: time.Now(), + }, + }, }, }, Groups: map[string]*Group{ From bc3cec23ec1ba3a426f436e65ae1519f918aab5c Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Thu, 16 Mar 2023 11:32:55 +0100 Subject: [PATCH 14/16] use slice copy --- management/server/user.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/management/server/user.go b/management/server/user.go index 369df5238..a9aaa1b61 100644 --- a/management/server/user.go +++ b/management/server/user.go @@ -92,10 +92,10 @@ func (u *User) toUserInfo(userData *idp.UserData) (*UserInfo, error) { // Copy the user func (u *User) Copy() *User { - autoGroups := make([]string, 0) - autoGroups = append(autoGroups, u.AutoGroups...) - pats := make([]PersonalAccessToken, 0) - pats = append(pats, u.PATs...) + autoGroups := make([]string, len(u.AutoGroups)) + copy(autoGroups, u.AutoGroups) + pats := make([]PersonalAccessToken, len(u.PATs)) + copy(pats, u.PATs) return &User{ Id: u.Id, Role: u.Role, From 1d4dfa41d22c79687b2500805403b14be2b9e17d Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Thu, 16 Mar 2023 11:46:53 +0100 Subject: [PATCH 15/16] clean dependencies --- go.sum | 1 - 1 file changed, 1 deletion(-) diff --git a/go.sum b/go.sum index f878c3235..e50c4a033 100644 --- a/go.sum +++ b/go.sum @@ -283,7 +283,6 @@ github.com/gopherjs/gopherjs v0.0.0-20220410123724-9e86199038b0 h1:fWY+zXdWhvWnd github.com/gorilla/mux v1.8.0 h1:i40aqfkR1h2SlN9hojwV5ZA91wcXFOvkdNIeFDP5koI= github.com/gorilla/mux v1.8.0/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB71So= github.com/gorilla/websocket v1.4.2/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE= -github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFbcEtjT1g+wF4CSlocrBnw= github.com/hashicorp/go-secure-stdlib/base62 v0.1.2 h1:ET4pqyjiGmY09R5y+rSd70J2w45CtbWDNvGqWp/R3Ng= github.com/hashicorp/go-secure-stdlib/base62 v0.1.2/go.mod h1:EdWO6czbmthiwZ3/PUsDV+UD1D5IRU4ActiaWGwt0Yw= github.com/hashicorp/go-uuid v1.0.2 h1:cfejS+Tpcp13yd5nYHWDI6qVCny6wyX2Mt5SGur2IGE= From 3b42d5e48a8e87b9632ea6e046c855dec3594e3e Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Thu, 16 Mar 2023 11:59:12 +0100 Subject: [PATCH 16/16] fix imports after merge --- management/server/account_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/management/server/account_test.go b/management/server/account_test.go index f4c3b186d..e40d5e5b8 100644 --- a/management/server/account_test.go +++ b/management/server/account_test.go @@ -12,8 +12,6 @@ import ( "github.com/netbirdio/netbird/management/server/activity" "github.com/netbirdio/netbird/route" - - "github.com/netbirdio/netbird/management/server/jwtclaims" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.zx2c4.com/wireguard/wgctrl/wgtypes"