From de8608f99fb2c1a47ad6366c573518ef3b1ca0ad Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Tue, 21 Mar 2023 16:02:19 +0100 Subject: [PATCH 01/18] add rest endpoints and update openapi doc --- management/server/http/api/openapi.yml | 176 +++++++++++++++++++ management/server/http/api/types.gen.go | 33 ++++ management/server/http/handler.go | 9 + management/server/http/pat_handler.go | 187 +++++++++++++++++++++ management/server/http/pat_handler_test.go | 37 ++++ 5 files changed, 442 insertions(+) create mode 100644 management/server/http/pat_handler.go create mode 100644 management/server/http/pat_handler_test.go diff --git a/management/server/http/api/openapi.yml b/management/server/http/api/openapi.yml index b3d954a4d..3f742b850 100644 --- a/management/server/http/api/openapi.yml +++ b/management/server/http/api/openapi.yml @@ -6,6 +6,8 @@ info: tags: - name: Users description: Interact with and view information about users. + - name: Tokens + description: Interact with and view information about tokens. - name: Peers description: Interact with and view information about peers. - name: Setup Keys @@ -284,6 +286,53 @@ components: - revoked - auto_groups - usage_limit + PersonalAccessToken: + type: object + properties: + id: + description: ID of a token + type: string + description: + description: Description of the token + type: string +# hashed_token: +# description: Hashed representation of the token +# type: string + expiration_date: + description: Date the token expires + type: string + format: date-time + created_by: + description: User ID of the user who created the token + type: string + created_at: + description: Date the token was created + type: string + format: date-time + last_used: + description: Date the token was last used + type: string + format: date-time + required: + - id + - description +# - hashed_token + - expiration_date + - created_by + - created_at + - last_used + PersonalAccessTokenRequest: + type: object + properties: + description: + description: Description of the token + type: string + expires_in: + description: Expiration in days + type: integer + required: + - description + - expires_in GroupMinimum: type: object properties: @@ -848,6 +897,133 @@ paths: "$ref": "#/components/responses/forbidden" '500': "$ref": "#/components/responses/internal_error" + /api/users/{userId}/tokens: + get: + summary: Returns a list of all tokens for a user + tags: [ Tokens ] + security: + - BearerAuth: [] + parameters: + - in: path + name: userId + required: true + schema: + type: string + description: The User ID + responses: + '200': + description: A JSON Array of PersonalAccessTokens + content: + application/json: + schema: + type: array + items: + $ref: '#/components/schemas/PersonalAccessToken' + '400': + "$ref": "#/components/responses/bad_request" + '401': + "$ref": "#/components/responses/requires_authentication" + '403': + "$ref": "#/components/responses/forbidden" + '500': + "$ref": "#/components/responses/internal_error" + post: + summary: Create a new token + tags: [ Tokens ] + security: + - BearerAuth: [ ] + parameters: + - in: path + name: userId + required: true + schema: + type: string + description: The User ID + requestBody: + description: PersonalAccessToken create parameters + content: + application/json: + schema: + $ref: '#/components/schemas/PersonalAccessTokenRequest' + responses: + '200': + description: The token in plain text + content: + text/plain: + schema: + type: string + '400': + "$ref": "#/components/responses/bad_request" + '401': + "$ref": "#/components/responses/requires_authentication" + '403': + "$ref": "#/components/responses/forbidden" + '500': + "$ref": "#/components/responses/internal_error" + /api/users/{userId}/tokens/{tokenId}: + get: + summary: Returns a specific token + tags: [ Tokens ] + security: + - BearerAuth: [ ] + parameters: + - in: path + name: userId + required: true + schema: + type: string + description: The User ID + - in: path + name: tokenId + required: true + schema: + type: string + description: The Token ID + responses: + '200': + description: A PersonalAccessTokens Object + content: + application/json: + schema: + $ref: '#/components/schemas/PersonalAccessToken' + '400': + "$ref": "#/components/responses/bad_request" + '401': + "$ref": "#/components/responses/requires_authentication" + '403': + "$ref": "#/components/responses/forbidden" + '500': + "$ref": "#/components/responses/internal_error" + delete: + summary: Delete a token + tags: [ Tokens ] + security: + - BearerAuth: [ ] + parameters: + - in: path + name: userId + required: true + schema: + type: string + description: The User ID + - in: path + name: tokenId + required: true + schema: + type: string + description: The Token ID + responses: + '200': + description: Delete status code + content: { } + '400': + "$ref": "#/components/responses/bad_request" + '401': + "$ref": "#/components/responses/requires_authentication" + '403': + "$ref": "#/components/responses/forbidden" + '500': + "$ref": "#/components/responses/internal_error" /api/peers: get: summary: Returns a list of all peers diff --git a/management/server/http/api/types.gen.go b/management/server/http/api/types.gen.go index 372ecd1a7..76c128d55 100644 --- a/management/server/http/api/types.gen.go +++ b/management/server/http/api/types.gen.go @@ -379,6 +379,36 @@ type PeerMinimum struct { Name string `json:"name"` } +// PersonalAccessToken defines model for PersonalAccessToken. +type PersonalAccessToken struct { + // CreatedAt Date the token was created + CreatedAt time.Time `json:"created_at"` + + // CreatedBy User ID of the user who created the token + CreatedBy string `json:"created_by"` + + // Description Description of the token + Description string `json:"description"` + + // ExpirationDate Date the token expires + ExpirationDate time.Time `json:"expiration_date"` + + // Id ID of a token + Id string `json:"id"` + + // LastUsed Date the token was last used + LastUsed time.Time `json:"last_used"` +} + +// PersonalAccessTokenRequest defines model for PersonalAccessTokenRequest. +type PersonalAccessTokenRequest struct { + // Description Description of the token + Description string `json:"description"` + + // ExpiresIn Expiration in days + ExpiresIn int `json:"expires_in"` +} + // Policy defines model for Policy. type Policy struct { // Description Policy friendly description @@ -808,3 +838,6 @@ type PostApiUsersJSONRequestBody = UserCreateRequest // PutApiUsersIdJSONRequestBody defines body for PutApiUsersId for application/json ContentType. type PutApiUsersIdJSONRequestBody = UserRequest + +// PostApiUsersUserIdTokensJSONRequestBody defines body for PostApiUsersUserIdTokens for application/json ContentType. +type PostApiUsersUserIdTokensJSONRequestBody = PersonalAccessTokenRequest diff --git a/management/server/http/handler.go b/management/server/http/handler.go index 90f62e700..e2ed927a3 100644 --- a/management/server/http/handler.go +++ b/management/server/http/handler.go @@ -57,6 +57,7 @@ func APIHandler(accountManager s.AccountManager, appMetrics telemetry.AppMetrics api.addAccountsEndpoint() api.addPeersEndpoint() api.addUsersEndpoint() + api.addUsersTokensEndpoint() api.addSetupKeysEndpoint() api.addRulesEndpoint() api.addPoliciesEndpoint() @@ -110,6 +111,14 @@ func (apiHandler *apiHandler) addUsersEndpoint() { apiHandler.Router.HandleFunc("/users", userHandler.CreateUser).Methods("POST", "OPTIONS") } +func (apiHandler *apiHandler) addUsersTokensEndpoint() { + tokenHandler := NewPATsHandler(apiHandler.AccountManager, apiHandler.AuthCfg) + apiHandler.Router.HandleFunc("/users/{userId}/tokens", tokenHandler.GetAllTokens).Methods("GET", "OPTIONS") + apiHandler.Router.HandleFunc("/users/{userId}/tokens", tokenHandler.CreateToken).Methods("POST", "OPTIONS") + apiHandler.Router.HandleFunc("/users/{userId}/tokens/{tokenId}", tokenHandler.GetToken).Methods("GET", "OPTIONS") + apiHandler.Router.HandleFunc("/users/{userId}/tokens/{tokenId}", tokenHandler.DeleteToken).Methods("DELETE", "OPTIONS") +} + func (apiHandler *apiHandler) addSetupKeysEndpoint() { keysHandler := NewSetupKeysHandler(apiHandler.AccountManager, apiHandler.AuthCfg) apiHandler.Router.HandleFunc("/setup-keys", keysHandler.GetAllSetupKeys).Methods("GET", "OPTIONS") diff --git a/management/server/http/pat_handler.go b/management/server/http/pat_handler.go new file mode 100644 index 000000000..8cdef141a --- /dev/null +++ b/management/server/http/pat_handler.go @@ -0,0 +1,187 @@ +package http + +import ( + "encoding/json" + "net/http" + + "github.com/gorilla/mux" + + "github.com/netbirdio/netbird/management/server" + "github.com/netbirdio/netbird/management/server/http/api" + "github.com/netbirdio/netbird/management/server/http/util" + "github.com/netbirdio/netbird/management/server/jwtclaims" + "github.com/netbirdio/netbird/management/server/status" +) + +// PATHandler is the nameserver group handler of the account +type PATHandler struct { + accountManager server.AccountManager + claimsExtractor *jwtclaims.ClaimsExtractor +} + +func NewPATsHandler(accountManager server.AccountManager, authCfg AuthCfg) *PATHandler { + return &PATHandler{ + accountManager: accountManager, + claimsExtractor: jwtclaims.NewClaimsExtractor( + jwtclaims.WithAudience(authCfg.Audience), + jwtclaims.WithUserIDClaim(authCfg.UserIDClaim), + ), + } +} + +func (h *PATHandler) GetAllTokens(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + util.WriteErrorResponse("wrong HTTP method", http.StatusMethodNotAllowed, w) + return + } + + claims := h.claimsExtractor.FromRequestContext(r) + account, user, err := h.accountManager.GetAccountFromToken(claims) + if err != nil { + util.WriteError(err, w) + return + } + + vars := mux.Vars(r) + userID := vars["userId"] + if len(userID) == 0 { + util.WriteError(status.Errorf(status.InvalidArgument, "invalid user ID"), w) + return + } + if userID != user.Id { + util.WriteErrorResponse("User not authorized to get tokens", http.StatusUnauthorized, w) + return + } + + var pats []*api.PersonalAccessToken + for _, pat := range account.Users[userID].PATs { + pats = append(pats, toPATResponse(pat)) + } + + util.WriteJSONObject(w, pats) +} + +func (h *PATHandler) GetToken(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + util.WriteErrorResponse("wrong HTTP method", http.StatusMethodNotAllowed, w) + return + } + + claims := h.claimsExtractor.FromRequestContext(r) + account, user, err := h.accountManager.GetAccountFromToken(claims) + if err != nil { + util.WriteError(err, w) + return + } + + vars := mux.Vars(r) + userID := vars["userId"] + if len(userID) == 0 { + util.WriteError(status.Errorf(status.InvalidArgument, "invalid user ID"), w) + return + } + if userID != user.Id { + util.WriteErrorResponse("User not authorized to get token", http.StatusUnauthorized, w) + return + } + + tokenID := vars["tokenId"] + if len(tokenID) == 0 { + util.WriteError(status.Errorf(status.InvalidArgument, "invalid token ID"), w) + return + } + + pat := account.Users[userID].PATs[tokenID] + util.WriteJSONObject(w, toPATResponse(pat)) +} + +func (h *PATHandler) CreateToken(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPut { + util.WriteErrorResponse("wrong HTTP method", http.StatusMethodNotAllowed, w) + return + } + + claims := h.claimsExtractor.FromRequestContext(r) + account, user, err := h.accountManager.GetAccountFromToken(claims) + if err != nil { + util.WriteError(err, w) + return + } + + vars := mux.Vars(r) + userID := vars["userId"] + if len(userID) == 0 { + util.WriteError(status.Errorf(status.InvalidArgument, "invalid user ID"), w) + return + } + if userID != user.Id { + util.WriteErrorResponse("User not authorized to create token", http.StatusUnauthorized, w) + return + } + + var req api.PostApiUsersUserIdTokensJSONRequestBody + err = json.NewDecoder(r.Body).Decode(&req) + if err != nil { + util.WriteErrorResponse("couldn't parse JSON request", http.StatusBadRequest, w) + return + } + + pat, plainToken, err := server.CreateNewPAT(req.Description, req.ExpiresIn, user.Id) + err = h.accountManager.AddPATToUser(account.Id, userID, pat) + if err != nil { + util.WriteError(err, w) + return + } + + util.WriteJSONObject(w, plainToken) +} + +func (h *PATHandler) DeleteToken(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodDelete { + util.WriteErrorResponse("wrong HTTP method", http.StatusMethodNotAllowed, w) + return + } + + claims := h.claimsExtractor.FromRequestContext(r) + account, user, err := h.accountManager.GetAccountFromToken(claims) + if err != nil { + util.WriteError(err, w) + return + } + + vars := mux.Vars(r) + userID := vars["userId"] + if len(userID) == 0 { + util.WriteError(status.Errorf(status.InvalidArgument, "invalid user ID"), w) + return + } + if userID != user.Id { + util.WriteErrorResponse("User not authorized to delete token", http.StatusUnauthorized, w) + return + } + + tokenID := vars["tokenId"] + if len(tokenID) == 0 { + util.WriteError(status.Errorf(status.InvalidArgument, "invalid token ID"), w) + return + } + + err = h.accountManager.DeletePAT(account.Id, userID, tokenID) + if err != nil { + util.WriteError(err, w) + return + } + + util.WriteJSONObject(w, "") +} + +func toPATResponse(pat *server.PersonalAccessToken) *api.PersonalAccessToken { + return &api.PersonalAccessToken{ + CreatedAt: pat.CreatedAt, + CreatedBy: pat.CreatedBy, + Description: pat.Description, + ExpirationDate: pat.ExpirationDate, + Id: pat.ID, + LastUsed: pat.LastUsed, + } +} diff --git a/management/server/http/pat_handler_test.go b/management/server/http/pat_handler_test.go new file mode 100644 index 000000000..3d32e7c30 --- /dev/null +++ b/management/server/http/pat_handler_test.go @@ -0,0 +1,37 @@ +package http + +import ( + "net/http" + + "github.com/netbirdio/netbird/management/server" + "github.com/netbirdio/netbird/management/server/jwtclaims" + "github.com/netbirdio/netbird/management/server/mock_server" + "github.com/netbirdio/netbird/management/server/status" +) + +func initPATTestData() *PATHandler { + return &PATHandler{ + accountManager: &mock_server.MockAccountManager{ + + AddPATToUserFunc: func(accountID string, userID string, pat *server.PersonalAccessToken) error { + if nsGroupID == existingNSGroupID { + return baseExistingNSGroup.Copy(), nil + } + return nil, status.Errorf(status.NotFound, "nameserver group with ID %s not found", nsGroupID) + }, + + GetAccountFromTokenFunc: func(_ jwtclaims.AuthorizationClaims) (*server.Account, *server.User, error) { + return testingNSAccount, testingAccount.Users["test_user"], nil + }, + }, + claimsExtractor: jwtclaims.NewClaimsExtractor( + jwtclaims.WithFromRequestContext(func(r *http.Request) jwtclaims.AuthorizationClaims { + return jwtclaims.AuthorizationClaims{ + UserId: "test_user", + Domain: "hotmail.com", + AccountId: testNSGroupAccountID, + } + }), + ), + } +} From 9e74f30d2f1da8f24b5c2b563128e7a4374d1443 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Mon, 27 Mar 2023 15:19:19 +0200 Subject: [PATCH 02/18] fix delete token parameter lookup --- management/server/user.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/management/server/user.go b/management/server/user.go index c3011c317..572843fff 100644 --- a/management/server/user.go +++ b/management/server/user.go @@ -228,7 +228,7 @@ func (am *DefaultAccountManager) DeletePAT(accountID string, userID string, toke return status.Errorf(status.NotFound, "user not found") } - pat := user.PATs["tokenID"] + pat := user.PATs[tokenID] if pat == nil { return status.Errorf(status.NotFound, "PAT not found") } From c65a9341077cee4c2f98784ae103e4e8a7bb7917 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Mon, 27 Mar 2023 16:28:49 +0200 Subject: [PATCH 03/18] refactor to use name instead of description --- management/server/account_test.go | 2 +- management/server/http/api/openapi.yml | 16 ++++++--------- management/server/http/api/types.gen.go | 12 +++++------ management/server/http/pat_handler.go | 24 ++-------------------- management/server/personal_access_token.go | 6 +++--- 5 files changed, 18 insertions(+), 42 deletions(-) diff --git a/management/server/account_test.go b/management/server/account_test.go index 5b4b1cc17..57b1cf3da 100644 --- a/management/server/account_test.go +++ b/management/server/account_test.go @@ -1245,7 +1245,7 @@ func TestAccount_Copy(t *testing.T) { PATs: map[string]*PersonalAccessToken{ "pat1": { ID: "pat1", - Description: "First PAT", + Name: "First PAT", HashedToken: "SoMeHaShEdToKeN", ExpirationDate: time.Now().AddDate(0, 0, 7), CreatedBy: "user1", diff --git a/management/server/http/api/openapi.yml b/management/server/http/api/openapi.yml index 3f742b850..c9a373411 100644 --- a/management/server/http/api/openapi.yml +++ b/management/server/http/api/openapi.yml @@ -292,12 +292,9 @@ components: id: description: ID of a token type: string - description: - description: Description of the token + name: + description: Name of the token type: string -# hashed_token: -# description: Hashed representation of the token -# type: string expiration_date: description: Date the token expires type: string @@ -315,8 +312,7 @@ components: format: date-time required: - id - - description -# - hashed_token + - name - expiration_date - created_by - created_at @@ -324,14 +320,14 @@ components: PersonalAccessTokenRequest: type: object properties: - description: - description: Description of the token + name: + description: Name of the token type: string expires_in: description: Expiration in days type: integer required: - - description + - name - expires_in GroupMinimum: type: object diff --git a/management/server/http/api/types.gen.go b/management/server/http/api/types.gen.go index 76c128d55..4727e471e 100644 --- a/management/server/http/api/types.gen.go +++ b/management/server/http/api/types.gen.go @@ -387,9 +387,6 @@ type PersonalAccessToken struct { // CreatedBy User ID of the user who created the token CreatedBy string `json:"created_by"` - // Description Description of the token - Description string `json:"description"` - // ExpirationDate Date the token expires ExpirationDate time.Time `json:"expiration_date"` @@ -398,15 +395,18 @@ type PersonalAccessToken struct { // LastUsed Date the token was last used LastUsed time.Time `json:"last_used"` + + // Name Name of the token + Name string `json:"name"` } // PersonalAccessTokenRequest defines model for PersonalAccessTokenRequest. type PersonalAccessTokenRequest struct { - // Description Description of the token - Description string `json:"description"` - // ExpiresIn Expiration in days ExpiresIn int `json:"expires_in"` + + // Name Name of the token + Name string `json:"name"` } // Policy defines model for Policy. diff --git a/management/server/http/pat_handler.go b/management/server/http/pat_handler.go index 8cdef141a..04c1f369f 100644 --- a/management/server/http/pat_handler.go +++ b/management/server/http/pat_handler.go @@ -30,11 +30,6 @@ func NewPATsHandler(accountManager server.AccountManager, authCfg AuthCfg) *PATH } func (h *PATHandler) GetAllTokens(w http.ResponseWriter, r *http.Request) { - if r.Method != http.MethodGet { - util.WriteErrorResponse("wrong HTTP method", http.StatusMethodNotAllowed, w) - return - } - claims := h.claimsExtractor.FromRequestContext(r) account, user, err := h.accountManager.GetAccountFromToken(claims) if err != nil { @@ -62,11 +57,6 @@ func (h *PATHandler) GetAllTokens(w http.ResponseWriter, r *http.Request) { } func (h *PATHandler) GetToken(w http.ResponseWriter, r *http.Request) { - if r.Method != http.MethodGet { - util.WriteErrorResponse("wrong HTTP method", http.StatusMethodNotAllowed, w) - return - } - claims := h.claimsExtractor.FromRequestContext(r) account, user, err := h.accountManager.GetAccountFromToken(claims) if err != nil { @@ -96,11 +86,6 @@ func (h *PATHandler) GetToken(w http.ResponseWriter, r *http.Request) { } func (h *PATHandler) CreateToken(w http.ResponseWriter, r *http.Request) { - if r.Method != http.MethodPut { - util.WriteErrorResponse("wrong HTTP method", http.StatusMethodNotAllowed, w) - return - } - claims := h.claimsExtractor.FromRequestContext(r) account, user, err := h.accountManager.GetAccountFromToken(claims) if err != nil { @@ -126,7 +111,7 @@ func (h *PATHandler) CreateToken(w http.ResponseWriter, r *http.Request) { return } - pat, plainToken, err := server.CreateNewPAT(req.Description, req.ExpiresIn, user.Id) + pat, plainToken, err := server.CreateNewPAT(req.Name, req.ExpiresIn, user.Id) err = h.accountManager.AddPATToUser(account.Id, userID, pat) if err != nil { util.WriteError(err, w) @@ -137,11 +122,6 @@ func (h *PATHandler) CreateToken(w http.ResponseWriter, r *http.Request) { } func (h *PATHandler) DeleteToken(w http.ResponseWriter, r *http.Request) { - if r.Method != http.MethodDelete { - util.WriteErrorResponse("wrong HTTP method", http.StatusMethodNotAllowed, w) - return - } - claims := h.claimsExtractor.FromRequestContext(r) account, user, err := h.accountManager.GetAccountFromToken(claims) if err != nil { @@ -179,7 +159,7 @@ func toPATResponse(pat *server.PersonalAccessToken) *api.PersonalAccessToken { return &api.PersonalAccessToken{ CreatedAt: pat.CreatedAt, CreatedBy: pat.CreatedBy, - Description: pat.Description, + Name: pat.Name, ExpirationDate: pat.ExpirationDate, Id: pat.ID, LastUsed: pat.LastUsed, diff --git a/management/server/personal_access_token.go b/management/server/personal_access_token.go index 7416a9e0b..817605dce 100644 --- a/management/server/personal_access_token.go +++ b/management/server/personal_access_token.go @@ -25,7 +25,7 @@ const ( // PersonalAccessToken holds all information about a PAT including a hashed version of it for verification type PersonalAccessToken struct { ID string - Description string + Name string HashedToken string ExpirationDate time.Time // scope could be added in future @@ -36,7 +36,7 @@ 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, error) { +func CreateNewPAT(name string, expirationInDays int, createdBy string) (*PersonalAccessToken, string, error) { hashedToken, plainToken, err := generateNewToken() if err != nil { return nil, "", err @@ -44,7 +44,7 @@ func CreateNewPAT(description string, expirationInDays int, createdBy string) (* currentTime := time.Now().UTC() return &PersonalAccessToken{ ID: xid.New().String(), - Description: description, + Name: name, HashedToken: hashedToken, ExpirationDate: currentTime.AddDate(0, 0, expirationInDays), CreatedBy: createdBy, From b66e984dddc384fc8a21c25da0e8c4d4b0a0449c Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Mon, 27 Mar 2023 17:28:24 +0200 Subject: [PATCH 04/18] set limits for expiration --- management/server/http/pat_handler.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/management/server/http/pat_handler.go b/management/server/http/pat_handler.go index 04c1f369f..c7bcb92bc 100644 --- a/management/server/http/pat_handler.go +++ b/management/server/http/pat_handler.go @@ -111,6 +111,16 @@ func (h *PATHandler) CreateToken(w http.ResponseWriter, r *http.Request) { return } + if req.Name == "" { + util.WriteErrorResponse("name can't be empty", status.InvalidArgument, w) + return + } + + if req.ExpiresIn < 1 || req.ExpiresIn > 365 { + util.WriteErrorResponse("expiration has to be between 1 and 365", status.InvalidArgument, w) + return + } + pat, plainToken, err := server.CreateNewPAT(req.Name, req.ExpiresIn, user.Id) err = h.accountManager.AddPATToUser(account.Id, userID, pat) if err != nil { From 6a75ec4ab775cb14d68251a71be6830f042c36d9 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Mon, 27 Mar 2023 17:42:05 +0200 Subject: [PATCH 05/18] fix http error codes --- management/server/http/pat_handler.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/management/server/http/pat_handler.go b/management/server/http/pat_handler.go index c7bcb92bc..034caa4e9 100644 --- a/management/server/http/pat_handler.go +++ b/management/server/http/pat_handler.go @@ -112,12 +112,12 @@ func (h *PATHandler) CreateToken(w http.ResponseWriter, r *http.Request) { } if req.Name == "" { - util.WriteErrorResponse("name can't be empty", status.InvalidArgument, w) + util.WriteErrorResponse("name can't be empty", http.StatusBadRequest, w) return } if req.ExpiresIn < 1 || req.ExpiresIn > 365 { - util.WriteErrorResponse("expiration has to be between 1 and 365", status.InvalidArgument, w) + util.WriteErrorResponse("expiration has to be between 1 and 365", http.StatusBadRequest, w) return } From 514403db370cedc82e02a17db98bdab73d0d3430 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Tue, 28 Mar 2023 14:47:15 +0200 Subject: [PATCH 06/18] use object instead of plain token for create response + handler test --- management/server/http/api/openapi.yml | 15 +- management/server/http/api/types.gen.go | 8 + management/server/http/pat_handler.go | 26 ++- management/server/http/pat_handler_test.go | 208 ++++++++++++++++++++- management/server/personal_access_token.go | 32 ++-- 5 files changed, 265 insertions(+), 24 deletions(-) diff --git a/management/server/http/api/openapi.yml b/management/server/http/api/openapi.yml index c9a373411..2668198c4 100644 --- a/management/server/http/api/openapi.yml +++ b/management/server/http/api/openapi.yml @@ -317,6 +317,17 @@ components: - created_by - created_at - last_used + PersonalAccessTokenGenerated: + type: object + properties: + plain_token: + description: Plain text representation of the generated token + type: string + personal_access_token: + $ref: '#/components/schemas/PersonalAccessToken' + required: + - plain_token + - personal_access_token PersonalAccessTokenRequest: type: object properties: @@ -945,9 +956,9 @@ paths: '200': description: The token in plain text content: - text/plain: + application/json: schema: - type: string + $ref: '#/components/schemas/PersonalAccessTokenGenerated' '400': "$ref": "#/components/responses/bad_request" '401': diff --git a/management/server/http/api/types.gen.go b/management/server/http/api/types.gen.go index 4727e471e..24abaf829 100644 --- a/management/server/http/api/types.gen.go +++ b/management/server/http/api/types.gen.go @@ -400,6 +400,14 @@ type PersonalAccessToken struct { Name string `json:"name"` } +// PersonalAccessTokenGenerated defines model for PersonalAccessTokenGenerated. +type PersonalAccessTokenGenerated struct { + PersonalAccessToken PersonalAccessToken `json:"personal_access_token"` + + // PlainToken Plain text representation of the generated token + PlainToken string `json:"plain_token"` +} + // PersonalAccessTokenRequest defines model for PersonalAccessTokenRequest. type PersonalAccessTokenRequest struct { // ExpiresIn Expiration in days diff --git a/management/server/http/pat_handler.go b/management/server/http/pat_handler.go index 034caa4e9..ab356b8c9 100644 --- a/management/server/http/pat_handler.go +++ b/management/server/http/pat_handler.go @@ -81,7 +81,18 @@ func (h *PATHandler) GetToken(w http.ResponseWriter, r *http.Request) { return } - pat := account.Users[userID].PATs[tokenID] + user = account.Users[userID] + if user == nil { + util.WriteError(status.Errorf(status.NotFound, "user not found"), w) + return + } + + pat := user.PATs[tokenID] + if pat == nil { + util.WriteError(status.Errorf(status.NotFound, "PAT not found"), w) + return + } + util.WriteJSONObject(w, toPATResponse(pat)) } @@ -121,14 +132,14 @@ func (h *PATHandler) CreateToken(w http.ResponseWriter, r *http.Request) { return } - pat, plainToken, err := server.CreateNewPAT(req.Name, req.ExpiresIn, user.Id) - err = h.accountManager.AddPATToUser(account.Id, userID, pat) + pat, err := server.CreateNewPAT(req.Name, req.ExpiresIn, user.Id) + err = h.accountManager.AddPATToUser(account.Id, userID, &pat.PersonalAccessToken) if err != nil { util.WriteError(err, w) return } - util.WriteJSONObject(w, plainToken) + util.WriteJSONObject(w, toPATGeneratedResponse(pat)) } func (h *PATHandler) DeleteToken(w http.ResponseWriter, r *http.Request) { @@ -175,3 +186,10 @@ func toPATResponse(pat *server.PersonalAccessToken) *api.PersonalAccessToken { LastUsed: pat.LastUsed, } } + +func toPATGeneratedResponse(pat *server.PersonalAccessTokenGenerated) *api.PersonalAccessTokenGenerated { + return &api.PersonalAccessTokenGenerated{ + PlainToken: pat.PlainToken, + PersonalAccessToken: *toPATResponse(&pat.PersonalAccessToken), + } +} diff --git a/management/server/http/pat_handler_test.go b/management/server/http/pat_handler_test.go index 3d32e7c30..c0a7185cb 100644 --- a/management/server/http/pat_handler_test.go +++ b/management/server/http/pat_handler_test.go @@ -1,37 +1,231 @@ package http import ( + "bytes" + "encoding/json" + "fmt" + "io" "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/google/go-cmp/cmp" + "github.com/gorilla/mux" + "github.com/stretchr/testify/assert" "github.com/netbirdio/netbird/management/server" + "github.com/netbirdio/netbird/management/server/http/api" "github.com/netbirdio/netbird/management/server/jwtclaims" "github.com/netbirdio/netbird/management/server/mock_server" "github.com/netbirdio/netbird/management/server/status" ) +const ( + existingAccountID = "existingAccountID" + notFoundAccountID = "notFoundAccountID" + existingUserID = "existingUserID" + notFoundUserID = "notFoundUserID" + existingTokenID = "existingTokenID" + notFoundTokenID = "notFoundTokenID" + domain = "hotmail.com" +) + +var testAccount = &server.Account{ + Id: existingAccountID, + Domain: domain, + Users: map[string]*server.User{ + existingUserID: { + Id: existingUserID, + PATs: map[string]*server.PersonalAccessToken{ + existingTokenID: { + ID: existingTokenID, + Name: "My first token", + HashedToken: "someHash", + ExpirationDate: time.Now().AddDate(0, 0, 7), + CreatedBy: existingUserID, + CreatedAt: time.Now(), + LastUsed: time.Now(), + }, + "token2": { + ID: "token2", + Name: "My second token", + HashedToken: "someOtherHash", + ExpirationDate: time.Now().AddDate(0, 0, 7), + CreatedBy: existingUserID, + CreatedAt: time.Now(), + LastUsed: time.Now(), + }, + }, + }, + }, +} + func initPATTestData() *PATHandler { return &PATHandler{ accountManager: &mock_server.MockAccountManager{ - AddPATToUserFunc: func(accountID string, userID string, pat *server.PersonalAccessToken) error { - if nsGroupID == existingNSGroupID { - return baseExistingNSGroup.Copy(), nil + if accountID != existingAccountID { + return status.Errorf(status.NotFound, "account with ID %s not found", accountID) } - return nil, status.Errorf(status.NotFound, "nameserver group with ID %s not found", nsGroupID) + if userID != existingUserID { + return status.Errorf(status.NotFound, "user with ID %s not found", userID) + } + return nil }, GetAccountFromTokenFunc: func(_ jwtclaims.AuthorizationClaims) (*server.Account, *server.User, error) { - return testingNSAccount, testingAccount.Users["test_user"], nil + return testAccount, testAccount.Users[existingUserID], nil + }, + DeletePATFunc: func(accountID string, userID string, tokenID string) error { + if accountID != existingAccountID { + return status.Errorf(status.NotFound, "account with ID %s not found", accountID) + } + if userID != existingUserID { + return status.Errorf(status.NotFound, "user with ID %s not found", userID) + } + if tokenID != existingTokenID { + return status.Errorf(status.NotFound, "token with ID %s not found", tokenID) + } + return nil }, }, claimsExtractor: jwtclaims.NewClaimsExtractor( jwtclaims.WithFromRequestContext(func(r *http.Request) jwtclaims.AuthorizationClaims { return jwtclaims.AuthorizationClaims{ - UserId: "test_user", - Domain: "hotmail.com", + UserId: existingUserID, + Domain: domain, AccountId: testNSGroupAccountID, } }), ), } } + +func TestTokenHandlers(t *testing.T) { + tt := []struct { + name string + expectedStatus int + expectedBody bool + requestType string + requestPath string + requestBody io.Reader + }{ + { + name: "Get All Tokens", + requestType: http.MethodGet, + requestPath: "/api/users/" + existingUserID + "/tokens", + expectedStatus: http.StatusOK, + expectedBody: true, + }, + { + name: "Get Existing Token", + requestType: http.MethodGet, + requestPath: "/api/users/" + existingUserID + "/tokens/" + existingTokenID, + expectedStatus: http.StatusOK, + expectedBody: true, + }, + { + name: "Get Not Existing Token", + requestType: http.MethodGet, + requestPath: "/api/users/" + existingUserID + "/tokens/" + notFoundTokenID, + expectedStatus: http.StatusNotFound, + }, + { + name: "Delete Existing Token", + requestType: http.MethodDelete, + requestPath: "/api/users/" + existingUserID + "/tokens/" + existingTokenID, + expectedStatus: http.StatusOK, + }, + { + name: "Delete Not Existing Token", + requestType: http.MethodDelete, + requestPath: "/api/users/" + existingUserID + "/tokens/" + notFoundTokenID, + expectedStatus: http.StatusNotFound, + }, + { + name: "POST OK", + requestType: http.MethodPost, + requestPath: "/api/users/" + existingUserID + "/tokens", + requestBody: bytes.NewBuffer( + []byte(fmt.Sprint("{\"name\":\"name\",\"expires_in\":7}"))), + expectedStatus: http.StatusOK, + expectedBody: true, + }, + } + + p := initPATTestData() + + for _, tc := range tt { + t.Run(tc.name, func(t *testing.T) { + recorder := httptest.NewRecorder() + req := httptest.NewRequest(tc.requestType, tc.requestPath, tc.requestBody) + + router := mux.NewRouter() + router.HandleFunc("/api/users/{userId}/tokens", p.GetAllTokens).Methods("GET") + router.HandleFunc("/api/users/{userId}/tokens/{tokenId}", p.GetToken).Methods("GET") + router.HandleFunc("/api/users/{userId}/tokens", p.CreateToken).Methods("POST") + router.HandleFunc("/api/users/{userId}/tokens/{tokenId}", p.DeleteToken).Methods("DELETE") + router.ServeHTTP(recorder, req) + + res := recorder.Result() + defer res.Body.Close() + + content, err := io.ReadAll(res.Body) + if err != nil { + t.Fatalf("I don't know what I expected; %v", err) + } + + if status := recorder.Code; status != tc.expectedStatus { + t.Errorf("handler returned wrong status code: got %v want %v, content: %s", + status, tc.expectedStatus, string(content)) + return + } + + if !tc.expectedBody { + return + } + + switch tc.name { + case "POST OK": + got := &api.PersonalAccessTokenGenerated{} + if err = json.Unmarshal(content, &got); err != nil { + t.Fatalf("Sent content is not in correct json format; %v", err) + } + assert.NotEmpty(t, got.PlainToken) + assert.Equal(t, server.PATLength, len(got.PlainToken)) + case "Get All Tokens": + expectedTokens := []api.PersonalAccessToken{ + toTokenResponse(*testAccount.Users[existingUserID].PATs[existingTokenID]), + toTokenResponse(*testAccount.Users[existingUserID].PATs["token2"]), + } + + var got []api.PersonalAccessToken + if err = json.Unmarshal(content, &got); err != nil { + t.Fatalf("Sent content is not in correct json format; %v", err) + } + assert.True(t, cmp.Equal(got, expectedTokens)) + case "Get Existing Token": + expectedToken := toTokenResponse(*testAccount.Users[existingUserID].PATs[existingTokenID]) + got := &api.PersonalAccessToken{} + if err = json.Unmarshal(content, &got); err != nil { + t.Fatalf("Sent content is not in correct json format; %v", err) + } + + assert.True(t, cmp.Equal(*got, expectedToken)) + } + + }) + } +} + +func toTokenResponse(serverToken server.PersonalAccessToken) api.PersonalAccessToken { + return api.PersonalAccessToken{ + Id: serverToken.ID, + Name: serverToken.Name, + CreatedAt: serverToken.CreatedAt, + LastUsed: serverToken.LastUsed, + CreatedBy: serverToken.CreatedBy, + ExpirationDate: serverToken.ExpirationDate, + } +} diff --git a/management/server/personal_access_token.go b/management/server/personal_access_token.go index 817605dce..6eab840b8 100644 --- a/management/server/personal_access_token.go +++ b/management/server/personal_access_token.go @@ -34,23 +34,33 @@ type PersonalAccessToken struct { LastUsed time.Time } +// PersonalAccessTokenGenerated holds the new PersonalAccessToken and the plain text version of it +type PersonalAccessTokenGenerated struct { + PlainToken string + PersonalAccessToken +} + // 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(name string, expirationInDays int, createdBy string) (*PersonalAccessToken, string, error) { +func CreateNewPAT(name string, expirationInDays int, createdBy string) (*PersonalAccessTokenGenerated, error) { hashedToken, plainToken, err := generateNewToken() if err != nil { - return nil, "", err + return nil, err } currentTime := time.Now().UTC() - return &PersonalAccessToken{ - ID: xid.New().String(), - Name: name, - HashedToken: hashedToken, - ExpirationDate: currentTime.AddDate(0, 0, expirationInDays), - CreatedBy: createdBy, - CreatedAt: currentTime, - LastUsed: currentTime, - }, plainToken, nil + return &PersonalAccessTokenGenerated{ + PersonalAccessToken: PersonalAccessToken{ + ID: xid.New().String(), + Name: name, + HashedToken: hashedToken, + ExpirationDate: currentTime.AddDate(0, 0, expirationInDays), + CreatedBy: createdBy, + CreatedAt: currentTime, + LastUsed: currentTime, + }, + PlainToken: plainToken, + }, nil + } func generateNewToken() (string, string, error) { From 42ba0765c80ba173e0fad6a53d732f02fbfd7a21 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Tue, 28 Mar 2023 14:54:06 +0200 Subject: [PATCH 07/18] fix linter --- go.mod | 2 +- management/server/http/pat_handler.go | 5 +++++ management/server/http/pat_handler_test.go | 3 +-- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index d6518467f..c74fee409 100644 --- a/go.mod +++ b/go.mod @@ -37,6 +37,7 @@ require ( github.com/getlantern/systray v1.2.1 github.com/gliderlabs/ssh v0.3.4 github.com/godbus/dbus/v5 v5.1.0 + github.com/google/go-cmp v0.5.9 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 @@ -89,7 +90,6 @@ require ( github.com/go-stack/stack v1.8.0 // indirect github.com/gobwas/glob v0.2.3 // indirect 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 diff --git a/management/server/http/pat_handler.go b/management/server/http/pat_handler.go index ab356b8c9..654ea44f8 100644 --- a/management/server/http/pat_handler.go +++ b/management/server/http/pat_handler.go @@ -133,6 +133,11 @@ func (h *PATHandler) CreateToken(w http.ResponseWriter, r *http.Request) { } pat, err := server.CreateNewPAT(req.Name, req.ExpiresIn, user.Id) + if err != nil { + util.WriteError(err, w) + return + } + err = h.accountManager.AddPATToUser(account.Id, userID, &pat.PersonalAccessToken) if err != nil { util.WriteError(err, w) diff --git a/management/server/http/pat_handler_test.go b/management/server/http/pat_handler_test.go index c0a7185cb..0d29f1ad4 100644 --- a/management/server/http/pat_handler_test.go +++ b/management/server/http/pat_handler_test.go @@ -3,7 +3,6 @@ package http import ( "bytes" "encoding/json" - "fmt" "io" "net/http" "net/http/httptest" @@ -148,7 +147,7 @@ func TestTokenHandlers(t *testing.T) { requestType: http.MethodPost, requestPath: "/api/users/" + existingUserID + "/tokens", requestBody: bytes.NewBuffer( - []byte(fmt.Sprint("{\"name\":\"name\",\"expires_in\":7}"))), + []byte("{\"name\":\"name\",\"expires_in\":7}")), expectedStatus: http.StatusOK, expectedBody: true, }, From 726ffb57405c08a45024ec72b04d57d44c60f0cd Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Wed, 29 Mar 2023 15:06:54 +0200 Subject: [PATCH 08/18] add comments for exported functions --- management/server/http/pat_handler.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/management/server/http/pat_handler.go b/management/server/http/pat_handler.go index 654ea44f8..7a8175fbf 100644 --- a/management/server/http/pat_handler.go +++ b/management/server/http/pat_handler.go @@ -19,6 +19,7 @@ type PATHandler struct { claimsExtractor *jwtclaims.ClaimsExtractor } +// NewPATsHandler creates a new PATHandler HTTP handler func NewPATsHandler(accountManager server.AccountManager, authCfg AuthCfg) *PATHandler { return &PATHandler{ accountManager: accountManager, @@ -29,6 +30,7 @@ func NewPATsHandler(accountManager server.AccountManager, authCfg AuthCfg) *PATH } } +// GetAllTokens is HTTP GET handler that returns a list of all personal access tokens for the given user func (h *PATHandler) GetAllTokens(w http.ResponseWriter, r *http.Request) { claims := h.claimsExtractor.FromRequestContext(r) account, user, err := h.accountManager.GetAccountFromToken(claims) @@ -56,6 +58,7 @@ func (h *PATHandler) GetAllTokens(w http.ResponseWriter, r *http.Request) { util.WriteJSONObject(w, pats) } +// GetToken is HTTP GET handler that returns a personal access token for the given user func (h *PATHandler) GetToken(w http.ResponseWriter, r *http.Request) { claims := h.claimsExtractor.FromRequestContext(r) account, user, err := h.accountManager.GetAccountFromToken(claims) @@ -96,6 +99,7 @@ func (h *PATHandler) GetToken(w http.ResponseWriter, r *http.Request) { util.WriteJSONObject(w, toPATResponse(pat)) } +// CreateToken is HTTP POST handler that creates a personal access token for the given user func (h *PATHandler) CreateToken(w http.ResponseWriter, r *http.Request) { claims := h.claimsExtractor.FromRequestContext(r) account, user, err := h.accountManager.GetAccountFromToken(claims) @@ -147,6 +151,7 @@ func (h *PATHandler) CreateToken(w http.ResponseWriter, r *http.Request) { util.WriteJSONObject(w, toPATGeneratedResponse(pat)) } +// DeleteToken is HTTP DELETE handler that deletes a personal access token for the given user func (h *PATHandler) DeleteToken(w http.ResponseWriter, r *http.Request) { claims := h.claimsExtractor.FromRequestContext(r) account, user, err := h.accountManager.GetAccountFromToken(claims) From c5942e6b33362684e7baf7dd56ea7c0cc7e1c2f6 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Wed, 29 Mar 2023 15:21:53 +0200 Subject: [PATCH 09/18] store hashed token base64 encoded --- management/server/account.go | 34 +++++++++++++------ management/server/account_test.go | 9 +++-- management/server/file_store.go | 12 ++++--- management/server/personal_access_token.go | 4 ++- .../server/personal_access_token_test.go | 4 ++- management/server/user_test.go | 2 +- 6 files changed, 43 insertions(+), 22 deletions(-) diff --git a/management/server/account.go b/management/server/account.go index 1d4c10721..55a5a0299 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -3,6 +3,7 @@ package server import ( "context" "crypto/sha256" + b64 "encoding/base64" "fmt" "hash/crc32" "math/rand" @@ -54,7 +55,7 @@ type AccountManager interface { GetSetupKey(accountID, userID, keyID string) (*SetupKey, error) GetAccountByUserOrAccountID(userID, accountID, domain string) (*Account, error) GetAccountFromToken(claims jwtclaims.AuthorizationClaims) (*Account, *User, error) - GetAccountFromPAT(pat string) (*Account, *User, error) + GetAccountFromPAT(pat string) (*Account, *User, *PersonalAccessToken, error) IsUserAdmin(claims jwtclaims.AuthorizationClaims) (bool, error) AccountExists(accountId string) (*bool, error) GetPeerByKey(peerKey string) (*Peer, error) @@ -1120,44 +1121,55 @@ func (am *DefaultAccountManager) redeemInvite(account *Account, userID string) e } // GetAccountFromPAT returns Account and User associated with a personal access token -func (am *DefaultAccountManager) GetAccountFromPAT(token string) (*Account, *User, error) { +func (am *DefaultAccountManager) GetAccountFromPAT(token string) (*Account, *User, *PersonalAccessToken, error) { if len(token) != PATLength { - return nil, nil, fmt.Errorf("token has wrong length") + return nil, nil, nil, fmt.Errorf("token has wrong length") } + log.Debugf("Token: %s", token) + prefix := token[:len(PATPrefix)] if prefix != PATPrefix { - return nil, nil, fmt.Errorf("token has wrong prefix") + return nil, nil, nil, fmt.Errorf("token has wrong prefix") } secret := token[len(PATPrefix) : len(PATPrefix)+PATSecretLength] encodedChecksum := token[len(PATPrefix)+PATSecretLength : len(PATPrefix)+PATSecretLength+PATChecksumLength] verificationChecksum, err := base62.Decode(encodedChecksum) if err != nil { - return nil, nil, fmt.Errorf("token checksum decoding failed: %w", err) + return nil, nil, nil, fmt.Errorf("token checksum decoding failed: %w", err) } secretChecksum := crc32.ChecksumIEEE([]byte(secret)) if secretChecksum != verificationChecksum { - return nil, nil, fmt.Errorf("token checksum does not match") + return nil, nil, nil, fmt.Errorf("token checksum does not match") } hashedToken := sha256.Sum256([]byte(token)) - tokenID, err := am.Store.GetTokenIDByHashedToken(string(hashedToken[:])) + encodedHashedToken := b64.StdEncoding.EncodeToString(hashedToken[:]) + tokenID, err := am.Store.GetTokenIDByHashedToken(encodedHashedToken) if err != nil { - return nil, nil, err + return nil, nil, nil, err } + log.Debugf("TokenID: %s", tokenID) user, err := am.Store.GetUserByTokenID(tokenID) + log.Debugf("User: %v", user) if err != nil { - return nil, nil, err + return nil, nil, nil, err } account, err := am.Store.GetAccountByUser(user.Id) if err != nil { - return nil, nil, err + return nil, nil, nil, err } - return account, user, nil + + pat := user.PATs[tokenID] + if pat == nil { + return nil, nil, nil, fmt.Errorf("personal access token not found") + } + + return account, user, pat, nil } // GetAccountFromToken returns an account associated with this token diff --git a/management/server/account_test.go b/management/server/account_test.go index 57b1cf3da..8eea04362 100644 --- a/management/server/account_test.go +++ b/management/server/account_test.go @@ -2,6 +2,7 @@ package server import ( "crypto/sha256" + b64 "encoding/base64" "fmt" "net" "reflect" @@ -465,12 +466,13 @@ func TestAccountManager_GetAccountFromPAT(t *testing.T) { token := "nbp_9999EUDNdkeusjentDLSJEn1902u84390W6W" hashedToken := sha256.Sum256([]byte(token)) + encodedHashedToken := b64.StdEncoding.EncodeToString(hashedToken[:]) account.Users["someUser"] = &User{ Id: "someUser", PATs: map[string]*PersonalAccessToken{ - "pat1": { + "tokenId": { ID: "tokenId", - HashedToken: string(hashedToken[:]), + HashedToken: encodedHashedToken, }, }, } @@ -483,13 +485,14 @@ func TestAccountManager_GetAccountFromPAT(t *testing.T) { Store: store, } - account, user, err := am.GetAccountFromPAT(token) + account, user, pat, err := am.GetAccountFromPAT(token) if err != nil { t.Fatalf("Error when getting Account from PAT: %s", err) } assert.Equal(t, "account_id", account.Id) assert.Equal(t, "someUser", user.Id) + assert.Equal(t, account.Users["someUser"].PATs["tokenId"], pat) } func TestAccountManager_PrivateAccount(t *testing.T) { diff --git a/management/server/file_store.go b/management/server/file_store.go index 4f8092cfb..b09dccf81 100644 --- a/management/server/file_store.go +++ b/management/server/file_store.go @@ -112,7 +112,7 @@ func restore(file string) (*FileStore, error) { store.UserID2AccountID[user.Id] = accountID for _, pat := range user.PATs { store.TokenID2UserID[pat.ID] = user.Id - store.HashedPAT2TokenID[pat.HashedToken[:]] = pat.ID + store.HashedPAT2TokenID[pat.HashedToken] = pat.ID } } @@ -268,7 +268,7 @@ func (s *FileStore) SaveAccount(account *Account) error { s.UserID2AccountID[user.Id] = accountCopy.Id for _, pat := range user.PATs { s.TokenID2UserID[pat.ID] = user.Id - s.HashedPAT2TokenID[pat.HashedToken[:]] = pat.ID + s.HashedPAT2TokenID[pat.HashedToken] = pat.ID } } @@ -349,11 +349,13 @@ func (s *FileStore) GetTokenIDByHashedToken(token string) (string, error) { s.mux.Lock() defer s.mux.Unlock() + log.Debugf("TOken still there: %v", token) + log.Debugf("TokenID2UserId %v", s.HashedPAT2TokenID) tokenID, ok := s.HashedPAT2TokenID[token] if !ok { return "", status.Errorf(status.NotFound, "tokenID not found: provided token doesn't exists") } - + log.Debugf("TokenID for token %s is %s", token, tokenID) return tokenID, nil } @@ -366,12 +368,12 @@ func (s *FileStore) GetUserByTokenID(tokenID string) (*User, error) { if !ok { return nil, status.Errorf(status.NotFound, "user not found: provided tokenID doesn't exists") } - + log.Debugf("UserID for tokenID %s is %s", tokenID, userID) accountID, ok := s.UserID2AccountID[userID] if !ok { return nil, status.Errorf(status.NotFound, "accountID not found: provided userID doesn't exists") } - + log.Debugf("AccountID for userID %s is %s", userID, accountID) account, err := s.getAccount(accountID) if err != nil { return nil, err diff --git a/management/server/personal_access_token.go b/management/server/personal_access_token.go index 6eab840b8..a7c55018f 100644 --- a/management/server/personal_access_token.go +++ b/management/server/personal_access_token.go @@ -2,6 +2,7 @@ package server import ( "crypto/sha256" + b64 "encoding/base64" "fmt" "hash/crc32" "time" @@ -74,5 +75,6 @@ func generateNewToken() (string, string, error) { paddedChecksum := fmt.Sprintf("%06s", encodedChecksum) plainToken := PATPrefix + secret + paddedChecksum hashedToken := sha256.Sum256([]byte(plainToken)) - return string(hashedToken[:]), plainToken, nil + encodedHashedToken := b64.StdEncoding.EncodeToString(hashedToken[:]) + return encodedHashedToken, plainToken, nil } diff --git a/management/server/personal_access_token_test.go b/management/server/personal_access_token_test.go index a4e02f750..03dd2ef4e 100644 --- a/management/server/personal_access_token_test.go +++ b/management/server/personal_access_token_test.go @@ -2,6 +2,7 @@ package server import ( "crypto/sha256" + b64 "encoding/base64" "hash/crc32" "strings" "testing" @@ -13,7 +14,8 @@ import ( func TestPAT_GenerateToken_Hashing(t *testing.T) { hashedToken, plainToken, _ := generateNewToken() expectedToken := sha256.Sum256([]byte(plainToken)) - assert.Equal(t, hashedToken, string(expectedToken[:])) + encodedExpectedToken := b64.StdEncoding.EncodeToString(expectedToken[:]) + assert.Equal(t, hashedToken, encodedExpectedToken) } func TestPAT_GenerateToken_Prefix(t *testing.T) { diff --git a/management/server/user_test.go b/management/server/user_test.go index 20f2ca4f1..ffbb71282 100644 --- a/management/server/user_test.go +++ b/management/server/user_test.go @@ -37,7 +37,7 @@ func TestUser_AddPATToUser(t *testing.T) { } fileStore := am.Store.(*FileStore) - tokenID := fileStore.HashedPAT2TokenID[mockToken[:]] + tokenID := fileStore.HashedPAT2TokenID[mockToken] if tokenID == "" { t.Fatal("GetTokenIDByHashedToken failed after adding PAT") From 0ca3d27a808b9ffb936e9a95a97d3f57faca9573 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Wed, 29 Mar 2023 15:25:44 +0200 Subject: [PATCH 10/18] update account mock --- management/server/mock_server/account_mock.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/management/server/mock_server/account_mock.go b/management/server/mock_server/account_mock.go index 2ae71d1a9..71870fd84 100644 --- a/management/server/mock_server/account_mock.go +++ b/management/server/mock_server/account_mock.go @@ -47,7 +47,7 @@ type MockAccountManager struct { DeletePolicyFunc func(accountID, policyID, userID string) error ListPoliciesFunc func(accountID, userID string) ([]*server.Policy, error) GetUsersFromAccountFunc func(accountID, userID string) ([]*server.UserInfo, error) - GetAccountFromPATFunc func(pat string) (*server.Account, *server.User, error) + GetAccountFromPATFunc func(pat string) (*server.Account, *server.User, *server.PersonalAccessToken, error) UpdatePeerMetaFunc func(peerID string, meta server.PeerSystemMeta) error UpdatePeerSSHKeyFunc func(peerID string, sshKey string) error UpdatePeerFunc func(accountID, userID string, peer *server.Peer) (*server.Peer, error) @@ -179,11 +179,11 @@ func (am *MockAccountManager) GetPeerByIP(accountId string, peerIP string) (*ser } // GetAccountFromPAT mock implementation of GetAccountFromPAT from server.AccountManager interface -func (am *MockAccountManager) GetAccountFromPAT(pat string) (*server.Account, *server.User, error) { +func (am *MockAccountManager) GetAccountFromPAT(pat string) (*server.Account, *server.User, *server.PersonalAccessToken, error) { if am.GetAccountFromPATFunc != nil { return am.GetAccountFromPATFunc(pat) } - return nil, nil, status.Errorf(codes.Unimplemented, "method GetAccountFromPAT is not implemented") + return nil, nil, nil, status.Errorf(codes.Unimplemented, "method GetAccountFromPAT is not implemented") } // AddPATToUser mock implementation of AddPATToUser from server.AccountManager interface From 3bab7451429f63f631f8f49bbba66efda906b0e7 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Wed, 29 Mar 2023 17:46:09 +0200 Subject: [PATCH 11/18] last_used can be nil --- management/server/http/api/openapi.yml | 1 - management/server/http/api/types.gen.go | 2 +- management/server/http/pat_handler.go | 7 ++++++- management/server/personal_access_token.go | 2 +- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/management/server/http/api/openapi.yml b/management/server/http/api/openapi.yml index 2668198c4..eaeb5693c 100644 --- a/management/server/http/api/openapi.yml +++ b/management/server/http/api/openapi.yml @@ -316,7 +316,6 @@ components: - expiration_date - created_by - created_at - - last_used PersonalAccessTokenGenerated: type: object properties: diff --git a/management/server/http/api/types.gen.go b/management/server/http/api/types.gen.go index 24abaf829..930a9df54 100644 --- a/management/server/http/api/types.gen.go +++ b/management/server/http/api/types.gen.go @@ -394,7 +394,7 @@ type PersonalAccessToken struct { Id string `json:"id"` // LastUsed Date the token was last used - LastUsed time.Time `json:"last_used"` + LastUsed *time.Time `json:"last_used,omitempty"` // Name Name of the token Name string `json:"name"` diff --git a/management/server/http/pat_handler.go b/management/server/http/pat_handler.go index 7a8175fbf..2f6cb1492 100644 --- a/management/server/http/pat_handler.go +++ b/management/server/http/pat_handler.go @@ -3,6 +3,7 @@ package http import ( "encoding/json" "net/http" + "time" "github.com/gorilla/mux" @@ -187,13 +188,17 @@ func (h *PATHandler) DeleteToken(w http.ResponseWriter, r *http.Request) { } func toPATResponse(pat *server.PersonalAccessToken) *api.PersonalAccessToken { + var lastUsed *time.Time + if !pat.LastUsed.IsZero() { + lastUsed = &pat.LastUsed + } return &api.PersonalAccessToken{ CreatedAt: pat.CreatedAt, CreatedBy: pat.CreatedBy, Name: pat.Name, ExpirationDate: pat.ExpirationDate, Id: pat.ID, - LastUsed: pat.LastUsed, + LastUsed: lastUsed, } } diff --git a/management/server/personal_access_token.go b/management/server/personal_access_token.go index a7c55018f..bdf34e9fd 100644 --- a/management/server/personal_access_token.go +++ b/management/server/personal_access_token.go @@ -57,7 +57,7 @@ func CreateNewPAT(name string, expirationInDays int, createdBy string) (*Persona ExpirationDate: currentTime.AddDate(0, 0, expirationInDays), CreatedBy: createdBy, CreatedAt: currentTime, - LastUsed: currentTime, + LastUsed: time.Time{}, }, PlainToken: plainToken, }, nil From 4ec6d5d20ba9b34661c3abcffff49093c56daa14 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Wed, 29 Mar 2023 18:23:10 +0200 Subject: [PATCH 12/18] remove debug logs --- management/server/file_store.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/management/server/file_store.go b/management/server/file_store.go index b09dccf81..9b4a9f47f 100644 --- a/management/server/file_store.go +++ b/management/server/file_store.go @@ -349,13 +349,11 @@ func (s *FileStore) GetTokenIDByHashedToken(token string) (string, error) { s.mux.Lock() defer s.mux.Unlock() - log.Debugf("TOken still there: %v", token) - log.Debugf("TokenID2UserId %v", s.HashedPAT2TokenID) tokenID, ok := s.HashedPAT2TokenID[token] if !ok { return "", status.Errorf(status.NotFound, "tokenID not found: provided token doesn't exists") } - log.Debugf("TokenID for token %s is %s", token, tokenID) + return tokenID, nil } @@ -368,12 +366,12 @@ func (s *FileStore) GetUserByTokenID(tokenID string) (*User, error) { if !ok { return nil, status.Errorf(status.NotFound, "user not found: provided tokenID doesn't exists") } - log.Debugf("UserID for tokenID %s is %s", tokenID, userID) + accountID, ok := s.UserID2AccountID[userID] if !ok { return nil, status.Errorf(status.NotFound, "accountID not found: provided userID doesn't exists") } - log.Debugf("AccountID for userID %s is %s", userID, accountID) + account, err := s.getAccount(accountID) if err != nil { return nil, err From 9746a7f61abe9fb9f0fce1682fed0766750a713e Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Wed, 29 Mar 2023 18:27:01 +0200 Subject: [PATCH 13/18] remove debug logs --- management/server/account.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/management/server/account.go b/management/server/account.go index 55a5a0299..3db6a2fe0 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -1126,8 +1126,6 @@ func (am *DefaultAccountManager) GetAccountFromPAT(token string) (*Account, *Use return nil, nil, nil, fmt.Errorf("token has wrong length") } - log.Debugf("Token: %s", token) - prefix := token[:len(PATPrefix)] if prefix != PATPrefix { return nil, nil, nil, fmt.Errorf("token has wrong prefix") @@ -1151,10 +1149,8 @@ func (am *DefaultAccountManager) GetAccountFromPAT(token string) (*Account, *Use if err != nil { return nil, nil, nil, err } - log.Debugf("TokenID: %s", tokenID) user, err := am.Store.GetUserByTokenID(tokenID) - log.Debugf("User: %v", user) if err != nil { return nil, nil, nil, err } From 03abdfa11211a346c3f2c837a881798786fdc9ec Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Wed, 29 Mar 2023 18:46:40 +0200 Subject: [PATCH 14/18] return empty object on all handlers instead of empty string --- management/server/http/groups_handler.go | 2 +- management/server/http/handler.go | 3 +++ management/server/http/nameservers_handler.go | 2 +- management/server/http/pat_handler.go | 2 +- management/server/http/peers_handler.go | 2 +- management/server/http/policies.go | 2 +- management/server/http/routes_handler.go | 2 +- management/server/http/rules_handler.go | 2 +- 8 files changed, 10 insertions(+), 7 deletions(-) diff --git a/management/server/http/groups_handler.go b/management/server/http/groups_handler.go index 9712d2e75..2464f47ef 100644 --- a/management/server/http/groups_handler.go +++ b/management/server/http/groups_handler.go @@ -300,7 +300,7 @@ func (h *GroupsHandler) DeleteGroup(w http.ResponseWriter, r *http.Request) { return } - util.WriteJSONObject(w, "") + util.WriteJSONObject(w, emptyObject{}) } // GetGroup returns a group diff --git a/management/server/http/handler.go b/management/server/http/handler.go index e2ed927a3..79028e6d2 100644 --- a/management/server/http/handler.go +++ b/management/server/http/handler.go @@ -25,6 +25,9 @@ type apiHandler struct { AuthCfg AuthCfg } +type emptyObject struct { +} + // APIHandler creates the Management service HTTP API handler registering all the available endpoints. func APIHandler(accountManager s.AccountManager, appMetrics telemetry.AppMetrics, authCfg AuthCfg) (http.Handler, error) { jwtMiddleware, err := middleware.NewJwtMiddleware( diff --git a/management/server/http/nameservers_handler.go b/management/server/http/nameservers_handler.go index e7be617e2..5ad52a426 100644 --- a/management/server/http/nameservers_handler.go +++ b/management/server/http/nameservers_handler.go @@ -243,7 +243,7 @@ func (h *NameserversHandler) DeleteNameserverGroup(w http.ResponseWriter, r *htt return } - util.WriteJSONObject(w, "") + util.WriteJSONObject(w, emptyObject{}) } // GetNameserverGroup handles a nameserver group Get request identified by ID diff --git a/management/server/http/pat_handler.go b/management/server/http/pat_handler.go index 2f6cb1492..d3e8b9ac5 100644 --- a/management/server/http/pat_handler.go +++ b/management/server/http/pat_handler.go @@ -184,7 +184,7 @@ func (h *PATHandler) DeleteToken(w http.ResponseWriter, r *http.Request) { return } - util.WriteJSONObject(w, "") + util.WriteJSONObject(w, emptyObject{}) } func toPATResponse(pat *server.PersonalAccessToken) *api.PersonalAccessToken { diff --git a/management/server/http/peers_handler.go b/management/server/http/peers_handler.go index 76c4f7502..7379277af 100644 --- a/management/server/http/peers_handler.go +++ b/management/server/http/peers_handler.go @@ -66,7 +66,7 @@ func (h *PeersHandler) deletePeer(accountID, userID string, peerID string, w htt util.WriteError(err, w) return } - util.WriteJSONObject(w, "") + util.WriteJSONObject(w, emptyObject{}) } // HandlePeer handles all peer requests for GET, PUT and DELETE operations diff --git a/management/server/http/policies.go b/management/server/http/policies.go index 275992d14..a0fe3b1e2 100644 --- a/management/server/http/policies.go +++ b/management/server/http/policies.go @@ -225,7 +225,7 @@ func (h *Policies) DeletePolicy(w http.ResponseWriter, r *http.Request) { return } - util.WriteJSONObject(w, "") + util.WriteJSONObject(w, emptyObject{}) } // GetPolicy handles a group Get request identified by ID diff --git a/management/server/http/routes_handler.go b/management/server/http/routes_handler.go index b29e5c261..aaaaaa854 100644 --- a/management/server/http/routes_handler.go +++ b/management/server/http/routes_handler.go @@ -321,7 +321,7 @@ func (h *RoutesHandler) DeleteRoute(w http.ResponseWriter, r *http.Request) { return } - util.WriteJSONObject(w, "") + util.WriteJSONObject(w, emptyObject{}) } // GetRoute handles a route Get request identified by ID diff --git a/management/server/http/rules_handler.go b/management/server/http/rules_handler.go index 8925c3763..f8bb5f0cb 100644 --- a/management/server/http/rules_handler.go +++ b/management/server/http/rules_handler.go @@ -222,7 +222,7 @@ func (h *RulesHandler) DeleteRule(w http.ResponseWriter, r *http.Request) { return } - util.WriteJSONObject(w, "") + util.WriteJSONObject(w, emptyObject{}) } // GetRule handles a group Get request identified by ID From ecc4f8a10d2466021a4862b6cc631a86c4425ea3 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Wed, 29 Mar 2023 19:13:01 +0200 Subject: [PATCH 15/18] fix Pat handler test --- management/server/http/pat_handler_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/management/server/http/pat_handler_test.go b/management/server/http/pat_handler_test.go index 0d29f1ad4..7b83c5db8 100644 --- a/management/server/http/pat_handler_test.go +++ b/management/server/http/pat_handler_test.go @@ -223,7 +223,7 @@ func toTokenResponse(serverToken server.PersonalAccessToken) api.PersonalAccessT Id: serverToken.ID, Name: serverToken.Name, CreatedAt: serverToken.CreatedAt, - LastUsed: serverToken.LastUsed, + LastUsed: &serverToken.LastUsed, CreatedBy: serverToken.CreatedBy, ExpirationDate: serverToken.ExpirationDate, } From 5c1acdbf2fdd380c29e84e4810b33bedb427413b Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Thu, 30 Mar 2023 13:58:44 +0200 Subject: [PATCH 16/18] move validation into account manager + func for get requests --- management/server/account.go | 6 +- management/server/http/pat_handler.go | 70 +++----- management/server/http/pat_handler_test.go | 40 ++++- management/server/mock_server/account_mock.go | 36 +++- management/server/user.go | 116 +++++++++++-- management/server/user_test.go | 160 +++++++++++++++--- 6 files changed, 322 insertions(+), 106 deletions(-) diff --git a/management/server/account.go b/management/server/account.go index 3db6a2fe0..ce2d2fc1e 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -67,8 +67,10 @@ type AccountManager interface { GetNetworkMap(peerID string) (*NetworkMap, error) GetPeerNetwork(peerID string) (*Network, error) AddPeer(setupKey, userID string, peer *Peer) (*Peer, *NetworkMap, error) - AddPATToUser(accountID string, userID string, pat *PersonalAccessToken) error - DeletePAT(accountID string, userID string, tokenID string) error + CreatePAT(accountID string, executingUserID string, targetUserId string, tokenName string, expiresIn int) (*PersonalAccessTokenGenerated, error) + DeletePAT(accountID string, executingUserID string, targetUserId string, tokenID string) error + GetPAT(accountID string, executingUserID string, targetUserId string, tokenID string) (*PersonalAccessToken, error) + GetAllPATs(accountID string, executingUserID string, targetUserId string) ([]*PersonalAccessToken, error) UpdatePeerSSHKey(peerID string, sshKey string) error GetUsersFromAccount(accountID, userID string) ([]*UserInfo, error) GetGroup(accountId, groupID string) (*Group, error) diff --git a/management/server/http/pat_handler.go b/management/server/http/pat_handler.go index d3e8b9ac5..d2398a7e1 100644 --- a/management/server/http/pat_handler.go +++ b/management/server/http/pat_handler.go @@ -46,17 +46,19 @@ func (h *PATHandler) GetAllTokens(w http.ResponseWriter, r *http.Request) { util.WriteError(status.Errorf(status.InvalidArgument, "invalid user ID"), w) return } - if userID != user.Id { - util.WriteErrorResponse("User not authorized to get tokens", http.StatusUnauthorized, w) + + pats, err := h.accountManager.GetAllPATs(account.Id, user.Id, userID) + if err != nil { + util.WriteError(err, w) return } - var pats []*api.PersonalAccessToken - for _, pat := range account.Users[userID].PATs { - pats = append(pats, toPATResponse(pat)) + var patResponse []*api.PersonalAccessToken + for _, pat := range pats { + patResponse = append(patResponse, toPATResponse(pat)) } - util.WriteJSONObject(w, pats) + util.WriteJSONObject(w, patResponse) } // GetToken is HTTP GET handler that returns a personal access token for the given user @@ -69,15 +71,11 @@ func (h *PATHandler) GetToken(w http.ResponseWriter, r *http.Request) { } vars := mux.Vars(r) - userID := vars["userId"] - if len(userID) == 0 { + targetUserID := vars["userId"] + if len(targetUserID) == 0 { util.WriteError(status.Errorf(status.InvalidArgument, "invalid user ID"), w) return } - if userID != user.Id { - util.WriteErrorResponse("User not authorized to get token", http.StatusUnauthorized, w) - return - } tokenID := vars["tokenId"] if len(tokenID) == 0 { @@ -85,15 +83,9 @@ func (h *PATHandler) GetToken(w http.ResponseWriter, r *http.Request) { return } - user = account.Users[userID] - if user == nil { - util.WriteError(status.Errorf(status.NotFound, "user not found"), w) - return - } - - pat := user.PATs[tokenID] - if pat == nil { - util.WriteError(status.Errorf(status.NotFound, "PAT not found"), w) + pat, err := h.accountManager.GetPAT(account.Id, user.Id, targetUserID, tokenID) + if err != nil { + util.WriteError(err, w) return } @@ -110,15 +102,11 @@ func (h *PATHandler) CreateToken(w http.ResponseWriter, r *http.Request) { } vars := mux.Vars(r) - userID := vars["userId"] - if len(userID) == 0 { + targetUserID := vars["userId"] + if len(targetUserID) == 0 { util.WriteError(status.Errorf(status.InvalidArgument, "invalid user ID"), w) return } - if userID != user.Id { - util.WriteErrorResponse("User not authorized to create token", http.StatusUnauthorized, w) - return - } var req api.PostApiUsersUserIdTokensJSONRequestBody err = json.NewDecoder(r.Body).Decode(&req) @@ -127,23 +115,7 @@ func (h *PATHandler) CreateToken(w http.ResponseWriter, r *http.Request) { return } - if req.Name == "" { - util.WriteErrorResponse("name can't be empty", http.StatusBadRequest, w) - return - } - - if req.ExpiresIn < 1 || req.ExpiresIn > 365 { - util.WriteErrorResponse("expiration has to be between 1 and 365", http.StatusBadRequest, w) - return - } - - pat, err := server.CreateNewPAT(req.Name, req.ExpiresIn, user.Id) - if err != nil { - util.WriteError(err, w) - return - } - - err = h.accountManager.AddPATToUser(account.Id, userID, &pat.PersonalAccessToken) + pat, err := h.accountManager.CreatePAT(account.Id, user.Id, targetUserID, req.Name, req.ExpiresIn) if err != nil { util.WriteError(err, w) return @@ -162,15 +134,11 @@ func (h *PATHandler) DeleteToken(w http.ResponseWriter, r *http.Request) { } vars := mux.Vars(r) - userID := vars["userId"] - if len(userID) == 0 { + targetUserID := vars["userId"] + if len(targetUserID) == 0 { util.WriteError(status.Errorf(status.InvalidArgument, "invalid user ID"), w) return } - if userID != user.Id { - util.WriteErrorResponse("User not authorized to delete token", http.StatusUnauthorized, w) - return - } tokenID := vars["tokenId"] if len(tokenID) == 0 { @@ -178,7 +146,7 @@ func (h *PATHandler) DeleteToken(w http.ResponseWriter, r *http.Request) { return } - err = h.accountManager.DeletePAT(account.Id, userID, tokenID) + err = h.accountManager.DeletePAT(account.Id, user.Id, targetUserID, tokenID) if err != nil { util.WriteError(err, w) return diff --git a/management/server/http/pat_handler_test.go b/management/server/http/pat_handler_test.go index 7b83c5db8..de79f1006 100644 --- a/management/server/http/pat_handler_test.go +++ b/management/server/http/pat_handler_test.go @@ -63,31 +63,55 @@ var testAccount = &server.Account{ func initPATTestData() *PATHandler { return &PATHandler{ accountManager: &mock_server.MockAccountManager{ - AddPATToUserFunc: func(accountID string, userID string, pat *server.PersonalAccessToken) error { + CreatePATFunc: func(accountID string, executingUserID string, targetUserID string, tokenName string, expiresIn int) (*server.PersonalAccessTokenGenerated, error) { if accountID != existingAccountID { - return status.Errorf(status.NotFound, "account with ID %s not found", accountID) + return nil, status.Errorf(status.NotFound, "account with ID %s not found", accountID) } - if userID != existingUserID { - return status.Errorf(status.NotFound, "user with ID %s not found", userID) + if targetUserID != existingUserID { + return nil, status.Errorf(status.NotFound, "user with ID %s not found", targetUserID) } - return nil + return &server.PersonalAccessTokenGenerated{ + PlainToken: "nbp_z1pvsg2wP3EzmEou4S679KyTNhov632eyrXe", + PersonalAccessToken: server.PersonalAccessToken{}, + }, nil }, GetAccountFromTokenFunc: func(_ jwtclaims.AuthorizationClaims) (*server.Account, *server.User, error) { return testAccount, testAccount.Users[existingUserID], nil }, - DeletePATFunc: func(accountID string, userID string, tokenID string) error { + DeletePATFunc: func(accountID string, executingUserID string, targetUserID string, tokenID string) error { if accountID != existingAccountID { return status.Errorf(status.NotFound, "account with ID %s not found", accountID) } - if userID != existingUserID { - return status.Errorf(status.NotFound, "user with ID %s not found", userID) + if targetUserID != existingUserID { + return status.Errorf(status.NotFound, "user with ID %s not found", targetUserID) } if tokenID != existingTokenID { return status.Errorf(status.NotFound, "token with ID %s not found", tokenID) } return nil }, + GetPATFunc: func(accountID string, executingUserID string, targetUserID string, tokenID string) (*server.PersonalAccessToken, error) { + if accountID != existingAccountID { + return nil, status.Errorf(status.NotFound, "account with ID %s not found", accountID) + } + if targetUserID != existingUserID { + return nil, status.Errorf(status.NotFound, "user with ID %s not found", targetUserID) + } + if tokenID != existingTokenID { + return nil, status.Errorf(status.NotFound, "token with ID %s not found", tokenID) + } + return testAccount.Users[existingUserID].PATs[existingTokenID], nil + }, + GetAllPATsFunc: func(accountID string, executingUserID string, targetUserID string) ([]*server.PersonalAccessToken, error) { + if accountID != existingAccountID { + return nil, status.Errorf(status.NotFound, "account with ID %s not found", accountID) + } + if targetUserID != existingUserID { + return nil, status.Errorf(status.NotFound, "user with ID %s not found", targetUserID) + } + return []*server.PersonalAccessToken{testAccount.Users[existingUserID].PATs[existingTokenID], testAccount.Users[existingUserID].PATs["token2"]}, nil + }, }, claimsExtractor: jwtclaims.NewClaimsExtractor( jwtclaims.WithFromRequestContext(func(r *http.Request) jwtclaims.AuthorizationClaims { diff --git a/management/server/mock_server/account_mock.go b/management/server/mock_server/account_mock.go index 71870fd84..53cd2d672 100644 --- a/management/server/mock_server/account_mock.go +++ b/management/server/mock_server/account_mock.go @@ -60,8 +60,10 @@ type MockAccountManager struct { SaveSetupKeyFunc func(accountID string, key *server.SetupKey, userID string) (*server.SetupKey, error) ListSetupKeysFunc func(accountID, userID string) ([]*server.SetupKey, error) SaveUserFunc func(accountID, userID string, user *server.User) (*server.UserInfo, error) - AddPATToUserFunc func(accountID string, userID string, pat *server.PersonalAccessToken) error - DeletePATFunc func(accountID string, userID string, tokenID string) error + CreatePATFunc func(accountID string, executingUserID string, targetUserId string, tokenName string, expiresIn int) (*server.PersonalAccessTokenGenerated, error) + DeletePATFunc func(accountID string, executingUserID string, targetUserId string, tokenID string) error + GetPATFunc func(accountID string, executingUserID string, targetUserId string, tokenID string) (*server.PersonalAccessToken, error) + GetAllPATsFunc func(accountID string, executingUserID string, targetUserId string) ([]*server.PersonalAccessToken, error) GetNameServerGroupFunc func(accountID, nsGroupID string) (*nbdns.NameServerGroup, error) CreateNameServerGroupFunc func(accountID string, name, description string, nameServerList []nbdns.NameServer, groups []string, primary bool, domains []string, enabled bool, userID string) (*nbdns.NameServerGroup, error) SaveNameServerGroupFunc func(accountID, userID string, nsGroupToSave *nbdns.NameServerGroup) error @@ -186,22 +188,38 @@ func (am *MockAccountManager) GetAccountFromPAT(pat string) (*server.Account, *s return nil, nil, nil, status.Errorf(codes.Unimplemented, "method GetAccountFromPAT is not implemented") } -// AddPATToUser mock implementation of AddPATToUser from server.AccountManager interface -func (am *MockAccountManager) AddPATToUser(accountID string, userID string, pat *server.PersonalAccessToken) error { - if am.AddPATToUserFunc != nil { - return am.AddPATToUserFunc(accountID, userID, pat) +// CreatePAT mock implementation of GetPAT from server.AccountManager interface +func (am *MockAccountManager) CreatePAT(accountID string, executingUserID string, targetUserID string, name string, expiresIn int) (*server.PersonalAccessTokenGenerated, error) { + if am.CreatePATFunc != nil { + return am.CreatePATFunc(accountID, executingUserID, targetUserID, name, expiresIn) } - return status.Errorf(codes.Unimplemented, "method AddPATToUser is not implemented") + return nil, status.Errorf(codes.Unimplemented, "method CreatePAT is not implemented") } // DeletePAT mock implementation of DeletePAT from server.AccountManager interface -func (am *MockAccountManager) DeletePAT(accountID string, userID string, tokenID string) error { +func (am *MockAccountManager) DeletePAT(accountID string, executingUserID string, targetUserID string, tokenID string) error { if am.DeletePATFunc != nil { - return am.DeletePATFunc(accountID, userID, tokenID) + return am.DeletePATFunc(accountID, executingUserID, targetUserID, tokenID) } return status.Errorf(codes.Unimplemented, "method DeletePAT is not implemented") } +// GetPAT mock implementation of GetPAT from server.AccountManager interface +func (am *MockAccountManager) GetPAT(accountID string, executingUserID string, targetUserID string, tokenID string) (*server.PersonalAccessToken, error) { + if am.GetPATFunc != nil { + return am.GetPATFunc(accountID, executingUserID, targetUserID, tokenID) + } + return nil, status.Errorf(codes.Unimplemented, "method GetPAT is not implemented") +} + +// GetAllPATs mock implementation of GetAllPATs from server.AccountManager interface +func (am *MockAccountManager) GetAllPATs(accountID string, executingUserID string, targetUserID string) ([]*server.PersonalAccessToken, error) { + if am.GetAllPATsFunc != nil { + return am.GetAllPATsFunc(accountID, executingUserID, targetUserID) + } + return nil, status.Errorf(codes.Unimplemented, "method GetAllPATs is not implemented") +} + // GetNetworkMap mock implementation of GetNetworkMap from server.AccountManager interface func (am *MockAccountManager) GetNetworkMap(peerKey string) (*server.NetworkMap, error) { if am.GetNetworkMapFunc != nil { diff --git a/management/server/user.go b/management/server/user.go index 572843fff..97e9cd0c7 100644 --- a/management/server/user.go +++ b/management/server/user.go @@ -193,37 +193,63 @@ func (am *DefaultAccountManager) CreateUser(accountID, userID string, invite *Us } -// AddPATToUser takes the userID and the accountID the user belongs to and assigns a provided PersonalAccessToken to that user -func (am *DefaultAccountManager) AddPATToUser(accountID string, userID string, pat *PersonalAccessToken) error { +// CreatePAT creates a new PAT for the given user +func (am *DefaultAccountManager) CreatePAT(accountID string, executingUserID string, targetUserId string, tokenName string, expiresIn int) (*PersonalAccessTokenGenerated, error) { unlock := am.Store.AcquireAccountLock(accountID) defer unlock() + if tokenName == "" { + return nil, status.Errorf(status.InvalidArgument, "token name can't be empty") + } + + if expiresIn < 1 || expiresIn > 365 { + return nil, status.Errorf(status.InvalidArgument, "expiration has to be between 1 and 365") + } + + if executingUserID != targetUserId { + return nil, status.Errorf(status.PermissionDenied, "no permission to create PAT for this user") + } + account, err := am.Store.GetAccount(accountID) if err != nil { - return err + return nil, err } - user := account.Users[userID] - if user == nil { - return status.Errorf(status.NotFound, "user not found") + targetUser := account.Users[targetUserId] + if targetUser == nil { + return nil, status.Errorf(status.NotFound, "targetUser not found") } - user.PATs[pat.ID] = pat + pat, err := CreateNewPAT(tokenName, expiresIn, targetUser.Id) + if err != nil { + return nil, status.Errorf(status.Internal, "failed to create PAT: %v", err) + } - return am.Store.SaveAccount(account) + targetUser.PATs[pat.ID] = &pat.PersonalAccessToken + + err = am.Store.SaveAccount(account) + if err != nil { + return nil, status.Errorf(status.Internal, "failed to save account: %v", err) + } + + return pat, nil } // DeletePAT deletes a specific PAT from a user -func (am *DefaultAccountManager) DeletePAT(accountID string, userID string, tokenID string) error { +func (am *DefaultAccountManager) DeletePAT(accountID string, executingUserID string, targetUserID string, tokenID string) error { unlock := am.Store.AcquireAccountLock(accountID) defer unlock() - account, err := am.Store.GetAccount(accountID) - if err != nil { - return err + if executingUserID != targetUserID { + return status.Errorf(status.PermissionDenied, "no permission to delete PAT for this user") } - user := account.Users[userID] + account, err := am.Store.GetAccount(accountID) + if err != nil { + return status.Errorf(status.NotFound, "account not found: %s", err) + } + + user := account.Users[targetUserID] if user == nil { return status.Errorf(status.NotFound, "user not found") } @@ -235,15 +261,73 @@ func (am *DefaultAccountManager) DeletePAT(accountID string, userID string, toke err = am.Store.DeleteTokenID2UserIDIndex(pat.ID) if err != nil { - return err + return status.Errorf(status.Internal, "Failed to delete token id index: %s", err) } err = am.Store.DeleteHashedPAT2TokenIDIndex(pat.HashedToken) if err != nil { - return err + return status.Errorf(status.Internal, "Failed to delete hashed token index: %s", err) } delete(user.PATs, tokenID) - return am.Store.SaveAccount(account) + err = am.Store.SaveAccount(account) + if err != nil { + return status.Errorf(status.Internal, "Failed to save account: %s", err) + } + return nil +} + +// GetPAT returns a specific PAT from a user +func (am *DefaultAccountManager) GetPAT(accountID string, executingUserID string, targetUserID string, tokenID string) (*PersonalAccessToken, error) { + unlock := am.Store.AcquireAccountLock(accountID) + defer unlock() + + if executingUserID != targetUserID { + return nil, status.Errorf(status.PermissionDenied, "no permission to get PAT for this user") + } + + account, err := am.Store.GetAccount(accountID) + if err != nil { + return nil, status.Errorf(status.NotFound, "account not found: %s", err) + } + + user := account.Users[targetUserID] + if user == nil { + return nil, status.Errorf(status.NotFound, "user not found") + } + + pat := user.PATs[tokenID] + if pat == nil { + return nil, status.Errorf(status.NotFound, "PAT not found") + } + + return pat, nil +} + +// GetAllPATs returns all PATs for a user +func (am *DefaultAccountManager) GetAllPATs(accountID string, executingUserID string, targetUserID string) ([]*PersonalAccessToken, error) { + unlock := am.Store.AcquireAccountLock(accountID) + defer unlock() + + if executingUserID != targetUserID { + return nil, status.Errorf(status.PermissionDenied, "no permission to get PAT for this user") + } + + account, err := am.Store.GetAccount(accountID) + if err != nil { + return nil, status.Errorf(status.NotFound, "account not found: %s", err) + } + + user := account.Users[targetUserID] + if user == nil { + return nil, status.Errorf(status.NotFound, "user not found") + } + + var pats []*PersonalAccessToken + for _, pat := range user.PATs { + pats = append(pats, pat) + } + + return pats, nil } // SaveUser saves updates a given user. If the user doesn't exit it will throw status.NotFound error. diff --git a/management/server/user_test.go b/management/server/user_test.go index ffbb71282..1dd12e57b 100644 --- a/management/server/user_test.go +++ b/management/server/user_test.go @@ -7,13 +7,20 @@ import ( ) const ( - mockAccountID = "accountID" - mockUserID = "userID" - mockTokenID = "tokenID" - mockToken = "SoMeHaShEdToKeN" + mockAccountID = "accountID" + mockUserID = "userID" + mockTargetUserId = "targetUserID" + mockTokenID1 = "tokenID1" + mockToken1 = "SoMeHaShEdToKeN1" + mockTokenID2 = "tokenID2" + mockToken2 = "SoMeHaShEdToKeN2" + mockTokenName = "tokenName" + mockEmptyTokenName = "" + mockExpiresIn = 7 + mockWrongExpiresIn = 4506 ) -func TestUser_AddPATToUser(t *testing.T) { +func TestUser_CreatePAT_ForSameUser(t *testing.T) { store := newStore(t) account := newAccountWithId(mockAccountID, mockUserID, "") @@ -26,24 +33,19 @@ func TestUser_AddPATToUser(t *testing.T) { Store: store, } - pat := PersonalAccessToken{ - ID: mockTokenID, - HashedToken: mockToken, - } - - err = am.AddPATToUser(mockAccountID, mockUserID, &pat) + pat, err := am.CreatePAT(mockAccountID, mockUserID, mockUserID, mockTokenName, mockExpiresIn) if err != nil { t.Fatalf("Error when adding PAT to user: %s", err) } fileStore := am.Store.(*FileStore) - tokenID := fileStore.HashedPAT2TokenID[mockToken] + tokenID := fileStore.HashedPAT2TokenID[pat.HashedToken] if tokenID == "" { t.Fatal("GetTokenIDByHashedToken failed after adding PAT") } - assert.Equal(t, mockTokenID, tokenID) + assert.Equal(t, pat.ID, tokenID) userID := fileStore.TokenID2UserID[tokenID] if userID == "" { @@ -52,15 +54,66 @@ func TestUser_AddPATToUser(t *testing.T) { assert.Equal(t, mockUserID, userID) } +func TestUser_CreatePAT_ForDifferentUser(t *testing.T) { + store := newStore(t) + account := newAccountWithId(mockAccountID, mockUserID, "") + + err := store.SaveAccount(account) + if err != nil { + t.Fatalf("Error when saving account: %s", err) + } + + am := DefaultAccountManager{ + Store: store, + } + + _, err = am.CreatePAT(mockAccountID, mockUserID, mockTargetUserId, mockTokenName, mockExpiresIn) + assert.Errorf(t, err, "Creating PAT for different user should thorw error") +} + +func TestUser_CreatePAT_WithWrongExpiration(t *testing.T) { + store := newStore(t) + account := newAccountWithId(mockAccountID, mockUserID, "") + + err := store.SaveAccount(account) + if err != nil { + t.Fatalf("Error when saving account: %s", err) + } + + am := DefaultAccountManager{ + Store: store, + } + + _, err = am.CreatePAT(mockAccountID, mockUserID, mockUserID, mockTokenName, mockWrongExpiresIn) + assert.Errorf(t, err, "Wrong expiration should thorw error") +} + +func TestUser_CreatePAT_WithEmptyName(t *testing.T) { + store := newStore(t) + account := newAccountWithId(mockAccountID, mockUserID, "") + + err := store.SaveAccount(account) + if err != nil { + t.Fatalf("Error when saving account: %s", err) + } + + am := DefaultAccountManager{ + Store: store, + } + + _, err = am.CreatePAT(mockAccountID, mockUserID, mockUserID, mockEmptyTokenName, mockExpiresIn) + assert.Errorf(t, err, "Wrong expiration should thorw error") +} + func TestUser_DeletePAT(t *testing.T) { store := newStore(t) account := newAccountWithId(mockAccountID, mockUserID, "") account.Users[mockUserID] = &User{ Id: mockUserID, PATs: map[string]*PersonalAccessToken{ - mockTokenID: { - ID: mockTokenID, - HashedToken: mockToken, + mockTokenID1: { + ID: mockTokenID1, + HashedToken: mockToken1, }, }, } @@ -73,12 +126,79 @@ func TestUser_DeletePAT(t *testing.T) { Store: store, } - err = am.DeletePAT(mockAccountID, mockUserID, mockTokenID) + err = am.DeletePAT(mockAccountID, mockUserID, mockUserID, mockTokenID1) if err != nil { t.Fatalf("Error when adding PAT to user: %s", err) } - assert.Nil(t, store.Accounts[mockAccountID].Users[mockUserID].PATs[mockTokenID]) - assert.Empty(t, store.HashedPAT2TokenID[mockToken]) - assert.Empty(t, store.TokenID2UserID[mockTokenID]) + assert.Nil(t, store.Accounts[mockAccountID].Users[mockUserID].PATs[mockTokenID1]) + assert.Empty(t, store.HashedPAT2TokenID[mockToken1]) + assert.Empty(t, store.TokenID2UserID[mockTokenID1]) +} + +func TestUser_GetPAT(t *testing.T) { + store := newStore(t) + account := newAccountWithId(mockAccountID, mockUserID, "") + account.Users[mockUserID] = &User{ + Id: mockUserID, + PATs: map[string]*PersonalAccessToken{ + mockTokenID1: { + ID: mockTokenID1, + HashedToken: mockToken1, + }, + }, + } + err := store.SaveAccount(account) + if err != nil { + t.Fatalf("Error when saving account: %s", err) + } + + am := DefaultAccountManager{ + Store: store, + } + + pat, err := am.GetPAT(mockAccountID, mockUserID, mockUserID, mockTokenID1) + if err != nil { + t.Fatalf("Error when adding PAT to user: %s", err) + } + + assert.Equal(t, mockTokenID1, pat.ID) + assert.Equal(t, mockToken1, pat.HashedToken) +} + +func TestUser_GetAllPATs(t *testing.T) { + store := newStore(t) + account := newAccountWithId(mockAccountID, mockUserID, "") + account.Users[mockUserID] = &User{ + Id: mockUserID, + PATs: map[string]*PersonalAccessToken{ + mockTokenID1: { + ID: mockTokenID1, + HashedToken: mockToken1, + }, + mockTokenID2: { + ID: mockTokenID2, + HashedToken: mockToken2, + }, + }, + } + err := store.SaveAccount(account) + if err != nil { + t.Fatalf("Error when saving account: %s", err) + } + + am := DefaultAccountManager{ + Store: store, + } + + pats, err := am.GetAllPATs(mockAccountID, mockUserID, mockUserID) + if err != nil { + t.Fatalf("Error when adding PAT to user: %s", err) + } + + assert.Equal(t, 2, len(pats)) + assert.Equal(t, mockTokenID1, pats[0].ID) + assert.Equal(t, mockToken1, pats[0].HashedToken) + assert.Equal(t, mockTokenID2, pats[1].ID) + assert.Equal(t, mockToken2, pats[1].HashedToken) } From a7519859bccf2f4a02c672437ed3bd4196ace724 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Thu, 30 Mar 2023 14:15:44 +0200 Subject: [PATCH 17/18] fix test --- management/server/user_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/management/server/user_test.go b/management/server/user_test.go index 1dd12e57b..238aa2bff 100644 --- a/management/server/user_test.go +++ b/management/server/user_test.go @@ -197,8 +197,4 @@ func TestUser_GetAllPATs(t *testing.T) { } assert.Equal(t, 2, len(pats)) - assert.Equal(t, mockTokenID1, pats[0].ID) - assert.Equal(t, mockToken1, pats[0].HashedToken) - assert.Equal(t, mockTokenID2, pats[1].ID) - assert.Equal(t, mockToken2, pats[1].HashedToken) } From 5e2f66d59142750c2ce9491af4b5d2dd607e12c4 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Thu, 30 Mar 2023 15:23:24 +0200 Subject: [PATCH 18/18] fix codacy --- management/server/account.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/management/server/account.go b/management/server/account.go index ce2d2fc1e..2c146b3ad 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -67,10 +67,10 @@ type AccountManager interface { GetNetworkMap(peerID string) (*NetworkMap, error) GetPeerNetwork(peerID string) (*Network, error) AddPeer(setupKey, userID string, peer *Peer) (*Peer, *NetworkMap, error) - CreatePAT(accountID string, executingUserID string, targetUserId string, tokenName string, expiresIn int) (*PersonalAccessTokenGenerated, error) - DeletePAT(accountID string, executingUserID string, targetUserId string, tokenID string) error - GetPAT(accountID string, executingUserID string, targetUserId string, tokenID string) (*PersonalAccessToken, error) - GetAllPATs(accountID string, executingUserID string, targetUserId string) ([]*PersonalAccessToken, error) + CreatePAT(accountID string, executingUserID string, targetUserID string, tokenName string, expiresIn int) (*PersonalAccessTokenGenerated, error) + DeletePAT(accountID string, executingUserID string, targetUserID string, tokenID string) error + GetPAT(accountID string, executingUserID string, targetUserID string, tokenID string) (*PersonalAccessToken, error) + GetAllPATs(accountID string, executingUserID string, targetUserID string) ([]*PersonalAccessToken, error) UpdatePeerSSHKey(peerID string, sshKey string) error GetUsersFromAccount(accountID, userID string) ([]*UserInfo, error) GetGroup(accountId, groupID string) (*Group, error)