From 86b62d6449cb10326eeb04f1aa1a9f2eb65acc6f Mon Sep 17 00:00:00 2001 From: Ivan Kapelyukhin Date: Tue, 18 Aug 2020 21:23:49 +0200 Subject: [PATCH 1/8] Add backend support for API keys --- src/jetstream/apikeys.go | 78 +++++++ .../datastore/20200814140918_ApiKeys.go | 21 ++ src/jetstream/main.go | 19 +- src/jetstream/middleware.go | 216 +++++++++++++----- src/jetstream/repository/apikeys/apikeys.go | 11 + .../repository/apikeys/psql_apikeys.go | 138 +++++++++++ .../repository/interfaces/apikeys.go | 8 + 7 files changed, 431 insertions(+), 60 deletions(-) create mode 100644 src/jetstream/apikeys.go create mode 100644 src/jetstream/datastore/20200814140918_ApiKeys.go create mode 100644 src/jetstream/repository/apikeys/apikeys.go create mode 100644 src/jetstream/repository/apikeys/psql_apikeys.go create mode 100644 src/jetstream/repository/interfaces/apikeys.go diff --git a/src/jetstream/apikeys.go b/src/jetstream/apikeys.go new file mode 100644 index 0000000000..5ce7c98ec0 --- /dev/null +++ b/src/jetstream/apikeys.go @@ -0,0 +1,78 @@ +package main + +import ( + "net/http" + + "github.com/cloudfoundry-incubator/stratos/src/jetstream/repository/apikeys" + "github.com/labstack/echo" + log "github.com/sirupsen/logrus" +) + +func (p *portalProxy) addAPIKey(c echo.Context) error { + log.Debug("addAPIKey") + + userGUID := c.Get("user_id").(string) + comment := c.FormValue("comment") + + if len(comment) == 0 { + return echo.NewHTTPError(http.StatusBadRequest, "Comment can't be empty") + } + + apiKeysRepo, err := apikeys.NewPgsqlAPIKeysRepository(p.DatabaseConnectionPool) + if err != nil { + log.Errorf("Database error getting repo for API keys: %v", err) + return err + } + + apiKey, err := apiKeysRepo.AddAPIKey(userGUID, comment) + if err != nil { + log.Errorf("Error adding API key %v", err) + return err + } + + return c.JSON(http.StatusOK, apiKey) +} + +func (p *portalProxy) listAPIKeys(c echo.Context) error { + log.Debug("listAPIKeys") + + apiKeysRepo, err := apikeys.NewPgsqlAPIKeysRepository(p.DatabaseConnectionPool) + if err != nil { + log.Errorf("Database error getting repo for API keys: %v", err) + return err + } + + userGUID := c.Get("user_id").(string) + + apiKeys, err := apiKeysRepo.ListAPIKeys(userGUID) + if err != nil { + log.Errorf("Error listing API keys %v", err) + return nil + } + + return c.JSON(http.StatusOK, apiKeys) +} + +func (p *portalProxy) deleteAPIKey(c echo.Context) error { + log.Debug("deleteAPIKey") + + userGUID := c.Get("user_id").(string) + keyGUID := c.FormValue("guid") + + if len(keyGUID) == 0 { + return echo.NewHTTPError(http.StatusBadRequest, "API key guid can't be empty") + } + + apiKeysRepo, err := apikeys.NewPgsqlAPIKeysRepository(p.DatabaseConnectionPool) + if err != nil { + log.Errorf("Database error getting repo for API keys: %v", err) + return err + } + + if err = apiKeysRepo.DeleteAPIKey(userGUID, keyGUID); err != nil { + log.Errorf("Error deleting API key %v", err) + return echo.NewHTTPError(http.StatusBadRequest, "Error deleting API key") + } + + return nil +} diff --git a/src/jetstream/datastore/20200814140918_ApiKeys.go b/src/jetstream/datastore/20200814140918_ApiKeys.go new file mode 100644 index 0000000000..ba9566dda2 --- /dev/null +++ b/src/jetstream/datastore/20200814140918_ApiKeys.go @@ -0,0 +1,21 @@ +package datastore + +import ( + "database/sql" + + "bitbucket.org/liamstask/goose/lib/goose" +) + +func init() { + RegisterMigration(20200814140918, "ApiKeys", func(txn *sql.Tx, conf *goose.DBConf) error { + apiTokenTable := "CREATE TABLE IF NOT EXISTS api_keys (" + apiTokenTable += "guid VARCHAR(36) NOT NULL UNIQUE," + apiTokenTable += "secret VARCHAR(36) NOT NULL UNIQUE," + apiTokenTable += "user_guid VARCHAR(36) NOT NULL," + apiTokenTable += "comment VARCHAR(255) NOT NULL," + apiTokenTable += "PRIMARY KEY (guid) );" + + _, err := txn.Exec(apiTokenTable) + return err + }) +} diff --git a/src/jetstream/main.go b/src/jetstream/main.go index 955f6f0000..13dd1c7587 100644 --- a/src/jetstream/main.go +++ b/src/jetstream/main.go @@ -905,8 +905,19 @@ func (p *portalProxy) registerRoutes(e *echo.Echo, needSetupMiddleware bool) { // All routes in the session group need the user to be authenticated sessionGroup := pp.Group("/v1") - sessionGroup.Use(p.sessionMiddleware) - sessionGroup.Use(p.xsrfMiddleware) + sessionGroup.Use(p.sessionMiddleware()) + sessionGroup.Use(p.xsrfMiddleware()) + + sessionGroup.POST("/api_keys", p.addAPIKey) + sessionGroup.GET("/api_keys", p.listAPIKeys) + sessionGroup.DELETE("/api_keys", p.deleteAPIKey) + + apiKeyGroupConfig := MiddlewareConfig{Skipper: p.apiKeySkipper} + + apiKeyGroup := pp.Group("/v1") + apiKeyGroup.Use(p.apiKeyMiddleware) + apiKeyGroup.Use(p.sessionMiddlewareWithConfig(apiKeyGroupConfig)) + apiKeyGroup.Use(p.xsrfMiddlewareWithConfig(apiKeyGroupConfig)) for _, plugin := range p.Plugins { middlewarePlugin, err := plugin.GetMiddlewarePlugin() @@ -932,8 +943,8 @@ func (p *portalProxy) registerRoutes(e *echo.Echo, needSetupMiddleware bool) { sessionAuthGroup.GET("/session/verify", p.verifySession) // CNSI operations - sessionGroup.GET("/cnsis", p.listCNSIs) - sessionGroup.GET("/cnsis/registered", p.listRegisteredCNSIs) + apiKeyGroup.GET("/cnsis", p.listCNSIs) + apiKeyGroup.GET("/cnsis/registered", p.listRegisteredCNSIs) // Info sessionGroup.GET("/info", p.info) diff --git a/src/jetstream/middleware.go b/src/jetstream/middleware.go index 723ed2ad49..c7e1b115b0 100644 --- a/src/jetstream/middleware.go +++ b/src/jetstream/middleware.go @@ -2,6 +2,7 @@ package main import ( "crypto/subtle" + "database/sql" "errors" "fmt" "net/http" @@ -14,6 +15,7 @@ import ( "github.com/labstack/echo" log "github.com/sirupsen/logrus" + "github.com/cloudfoundry-incubator/stratos/src/jetstream/repository/apikeys" "github.com/cloudfoundry-incubator/stratos/src/jetstream/repository/interfaces" ) @@ -28,6 +30,15 @@ const StratosSSOHeader = "x-stratos-sso-login" // Header to communicate any error during SSO const StratosSSOErrorHeader = "x-stratos-sso-error" +// APIKeyContextKey - context +const APIKeySkipperContextKey = "valid_api_key" + +// APIKeyHeader - API key authentication header name +const APIKeyHeader = "Authentication" + +// APIKeyAuthScheme - API key authentication scheme +const APIKeyAuthScheme = "Bearer" + func handleSessionError(config interfaces.PortalConfig, c echo.Context, err error, doNotLog bool, msg string) error { log.Debug("handleSessionError") @@ -65,75 +76,113 @@ func handleSessionError(config interfaces.PortalConfig, c echo.Context, err erro ) } -func (p *portalProxy) sessionMiddleware(h echo.HandlerFunc) echo.HandlerFunc { - return func(c echo.Context) error { - log.Debug("sessionMiddleware") +type ( + // Skipper - skipper function for middlewares + Skipper func(echo.Context) bool - p.removeEmptyCookie(c) + // MiddlewareConfig defines the config for Logger middleware. + MiddlewareConfig struct { + // Skipper defines a function to skip middleware. + Skipper Skipper + } +) - userID, err := p.GetSessionValue(c, "user_id") - if err == nil { - c.Set("user_id", userID) - return h(c) - } +func (p *portalProxy) sessionMiddleware() echo.MiddlewareFunc { - // Don't log an error if we are verifying the session, as a failure is not an error - isVerify := strings.HasSuffix(c.Request().RequestURI, "/auth/session/verify") - if isVerify { - // Tell the frontend what the Cookie Domain is so it can check if sessions will work - c.Response().Header().Set(StratosDomainHeader, p.Config.CookieDomain) - } + return p.sessionMiddlewareWithConfig(MiddlewareConfig{}) +} - // Clear any session cookie - cookie := new(http.Cookie) - cookie.Name = p.SessionCookieName - cookie.Value = "" - cookie.Expires = time.Now().Add(-24 * time.Hour) - cookie.Domain = p.SessionStoreOptions.Domain - cookie.HttpOnly = p.SessionStoreOptions.HttpOnly - cookie.Secure = p.SessionStoreOptions.Secure - cookie.Path = p.SessionStoreOptions.Path - cookie.MaxAge = 0 - c.SetCookie(cookie) - - return handleSessionError(p.Config, c, err, isVerify, "User session could not be found") +func (p *portalProxy) sessionMiddlewareWithConfig(config MiddlewareConfig) echo.MiddlewareFunc { + // Default skipper function always returns false + if config.Skipper == nil { + config.Skipper = func(c echo.Context) bool { return false } } -} -// Support for Angular XSRF -func (p *portalProxy) xsrfMiddleware(h echo.HandlerFunc) echo.HandlerFunc { - return func(c echo.Context) error { - log.Debug("xsrfMiddleware") + return func(h echo.HandlerFunc) echo.HandlerFunc { + return func(c echo.Context) error { + log.Debug("sessionMiddleware") - // Only do this for mutating requests - i.e. we can ignore for GET or HEAD requests - if c.Request().Method == "GET" || c.Request().Method == "HEAD" { - return h(c) - } + if config.Skipper(c) { + log.Debug("Skipping sessionMiddleware") + return h(c) + } - // Routes registered with /apps are assumed to be web apps that do their own XSRF - if strings.HasPrefix(c.Request().URL.String(), "/pp/v1/apps/") { - return h(c) + p.removeEmptyCookie(c) + + userID, err := p.GetSessionValue(c, "user_id") + if err == nil { + c.Set("user_id", userID) + return h(c) + } + + // Don't log an error if we are verifying the session, as a failure is not an error + isVerify := strings.HasSuffix(c.Request().RequestURI, "/auth/session/verify") + if isVerify { + // Tell the frontend what the Cookie Domain is so it can check if sessions will work + c.Response().Header().Set(StratosDomainHeader, p.Config.CookieDomain) + } + + // Clear any session cookie + cookie := new(http.Cookie) + cookie.Name = p.SessionCookieName + cookie.Value = "" + cookie.Expires = time.Now().Add(-24 * time.Hour) + cookie.Domain = p.SessionStoreOptions.Domain + cookie.HttpOnly = p.SessionStoreOptions.HttpOnly + cookie.Secure = p.SessionStoreOptions.Secure + cookie.Path = p.SessionStoreOptions.Path + cookie.MaxAge = 0 + c.SetCookie(cookie) + + return handleSessionError(p.Config, c, err, isVerify, "User session could not be found") } + } +} - errMsg := "Failed to get stored XSRF token from user session" - token, err := p.GetSessionStringValue(c, XSRFTokenSessionName) - if err == nil { - // Check the token against the header - requestToken := c.Request().Header.Get(XSRFTokenHeader) - if len(requestToken) > 0 { - if compareTokens(requestToken, token) { - return h(c) +func (p *portalProxy) xsrfMiddleware() echo.MiddlewareFunc { + return p.xsrfMiddlewareWithConfig(MiddlewareConfig{}) +} + +func (p *portalProxy) xsrfMiddlewareWithConfig(config MiddlewareConfig) echo.MiddlewareFunc { + // Default skipper function always returns false + if config.Skipper == nil { + config.Skipper = func(c echo.Context) bool { return false } + } + + return func(h echo.HandlerFunc) echo.HandlerFunc { + return func(c echo.Context) error { + log.Debug("xsrfMiddleware") + + // Only do this for mutating requests - i.e. we can ignore for GET or HEAD requests + if c.Request().Method == "GET" || c.Request().Method == "HEAD" { + return h(c) + } + + // Routes registered with /apps are assumed to be web apps that do their own XSRF + if strings.HasPrefix(c.Request().URL.String(), "/pp/v1/apps/") { + return h(c) + } + + errMsg := "Failed to get stored XSRF token from user session" + token, err := p.GetSessionStringValue(c, XSRFTokenSessionName) + if err == nil { + // Check the token against the header + requestToken := c.Request().Header.Get(XSRFTokenHeader) + if len(requestToken) > 0 { + if compareTokens(requestToken, token) { + return h(c) + } + errMsg = "Supplied XSRF Token does not match" + } else { + errMsg = "XSRF Token was not supplied in the header" } - errMsg = "Supplied XSRF Token does not match" - } else { - errMsg = "XSRF Token was not supplied in the header" } + return interfaces.NewHTTPShadowError( + http.StatusUnauthorized, + "XSRF Token could not be found or does not match", + "XSRF Token error: %s", errMsg, + ) } - return interfaces.NewHTTPShadowError( - http.StatusUnauthorized, - "XSRF Token could not be found or does not match", - "XSRF Token error: %s", errMsg, - ) } } @@ -254,3 +303,58 @@ func retryAfterUpgradeMiddleware(h echo.HandlerFunc, env *env.VarSet) echo.Handl return h(c) } } + +func getAPIKeyFromHeader(c echo.Context) (string, error) { + header := c.Request().Header.Get(APIKeyHeader) + + l := len(APIKeyAuthScheme) + if len(header) > l+1 && header[:l] == APIKeyAuthScheme { + return header[l+1:], nil + } + + return "", errors.New("No API key in the header") +} + +func (p *portalProxy) apiKeyMiddleware(h echo.HandlerFunc) echo.HandlerFunc { + return func(c echo.Context) error { + log.Debug("apiKeyMiddleware") + + apiKey, err := getAPIKeyFromHeader(c) + if err != nil { + log.Debugf("apiKeyMiddleware: %v", err) + return h(c) + } + + apiKeysRepo, err := apikeys.NewPgsqlAPIKeysRepository(p.DatabaseConnectionPool) + if err != nil { + log.Errorf("apiKeyMiddleware: %v", err) + return h(c) + } + + userID, err := apiKeysRepo.GetAPIKeyUserID(apiKey) + if err != nil { + switch { + case err == sql.ErrNoRows: + log.Debug("apiKeyMiddleware: Invalid API key supplied") + default: + log.Warnf("apiKeyMiddleware: %v", err) + } + + return h(c) + } + + c.Set(APIKeySkipperContextKey, true) + c.Set("user_id", userID) + + // some endpoints check not only the context store, but also the contents of the session store + sessionValues := make(map[string]interface{}) + sessionValues["user_id"] = userID + p.setSessionValues(c, sessionValues) + + return h(c) + } +} + +func (p *portalProxy) apiKeySkipper(c echo.Context) bool { + return c.Get(APIKeySkipperContextKey) != nil && c.Get(APIKeySkipperContextKey).(bool) == true +} diff --git a/src/jetstream/repository/apikeys/apikeys.go b/src/jetstream/repository/apikeys/apikeys.go new file mode 100644 index 0000000000..f64cba0b67 --- /dev/null +++ b/src/jetstream/repository/apikeys/apikeys.go @@ -0,0 +1,11 @@ +package apikeys + +import "github.com/cloudfoundry-incubator/stratos/src/jetstream/repository/interfaces" + +// Repository - API keys repository +type Repository interface { + AddAPIKey(userID string, comment string) (*interfaces.APIKey, error) + GetAPIKeyUserID(keySecret string) (string, error) + ListAPIKeys(userID string) ([]interfaces.APIKey, error) + DeleteAPIKey(userGUID string, keyGUID string) error +} diff --git a/src/jetstream/repository/apikeys/psql_apikeys.go b/src/jetstream/repository/apikeys/psql_apikeys.go new file mode 100644 index 0000000000..e53b7c41b3 --- /dev/null +++ b/src/jetstream/repository/apikeys/psql_apikeys.go @@ -0,0 +1,138 @@ +package apikeys + +import ( + "database/sql" + "errors" + + "github.com/cloudfoundry-incubator/stratos/src/jetstream/datastore" + "github.com/cloudfoundry-incubator/stratos/src/jetstream/repository/interfaces" + uuid "github.com/satori/go.uuid" + log "github.com/sirupsen/logrus" +) + +var insertAPIKey = `INSERT INTO api_keys (guid, secret, user_guid, comment) VALUES ($1, $2, $3, $4)` +var getAPIKeyUserID = `SELECT user_guid FROM api_keys WHERE secret = $1` +var listAPIKeys = `SELECT guid, user_guid, comment FROM api_keys WHERE user_guid = $1` +var deleteAPIKey = `DELETE FROM api_keys WHERE user_guid = $1 AND guid = $2` + +// PgsqlAPIKeysRepository - Postgresql-backed API keys repository +type PgsqlAPIKeysRepository struct { + db *sql.DB +} + +// NewPgsqlAPIKeysRepository - get a reference to the API keys data source +func NewPgsqlAPIKeysRepository(dcp *sql.DB) (Repository, error) { + log.Debug("NewPgsqlAPIKeysRepository") + return &PgsqlAPIKeysRepository{db: dcp}, nil +} + +// InitRepositoryProvider - One time init for the given DB Provider +func InitRepositoryProvider(databaseProvider string) { + // Modify the database statements if needed, for the given database type + insertAPIKey = datastore.ModifySQLStatement(insertAPIKey, databaseProvider) + getAPIKeyUserID = datastore.ModifySQLStatement(getAPIKeyUserID, databaseProvider) + deleteAPIKey = datastore.ModifySQLStatement(deleteAPIKey, databaseProvider) + listAPIKeys = datastore.ModifySQLStatement(listAPIKeys, databaseProvider) +} + +// AddAPIKey - Add a new API key to the datastore. +func (p *PgsqlAPIKeysRepository) AddAPIKey(userID string, comment string) (*interfaces.APIKey, error) { + log.Debug("AddAPIKey") + + var err error + + // Validate args + if len(comment) > 255 { + msg := "comment must be less than 255 characters long" + log.Debug(msg) + err = errors.New(msg) + } + if err != nil { + return nil, err + } + + keyGUID := uuid.NewV4().String() + keySecret := uuid.NewV4().String() + + var result sql.Result + if result, err = p.db.Exec(insertAPIKey, keyGUID, keySecret, userID, comment); err != nil { + log.Errorf("unable to INSERT API key: %v", err) + return nil, err + } + + //Validate that 1 row has been updated + rowsUpdates, err := result.RowsAffected() + if err != nil { + return nil, errors.New("unable to INSERT api key: could not determine number of rows that were updated") + } else if rowsUpdates < 1 { + return nil, errors.New("unable to INSERT api key: no rows were updated") + } + + apiKey := &interfaces.APIKey{ + GUID: keyGUID, + Secret: keySecret, + UserGUID: userID, + Comment: comment, + } + + return apiKey, err +} + +// GetAPIKeyUserID - gets user ID for an API key +func (p *PgsqlAPIKeysRepository) GetAPIKeyUserID(keySecret string) (string, error) { + log.Debug("GetAPIKeyUserID") + + var ( + err error + userGUID string + ) + + if err = p.db.QueryRow(getAPIKeyUserID, keySecret).Scan(&userGUID); err != nil { + return "", err + } + + return userGUID, nil +} + +// ListAPIKeys - list API keys for a given user GUID +func (p *PgsqlAPIKeysRepository) ListAPIKeys(userID string) ([]interfaces.APIKey, error) { + log.Debug("ListAPIKeys") + + rows, err := p.db.Query(listAPIKeys, userID) + if err != nil { + log.Errorf("unable to list API keys: %v", err) + return nil, err + } + + result := []interfaces.APIKey{} + for rows.Next() { + var apiKey interfaces.APIKey + err = rows.Scan(&apiKey.GUID, &apiKey.UserGUID, &apiKey.Comment) + if err != nil { + log.Errorf("Scan: %v", err) + return nil, err + } + result = append(result, apiKey) + } + + return result, nil +} + +// DeleteAPIKey - delete an API key identified by its GUID +func (p *PgsqlAPIKeysRepository) DeleteAPIKey(userGUID string, keyGUID string) error { + log.Debug("DeleteAPIKey") + + result, err := p.db.Exec(deleteAPIKey, userGUID, keyGUID) + if err != nil { + return err + } + + rowsUpdates, err := result.RowsAffected() + if err != nil { + return errors.New("unable to DELETE api key: could not determine number of rows that were updated") + } else if rowsUpdates < 1 { + return errors.New("unable to DELETE api key: no rows were updated") + } + + return nil +} diff --git a/src/jetstream/repository/interfaces/apikeys.go b/src/jetstream/repository/interfaces/apikeys.go new file mode 100644 index 0000000000..fd8b1c71b2 --- /dev/null +++ b/src/jetstream/repository/interfaces/apikeys.go @@ -0,0 +1,8 @@ +package interfaces + +type APIKey struct { + GUID string `json:"guid"` + Secret string `json:"secret"` + UserGUID string `json:"user_guid"` + Comment string `json:"comment"` +} From 96248c09ac3673426458c129e9dfe3d81227e2fd Mon Sep 17 00:00:00 2001 From: Ivan Kapelyukhin Date: Wed, 19 Aug 2020 14:41:18 +0200 Subject: [PATCH 2/8] Add last_used field to API keys --- .../datastore/20200814140918_ApiKeys.go | 1 + src/jetstream/middleware.go | 15 ++++-- src/jetstream/repository/apikeys/apikeys.go | 3 +- .../repository/apikeys/psql_apikeys.go | 54 ++++++++++++++----- .../repository/interfaces/apikeys.go | 12 +++-- 5 files changed, 62 insertions(+), 23 deletions(-) diff --git a/src/jetstream/datastore/20200814140918_ApiKeys.go b/src/jetstream/datastore/20200814140918_ApiKeys.go index ba9566dda2..2a00b44365 100644 --- a/src/jetstream/datastore/20200814140918_ApiKeys.go +++ b/src/jetstream/datastore/20200814140918_ApiKeys.go @@ -13,6 +13,7 @@ func init() { apiTokenTable += "secret VARCHAR(36) NOT NULL UNIQUE," apiTokenTable += "user_guid VARCHAR(36) NOT NULL," apiTokenTable += "comment VARCHAR(255) NOT NULL," + apiTokenTable += "last_used TIMESTAMP," apiTokenTable += "PRIMARY KEY (guid) );" _, err := txn.Exec(apiTokenTable) diff --git a/src/jetstream/middleware.go b/src/jetstream/middleware.go index c7e1b115b0..d41f3bc540 100644 --- a/src/jetstream/middleware.go +++ b/src/jetstream/middleware.go @@ -319,7 +319,7 @@ func (p *portalProxy) apiKeyMiddleware(h echo.HandlerFunc) echo.HandlerFunc { return func(c echo.Context) error { log.Debug("apiKeyMiddleware") - apiKey, err := getAPIKeyFromHeader(c) + apiKeySecret, err := getAPIKeyFromHeader(c) if err != nil { log.Debugf("apiKeyMiddleware: %v", err) return h(c) @@ -331,26 +331,31 @@ func (p *portalProxy) apiKeyMiddleware(h echo.HandlerFunc) echo.HandlerFunc { return h(c) } - userID, err := apiKeysRepo.GetAPIKeyUserID(apiKey) + apiKey, err := apiKeysRepo.GetAPIKeyBySecret(apiKeySecret) if err != nil { switch { case err == sql.ErrNoRows: log.Debug("apiKeyMiddleware: Invalid API key supplied") default: - log.Warnf("apiKeyMiddleware: %v", err) + log.Errorf("apiKeyMiddleware: %v", err) } return h(c) } c.Set(APIKeySkipperContextKey, true) - c.Set("user_id", userID) + c.Set("user_id", apiKey.UserGUID) // some endpoints check not only the context store, but also the contents of the session store sessionValues := make(map[string]interface{}) - sessionValues["user_id"] = userID + sessionValues["user_id"] = apiKey.UserGUID p.setSessionValues(c, sessionValues) + err = apiKeysRepo.UpdateAPIKeyLastUsed(apiKey.GUID) + if err != nil { + log.Errorf("apiKeyMiddleware: %v", err) + } + return h(c) } } diff --git a/src/jetstream/repository/apikeys/apikeys.go b/src/jetstream/repository/apikeys/apikeys.go index f64cba0b67..bf783990a0 100644 --- a/src/jetstream/repository/apikeys/apikeys.go +++ b/src/jetstream/repository/apikeys/apikeys.go @@ -5,7 +5,8 @@ import "github.com/cloudfoundry-incubator/stratos/src/jetstream/repository/inter // Repository - API keys repository type Repository interface { AddAPIKey(userID string, comment string) (*interfaces.APIKey, error) - GetAPIKeyUserID(keySecret string) (string, error) + GetAPIKeyBySecret(keySecret string) (*interfaces.APIKey, error) ListAPIKeys(userID string) ([]interfaces.APIKey, error) DeleteAPIKey(userGUID string, keyGUID string) error + UpdateAPIKeyLastUsed(keyGUID string) error } diff --git a/src/jetstream/repository/apikeys/psql_apikeys.go b/src/jetstream/repository/apikeys/psql_apikeys.go index e53b7c41b3..eee946696d 100644 --- a/src/jetstream/repository/apikeys/psql_apikeys.go +++ b/src/jetstream/repository/apikeys/psql_apikeys.go @@ -3,6 +3,7 @@ package apikeys import ( "database/sql" "errors" + "time" "github.com/cloudfoundry-incubator/stratos/src/jetstream/datastore" "github.com/cloudfoundry-incubator/stratos/src/jetstream/repository/interfaces" @@ -11,9 +12,12 @@ import ( ) var insertAPIKey = `INSERT INTO api_keys (guid, secret, user_guid, comment) VALUES ($1, $2, $3, $4)` -var getAPIKeyUserID = `SELECT user_guid FROM api_keys WHERE secret = $1` -var listAPIKeys = `SELECT guid, user_guid, comment FROM api_keys WHERE user_guid = $1` +var getAPIKeyBySecret = `SELECT guid, user_guid, comment, last_used FROM api_keys WHERE secret = $1` +var listAPIKeys = `SELECT guid, user_guid, comment, last_used FROM api_keys WHERE user_guid = $1` var deleteAPIKey = `DELETE FROM api_keys WHERE user_guid = $1 AND guid = $2` +var updateAPIKeyLastUsed = `UPDATE api_keys SET last_used = $1 WHERE guid = $2` + +// UpdateAPIKeyLastUsed // PgsqlAPIKeysRepository - Postgresql-backed API keys repository type PgsqlAPIKeysRepository struct { @@ -30,9 +34,10 @@ func NewPgsqlAPIKeysRepository(dcp *sql.DB) (Repository, error) { func InitRepositoryProvider(databaseProvider string) { // Modify the database statements if needed, for the given database type insertAPIKey = datastore.ModifySQLStatement(insertAPIKey, databaseProvider) - getAPIKeyUserID = datastore.ModifySQLStatement(getAPIKeyUserID, databaseProvider) + getAPIKeyBySecret = datastore.ModifySQLStatement(getAPIKeyBySecret, databaseProvider) deleteAPIKey = datastore.ModifySQLStatement(deleteAPIKey, databaseProvider) listAPIKeys = datastore.ModifySQLStatement(listAPIKeys, databaseProvider) + updateAPIKeyLastUsed = datastore.ModifySQLStatement(updateAPIKeyLastUsed, databaseProvider) } // AddAPIKey - Add a new API key to the datastore. @@ -78,20 +83,24 @@ func (p *PgsqlAPIKeysRepository) AddAPIKey(userID string, comment string) (*inte return apiKey, err } -// GetAPIKeyUserID - gets user ID for an API key -func (p *PgsqlAPIKeysRepository) GetAPIKeyUserID(keySecret string) (string, error) { - log.Debug("GetAPIKeyUserID") +// GetAPIKeyBySecret - gets user ID for an API key +func (p *PgsqlAPIKeysRepository) GetAPIKeyBySecret(keySecret string) (*interfaces.APIKey, error) { + log.Debug("GetAPIKeyBySecret") + + var apiKey interfaces.APIKey - var ( - err error - userGUID string + err := p.db.QueryRow(getAPIKeyBySecret, keySecret).Scan( + &apiKey.GUID, + &apiKey.UserGUID, + &apiKey.Comment, + &apiKey.LastUsed, ) - if err = p.db.QueryRow(getAPIKeyUserID, keySecret).Scan(&userGUID); err != nil { - return "", err + if err != nil { + return nil, err } - return userGUID, nil + return &apiKey, nil } // ListAPIKeys - list API keys for a given user GUID @@ -107,7 +116,7 @@ func (p *PgsqlAPIKeysRepository) ListAPIKeys(userID string) ([]interfaces.APIKey result := []interfaces.APIKey{} for rows.Next() { var apiKey interfaces.APIKey - err = rows.Scan(&apiKey.GUID, &apiKey.UserGUID, &apiKey.Comment) + err = rows.Scan(&apiKey.GUID, &apiKey.UserGUID, &apiKey.Comment, &apiKey.LastUsed) if err != nil { log.Errorf("Scan: %v", err) return nil, err @@ -136,3 +145,22 @@ func (p *PgsqlAPIKeysRepository) DeleteAPIKey(userGUID string, keyGUID string) e return nil } + +// UpdateAPIKeyLastUsed - sets API key last_used field to current time +func (p *PgsqlAPIKeysRepository) UpdateAPIKeyLastUsed(keyGUID string) error { + log.Debug("UpdateAPIKeyLastUsed") + + result, err := p.db.Exec(updateAPIKeyLastUsed, time.Now(), keyGUID) + if err != nil { + return err + } + + rowsUpdates, err := result.RowsAffected() + if err != nil { + return errors.New("unable to UPDATE api key: could not determine number of rows that were updated") + } else if rowsUpdates < 1 { + return errors.New("unable to UPDATE api key: no rows were updated") + } + + return nil +} diff --git a/src/jetstream/repository/interfaces/apikeys.go b/src/jetstream/repository/interfaces/apikeys.go index fd8b1c71b2..a0ead69e35 100644 --- a/src/jetstream/repository/interfaces/apikeys.go +++ b/src/jetstream/repository/interfaces/apikeys.go @@ -1,8 +1,12 @@ package interfaces +import "time" + +// APIKey - represents API key DB entry type APIKey struct { - GUID string `json:"guid"` - Secret string `json:"secret"` - UserGUID string `json:"user_guid"` - Comment string `json:"comment"` + GUID string `json:"guid"` + Secret string `json:"secret"` + UserGUID string `json:"user_guid"` + Comment string `json:"comment"` + LastUsed *time.Time `json:"last_used"` } From e93d550da2233ef37b0d2b9c7f26be162a63caf0 Mon Sep 17 00:00:00 2001 From: Ivan Kapelyukhin Date: Thu, 20 Aug 2020 17:42:51 +0200 Subject: [PATCH 3/8] Use secure random value as key secret --- src/jetstream/repository/apikeys/psql_apikeys.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/jetstream/repository/apikeys/psql_apikeys.go b/src/jetstream/repository/apikeys/psql_apikeys.go index eee946696d..84b4e626c6 100644 --- a/src/jetstream/repository/apikeys/psql_apikeys.go +++ b/src/jetstream/repository/apikeys/psql_apikeys.go @@ -2,9 +2,11 @@ package apikeys import ( "database/sql" + "encoding/base64" "errors" "time" + "github.com/cloudfoundry-incubator/stratos/src/jetstream/crypto" "github.com/cloudfoundry-incubator/stratos/src/jetstream/datastore" "github.com/cloudfoundry-incubator/stratos/src/jetstream/repository/interfaces" uuid "github.com/satori/go.uuid" @@ -52,12 +54,18 @@ func (p *PgsqlAPIKeysRepository) AddAPIKey(userID string, comment string) (*inte log.Debug(msg) err = errors.New(msg) } + + if err != nil { + return nil, err + } + + randomBytes, err := crypto.GenerateRandomBytes(48) if err != nil { return nil, err } keyGUID := uuid.NewV4().String() - keySecret := uuid.NewV4().String() + keySecret := base64.URLEncoding.EncodeToString(randomBytes) var result sql.Result if result, err = p.db.Exec(insertAPIKey, keyGUID, keySecret, userID, comment); err != nil { From 830fa3aaeb3f1fcc59152ef64f558c151715a3b9 Mon Sep 17 00:00:00 2001 From: Ivan Kapelyukhin Date: Thu, 20 Aug 2020 23:45:53 +0200 Subject: [PATCH 4/8] Add tests for ListAPIKeys and AddAPIKey --- .../repository/apikeys/psql_apikeys.go | 2 +- .../repository/apikeys/psql_apikeys_test.go | 154 ++++++++++++++++++ 2 files changed, 155 insertions(+), 1 deletion(-) create mode 100644 src/jetstream/repository/apikeys/psql_apikeys_test.go diff --git a/src/jetstream/repository/apikeys/psql_apikeys.go b/src/jetstream/repository/apikeys/psql_apikeys.go index 84b4e626c6..881ced5975 100644 --- a/src/jetstream/repository/apikeys/psql_apikeys.go +++ b/src/jetstream/repository/apikeys/psql_apikeys.go @@ -50,7 +50,7 @@ func (p *PgsqlAPIKeysRepository) AddAPIKey(userID string, comment string) (*inte // Validate args if len(comment) > 255 { - msg := "comment must be less than 255 characters long" + msg := "comment maximum length is 255 characters" log.Debug(msg) err = errors.New(msg) } diff --git a/src/jetstream/repository/apikeys/psql_apikeys_test.go b/src/jetstream/repository/apikeys/psql_apikeys_test.go new file mode 100644 index 0000000000..2ea36e1912 --- /dev/null +++ b/src/jetstream/repository/apikeys/psql_apikeys_test.go @@ -0,0 +1,154 @@ +package apikeys + +import ( + "errors" + "strings" + "testing" + "time" + + "github.com/cloudfoundry-incubator/stratos/src/jetstream/repository/interfaces" + . "github.com/smartystreets/goconvey/convey" + "gopkg.in/DATA-DOG/go-sqlmock.v1" +) + +func TestNewPgsqlAPIKeysRepository(t *testing.T) { + Convey("Given a request for a new reference to a API keys Repository", t, func() { + db, _, err := sqlmock.New() + if err != nil { + t.Errorf("an error '%s' was not expected when opening a stub database connection", err) + } + defer db.Close() + + repository, err := NewPgsqlAPIKeysRepository(db) + + Convey("no error should be returned", func() { + So(err, ShouldBeNil) + }) + + Convey("result should be of valid type", func() { + So(repository, ShouldHaveSameTypeAs, &PgsqlAPIKeysRepository{}) + }) + }) +} + +func TestAddAPIKey(t *testing.T) { + var ( + userID = "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa" + insertIntoAPIKeys = `INSERT INTO api_keys` + ) + + Convey("Given a request to add an API key", t, func() { + db, mock, err := sqlmock.New() + if err != nil { + t.Errorf("an error '%s' was not expected when opening a stub database connection", err) + } + defer db.Close() + + repository, _ := NewPgsqlAPIKeysRepository(db) + + _ = mock + + Convey("when the comment exceeds maximal length", func() { + _, err := repository.AddAPIKey(userID, strings.Repeat("a", 256)) + + Convey("there should be no error returned", func() { + So(err, ShouldResemble, errors.New("comment maximum length is 255 characters")) + }) + }) + + Convey("when the comment is not empty", func() { + comment := "test" + + mock.ExpectExec(insertIntoAPIKeys). + WithArgs(sqlmock.AnyArg(), sqlmock.AnyArg(), userID, comment). + WillReturnResult(sqlmock.NewResult(1, 1)) + + apiKey, err := repository.AddAPIKey(userID, comment) + + Convey("there should be no error returned", func() { + So(err, ShouldBeNil) + }) + + Convey("should return an API key", func() { + So(apiKey, ShouldHaveSameTypeAs, &interfaces.APIKey{}) + }) + + Convey("API key secret should not be empty", func() { + So(len(apiKey.Secret), ShouldBeGreaterThan, 0) + }) + }) + }) +} + +func TestListAPIKeys(t *testing.T) { + var ( + userID = "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa" + rowFields = []string{"guid", "user_guid", "comment", "last_used"} + selectUserAPIKeys = `SELECT (.+) FROM api_keys WHERE user_guid = (.+)` + ) + + Convey("Given a request to list API keys", t, func() { + db, mock, err := sqlmock.New() + if err != nil { + t.Errorf("an error '%s' was not expected when opening a stub database connection", err) + } + defer db.Close() + + repository, err := NewPgsqlAPIKeysRepository(db) + + Convey("if no records exist in the DB", func() { + rs := sqlmock.NewRows(rowFields) + mock.ExpectQuery(selectUserAPIKeys).WillReturnRows(rs) + results, err := repository.ListAPIKeys(userID) + + Convey("DB query expectations should be met", func() { + So(mock.ExpectationsWereMet(), ShouldBeNil) + }) + + Convey("there should be no error returned", func() { + So(err, ShouldBeNil) + }) + + Convey("result should be empty", func() { + So(len(results), ShouldEqual, 0) + }) + + Convey("result should be of correct type", func() { + expectedList := make([]interfaces.APIKey, 0) + So(results, ShouldResemble, expectedList) + }) + }) + + Convey("if records exist in the DB", func() { + t := time.Now() + r1 := &interfaces.APIKey{GUID: "00000000-0000-0000-0000-000000000000", Secret: "", UserGUID: userID, Comment: "First key", LastUsed: &t} + r2 := &interfaces.APIKey{GUID: "11111111-1111-1111-1111-111111111111", Secret: "", UserGUID: userID, Comment: "Second key", LastUsed: nil} + + expectedList := []interfaces.APIKey{*r1, *r2} + + mockRows := sqlmock.NewRows(rowFields). + AddRow(r1.GUID, r1.UserGUID, r1.Comment, r1.LastUsed). + AddRow(r2.GUID, r2.UserGUID, r2.Comment, r2.LastUsed) + + mock.ExpectQuery(selectUserAPIKeys).WillReturnRows(mockRows) + + results, err := repository.ListAPIKeys(userID) + + Convey("DB query expectations should be met", func() { + So(mock.ExpectationsWereMet(), ShouldBeNil) + }) + + Convey("there should be no error returned", func() { + So(err, ShouldBeNil) + }) + + Convey("2 API keys should be returned", func() { + So(len(results), ShouldEqual, 2) + }) + + Convey("result should be of correct type", func() { + So(results, ShouldResemble, expectedList) + }) + }) + }) +} From 94840939d4390975c03ca91997f9a8928c5e7e2a Mon Sep 17 00:00:00 2001 From: Ivan Kapelyukhin Date: Fri, 21 Aug 2020 14:04:20 +0200 Subject: [PATCH 5/8] Cover the rest of psql_apikeys.go with tests --- .../repository/apikeys/psql_apikeys_test.go | 199 +++++++++++++++++- 1 file changed, 192 insertions(+), 7 deletions(-) diff --git a/src/jetstream/repository/apikeys/psql_apikeys_test.go b/src/jetstream/repository/apikeys/psql_apikeys_test.go index 2ea36e1912..0064b992eb 100644 --- a/src/jetstream/repository/apikeys/psql_apikeys_test.go +++ b/src/jetstream/repository/apikeys/psql_apikeys_test.go @@ -35,6 +35,7 @@ func TestAddAPIKey(t *testing.T) { var ( userID = "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa" insertIntoAPIKeys = `INSERT INTO api_keys` + comment = "test" ) Convey("Given a request to add an API key", t, func() { @@ -46,19 +47,31 @@ func TestAddAPIKey(t *testing.T) { repository, _ := NewPgsqlAPIKeysRepository(db) - _ = mock - Convey("when the comment exceeds maximal length", func() { _, err := repository.AddAPIKey(userID, strings.Repeat("a", 256)) - Convey("there should be no error returned", func() { + Convey("an error should be returned", func() { So(err, ShouldResemble, errors.New("comment maximum length is 255 characters")) }) }) - Convey("when the comment is not empty", func() { - comment := "test" + Convey("when a key can't be inserted", func() { + mock.ExpectExec(insertIntoAPIKeys). + WithArgs(sqlmock.AnyArg(), sqlmock.AnyArg(), userID, comment). + WillReturnResult(sqlmock.NewResult(0, 0)) + + apiKey, err := repository.AddAPIKey(userID, comment) + Convey("an error should be returned", func() { + So(err, ShouldResemble, errors.New("unable to INSERT api key: no rows were updated")) + }) + + Convey("should return nil", func() { + So(apiKey, ShouldBeNil) + }) + }) + + Convey("when the comment is not empty", func() { mock.ExpectExec(insertIntoAPIKeys). WithArgs(sqlmock.AnyArg(), sqlmock.AnyArg(), userID, comment). WillReturnResult(sqlmock.NewResult(1, 1)) @@ -121,8 +134,22 @@ func TestListAPIKeys(t *testing.T) { Convey("if records exist in the DB", func() { t := time.Now() - r1 := &interfaces.APIKey{GUID: "00000000-0000-0000-0000-000000000000", Secret: "", UserGUID: userID, Comment: "First key", LastUsed: &t} - r2 := &interfaces.APIKey{GUID: "11111111-1111-1111-1111-111111111111", Secret: "", UserGUID: userID, Comment: "Second key", LastUsed: nil} + + r1 := &interfaces.APIKey{ + GUID: "00000000-0000-0000-0000-000000000000", + Secret: "", + UserGUID: userID, + Comment: "First key", + LastUsed: &t, + } + + r2 := &interfaces.APIKey{ + GUID: "11111111-1111-1111-1111-111111111111", + Secret: "", + UserGUID: userID, + Comment: "Second key", + LastUsed: nil, + } expectedList := []interfaces.APIKey{*r1, *r2} @@ -152,3 +179,161 @@ func TestListAPIKeys(t *testing.T) { }) }) } + +func TestGetAPIKeyBySecret(t *testing.T) { + var ( + rowFields = []string{"guid", "user_guid", "comment", "last_used"} + selectAPIKeyBySecret = `SELECT (.+) FROM api_keys WHERE secret = (.+)` + ) + + Convey("Given a request to get an API key by its secret", t, func() { + db, mock, err := sqlmock.New() + if err != nil { + t.Errorf("an error '%s' was not expected when opening a stub database connection", err) + } + defer db.Close() + + repository, err := NewPgsqlAPIKeysRepository(db) + + Convey("if no matching record exists in the DB", func() { + rs := sqlmock.NewRows(rowFields) + mock.ExpectQuery(selectAPIKeyBySecret).WillReturnRows(rs) + results, err := repository.GetAPIKeyBySecret("test") + + Convey("DB query expectations should be met", func() { + So(mock.ExpectationsWereMet(), ShouldBeNil) + }) + + Convey("an error should be returned", func() { + So(err, ShouldResemble, errors.New("sql: no rows in result set")) + }) + + Convey("result should be nil", func() { + So(results, ShouldBeNil) + }) + }) + + Convey("if records exist in the DB", func() { + t := time.Now() + + r := &interfaces.APIKey{ + GUID: "00000000-0000-0000-0000-000000000000", + Secret: "", + UserGUID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", + Comment: "First key", + LastUsed: &t, + } + + mockRows := sqlmock.NewRows(rowFields). + AddRow(r.GUID, r.UserGUID, r.Comment, r.LastUsed) + + mock.ExpectQuery(selectAPIKeyBySecret).WillReturnRows(mockRows) + + results, err := repository.GetAPIKeyBySecret("test") + + Convey("DB query expectations should be met", func() { + So(mock.ExpectationsWereMet(), ShouldBeNil) + }) + + Convey("there should be no error returned", func() { + So(err, ShouldBeNil) + }) + + Convey("result should be of correct type", func() { + So(results, ShouldResemble, r) + }) + }) + }) +} + +func TestDeleteAPIKey(t *testing.T) { + var ( + userID = "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa" + keyID = "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb" + deleteFromAPIKeys = `DELETE FROM api_keys WHERE user_guid = (.+) AND guid = (.+)` + ) + + Convey("Given a request to add an API key", t, func() { + db, mock, err := sqlmock.New() + if err != nil { + t.Errorf("an error '%s' was not expected when opening a stub database connection", err) + } + defer db.Close() + + repository, _ := NewPgsqlAPIKeysRepository(db) + + Convey("when a matching key doesn't exist", func() { + mock.ExpectExec(deleteFromAPIKeys). + WithArgs(userID, keyID). + WillReturnResult(sqlmock.NewResult(0, 0)) + + err := repository.DeleteAPIKey(userID, keyID) + + Convey("an error should be returned", func() { + So(err, ShouldResemble, errors.New("unable to DELETE api key: no rows were updated")) + }) + }) + + Convey("when a matching key exists", func() { + mock.ExpectExec(deleteFromAPIKeys). + WithArgs(userID, keyID). + WillReturnResult(sqlmock.NewResult(1, 1)) + + err := repository.DeleteAPIKey(userID, keyID) + + Convey("DB query expectations should be met", func() { + So(mock.ExpectationsWereMet(), ShouldBeNil) + }) + + Convey("there should be no error returned", func() { + So(err, ShouldBeNil) + }) + }) + }) +} + +// +func TestUpdateAPIKeyLastUsed(t *testing.T) { + var ( + keyID = "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb" + updateLastUsed = `UPDATE api_keys SET last_used = (.+) WHERE guid = (.+)` + ) + + Convey("Given a request to update API key last used", t, func() { + db, mock, err := sqlmock.New() + if err != nil { + t.Errorf("an error '%s' was not expected when opening a stub database connection", err) + } + defer db.Close() + + repository, _ := NewPgsqlAPIKeysRepository(db) + + Convey("when a matching key doesn't exist", func() { + mock.ExpectExec(updateLastUsed). + WithArgs(sqlmock.AnyArg(), keyID). + WillReturnResult(sqlmock.NewResult(0, 0)) + + err := repository.UpdateAPIKeyLastUsed(keyID) + + Convey("an error should be returned", func() { + So(err, ShouldResemble, errors.New("unable to UPDATE api key: no rows were updated")) + }) + }) + + Convey("when a matching key exists", func() { + mock.ExpectExec(updateLastUsed). + WithArgs(sqlmock.AnyArg(), keyID). + WillReturnResult(sqlmock.NewResult(1, 1)) + + err := repository.UpdateAPIKeyLastUsed(keyID) + + Convey("DB query expectations should be met", func() { + So(mock.ExpectationsWereMet(), ShouldBeNil) + }) + + Convey("there should be no error returned", func() { + So(err, ShouldBeNil) + }) + }) + }) +} From 541b8f324ea5951be28167f440863b26fe8a177f Mon Sep 17 00:00:00 2001 From: Ivan Kapelyukhin Date: Fri, 21 Aug 2020 14:47:29 +0200 Subject: [PATCH 6/8] Refactor the code a bit * Storing SQL queries in a struct should ensure that `datastore.ModifySQLStatement` gets called on all of them. * A wrapper func around `db.Exec` reduces copypasta. * Actually call `InitRepositoryProvider` for API keys package --- src/jetstream/main.go | 2 + .../repository/apikeys/psql_apikeys.go | 84 +++++++++++-------- .../repository/apikeys/psql_apikeys_test.go | 6 +- 3 files changed, 52 insertions(+), 40 deletions(-) diff --git a/src/jetstream/main.go b/src/jetstream/main.go index 13dd1c7587..16f253c467 100644 --- a/src/jetstream/main.go +++ b/src/jetstream/main.go @@ -37,6 +37,7 @@ import ( "github.com/cloudfoundry-incubator/stratos/src/jetstream/crypto" "github.com/cloudfoundry-incubator/stratos/src/jetstream/datastore" + "github.com/cloudfoundry-incubator/stratos/src/jetstream/repository/apikeys" "github.com/cloudfoundry-incubator/stratos/src/jetstream/repository/cnsis" "github.com/cloudfoundry-incubator/stratos/src/jetstream/repository/console_config" "github.com/cloudfoundry-incubator/stratos/src/jetstream/repository/interfaces" @@ -178,6 +179,7 @@ func main() { console_config.InitRepositoryProvider(dc.DatabaseProvider) localusers.InitRepositoryProvider(dc.DatabaseProvider) sessiondata.InitRepositoryProvider(dc.DatabaseProvider) + apikeys.InitRepositoryProvider(dc.DatabaseProvider) // Establish a Postgresql connection pool databaseConnectionPool, migratorConf, err := initConnPool(dc, envLookup) diff --git a/src/jetstream/repository/apikeys/psql_apikeys.go b/src/jetstream/repository/apikeys/psql_apikeys.go index 881ced5975..d9ca91c42c 100644 --- a/src/jetstream/repository/apikeys/psql_apikeys.go +++ b/src/jetstream/repository/apikeys/psql_apikeys.go @@ -4,6 +4,8 @@ import ( "database/sql" "encoding/base64" "errors" + "fmt" + "reflect" "time" "github.com/cloudfoundry-incubator/stratos/src/jetstream/crypto" @@ -13,13 +15,19 @@ import ( log "github.com/sirupsen/logrus" ) -var insertAPIKey = `INSERT INTO api_keys (guid, secret, user_guid, comment) VALUES ($1, $2, $3, $4)` -var getAPIKeyBySecret = `SELECT guid, user_guid, comment, last_used FROM api_keys WHERE secret = $1` -var listAPIKeys = `SELECT guid, user_guid, comment, last_used FROM api_keys WHERE user_guid = $1` -var deleteAPIKey = `DELETE FROM api_keys WHERE user_guid = $1 AND guid = $2` -var updateAPIKeyLastUsed = `UPDATE api_keys SET last_used = $1 WHERE guid = $2` - -// UpdateAPIKeyLastUsed +var sqlQueries = struct { + InsertAPIKey string + GetAPIKeyBySecret string + ListAPIKeys string + DeleteAPIKey string + UpdateAPIKeyLastUsed string +}{ + InsertAPIKey: `INSERT INTO api_keys (guid, secret, user_guid, comment) VALUES ($1, $2, $3, $4)`, + GetAPIKeyBySecret: `SELECT guid, user_guid, comment, last_used FROM api_keys WHERE secret = $1`, + ListAPIKeys: `SELECT guid, user_guid, comment, last_used FROM api_keys WHERE user_guid = $1`, + DeleteAPIKey: `DELETE FROM api_keys WHERE user_guid = $1 AND guid = $2`, + UpdateAPIKeyLastUsed: `UPDATE api_keys SET last_used = $1 WHERE guid = $2`, +} // PgsqlAPIKeysRepository - Postgresql-backed API keys repository type PgsqlAPIKeysRepository struct { @@ -35,11 +43,19 @@ func NewPgsqlAPIKeysRepository(dcp *sql.DB) (Repository, error) { // InitRepositoryProvider - One time init for the given DB Provider func InitRepositoryProvider(databaseProvider string) { // Modify the database statements if needed, for the given database type - insertAPIKey = datastore.ModifySQLStatement(insertAPIKey, databaseProvider) - getAPIKeyBySecret = datastore.ModifySQLStatement(getAPIKeyBySecret, databaseProvider) - deleteAPIKey = datastore.ModifySQLStatement(deleteAPIKey, databaseProvider) - listAPIKeys = datastore.ModifySQLStatement(listAPIKeys, databaseProvider) - updateAPIKeyLastUsed = datastore.ModifySQLStatement(updateAPIKeyLastUsed, databaseProvider) + // Iterating over the struct to ensure that all of the queries are updated + v := reflect.ValueOf(sqlQueries) + for i := 0; i < v.NumField(); i++ { + q := v.Field(i).Interface().(string) + + reflect. + ValueOf(&sqlQueries). + Elem(). + FieldByIndex([]int{i}). + SetString( + datastore.ModifySQLStatement(q, databaseProvider), + ) + } } // AddAPIKey - Add a new API key to the datastore. @@ -67,18 +83,9 @@ func (p *PgsqlAPIKeysRepository) AddAPIKey(userID string, comment string) (*inte keyGUID := uuid.NewV4().String() keySecret := base64.URLEncoding.EncodeToString(randomBytes) - var result sql.Result - if result, err = p.db.Exec(insertAPIKey, keyGUID, keySecret, userID, comment); err != nil { - log.Errorf("unable to INSERT API key: %v", err) - return nil, err - } - - //Validate that 1 row has been updated - rowsUpdates, err := result.RowsAffected() + err = execQuery(p, sqlQueries.InsertAPIKey, keyGUID, keySecret, userID, comment) if err != nil { - return nil, errors.New("unable to INSERT api key: could not determine number of rows that were updated") - } else if rowsUpdates < 1 { - return nil, errors.New("unable to INSERT api key: no rows were updated") + return nil, fmt.Errorf("AddAPIKey: %v", err) } apiKey := &interfaces.APIKey{ @@ -97,7 +104,7 @@ func (p *PgsqlAPIKeysRepository) GetAPIKeyBySecret(keySecret string) (*interface var apiKey interfaces.APIKey - err := p.db.QueryRow(getAPIKeyBySecret, keySecret).Scan( + err := p.db.QueryRow(sqlQueries.GetAPIKeyBySecret, keySecret).Scan( &apiKey.GUID, &apiKey.UserGUID, &apiKey.Comment, @@ -115,7 +122,7 @@ func (p *PgsqlAPIKeysRepository) GetAPIKeyBySecret(keySecret string) (*interface func (p *PgsqlAPIKeysRepository) ListAPIKeys(userID string) ([]interfaces.APIKey, error) { log.Debug("ListAPIKeys") - rows, err := p.db.Query(listAPIKeys, userID) + rows, err := p.db.Query(sqlQueries.ListAPIKeys, userID) if err != nil { log.Errorf("unable to list API keys: %v", err) return nil, err @@ -139,16 +146,9 @@ func (p *PgsqlAPIKeysRepository) ListAPIKeys(userID string) ([]interfaces.APIKey func (p *PgsqlAPIKeysRepository) DeleteAPIKey(userGUID string, keyGUID string) error { log.Debug("DeleteAPIKey") - result, err := p.db.Exec(deleteAPIKey, userGUID, keyGUID) - if err != nil { - return err - } - - rowsUpdates, err := result.RowsAffected() + err := execQuery(p, sqlQueries.DeleteAPIKey, userGUID, keyGUID) if err != nil { - return errors.New("unable to DELETE api key: could not determine number of rows that were updated") - } else if rowsUpdates < 1 { - return errors.New("unable to DELETE api key: no rows were updated") + return fmt.Errorf("DeleteAPIKey: %v", err) } return nil @@ -158,16 +158,26 @@ func (p *PgsqlAPIKeysRepository) DeleteAPIKey(userGUID string, keyGUID string) e func (p *PgsqlAPIKeysRepository) UpdateAPIKeyLastUsed(keyGUID string) error { log.Debug("UpdateAPIKeyLastUsed") - result, err := p.db.Exec(updateAPIKeyLastUsed, time.Now(), keyGUID) + err := execQuery(p, sqlQueries.UpdateAPIKeyLastUsed, time.Now(), keyGUID) + if err != nil { + return fmt.Errorf("UpdateAPIKeyLastUsed: %v", err) + } + + return nil +} + +// A wrapper around db.Exec that validates that exactly 1 row has been inserted/deleted/updated +func execQuery(p *PgsqlAPIKeysRepository, query string, args ...interface{}) error { + result, err := p.db.Exec(query, args...) if err != nil { return err } rowsUpdates, err := result.RowsAffected() if err != nil { - return errors.New("unable to UPDATE api key: could not determine number of rows that were updated") + return errors.New("could not determine number of rows that were updated") } else if rowsUpdates < 1 { - return errors.New("unable to UPDATE api key: no rows were updated") + return errors.New("no rows were updated") } return nil diff --git a/src/jetstream/repository/apikeys/psql_apikeys_test.go b/src/jetstream/repository/apikeys/psql_apikeys_test.go index 0064b992eb..3534e4619c 100644 --- a/src/jetstream/repository/apikeys/psql_apikeys_test.go +++ b/src/jetstream/repository/apikeys/psql_apikeys_test.go @@ -63,7 +63,7 @@ func TestAddAPIKey(t *testing.T) { apiKey, err := repository.AddAPIKey(userID, comment) Convey("an error should be returned", func() { - So(err, ShouldResemble, errors.New("unable to INSERT api key: no rows were updated")) + So(err, ShouldResemble, errors.New("AddAPIKey: no rows were updated")) }) Convey("should return nil", func() { @@ -270,7 +270,7 @@ func TestDeleteAPIKey(t *testing.T) { err := repository.DeleteAPIKey(userID, keyID) Convey("an error should be returned", func() { - So(err, ShouldResemble, errors.New("unable to DELETE api key: no rows were updated")) + So(err, ShouldResemble, errors.New("DeleteAPIKey: no rows were updated")) }) }) @@ -316,7 +316,7 @@ func TestUpdateAPIKeyLastUsed(t *testing.T) { err := repository.UpdateAPIKeyLastUsed(keyID) Convey("an error should be returned", func() { - So(err, ShouldResemble, errors.New("unable to UPDATE api key: no rows were updated")) + So(err, ShouldResemble, errors.New("UpdateAPIKeyLastUsed: no rows were updated")) }) }) From 51d82ee4c02aef5ef77f9514fc45e7530ec41e93 Mon Sep 17 00:00:00 2001 From: Ivan Kapelyukhin Date: Fri, 21 Aug 2020 20:27:12 +0200 Subject: [PATCH 7/8] Add route handler tests using gomock --- src/jetstream/apikeys.go | 38 +-- src/jetstream/apikeys_test.go | 289 ++++++++++++++++++ src/jetstream/go.mod | 1 + src/jetstream/go.sum | 5 + src/jetstream/main.go | 6 + src/jetstream/portal_proxy.go | 2 + .../repository/apikeys/mock_apikeys.go | 107 +++++++ 7 files changed, 420 insertions(+), 28 deletions(-) create mode 100644 src/jetstream/apikeys_test.go create mode 100644 src/jetstream/repository/apikeys/mock_apikeys.go diff --git a/src/jetstream/apikeys.go b/src/jetstream/apikeys.go index 5ce7c98ec0..3985f6fda7 100644 --- a/src/jetstream/apikeys.go +++ b/src/jetstream/apikeys.go @@ -1,9 +1,9 @@ package main import ( + "errors" "net/http" - "github.com/cloudfoundry-incubator/stratos/src/jetstream/repository/apikeys" "github.com/labstack/echo" log "github.com/sirupsen/logrus" ) @@ -18,16 +18,10 @@ func (p *portalProxy) addAPIKey(c echo.Context) error { return echo.NewHTTPError(http.StatusBadRequest, "Comment can't be empty") } - apiKeysRepo, err := apikeys.NewPgsqlAPIKeysRepository(p.DatabaseConnectionPool) + apiKey, err := p.APIKeysRepository.AddAPIKey(userGUID, comment) if err != nil { - log.Errorf("Database error getting repo for API keys: %v", err) - return err - } - - apiKey, err := apiKeysRepo.AddAPIKey(userGUID, comment) - if err != nil { - log.Errorf("Error adding API key %v", err) - return err + log.Errorf("Error adding API key: %v", err) + return errors.New("Error adding API key") } return c.JSON(http.StatusOK, apiKey) @@ -36,18 +30,12 @@ func (p *portalProxy) addAPIKey(c echo.Context) error { func (p *portalProxy) listAPIKeys(c echo.Context) error { log.Debug("listAPIKeys") - apiKeysRepo, err := apikeys.NewPgsqlAPIKeysRepository(p.DatabaseConnectionPool) - if err != nil { - log.Errorf("Database error getting repo for API keys: %v", err) - return err - } - userGUID := c.Get("user_id").(string) - apiKeys, err := apiKeysRepo.ListAPIKeys(userGUID) + apiKeys, err := p.APIKeysRepository.ListAPIKeys(userGUID) if err != nil { - log.Errorf("Error listing API keys %v", err) - return nil + log.Errorf("Error listing API keys: %v", err) + return errors.New("Error listing API keys") } return c.JSON(http.StatusOK, apiKeys) @@ -63,15 +51,9 @@ func (p *portalProxy) deleteAPIKey(c echo.Context) error { return echo.NewHTTPError(http.StatusBadRequest, "API key guid can't be empty") } - apiKeysRepo, err := apikeys.NewPgsqlAPIKeysRepository(p.DatabaseConnectionPool) - if err != nil { - log.Errorf("Database error getting repo for API keys: %v", err) - return err - } - - if err = apiKeysRepo.DeleteAPIKey(userGUID, keyGUID); err != nil { - log.Errorf("Error deleting API key %v", err) - return echo.NewHTTPError(http.StatusBadRequest, "Error deleting API key") + if err := p.APIKeysRepository.DeleteAPIKey(userGUID, keyGUID); err != nil { + log.Errorf("Error deleting API key: %v", err) + return errors.New("Error deleting API key") } return nil diff --git a/src/jetstream/apikeys_test.go b/src/jetstream/apikeys_test.go new file mode 100644 index 0000000000..6fb072f1cc --- /dev/null +++ b/src/jetstream/apikeys_test.go @@ -0,0 +1,289 @@ +package main + +import ( + "encoding/json" + "errors" + "net/http" + "testing" + + "github.com/cloudfoundry-incubator/stratos/src/jetstream/repository/apikeys" + "github.com/cloudfoundry-incubator/stratos/src/jetstream/repository/interfaces" + "github.com/golang/mock/gomock" + "github.com/labstack/echo" + log "github.com/sirupsen/logrus" + . "github.com/smartystreets/goconvey/convey" +) + +func Test_addAPIKey(t *testing.T) { + t.Parallel() + + Convey("Given a request to add an API key", t, func() { + log.SetLevel(log.WarnLevel) + + userID := "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa" + + Convey("when comment is not specified", func() { + req := setupMockReq("POST", "", map[string]string{}) + + _, _, ctx, pp, db, _ := setupHTTPTest(req) + defer db.Close() + + ctx.Set("user_id", userID) + + err := pp.addAPIKey(ctx) + + Convey("should return an error", func() { + So(err, ShouldResemble, echo.NewHTTPError(http.StatusBadRequest, "Comment can't be empty")) + }) + }) + + Convey("when a DB error occurs", func() { + comment := "Test API key" + + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + m := apikeys.NewMockRepository(ctrl) + m. + EXPECT(). + AddAPIKey(gomock.Eq(userID), gomock.Eq(comment)). + Return(nil, errors.New("Something went wrong")) + + req := setupMockReq("POST", "", map[string]string{ + "comment": comment, + }) + + _, _, ctx, pp, db, _ := setupHTTPTest(req) + defer db.Close() + + pp.APIKeysRepository = m + + ctx.Set("user_id", userID) + + err := pp.addAPIKey(ctx) + + Convey("should return an error", func() { + So(err, ShouldResemble, errors.New("Error adding API key")) + }) + }) + + Convey("when API key comment was added successfully", func() { + comment := "Test API key" + retval := interfaces.APIKey{UserGUID: userID, Comment: comment} + + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + m := apikeys.NewMockRepository(ctrl) + m. + EXPECT(). + AddAPIKey(gomock.Eq(userID), gomock.Eq(comment)). + Return(&retval, nil) + + req := setupMockReq("POST", "", map[string]string{ + "comment": comment, + }) + + rec, _, ctx, pp, db, _ := setupHTTPTest(req) + defer db.Close() + + pp.APIKeysRepository = m + + ctx.Set("user_id", userID) + + err := pp.addAPIKey(ctx) + + var data map[string]interface{} + if jsonErr := json.Unmarshal(rec.Body.Bytes(), &data); jsonErr != nil { + panic(jsonErr) + } + + Convey("there should be no error", func() { + So(err, ShouldBeNil) + }) + + Convey("should return HTTP code 200", func() { + So(rec.Code, ShouldEqual, 200) + }) + + Convey("API key user_guid should equal context user", func() { + So(data["user_guid"], ShouldEqual, userID) + }) + + Convey("API key comment should equal request comment", func() { + So(data["comment"], ShouldEqual, comment) + }) + + Convey("API key last_used should be nil", func() { + So(data["last_used"], ShouldBeNil) + }) + }) + + }) +} + +func Test_listAPIKeys(t *testing.T) { + t.Parallel() + + Convey("Given a request to list API keys", t, func() { + log.SetLevel(log.WarnLevel) + + userID := "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa" + + Convey("when a DB error occurs", func() { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + m := apikeys.NewMockRepository(ctrl) + m. + EXPECT(). + ListAPIKeys(gomock.Eq(userID)). + Return(nil, errors.New("Something went wrong")) + + req := setupMockReq("GET", "", map[string]string{}) + + _, _, ctx, pp, db, _ := setupHTTPTest(req) + defer db.Close() + pp.APIKeysRepository = m + + ctx.Set("user_id", userID) + + err := pp.listAPIKeys(ctx) + + Convey("should return an error", func() { + So(err, ShouldResemble, errors.New("Error listing API keys")) + }) + }) + + Convey("when DB no errors occur", func() { + r1 := &interfaces.APIKey{ + GUID: "00000000-0000-0000-0000-000000000000", + Secret: "", + UserGUID: userID, + Comment: "First key", + LastUsed: nil, + } + + r2 := &interfaces.APIKey{ + GUID: "11111111-1111-1111-1111-111111111111", + Secret: "", + UserGUID: userID, + Comment: "Second key", + LastUsed: nil, + } + + retval := []interfaces.APIKey{*r1, *r2} + + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + m := apikeys.NewMockRepository(ctrl) + m. + EXPECT(). + ListAPIKeys(gomock.Eq(userID)). + Return(retval, nil) + + req := setupMockReq("GET", "", map[string]string{}) + + rec, _, ctx, pp, db, _ := setupHTTPTest(req) + defer db.Close() + pp.APIKeysRepository = m + + ctx.Set("user_id", userID) + + err := pp.listAPIKeys(ctx) + + Convey("there should be no error", func() { + So(err, ShouldBeNil) + }) + + Convey("return valid JSON", func() { + So(rec.Body.String(), ShouldEqual, jsonMust(retval)+"\n") + }) + }) + }) +} + +func Test_deleteAPIKeys(t *testing.T) { + t.Parallel() + + userID := "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa" + keyID := "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb" + + Convey("Given a request to delete an API key", t, func() { + log.SetLevel(log.PanicLevel) + + Convey("when no API key GUID is supplied", func() { + req := setupMockReq("POST", "", map[string]string{ + "guid": "", + }) + + _, _, ctx, pp, db, _ := setupHTTPTest(req) + defer db.Close() + + ctx.Set("user_id", userID) + + err := pp.deleteAPIKey(ctx) + + Convey("should return an error", func() { + So(err, ShouldResemble, echo.NewHTTPError(http.StatusBadRequest, "API key guid can't be empty")) + }) + }) + + Convey("when an error occured during API key deletion", func() { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + m := apikeys.NewMockRepository(ctrl) + m. + EXPECT(). + DeleteAPIKey(gomock.Eq(userID), gomock.Eq(keyID)). + Return(errors.New("Something went wrong")) + + req := setupMockReq("POST", "", map[string]string{ + "guid": keyID, + }) + + _, _, ctx, pp, db, _ := setupHTTPTest(req) + defer db.Close() + + pp.APIKeysRepository = m + + ctx.Set("user_id", userID) + + err := pp.deleteAPIKey(ctx) + + Convey("should return an error", func() { + So(err, ShouldResemble, errors.New("Error deleting API key")) + }) + }) + + Convey("when an API key is deleted succesfully", func() { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + m := apikeys.NewMockRepository(ctrl) + m. + EXPECT(). + DeleteAPIKey(gomock.Eq(userID), gomock.Eq(keyID)). + Return(nil) + + req := setupMockReq("POST", "", map[string]string{ + "guid": keyID, + }) + + _, _, ctx, pp, db, _ := setupHTTPTest(req) + defer db.Close() + + pp.APIKeysRepository = m + + ctx.Set("user_id", userID) + + err := pp.deleteAPIKey(ctx) + + Convey("there should be no error", func() { + So(err, ShouldBeNil) + }) + }) + }) +} diff --git a/src/jetstream/go.mod b/src/jetstream/go.mod index 9da9655c41..a445d58dfd 100644 --- a/src/jetstream/go.mod +++ b/src/jetstream/go.mod @@ -40,6 +40,7 @@ require ( github.com/fatih/color v1.7.0 // indirect github.com/go-sql-driver/mysql v1.4.1 github.com/gogo/protobuf v1.2.1 // indirect + github.com/golang/mock v1.4.4 github.com/golang/snappy v0.0.1 // indirect github.com/google/go-querystring v1.0.0 // indirect github.com/google/martian v2.1.0+incompatible // indirect diff --git a/src/jetstream/go.sum b/src/jetstream/go.sum index a37e718a67..2902c87b8a 100644 --- a/src/jetstream/go.sum +++ b/src/jetstream/go.sum @@ -135,6 +135,8 @@ github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfU github.com/golang/groupcache v0.0.0-20190129154638-5b532d6fd5ef h1:veQD95Isof8w9/WXiA+pa3tz3fJXkt5B7QaRBrM62gk= github.com/golang/groupcache v0.0.0-20190129154638-5b532d6fd5ef/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc= github.com/golang/mock v1.1.1/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfbm0A= +github.com/golang/mock v1.4.4 h1:l75CXGRSwbaYNpl/Z2X1XIIAMSCquvXgpVZDhwEIJsc= +github.com/golang/mock v1.4.4/go.mod h1:l3mdAwkq5BuhzHwde/uurv3sEJeZMXNpwsxVWU71h+4= github.com/golang/protobuf v1.2.0 h1:P3YflyNX/ehuJFLhxviNdFxQPkGK5cDcApsge1SqnvM= github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/snappy v0.0.1 h1:Qgr9rKW7uDUkrbSmQeiDsGa8SjGyCOGtuasMWwvp2P4= @@ -359,6 +361,7 @@ golang.org/x/net v0.0.0-20181005035420-146acd28ed58/go.mod h1:mL1N/T3taQHkDXs73r golang.org/x/net v0.0.0-20181201002055-351d144fa1fc/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190108225652-1e06a53dbb7e/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190225153610-fe579d43d832/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= +golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190420063019-afa5a82059c6 h1:HdqqaWmYAUI7/dmByKKEw+yxDksGSo+9GjkUc9Zp34E= golang.org/x/net v0.0.0-20190420063019-afa5a82059c6/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= @@ -369,6 +372,7 @@ golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f h1:wMNYb4v58l5UBM7MYRLPG6Zh golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180909124046-d0be0721c37e/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= @@ -386,6 +390,7 @@ golang.org/x/time v0.0.0-20181108054448-85acf8d2951c h1:fqgJT0MGcGpPgpWU7VRdRjuA golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/tools v0.0.0-20180221164845-07fd8470d635/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20190114222345-bf090417da8b/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= +golang.org/x/tools v0.0.0-20190425150028-36563e24a262/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7 h1:9zdDQZ7Thm29KFXgAX/+yaf3eVbP7djjWp/dXAppNCc= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM= diff --git a/src/jetstream/main.go b/src/jetstream/main.go index 16f253c467..1c26e2069d 100644 --- a/src/jetstream/main.go +++ b/src/jetstream/main.go @@ -683,6 +683,12 @@ func newPortalProxy(pc interfaces.PortalConfig, dcp *sql.DB, ss HttpSessionStore Handler: pp.DoOidcFlowRequest, }) + var err error + pp.APIKeysRepository, err = apikeys.NewPgsqlAPIKeysRepository(pp.DatabaseConnectionPool) + if err != nil { + panic(fmt.Errorf("Can't initialize APIKeysRepository: %v", err)) + } + return pp } diff --git a/src/jetstream/portal_proxy.go b/src/jetstream/portal_proxy.go index 7ed43b1022..4dc8ead88b 100644 --- a/src/jetstream/portal_proxy.go +++ b/src/jetstream/portal_proxy.go @@ -5,6 +5,7 @@ import ( "regexp" "time" + "github.com/cloudfoundry-incubator/stratos/src/jetstream/repository/apikeys" "github.com/cloudfoundry-incubator/stratos/src/jetstream/repository/interfaces" "github.com/gorilla/sessions" "github.com/govau/cf-common/env" @@ -24,6 +25,7 @@ type portalProxy struct { AuthProviders map[string]interfaces.AuthProvider env *env.VarSet StratosAuthService interfaces.StratosAuth + APIKeysRepository apikeys.Repository } // HttpSessionStore - Interface for a store that can manage HTTP Sessions diff --git a/src/jetstream/repository/apikeys/mock_apikeys.go b/src/jetstream/repository/apikeys/mock_apikeys.go new file mode 100644 index 0000000000..d507eb5991 --- /dev/null +++ b/src/jetstream/repository/apikeys/mock_apikeys.go @@ -0,0 +1,107 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: apikeys.go + +// Package apikeys is a generated GoMock package. +package apikeys + +import ( + interfaces "github.com/cloudfoundry-incubator/stratos/src/jetstream/repository/interfaces" + gomock "github.com/golang/mock/gomock" + reflect "reflect" +) + +// MockRepository is a mock of Repository interface +type MockRepository struct { + ctrl *gomock.Controller + recorder *MockRepositoryMockRecorder +} + +// MockRepositoryMockRecorder is the mock recorder for MockRepository +type MockRepositoryMockRecorder struct { + mock *MockRepository +} + +// NewMockRepository creates a new mock instance +func NewMockRepository(ctrl *gomock.Controller) *MockRepository { + mock := &MockRepository{ctrl: ctrl} + mock.recorder = &MockRepositoryMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use +func (m *MockRepository) EXPECT() *MockRepositoryMockRecorder { + return m.recorder +} + +// AddAPIKey mocks base method +func (m *MockRepository) AddAPIKey(userID, comment string) (*interfaces.APIKey, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AddAPIKey", userID, comment) + ret0, _ := ret[0].(*interfaces.APIKey) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// AddAPIKey indicates an expected call of AddAPIKey +func (mr *MockRepositoryMockRecorder) AddAPIKey(userID, comment interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddAPIKey", reflect.TypeOf((*MockRepository)(nil).AddAPIKey), userID, comment) +} + +// GetAPIKeyBySecret mocks base method +func (m *MockRepository) GetAPIKeyBySecret(keySecret string) (*interfaces.APIKey, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetAPIKeyBySecret", keySecret) + ret0, _ := ret[0].(*interfaces.APIKey) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetAPIKeyBySecret indicates an expected call of GetAPIKeyBySecret +func (mr *MockRepositoryMockRecorder) GetAPIKeyBySecret(keySecret interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAPIKeyBySecret", reflect.TypeOf((*MockRepository)(nil).GetAPIKeyBySecret), keySecret) +} + +// ListAPIKeys mocks base method +func (m *MockRepository) ListAPIKeys(userID string) ([]interfaces.APIKey, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ListAPIKeys", userID) + ret0, _ := ret[0].([]interfaces.APIKey) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ListAPIKeys indicates an expected call of ListAPIKeys +func (mr *MockRepositoryMockRecorder) ListAPIKeys(userID interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListAPIKeys", reflect.TypeOf((*MockRepository)(nil).ListAPIKeys), userID) +} + +// DeleteAPIKey mocks base method +func (m *MockRepository) DeleteAPIKey(userGUID, keyGUID string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteAPIKey", userGUID, keyGUID) + ret0, _ := ret[0].(error) + return ret0 +} + +// DeleteAPIKey indicates an expected call of DeleteAPIKey +func (mr *MockRepositoryMockRecorder) DeleteAPIKey(userGUID, keyGUID interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAPIKey", reflect.TypeOf((*MockRepository)(nil).DeleteAPIKey), userGUID, keyGUID) +} + +// UpdateAPIKeyLastUsed mocks base method +func (m *MockRepository) UpdateAPIKeyLastUsed(keyGUID string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdateAPIKeyLastUsed", keyGUID) + ret0, _ := ret[0].(error) + return ret0 +} + +// UpdateAPIKeyLastUsed indicates an expected call of UpdateAPIKeyLastUsed +func (mr *MockRepositoryMockRecorder) UpdateAPIKeyLastUsed(keyGUID interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateAPIKeyLastUsed", reflect.TypeOf((*MockRepository)(nil).UpdateAPIKeyLastUsed), keyGUID) +} From 8ab25a4864e1372910aba061e3fb23ce42c5b68e Mon Sep 17 00:00:00 2001 From: Ivan Kapelyukhin Date: Mon, 24 Aug 2020 22:04:58 +0200 Subject: [PATCH 8/8] Actually use skipper in xsrfMiddleware; minor clean-up --- src/jetstream/middleware.go | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/jetstream/middleware.go b/src/jetstream/middleware.go index d41f3bc540..635afc2260 100644 --- a/src/jetstream/middleware.go +++ b/src/jetstream/middleware.go @@ -15,7 +15,6 @@ import ( "github.com/labstack/echo" log "github.com/sirupsen/logrus" - "github.com/cloudfoundry-incubator/stratos/src/jetstream/repository/apikeys" "github.com/cloudfoundry-incubator/stratos/src/jetstream/repository/interfaces" ) @@ -30,7 +29,7 @@ const StratosSSOHeader = "x-stratos-sso-login" // Header to communicate any error during SSO const StratosSSOErrorHeader = "x-stratos-sso-error" -// APIKeyContextKey - context +// APIKeySkipperContextKey - name of a context key that indicates that valid API key was supplied const APIKeySkipperContextKey = "valid_api_key" // APIKeyHeader - API key authentication header name @@ -80,7 +79,7 @@ type ( // Skipper - skipper function for middlewares Skipper func(echo.Context) bool - // MiddlewareConfig defines the config for Logger middleware. + // MiddlewareConfig defines the config for the middleware. MiddlewareConfig struct { // Skipper defines a function to skip middleware. Skipper Skipper @@ -153,6 +152,11 @@ func (p *portalProxy) xsrfMiddlewareWithConfig(config MiddlewareConfig) echo.Mid return func(c echo.Context) error { log.Debug("xsrfMiddleware") + if config.Skipper(c) { + log.Debug("Skipping xsrfMiddleware") + return h(c) + } + // Only do this for mutating requests - i.e. we can ignore for GET or HEAD requests if c.Request().Method == "GET" || c.Request().Method == "HEAD" { return h(c) @@ -325,13 +329,7 @@ func (p *portalProxy) apiKeyMiddleware(h echo.HandlerFunc) echo.HandlerFunc { return h(c) } - apiKeysRepo, err := apikeys.NewPgsqlAPIKeysRepository(p.DatabaseConnectionPool) - if err != nil { - log.Errorf("apiKeyMiddleware: %v", err) - return h(c) - } - - apiKey, err := apiKeysRepo.GetAPIKeyBySecret(apiKeySecret) + apiKey, err := p.APIKeysRepository.GetAPIKeyBySecret(apiKeySecret) if err != nil { switch { case err == sql.ErrNoRows: @@ -351,7 +349,7 @@ func (p *portalProxy) apiKeyMiddleware(h echo.HandlerFunc) echo.HandlerFunc { sessionValues["user_id"] = apiKey.UserGUID p.setSessionValues(c, sessionValues) - err = apiKeysRepo.UpdateAPIKeyLastUsed(apiKey.GUID) + err = p.APIKeysRepository.UpdateAPIKeyLastUsed(apiKey.GUID) if err != nil { log.Errorf("apiKeyMiddleware: %v", err) }