Encryption: Extract encryption into service (#38442)
* Add encryption service * Add tests for encryption service * Inject encryption service into http server * Replace encryption global function usage in login tests * Apply suggestions from code review Co-authored-by: Emil Tullstedt <emil.tullstedt@grafana.com> * Migrate to Wire * Undo non-desired changes * Move Encryption bindings to OSS Wire set Co-authored-by: Joan López de la Franca Beltran <joanjan14@gmail.com> Co-authored-by: Joan López de la Franca Beltran <5459617+joanlopez@users.noreply.github.com> Co-authored-by: Emil Tullstedt <emil.tullstedt@grafana.com>
This commit is contained in:
co-authored by
Emil Tullstedt
Joan López de la Franca Beltran
Joan López de la Franca Beltran
parent
79e79fe244
commit
a0108a1e5b
+12
-11
@@ -13,15 +13,6 @@ import (
|
||||
"strings"
|
||||
"sync"
|
||||
|
||||
"github.com/grafana/grafana/pkg/login/social"
|
||||
"github.com/grafana/grafana/pkg/services/cleanup"
|
||||
"github.com/grafana/grafana/pkg/services/ngalert"
|
||||
"github.com/grafana/grafana/pkg/services/notifications"
|
||||
|
||||
"github.com/grafana/grafana/pkg/services/libraryelements"
|
||||
"github.com/grafana/grafana/pkg/services/librarypanels"
|
||||
"github.com/grafana/grafana/pkg/services/oauthtoken"
|
||||
|
||||
"github.com/grafana/grafana/pkg/api/routing"
|
||||
httpstatic "github.com/grafana/grafana/pkg/api/static"
|
||||
"github.com/grafana/grafana/pkg/bus"
|
||||
@@ -32,6 +23,7 @@ import (
|
||||
"github.com/grafana/grafana/pkg/infra/remotecache"
|
||||
"github.com/grafana/grafana/pkg/infra/tracing"
|
||||
"github.com/grafana/grafana/pkg/infra/usagestats"
|
||||
"github.com/grafana/grafana/pkg/login/social"
|
||||
"github.com/grafana/grafana/pkg/middleware"
|
||||
"github.com/grafana/grafana/pkg/models"
|
||||
"github.com/grafana/grafana/pkg/plugins"
|
||||
@@ -40,13 +32,20 @@ import (
|
||||
"github.com/grafana/grafana/pkg/plugins/plugincontext"
|
||||
"github.com/grafana/grafana/pkg/services/accesscontrol"
|
||||
"github.com/grafana/grafana/pkg/services/alerting"
|
||||
"github.com/grafana/grafana/pkg/services/cleanup"
|
||||
"github.com/grafana/grafana/pkg/services/contexthandler"
|
||||
"github.com/grafana/grafana/pkg/services/datasourceproxy"
|
||||
"github.com/grafana/grafana/pkg/services/datasources"
|
||||
"github.com/grafana/grafana/pkg/services/encryption"
|
||||
"github.com/grafana/grafana/pkg/services/hooks"
|
||||
"github.com/grafana/grafana/pkg/services/libraryelements"
|
||||
"github.com/grafana/grafana/pkg/services/librarypanels"
|
||||
"github.com/grafana/grafana/pkg/services/live"
|
||||
"github.com/grafana/grafana/pkg/services/live/pushhttp"
|
||||
"github.com/grafana/grafana/pkg/services/login"
|
||||
"github.com/grafana/grafana/pkg/services/ngalert"
|
||||
"github.com/grafana/grafana/pkg/services/notifications"
|
||||
"github.com/grafana/grafana/pkg/services/oauthtoken"
|
||||
"github.com/grafana/grafana/pkg/services/provisioning"
|
||||
"github.com/grafana/grafana/pkg/services/quota"
|
||||
"github.com/grafana/grafana/pkg/services/rendering"
|
||||
@@ -56,7 +55,6 @@ import (
|
||||
"github.com/grafana/grafana/pkg/services/sqlstore"
|
||||
"github.com/grafana/grafana/pkg/setting"
|
||||
"github.com/grafana/grafana/pkg/tsdb"
|
||||
|
||||
"github.com/grafana/grafana/pkg/util/errutil"
|
||||
"github.com/prometheus/client_golang/prometheus"
|
||||
"github.com/prometheus/client_golang/prometheus/promhttp"
|
||||
@@ -107,6 +105,7 @@ type HTTPServer struct {
|
||||
SocialService social.Service
|
||||
OAuthTokenService oauthtoken.OAuthTokenService
|
||||
Listener net.Listener
|
||||
EncryptionService encryption.Service
|
||||
cleanUpService *cleanup.CleanUpService
|
||||
tracingService *tracing.TracingService
|
||||
internalMetricsSvc *metrics.InternalMetricsService
|
||||
@@ -133,7 +132,8 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi
|
||||
libraryPanelService librarypanels.Service, libraryElementService libraryelements.Service,
|
||||
notificationService *notifications.NotificationService, tracingService *tracing.TracingService,
|
||||
internalMetricsSvc *metrics.InternalMetricsService, quotaService *quota.QuotaService,
|
||||
socialService social.Service, oauthTokenService oauthtoken.OAuthTokenService) (*HTTPServer, error) {
|
||||
socialService social.Service, oauthTokenService oauthtoken.OAuthTokenService,
|
||||
encryptionService encryption.Service) (*HTTPServer, error) {
|
||||
macaron.Env = cfg.Env
|
||||
m := macaron.New()
|
||||
|
||||
@@ -180,6 +180,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi
|
||||
Listener: opts.Listener,
|
||||
SocialService: socialService,
|
||||
OAuthTokenService: oauthTokenService,
|
||||
EncryptionService: encryptionService,
|
||||
}
|
||||
if hs.Listener != nil {
|
||||
hs.log.Debug("Using provided listener")
|
||||
|
||||
+4
-5
@@ -18,7 +18,6 @@ import (
|
||||
"github.com/grafana/grafana/pkg/middleware/cookies"
|
||||
"github.com/grafana/grafana/pkg/models"
|
||||
"github.com/grafana/grafana/pkg/setting"
|
||||
"github.com/grafana/grafana/pkg/util"
|
||||
"github.com/grafana/grafana/pkg/util/errutil"
|
||||
)
|
||||
|
||||
@@ -99,7 +98,7 @@ func (hs *HTTPServer) LoginView(c *models.ReqContext) {
|
||||
viewData.Settings["oauth"] = enabledOAuths
|
||||
viewData.Settings["samlEnabled"] = hs.samlEnabled()
|
||||
|
||||
if loginError, ok := tryGetEncryptedCookie(c, loginErrorCookieName); ok {
|
||||
if loginError, ok := hs.tryGetEncryptedCookie(c, loginErrorCookieName); ok {
|
||||
// this cookie is only set whenever an OAuth login fails
|
||||
// therefore the loginError should be passed to the view data
|
||||
// and the view should return immediately before attempting
|
||||
@@ -299,7 +298,7 @@ func (hs *HTTPServer) Logout(c *models.ReqContext) {
|
||||
}
|
||||
}
|
||||
|
||||
func tryGetEncryptedCookie(ctx *models.ReqContext, cookieName string) (string, bool) {
|
||||
func (hs *HTTPServer) tryGetEncryptedCookie(ctx *models.ReqContext, cookieName string) (string, bool) {
|
||||
cookie := ctx.GetCookie(cookieName)
|
||||
if cookie == "" {
|
||||
return "", false
|
||||
@@ -310,12 +309,12 @@ func tryGetEncryptedCookie(ctx *models.ReqContext, cookieName string) (string, b
|
||||
return "", false
|
||||
}
|
||||
|
||||
decryptedError, err := util.Decrypt(decoded, setting.SecretKey)
|
||||
decryptedError, err := hs.EncryptionService.Decrypt(decoded, setting.SecretKey)
|
||||
return string(decryptedError), err == nil
|
||||
}
|
||||
|
||||
func (hs *HTTPServer) trySetEncryptedCookie(ctx *models.ReqContext, cookieName string, value string, maxAge int) error {
|
||||
encryptedError, err := util.Encrypt([]byte(value), setting.SecretKey)
|
||||
encryptedError, err := hs.EncryptionService.Encrypt([]byte(value), setting.SecretKey)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
@@ -10,6 +10,8 @@ import (
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/grafana/grafana/pkg/services/encryption/ossencryption"
|
||||
|
||||
"github.com/grafana/grafana/pkg/api/dtos"
|
||||
"github.com/grafana/grafana/pkg/api/response"
|
||||
"github.com/grafana/grafana/pkg/api/routing"
|
||||
@@ -23,7 +25,6 @@ import (
|
||||
"github.com/grafana/grafana/pkg/services/hooks"
|
||||
"github.com/grafana/grafana/pkg/services/licensing"
|
||||
"github.com/grafana/grafana/pkg/setting"
|
||||
"github.com/grafana/grafana/pkg/util"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
@@ -105,10 +106,11 @@ func TestLoginErrorCookieAPIEndpoint(t *testing.T) {
|
||||
sc := setupScenarioContext(t, "/login")
|
||||
cfg := setting.NewCfg()
|
||||
hs := &HTTPServer{
|
||||
Cfg: cfg,
|
||||
SettingsProvider: &setting.OSSImpl{Cfg: cfg},
|
||||
License: &licensing.OSSLicensingService{},
|
||||
SocialService: &mockSocialService{},
|
||||
Cfg: cfg,
|
||||
SettingsProvider: &setting.OSSImpl{Cfg: cfg},
|
||||
License: &licensing.OSSLicensingService{},
|
||||
SocialService: &mockSocialService{},
|
||||
EncryptionService: &ossencryption.Service{},
|
||||
}
|
||||
|
||||
sc.defaultHandler = routing.Wrap(func(w http.ResponseWriter, c *models.ReqContext) {
|
||||
@@ -121,7 +123,7 @@ func TestLoginErrorCookieAPIEndpoint(t *testing.T) {
|
||||
setting.OAuthAutoLogin = true
|
||||
|
||||
oauthError := errors.New("User not a member of one of the required organizations")
|
||||
encryptedError, err := util.Encrypt([]byte(oauthError.Error()), setting.SecretKey)
|
||||
encryptedError, err := hs.EncryptionService.Encrypt([]byte(oauthError.Error()), setting.SecretKey)
|
||||
require.NoError(t, err)
|
||||
expCookiePath := "/"
|
||||
if len(setting.AppSubUrl) > 0 {
|
||||
|
||||
Reference in New Issue
Block a user