From eb2fe200251e4222e69f2baafeb9be05db1da757 Mon Sep 17 00:00:00 2001 From: Ahsan Barkati Date: Fri, 24 Feb 2023 01:19:01 +0530 Subject: [PATCH] Address review comments --- ee/query-service/app/api/pat.go | 12 +++--- ee/query-service/app/server.go | 56 +++++++------------------ ee/query-service/dao/interface.go | 2 + ee/query-service/dao/sqlite/modelDao.go | 2 +- ee/query-service/dao/sqlite/pat.go | 45 ++++++++++++++++++++ ee/query-service/model/pat.go | 2 +- 6 files changed, 71 insertions(+), 48 deletions(-) diff --git a/ee/query-service/app/api/pat.go b/ee/query-service/app/api/pat.go index d708ba0606..619c875c8f 100644 --- a/ee/query-service/app/api/pat.go +++ b/ee/query-service/app/api/pat.go @@ -41,11 +41,13 @@ func (ah *APIHandler) createPAT(w http.ResponseWriter, r *http.Request) { return } + // All the PATs are associated with the user creating the PAT. Hence, the permissions + // associated with the PAT is also equivalent to that of the user. req.UserID = user.Id req.CreatedAt = time.Now().Unix() req.Token = generatePATToken() - zap.S().Infof("Got PAT request: %+v", req) + zap.S().Debugf("Got PAT request: %+v", req) if apierr := ah.AppDao().CreatePAT(ctx, &req); apierr != nil { RespondError(w, apierr, nil) return @@ -70,7 +72,7 @@ func (ah *APIHandler) getPATs(w http.ResponseWriter, r *http.Request) { RespondError(w, apierr, nil) return } - ah.WriteJSON(w, r, pats) + ah.Respond(w, pats) } func (ah *APIHandler) deletePAT(w http.ResponseWriter, r *http.Request) { @@ -84,7 +86,7 @@ func (ah *APIHandler) deletePAT(w http.ResponseWriter, r *http.Request) { }, nil) return } - pat, apierr := ah.AppDao().GetPAT(ctx, id) + pat, apierr := ah.AppDao().GetPATByID(ctx, id) if apierr != nil { RespondError(w, apierr, nil) return @@ -96,10 +98,10 @@ func (ah *APIHandler) deletePAT(w http.ResponseWriter, r *http.Request) { }, nil) return } - zap.S().Infof("Delete PAT with id: %+v", id) + zap.S().Debugf("Delete PAT with id: %+v", id) if apierr := ah.AppDao().DeletePAT(ctx, id); apierr != nil { RespondError(w, apierr, nil) return } - ah.WriteJSON(w, r, map[string]string{"data": "pat deleted successfully"}) + ah.Respond(w, map[string]string{"data": "pat deleted successfully"}) } diff --git a/ee/query-service/app/server.go b/ee/query-service/app/server.go index 42a8f2496e..7a0d794a88 100644 --- a/ee/query-service/app/server.go +++ b/ee/query-service/app/server.go @@ -10,7 +10,6 @@ import ( "net/http" _ "net/http/pprof" // http profiler "os" - "strings" "time" "github.com/gorilla/handlers" @@ -191,7 +190,7 @@ func (s *Server) createPrivateServer(apiHandler *api.APIHandler) (*http.Server, // ip here for alert manager AllowedOrigins: []string{"*"}, AllowedMethods: []string{"GET", "DELETE", "POST", "PUT", "PATCH"}, - AllowedHeaders: []string{"Accept", "Authorization", "Content-Type"}, + AllowedHeaders: []string{"Accept", "Authorization", "Content-Type", "SIGNOZ-API-KEY"}, }) handler := c.Handler(r) @@ -202,50 +201,25 @@ func (s *Server) createPrivateServer(apiHandler *api.APIHandler) (*http.Server, }, nil } -func getPATToken(r *http.Request) (string, error) { - patHeader := r.Header.Get("SIGNOZ-API-KEY") - if patHeader == "" { - return "", nil - } - - authHeaderParts := strings.Fields(patHeader) - if len(authHeaderParts) != 2 || strings.ToLower(authHeaderParts[0]) != "bearer" { - return "", fmt.Errorf("PAT authorization header format must be bearer {token}") - } - - return authHeaderParts[1], nil -} - func (s *Server) createPublicServer(apiHandler *api.APIHandler) (*http.Server, error) { r := mux.NewRouter() - getUserFromPAT := func(r *http.Request) (*model.UserPayload, error) { - patToken, err := getPATToken(r) - if err != nil { - return nil, fmt.Errorf("failed to get PAT token in request headers, err: %v", err) - } - ctx := context.Background() - dao := apiHandler.AppDao() - pat, err := dao.GetPAT(ctx, patToken) - if err != nil { - return nil, fmt.Errorf("failed to fetch PAT token from DB, err %v", err) - } - user, apierr := dao.GetUser(ctx, pat.UserID) - if apierr != nil { - return nil, fmt.Errorf("failed to fetch user for PAT from DB, err: %v", apierr) - } - return user, nil - } - getUserFromRequest := func(r *http.Request) (*model.UserPayload, error) { - user, err := getUserFromPAT(r) - if err == nil && user != nil { - zap.S().Debugf("Found valid PAT user: %+v", user) - return user, nil - } - if err != nil { - zap.S().Debugf("Error while getting user for PAT: %+v", err) + patToken := r.Header.Get("SIGNOZ-API-KEY") + if len(patToken) > 0 { + zap.S().Debugf("Received a non-zero length PAT token") + ctx := context.Background() + dao := apiHandler.AppDao() + + user, err := dao.GetUserByPAT(ctx, patToken) + if err == nil && user != nil { + zap.S().Debugf("Found valid PAT user: %+v", user) + return user, nil + } + if err != nil { + zap.S().Debugf("Error while getting user for PAT: %+v", err) + } } return baseauth.GetUserFromRequest(r) } diff --git a/ee/query-service/dao/interface.go b/ee/query-service/dao/interface.go index da6963f68c..2303bb72d4 100644 --- a/ee/query-service/dao/interface.go +++ b/ee/query-service/dao/interface.go @@ -36,6 +36,8 @@ type ModelDao interface { CreatePAT(ctx context.Context, p *model.PAT) basemodel.BaseApiError GetPAT(ctx context.Context, pat string) (*model.PAT, basemodel.BaseApiError) + GetPATByID(ctx context.Context, id string) (*model.PAT, basemodel.BaseApiError) + GetUserByPAT(ctx context.Context, token string) (*basemodel.UserPayload, basemodel.BaseApiError) ListPATs(ctx context.Context, userID string) ([]model.PAT, basemodel.BaseApiError) DeletePAT(ctx context.Context, id string) basemodel.BaseApiError } diff --git a/ee/query-service/dao/sqlite/modelDao.go b/ee/query-service/dao/sqlite/modelDao.go index 9b1d74c034..3c195ea9bf 100644 --- a/ee/query-service/dao/sqlite/modelDao.go +++ b/ee/query-service/dao/sqlite/modelDao.go @@ -52,7 +52,7 @@ func InitDB(dataSourceName string) (*modelDao, error) { CREATE TABLE IF NOT EXISTS personal_access_tokens ( id INTEGER PRIMARY KEY AUTOINCREMENT, user_id TEXT NOT NULL, - token TEXT NOT NULL, + token TEXT NOT NULL UNIQUE, name TEXT NOT NULL, created_at INTEGER NOT NULL, expires_at INTEGER NOT NULL, diff --git a/ee/query-service/dao/sqlite/pat.go b/ee/query-service/dao/sqlite/pat.go index 7b3569db66..cc4de546c5 100644 --- a/ee/query-service/dao/sqlite/pat.go +++ b/ee/query-service/dao/sqlite/pat.go @@ -59,3 +59,48 @@ func (m *modelDao) GetPAT(ctx context.Context, token string) (*model.PAT, basemo return &pats[0], nil } + +func (m *modelDao) GetPATByID(ctx context.Context, id string) (*model.PAT, basemodel.BaseApiError) { + pats := []model.PAT{} + + if err := m.DB().Select(&pats, `SELECT * FROM personal_access_tokens WHERE id=?;`, id); err != nil { + return nil, model.InternalError(fmt.Errorf("failed to fetch PAT")) + } + + if len(pats) != 1 { + return nil, &model.ApiError{ + Typ: model.ErrorInternal, + Err: fmt.Errorf("found zero or multiple PATs with same token"), + } + } + + return &pats[0], nil +} + +func (m *modelDao) GetUserByPAT(ctx context.Context, token string) (*basemodel.UserPayload, basemodel.BaseApiError) { + users := []basemodel.UserPayload{} + + query := `SELECT + u.id, + u.name, + u.email, + u.password, + u.created_at, + u.profile_picture_url, + u.org_id, + u.group_id + FROM users u, personal_access_tokens p + WHERE u.id = p.user_id and p.token=?;` + + if err := m.DB().Select(&users, query, token); err != nil { + return nil, model.InternalError(fmt.Errorf("failed to fetch user from PAT, err: %v", err)) + } + + if len(users) != 1 { + return nil, &model.ApiError{ + Typ: model.ErrorInternal, + Err: fmt.Errorf("found zero or multiple users with same PAT token"), + } + } + return &users[0], nil +} diff --git a/ee/query-service/model/pat.go b/ee/query-service/model/pat.go index f320d0be7c..c22282060b 100644 --- a/ee/query-service/model/pat.go +++ b/ee/query-service/model/pat.go @@ -6,5 +6,5 @@ type PAT struct { Token string `json:"token" db:"token"` Name string `json:"name" db:"name"` CreatedAt int64 `json:"createdAt" db:"created_at"` - ExpiresAt int64 `json:"expiresAt" db:"expires_at"` + ExpiresAt int64 `json:"expiresAt" db:"expires_at"` // unused as of now }