From 3cb1f4d1c624f881f6424f2e895852d4c4459423 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Va=C5=A1ko?= Date: Thu, 13 Jun 2024 14:36:16 +0200 Subject: [PATCH 01/15] fix: Add mocked sandboxes API response with `app` data app config. Mock response with `simpleAuth` example instead of OIDC. --- docker-compose.yml | 12 ++++++++++ initializer.json | 33 +++++++++++++++++++++++++++ provisioning/apps-proxy/dev/.air.toml | 2 +- 3 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 initializer.json diff --git a/docker-compose.yml b/docker-compose.yml index 63406f4816..0cf28758b2 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -8,6 +8,7 @@ services: links: - etcd - redis + - sandboxesMock volumes: - ./:/code:z - cache:/tmp/cache @@ -102,5 +103,16 @@ services: - K6_USERS - K6_DURATION + sandboxesMock: + image: mockserver/mockserver:latest + ports: + - 1080:1080 + environment: + MOCKSERVER_WATCH_INITIALIZATION_JSON: "true" + MOCKSERVER_PROPERTY_FILE: /config/mockserver.properties + MOCKSERVER_INITIALIZATION_JSON_PATH: /config/initializerJson.json + volumes: + - ./initializer.json:/config/initializerJson.json:Z + volumes: cache: diff --git a/initializer.json b/initializer.json new file mode 100644 index 0000000000..8fbfcd009b --- /dev/null +++ b/initializer.json @@ -0,0 +1,33 @@ +[ + { + "httpRequest": { + "method": "GET", + "path": "/apps/app/proxy-config" + }, + "httpResponse": { + "body": { + "appId": "123", + "appName": "app", + "projectId": "11", + "upstreamAppUrl": "http://localhost:1235", + "authProviders": [ + { + "type": "password", + "id": "simpleAuth", + "password": "a" + } + ], + "authRules": [ + { + "type": "pathPrefix", + "value": "/", + "auth": [ + "simpleAuth" + ] + } + ] + }, + "statusCode": 200 + } + } +] diff --git a/provisioning/apps-proxy/dev/.air.toml b/provisioning/apps-proxy/dev/.air.toml index 8866ff9b35..4a2db21389 100644 --- a/provisioning/apps-proxy/dev/.air.toml +++ b/provisioning/apps-proxy/dev/.air.toml @@ -3,7 +3,7 @@ tmp_dir = "target/.watcher" [build] bin = "./target/apps-proxy/proxy" - args_bin = ["--sandboxes-api-url", "http://localhost:1234", "--sandboxes-api-token", "my-token", "--api-public-url", "http://localhost:8000", "--cookie-secret-salt", "cookie"] + args_bin = ["--sandboxes-api-url", "http://localhost:1080", "--sandboxes-api-token", "my-token", "--api-public-url", "http://localhost:8000", "--cookie-secret-salt", "cookie"] cmd = "make build-apps-proxy" delay = 2000 exclude_dir = [] From b2512c810a83b04dae291a4b022a05bd6f0f138a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Va=C5=A1ko?= Date: Mon, 24 Jun 2024 08:26:53 +0200 Subject: [PATCH 02/15] feat: Mock basic auth provider Include refactor of oidcprovider to work through manager of authproxies. All providers are possible to select through selector page. --- .../appsproxy/dataapps/auth/provider/basic.go | 10 + .../dataapps/auth/provider/provider.go | 5 +- .../appsproxy/dependencies/dependencies.go | 12 +- .../appsproxy/proxy/apphandler/apphandler.go | 10 +- .../apphandler/authproxy/basicauth/handler.go | 102 +++++++ .../proxy/apphandler/authproxy/manager.go | 61 ++++ .../apphandler/authproxy/oidcproxy/config.go | 116 ++++++++ .../apphandler/authproxy/oidcproxy/doc.go | 3 + .../apphandler/authproxy/oidcproxy/handler.go | 98 +++++++ .../authproxy/oidcproxy/logging/writer.go | 35 +++ .../authproxy/oidcproxy/pagewriter.go | 107 +++++++ .../apphandler/authproxy/selector/handler.go | 13 + .../apphandler/authproxy/selector/selector.go | 273 ++++++++++++++++++ .../appsproxy/proxy/apphandler/manager.go | 19 +- 14 files changed, 838 insertions(+), 26 deletions(-) create mode 100644 internal/pkg/service/appsproxy/dataapps/auth/provider/basic.go create mode 100644 internal/pkg/service/appsproxy/proxy/apphandler/authproxy/basicauth/handler.go create mode 100644 internal/pkg/service/appsproxy/proxy/apphandler/authproxy/manager.go create mode 100644 internal/pkg/service/appsproxy/proxy/apphandler/authproxy/oidcproxy/config.go create mode 100644 internal/pkg/service/appsproxy/proxy/apphandler/authproxy/oidcproxy/doc.go create mode 100644 internal/pkg/service/appsproxy/proxy/apphandler/authproxy/oidcproxy/handler.go create mode 100644 internal/pkg/service/appsproxy/proxy/apphandler/authproxy/oidcproxy/logging/writer.go create mode 100644 internal/pkg/service/appsproxy/proxy/apphandler/authproxy/oidcproxy/pagewriter.go create mode 100644 internal/pkg/service/appsproxy/proxy/apphandler/authproxy/selector/handler.go create mode 100644 internal/pkg/service/appsproxy/proxy/apphandler/authproxy/selector/selector.go diff --git a/internal/pkg/service/appsproxy/dataapps/auth/provider/basic.go b/internal/pkg/service/appsproxy/dataapps/auth/provider/basic.go new file mode 100644 index 0000000000..b3f4a8bb0e --- /dev/null +++ b/internal/pkg/service/appsproxy/dataapps/auth/provider/basic.go @@ -0,0 +1,10 @@ +package provider + +type Basic struct { + Base + Password string `json:"password"` +} + +func (p *Basic) IsAuthorized(password string) bool { + return p.Password == password +} diff --git a/internal/pkg/service/appsproxy/dataapps/auth/provider/provider.go b/internal/pkg/service/appsproxy/dataapps/auth/provider/provider.go index 07b96bb8c7..9125c1d82b 100644 --- a/internal/pkg/service/appsproxy/dataapps/auth/provider/provider.go +++ b/internal/pkg/service/appsproxy/dataapps/auth/provider/provider.go @@ -9,7 +9,8 @@ import ( ) const ( - TypeOIDC = Type("oidc") + TypeOIDC Type = "oidc" + TypeBasic Type = "password" ) // ID is unique identifier of the authentication provider inside a data app. @@ -37,6 +38,8 @@ func (t Type) new() (Provider, error) { switch t { case TypeOIDC: return OIDC{}, nil + case TypeBasic: + return Basic{}, nil default: return nil, errors.Errorf(`unexpected type of data app auth provider "%v"`, t) } diff --git a/internal/pkg/service/appsproxy/dependencies/dependencies.go b/internal/pkg/service/appsproxy/dependencies/dependencies.go index 9d8a195882..801adb3023 100644 --- a/internal/pkg/service/appsproxy/dependencies/dependencies.go +++ b/internal/pkg/service/appsproxy/dependencies/dependencies.go @@ -28,7 +28,7 @@ import ( "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/dataapps/notify" "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/dataapps/wakeup" "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/proxy/apphandler" - "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/proxy/apphandler/oidcproxy" + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/proxy/apphandler/authproxy" "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/proxy/apphandler/upstream" "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/proxy/pagewriter" "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/proxy/transport" @@ -53,7 +53,7 @@ type ServiceScope interface { AppConfigLoader() *appconfig.Loader UpstreamTransport() http.RoundTripper UpstreamManager() *upstream.Manager - OidcProxyManager() *oidcproxy.Manager + AuthProxyManager() *authproxy.Manager PageWriter() *pagewriter.Writer NotifyManager() *notify.Manager WakeupManager() *wakeup.Manager @@ -73,7 +73,7 @@ type serviceScope struct { appHandlers *apphandler.Manager upstreamTransport http.RoundTripper upstreamManager *upstream.Manager - oidcProxyManager *oidcproxy.Manager + authProxyManager *authproxy.Manager pageWriter *pagewriter.Writer appConfigLoader *appconfig.Loader notifyManager *notify.Manager @@ -154,7 +154,7 @@ func newServiceScope(ctx context.Context, parentScp parentScopes, cfg config.Con d.appConfigLoader = appconfig.NewLoader(d) d.notifyManager = notify.NewManager(d) d.wakeupManager = wakeup.NewManager(d) - d.oidcProxyManager = oidcproxy.NewManager(d) + d.authProxyManager = authproxy.NewManager(d) d.upstreamManager = upstream.NewManager(d) d.appHandlers = apphandler.NewManager(d) @@ -185,8 +185,8 @@ func (v *serviceScope) UpstreamManager() *upstream.Manager { return v.upstreamManager } -func (v *serviceScope) OidcProxyManager() *oidcproxy.Manager { - return v.oidcProxyManager +func (v *serviceScope) AuthProxyManager() *authproxy.Manager { + return v.authProxyManager } func (v *serviceScope) PageWriter() *pagewriter.Writer { diff --git a/internal/pkg/service/appsproxy/proxy/apphandler/apphandler.go b/internal/pkg/service/appsproxy/proxy/apphandler/apphandler.go index 71621e2fc8..5c2ec57699 100644 --- a/internal/pkg/service/appsproxy/proxy/apphandler/apphandler.go +++ b/internal/pkg/service/appsproxy/proxy/apphandler/apphandler.go @@ -13,8 +13,8 @@ import ( "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/config" "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/dataapps/api" "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/dataapps/auth/provider" + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/selector" "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/proxy/apphandler/chain" - "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/proxy/apphandler/oidcproxy" "github.com/keboola/keboola-as-code/internal/pkg/service/common/ctxattr" svcErrors "github.com/keboola/keboola-as-code/internal/pkg/service/common/errors" "github.com/keboola/keboola-as-code/internal/pkg/service/common/httpserver/middleware" @@ -33,7 +33,7 @@ type appHandler struct { type ruleIndex int -func newAppHandler(manager *Manager, app api.AppConfig, appUpstream chain.Handler, authHandlers map[provider.ID]*oidcproxy.Handler) (http.Handler, error) { +func newAppHandler(manager *Manager, app api.AppConfig, appUpstream chain.Handler, authHandlers map[provider.ID]selector.Handler) (http.Handler, error) { handler := &appHandler{ manager: manager, app: app, @@ -45,7 +45,7 @@ func newAppHandler(manager *Manager, app api.AppConfig, appUpstream chain.Handle // Create handler with all auth handlers, to route internal URLs if len(authHandlers) > 0 { - if h, err := manager.oidcProxyManager.ProviderSelector().For(app, authHandlers); err == nil { + if h, err := manager.authProxyManager.ProviderSelector().For(app, authHandlers); err == nil { handler.allAuthHandlers = h } else { return nil, err @@ -78,7 +78,7 @@ func newAppHandler(manager *Manager, app api.AppConfig, appUpstream chain.Handle } // Filter authentication handlers - authHandlersPerRule := make(map[provider.ID]*oidcproxy.Handler) + authHandlersPerRule := make(map[provider.ID]selector.Handler) for _, providerID := range rule.Auth { if authHandler, found := authHandlers[providerID]; found { authHandlersPerRule[providerID] = authHandler @@ -88,7 +88,7 @@ func newAppHandler(manager *Manager, app api.AppConfig, appUpstream chain.Handle } // Merge authentication handlers for the rule to one selector handler - if h, err := manager.oidcProxyManager.ProviderSelector().For(app, authHandlersPerRule); err == nil { + if h, err := manager.authProxyManager.ProviderSelector().For(app, authHandlersPerRule); err == nil { handler.authHandlerPerRule[index] = h } else { return nil, err diff --git a/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/basicauth/handler.go b/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/basicauth/handler.go new file mode 100644 index 0000000000..fbf65ea36e --- /dev/null +++ b/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/basicauth/handler.go @@ -0,0 +1,102 @@ +package basicauth + +import ( + "net/http" + "time" + + "github.com/benbjohnson/clock" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util" + + "github.com/keboola/keboola-as-code/internal/pkg/log" + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/config" + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/dataapps/api" + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/dataapps/auth/provider" + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/proxy/apphandler/chain" + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/proxy/pagewriter" + "github.com/keboola/keboola-as-code/internal/pkg/utils/errors" +) + +const ( + basicAuthCookie = "proxyBasicAuth" + formPagePath = config.InternalPrefix + "/form" +) + +type Handler struct { + logger log.Logger + pageWriter *pagewriter.Writer + clock clock.Clock + app api.AppConfig + basicAuth provider.Basic + upstream chain.Handler +} + +func NewHandler( + logger log.Logger, + pageWriter *pagewriter.Writer, + clock clock.Clock, + app api.AppConfig, + auth provider.Basic, + upstream chain.Handler, +) *Handler { + return &Handler{ + logger: logger, + pageWriter: pageWriter, + clock: clock, + app: app, + basicAuth: auth, + upstream: upstream, + } +} + +func (h *Handler) Name() string { + return h.basicAuth.Name() +} + +func (h *Handler) SignInPath() string { + return formPagePath +} + +func (h *Handler) CookieExpiration() time.Duration { + return 5 * time.Minute +} + +func (h *Handler) ServeHTTPOrError(w http.ResponseWriter, req *http.Request) error { + cookie, _ := req.Cookie(basicAuthCookie) + if _, ok := req.Form["password"]; !ok || cookie == nil { + h.pageWriter.WriteFormPage(w, req, http.StatusOK) + return nil + } + + if req.URL.Path != formPagePath { + return nil + } + + if !h.basicAuth.IsAuthorized("") { + return errors.New("wrong password prompted") + } + + host, _ := util.SplitHostPort(req.Host) + if host == "" { + panic(errors.New("host cannot be empty")) + } + + v := &http.Cookie{ + Name: basicAuthCookie, + Path: "/", + Domain: host, + Secure: true, + HttpOnly: true, + SameSite: http.SameSiteStrictMode, + } + + expires := h.CookieExpiration() + if expires > 0 { + // If there is an expiration, set it + v.Expires = h.clock.Now().Add(expires) + } else { + // Otherwise clear the cookie + v.MaxAge = -1 + } + + return h.upstream.ServeHTTPOrError(w, req) +} diff --git a/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/manager.go b/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/manager.go new file mode 100644 index 0000000000..f3e957c95d --- /dev/null +++ b/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/manager.go @@ -0,0 +1,61 @@ +package authproxy + +import ( + "github.com/benbjohnson/clock" + + "github.com/keboola/keboola-as-code/internal/pkg/log" + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/config" + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/dataapps/api" + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/dataapps/auth/provider" + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/basicauth" + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/oidcproxy" + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/selector" + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/proxy/apphandler/chain" + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/proxy/pagewriter" +) + +type Manager struct { + logger log.Logger + config config.Config + pageWriter *pagewriter.Writer + clock clock.Clock + providerSelector *selector.Selector +} + +type dependencies interface { + Logger() log.Logger + Clock() clock.Clock + Config() config.Config + PageWriter() *pagewriter.Writer +} + +func NewManager(d dependencies) *Manager { + return &Manager{ + logger: d.Logger(), + config: d.Config(), + pageWriter: d.PageWriter(), + clock: d.Clock(), + providerSelector: selector.New(d), + } +} + +func (m *Manager) ProviderSelector() *selector.Selector { + return m.providerSelector +} + +func (m *Manager) NewHandlers(app api.AppConfig, upstream chain.Handler) map[provider.ID]selector.Handler { + authHandlers := make(map[provider.ID]selector.Handler, len(app.AuthProviders)) + for _, auth := range app.AuthProviders { + switch p := auth.(type) { + case provider.OIDC: + authHandlers[auth.ID()] = oidcproxy.NewHandler(m.logger, m.config, m.providerSelector, m.pageWriter, app, p, upstream) + + case provider.Basic: + authHandlers[auth.ID()] = basicauth.NewHandler(m.logger, m.pageWriter, m.clock, app, p, upstream) + + default: + panic("unknown auth provider type") + } + } + return authHandlers +} diff --git a/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/oidcproxy/config.go b/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/oidcproxy/config.go new file mode 100644 index 0000000000..7725d168eb --- /dev/null +++ b/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/oidcproxy/config.go @@ -0,0 +1,116 @@ +package oidcproxy + +import ( + "crypto/sha256" + "fmt" + "net/http" + "strings" + + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/validation" + + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/config" + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/dataapps/api" + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/dataapps/auth/provider" + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/selector" + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/proxy/apphandler/chain" + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/proxy/pagewriter" + "github.com/keboola/keboola-as-code/internal/pkg/utils/errors" +) + +func proxyConfig( + cfg config.Config, + selector *selector.Selector, + pageWriter *pagewriter.Writer, + app api.AppConfig, + authProvider provider.OIDC, + upstream chain.Handler, +) (*options.Options, error) { + // Generate unique cookies secret + secret, err := generateCookieSecret(cfg, app, authProvider.ID()) + if err != nil { + return nil, err + } + + proxyProvider, err := authProvider.ProxyProviderOptions() + if err != nil { + return nil, err + } + + v := options.NewOptions() + + // Connect to the app upstream + v.UpstreamHandler = http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + if err := upstream.ServeHTTPOrError(w, req); err != nil { + pageWriter.WriteError(w, req, &app, err) + } + }) + + // Render the selector page, if login is needed, it is not an internal URL + v.OnNeedsLogin = func(rw http.ResponseWriter, req *http.Request) (stop bool) { + // Bypass internal paths + if strings.HasPrefix(req.URL.Path, v.ProxyPrefix) { + return false + } + + return selector.OnNeedsLogin(&app, pageWriter.WriteError)(rw, req) + } + // Setup + domain := app.CookieDomain(cfg.API.PublicURL) + redirectURL := cfg.API.PublicURL.Scheme + "://" + domain + config.InternalPrefix + "/callback" + v.Logging.RequestIDHeader = config.RequestIDHeader + v.Logging.RequestEnabled = false // we have log middleware for all requests + v.Cookie.Secret = secret + v.Cookie.Domains = []string{domain} + v.Cookie.SameSite = "strict" + v.ProxyPrefix = config.InternalPrefix + v.RawRedirectURL = redirectURL + v.Providers = options.Providers{proxyProvider} + v.SkipProviderButton = true + v.Session = options.SessionOptions{Type: options.CookieSessionStoreType} + v.EmailDomains = []string{"*"} + v.InjectRequestHeaders = []options.Header{ + headerFromClaim("X-Kbc-User-Name", "name"), + headerFromClaim("X-Kbc-User-Email", options.OIDCEmailClaim), + headerFromClaim("X-Kbc-User-Roles", options.OIDCGroupsClaim), + } + + // Cannot separate errors from info because when ErrToInfo is false (default), + // oauthproxy keeps forcibly setting its global error writer to os.Stderr whenever a new proxy instance is created. + v.Logging.ErrToInfo = true + + if err := validation.Validate(v); err != nil { + return nil, err + } + + return v, nil +} + +// generateCookieSecret creates a unique cookie secret for each app and provider. +// This is necessary because otherwise cookies created by provider A would also be valid in a section that requires provider B but not A. +// To solve this we use the combination of the provider id and our salt. +func generateCookieSecret(cfg config.Config, app api.AppConfig, providerID provider.ID) (string, error) { + if cfg.CookieSecretSalt == "" { + return "", errors.New("missing cookie secret salt") + } + + h := sha256.New() + h.Write([]byte(app.ID.String() + "/" + providerID.String() + "/" + cfg.CookieSecretSalt)) + bs := h.Sum(nil) + + // Result must be 32 chars, 2 hex chars for each byte + return fmt.Sprintf("%x", bs[:16]), nil +} + +func headerFromClaim(header, claim string) options.Header { + return options.Header{ + Name: header, + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: claim, + }, + }, + }, + } +} diff --git a/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/oidcproxy/doc.go b/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/oidcproxy/doc.go new file mode 100644 index 0000000000..f1acc74ee2 --- /dev/null +++ b/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/oidcproxy/doc.go @@ -0,0 +1,3 @@ +// Package oidcproxy provides factory for OAuth2Proxy library. +// The logic is inserted before the handler from the "upstream" package. +package oidcproxy diff --git a/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/oidcproxy/handler.go b/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/oidcproxy/handler.go new file mode 100644 index 0000000000..7b0e18180a --- /dev/null +++ b/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/oidcproxy/handler.go @@ -0,0 +1,98 @@ +package oidcproxy + +import ( + "fmt" + "net/http" + "time" + + oauthproxy "github.com/oauth2-proxy/oauth2-proxy/v7" + proxyOptions "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" + + "github.com/keboola/keboola-as-code/internal/pkg/log" + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/config" + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/dataapps/api" + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/dataapps/auth/provider" + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/selector" + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/proxy/apphandler/chain" + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/proxy/pagewriter" + svcErrors "github.com/keboola/keboola-as-code/internal/pkg/service/common/errors" + "github.com/keboola/keboola-as-code/internal/pkg/utils/errors" +) + +type Handler struct { + providerName string + proxyConfig *proxyOptions.Options + proxyHandler *oauthproxy.OAuthProxy + initErr error +} + +func NewHandler( + logger log.Logger, + cfg config.Config, + selector *selector.Selector, + pw *pagewriter.Writer, + app api.AppConfig, + auth provider.OIDC, + upstream chain.Handler, +) *Handler { + var err error + handler := &Handler{providerName: auth.Name()} + + // Create proxy configuration + handler.proxyConfig, err = proxyConfig(cfg, selector, pw, app, auth, upstream) + if err != nil { + handler.initErr = wrapHandlerInitErr(app, auth, err) + return handler + } + + // Create proxy page writer adapter + pageWriter, err := newPageWriter(logger, pw, app, auth, handler.proxyConfig) + if err != nil { + handler.initErr = wrapHandlerInitErr(app, auth, err) + return handler + } + + // Create proxy HTTP handler + authValidator := func(email string) bool { return true } // there is no need to verify individual users + handler.proxyHandler, err = oauthproxy.NewOAuthProxyWithPageWriter(handler.proxyConfig, authValidator, pageWriter) + if err != nil { + handler.initErr = wrapHandlerInitErr(app, auth, err) + return handler + } + + return handler +} + +func (h *Handler) Name() string { + return h.providerName +} + +func (h *Handler) CookieExpiration() time.Duration { + if h.initErr != nil { + return 5 * time.Minute + } + return h.proxyConfig.Cookie.Expire +} + +func (h *Handler) SignInPath() string { + if h.initErr != nil { + return "/error" + } + return h.proxyHandler.SignInPath +} + +func (h *Handler) ServeHTTPOrError(w http.ResponseWriter, req *http.Request) error { + if h.initErr != nil { + return h.initErr + } + + // Pass request to OAuth2Proxy + h.proxyHandler.ServeHTTP(w, req) // errors are handled by the page writer + return nil +} + +func wrapHandlerInitErr(app api.AppConfig, auth provider.Provider, err error) error { + return svcErrors. + NewServiceUnavailableError(errors.PrefixErrorf(err, `application "%s" has invalid configuration for authentication provider "%s"`, app.IdAndName(), auth.ID())). + WithUserMessage(fmt.Sprintf(`Application "%s" has invalid configuration for authentication provider "%s".`, app.IdAndName(), auth.ID())) +} diff --git a/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/oidcproxy/logging/writer.go b/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/oidcproxy/logging/writer.go new file mode 100644 index 0000000000..ed451eeb4c --- /dev/null +++ b/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/oidcproxy/logging/writer.go @@ -0,0 +1,35 @@ +package logging + +import ( + "context" + "io" + + "github.com/keboola/keboola-as-code/internal/pkg/log" +) + +type loggerWriter struct { + logger log.Logger + level string + buffer []byte +} + +const newLine = byte(10) + +func (w *loggerWriter) Write(p []byte) (n int, err error) { + w.buffer = append(w.buffer, p...) + + if len(p) > 0 && p[len(p)-1] == newLine { + w.logger.Log(context.Background(), w.level, string(w.buffer)) + w.buffer = []byte{} + return 1, nil + } + + return len(p), nil +} + +func NewLoggerWriter(logger log.Logger, level string) io.Writer { + return &loggerWriter{ + logger: logger, + level: level, + } +} diff --git a/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/oidcproxy/pagewriter.go b/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/oidcproxy/pagewriter.go new file mode 100644 index 0000000000..df0248f088 --- /dev/null +++ b/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/oidcproxy/pagewriter.go @@ -0,0 +1,107 @@ +package oidcproxy + +import ( + "net/http" + "strings" + + oauthproxy "github.com/oauth2-proxy/oauth2-proxy/v7" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" + proxypw "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/app/pagewriter" + "github.com/spf13/cast" + "go.opentelemetry.io/otel/attribute" + semconv "go.opentelemetry.io/otel/semconv/v1.17.0" + + "github.com/keboola/keboola-as-code/internal/pkg/log" + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/dataapps/api" + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/dataapps/auth/provider" + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/proxy/pagewriter" + "github.com/keboola/keboola-as-code/internal/pkg/service/common/ctxattr" +) + +type proxyPageWriter proxypw.Writer + +// pageWriter is adapter between common page writer and OAuth2Proxy specific page writer. +type pageWriter struct { + proxyPageWriter + logger log.Logger + app api.AppConfig + authProvider provider.Provider + pageWriter *pagewriter.Writer +} + +func newPageWriter(logger log.Logger, pw *pagewriter.Writer, app api.AppConfig, authProvider provider.Provider, opts *options.Options) (*pageWriter, error) { + parent, err := proxypw.NewWriter( + proxypw.Opts{ + TemplatesPath: opts.Templates.Path, + CustomLogo: opts.Templates.CustomLogo, + ProxyPrefix: opts.ProxyPrefix, + Footer: opts.Templates.Footer, + Version: oauthproxy.VERSION, + Debug: opts.Templates.Debug, + ProviderName: opts.Providers[0].Name, + SignInMessage: opts.Templates.Banner, + DisplayLoginForm: opts.Templates.DisplayLoginForm, + }, + ) + if err != nil { + return nil, err + } + + return &pageWriter{ + logger: logger.WithComponent("oauth2proxy.pw"), + proxyPageWriter: parent, + app: app, + authProvider: authProvider, + pageWriter: pw, + }, nil +} + +func (pw *pageWriter) WriteErrorPage(w http.ResponseWriter, req *http.Request, opts proxypw.ErrorPageOpts) { + // Convert messages to string + var messages []string + for _, msg := range opts.Messages { + if str := cast.ToString(msg); str != "" { + messages = append(messages, str) + } + } + + // Default messages + if len(messages) == 0 { + switch opts.Status { + case http.StatusUnauthorized: + messages = []string{"You need to be logged in to access this resource."} + case http.StatusForbidden: + messages = []string{"You do not have permission to access this resource."} + case http.StatusInternalServerError: + messages = []string{"Oops! Something went wrong."} + default: + messages = []string{opts.AppError} + } + } + + // Exception ID + exceptionID := pagewriter.ExceptionIDPrefix + opts.RequestID + + joinedMessage := strings.Join(messages, "\n") + // Add attributes + req = req.WithContext(ctxattr.ContextWith( + req.Context(), + semconv.HTTPStatusCode(opts.Status), + attribute.String("exceptionId", exceptionID), + attribute.String("error.userMessages", joinedMessage), + attribute.String("error.details", opts.AppError), + )) + + // Log warning + pw.logger.Warn(req.Context(), strings.Join(messages, "\n")) //nolint:contextcheck // false positive + + pw.pageWriter.WriteErrorPage(w, req, &pw.app, opts.Status, joinedMessage+opts.AppError, exceptionID) +} + +func (pw *pageWriter) ProxyErrorHandler(w http.ResponseWriter, req *http.Request, err error) { + pw.pageWriter.ProxyErrorHandler(w, req, pw.app, err) +} + +func (pw *pageWriter) WriteRobotsTxt(w http.ResponseWriter, req *http.Request) { + pw.pageWriter.WriteRobotsTxt(w, req) +} diff --git a/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/selector/handler.go b/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/selector/handler.go new file mode 100644 index 0000000000..9d108210b8 --- /dev/null +++ b/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/selector/handler.go @@ -0,0 +1,13 @@ +package selector + +import ( + "net/http" + "time" +) + +type Handler interface { + Name() string + ServeHTTPOrError(w http.ResponseWriter, req *http.Request) error + CookieExpiration() time.Duration + SignInPath() string +} diff --git a/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/selector/selector.go b/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/selector/selector.go new file mode 100644 index 0000000000..ea408064e1 --- /dev/null +++ b/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/selector/selector.go @@ -0,0 +1,273 @@ +package selector + +import ( + "context" + "net/http" + "net/url" + "slices" + "strings" + "time" + + "github.com/benbjohnson/clock" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util" + + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/config" + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/dataapps/api" + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/dataapps/auth/provider" + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/proxy/pagewriter" + svcErrors "github.com/keboola/keboola-as-code/internal/pkg/service/common/errors" + "github.com/keboola/keboola-as-code/internal/pkg/utils/errors" +) + +const ( + providerCookie = "_oauth2_provider" + providerQueryParam = "provider" + continueAuthQueryParam = "continue_auth" + callbackQueryParam = "rd" // value match OAuth2Proxy internals and shouldn't be modified (see AppDirector there) + selectionPagePath = config.InternalPrefix + "/selection" + signOutPath = config.InternalPrefix + "/sign_out" + proxyCallbackPath = config.InternalPrefix + "/callback" + ignoreProviderCookieCtxKey = ctxKey("ignoreProviderCookieCtxKey") + selectorHandlerCtxKey = ctxKey("selectorHandlerCtxKey") +) + +type ctxKey string + +type Selector struct { + clock clock.Clock + config config.Config + pageWriter *pagewriter.Writer +} + +type SelectorForAppRule struct { + *Selector + app api.AppConfig + handlers map[provider.ID]Handler +} + +type dependencies interface { + Clock() clock.Clock + Config() config.Config + PageWriter() *pagewriter.Writer +} + +func New(d dependencies) *Selector { + return &Selector{ + clock: d.Clock(), + config: d.Config(), + pageWriter: d.PageWriter(), + } +} + +func (s *Selector) For(app api.AppConfig, handlers map[provider.ID]Handler) (*SelectorForAppRule, error) { + // Validate handlers count + if len(handlers) == 0 { + return nil, svcErrors.NewServiceUnavailableError(errors.New(`no authentication provider found`)) + } + + return &SelectorForAppRule{Selector: s, app: app, handlers: handlers}, nil +} + +// ServeHTTPOrError renders selector page if there is more than one authentication handler, +// and no handler is selected, or the selected handler is not allowed for the requested path (see api.AuthRule). +// +// The selector page is rendered: +// 1. If it is accessed directly using selectionPagePath, the status code is StatusOK. +// 2. If no handler is selected and the path requires authorization, the status code is StatusUnauthorized. +func (s *SelectorForAppRule) ServeHTTPOrError(w http.ResponseWriter, req *http.Request) error { + // To make the same site strict cookie work we need to replace the redirect from the auth provider with a page that does the redirect. + if req.URL.Path == proxyCallbackPath { + query := req.URL.Query() + if query.Get(continueAuthQueryParam) != "true" { + query.Set(continueAuthQueryParam, "true") + baseURL := s.app.BaseURL(s.config.API.PublicURL) + redirectURL := baseURL.ResolveReference(&url.URL{Path: req.URL.Path, RawQuery: query.Encode()}) + s.pageWriter.WriteRedirectPage(w, req, http.StatusOK, redirectURL.String()) + return nil + } + } + + // Store the selector to the context. + // It is used by the OnNeedsLogin callback, to render the selector page, if the provider needs login. + // Internal paths (it includes sing in) are bypassed, see Manager.proxyConfig for details. + req = req.WithContext(context.WithValue(req.Context(), selectorHandlerCtxKey, s)) + + // Clear cookie on logout + if req.URL.Path == signOutPath { + // This clears provider selection cookie while oauth2-proxy clears the session cookie. + // The user isn't logged out on the provider's side, but when redirected to the provider they're + // forced to select their account again because of the "select_account" flag in LoginURLParameters. + s.clearCookie(w, req) + } + + // Render selector page, if it is accessed directly + if req.URL.Path == selectionPagePath { + return s.writeSelectorPage(w, req, http.StatusOK) + } + + // Skip selector page, if there is only one provider + if len(s.handlers) == 1 { + // The handlers variable is a map, use the first handler via a for cycle + for id, handler := range s.handlers { + // Set cookie if needed + if providerID := s.providerIDFromCookie(req); providerID != id { + s.setCookie(w, req, id, handler) + } + return handler.ServeHTTPOrError(w, req) + } + } + + // Ignore cookie if we have already tried this provider, but the provider requires login. + providerID := s.providerIDFromCookie(req) + if ignore, _ := req.Context().Value(ignoreProviderCookieCtxKey).(bool); ignore { + providerID = "" + } + + // Identify the chosen provider by the cookie + if handler := s.handlers[providerID]; handler != nil { + return handler.ServeHTTPOrError(w, req) + } + + // No matching handler found + return s.writeSelectorPage(w, req, http.StatusUnauthorized) +} + +func (s *SelectorForAppRule) writeSelectorPage(w http.ResponseWriter, req *http.Request, status int) error { + // Mark provider selected + id := provider.ID(req.URL.Query().Get(providerQueryParam)) + if selected, found := s.handlers[id]; found { + // Set cookie with the same expiration as other provider cookies + s.setCookie(w, req, id, selected) + + // Get path for redirect after sign in, it must not refer to an external URL + query := make(url.Values) + callback := req.URL.Query().Get(callbackQueryParam) + if isAcceptedCallbackURL(callback) { + query.Set(callbackQueryParam, callback) + } + + // Render sign in page, set callback after login + s.redirect(w, req, selected.SignInPath(), query) + return nil + } + + // Render the page, if there is no cookie or the value is invalid + s.pageWriter.WriteSelectorPage(w, req, status, s.selectorPageData(req)) + return nil +} + +func (s *SelectorForAppRule) selectorPageData(req *http.Request) *pagewriter.SelectorPageData { + // Pass link back to the current page, if reasonable, otherwise the user will be redirected to / + var callback string + if req.Method == http.MethodGet { + callback = req.URL.Path + } + + // Base URL for all providers + pageURL := s.url(req, selectionPagePath, nil) + + // Generate link for each providers + data := &pagewriter.SelectorPageData{App: pagewriter.NewAppData(&s.app)} + for id, handler := range s.handlers { + query := make(url.Values) + query.Set(providerQueryParam, id.String()) + if isAcceptedCallbackURL(callback) { + query.Set(callbackQueryParam, callback) + } + data.Providers = append(data.Providers, pagewriter.ProviderData{ + Name: handler.Name(), + URL: pageURL.ResolveReference(&url.URL{RawQuery: query.Encode()}).String(), + }) + } + + // Sort items + slices.SortStableFunc(data.Providers, func(a, b pagewriter.ProviderData) int { + return strings.Compare(a.Name, b.Name) + }) + + return data +} + +func (s *Selector) OnNeedsLogin( + app *api.AppConfig, + writeErrorPage func(rw http.ResponseWriter, req *http.Request, app *api.AppConfig, err error), +) func(rw http.ResponseWriter, req *http.Request) bool { + return func(w http.ResponseWriter, req *http.Request) (stop bool) { + // Determine, if we should render the selector page using the selector instance from the context + if selector, ok := req.Context().Value(selectorHandlerCtxKey).(*SelectorForAppRule); ok { + // If there is only one provider, continue to the sing in page + if len(selector.handlers) <= 1 { + return false + } + + // Go back and render the selector page, ignore the cookie value + req = req.WithContext(context.WithValue(req.Context(), ignoreProviderCookieCtxKey, true)) + if err := selector.ServeHTTPOrError(w, req); err != nil { + writeErrorPage(w, req, app, err) + } + return true + } + + // Fallback, the selector instance is not found, it shouldn't happen. + // Clear the cookie and redirect to the same path, so the selector page is rendered. + s.clearCookie(w, req) + w.Header().Set("Location", req.URL.Path) + w.WriteHeader(http.StatusFound) + return true + } +} + +func (s *Selector) redirect(w http.ResponseWriter, req *http.Request, path string, query url.Values) { + w.Header().Set("Location", s.url(req, path, query).String()) + w.WriteHeader(http.StatusFound) +} + +func (s *Selector) url(req *http.Request, path string, query url.Values) *url.URL { + return &url.URL{Scheme: s.config.API.PublicURL.Scheme, Host: req.Host, Path: path, RawQuery: query.Encode()} +} + +func (s *Selector) providerIDFromCookie(req *http.Request) provider.ID { + if cookie, _ := req.Cookie(providerCookie); cookie != nil && cookie.Value != "" { + return provider.ID(cookie.Value) + } + return "" +} + +func (s *Selector) clearCookie(w http.ResponseWriter, req *http.Request) { + http.SetCookie(w, s.cookie(req, "", -1)) +} + +func (s *Selector) setCookie(w http.ResponseWriter, req *http.Request, id provider.ID, handler Handler) { + http.SetCookie(w, s.cookie(req, id.String(), handler.CookieExpiration())) +} + +func (s *Selector) cookie(req *http.Request, value string, expires time.Duration) *http.Cookie { + host, _ := util.SplitHostPort(req.Host) + if host == "" { + panic(errors.New("host cannot be empty")) + } + + v := &http.Cookie{ + Name: providerCookie, + Value: value, + Path: "/", + Domain: host, + Secure: true, + HttpOnly: true, + SameSite: http.SameSiteStrictMode, + } + + if expires > 0 { + // If there is an expiration, set it + v.Expires = s.clock.Now().Add(expires) + } else { + // Otherwise clear the cookie + v.MaxAge = -1 + } + + return v +} + +func isAcceptedCallbackURL(callback string) bool { + return callback != "" && callback != "/" && callback != selectionPagePath && strings.HasPrefix(callback, "/") +} diff --git a/internal/pkg/service/appsproxy/proxy/apphandler/manager.go b/internal/pkg/service/appsproxy/proxy/apphandler/manager.go index a22db18381..a170dec539 100644 --- a/internal/pkg/service/appsproxy/proxy/apphandler/manager.go +++ b/internal/pkg/service/appsproxy/proxy/apphandler/manager.go @@ -8,8 +8,7 @@ import ( "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/config" "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/dataapps/api" "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/dataapps/appconfig" - "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/dataapps/auth/provider" - "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/proxy/apphandler/oidcproxy" + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/proxy/apphandler/authproxy" "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/proxy/apphandler/upstream" "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/proxy/pagewriter" "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/syncmap" @@ -24,7 +23,7 @@ type Manager struct { telemetry telemetry.Telemetry configLoader *appconfig.Loader upstreamManager *upstream.Manager - oidcProxyManager *oidcproxy.Manager + authProxyManager *authproxy.Manager pageWriter *pagewriter.Writer handlers *syncmap.SyncMap[api.AppID, appHandlerWrapper] } @@ -39,7 +38,7 @@ type dependencies interface { Telemetry() telemetry.Telemetry PageWriter() *pagewriter.Writer UpstreamManager() *upstream.Manager - OidcProxyManager() *oidcproxy.Manager + AuthProxyManager() *authproxy.Manager AppConfigLoader() *appconfig.Loader } @@ -49,7 +48,7 @@ func NewManager(d dependencies) *Manager { telemetry: d.Telemetry(), configLoader: d.AppConfigLoader(), upstreamManager: d.UpstreamManager(), - oidcProxyManager: d.OidcProxyManager(), + authProxyManager: d.AuthProxyManager(), pageWriter: d.PageWriter(), handlers: syncmap.New[api.AppID, appHandlerWrapper](func(api.AppID) *appHandlerWrapper { return &appHandlerWrapper{lock: &sync.Mutex{}} @@ -87,15 +86,7 @@ func (m *Manager) newHandler(ctx context.Context, app api.AppConfig) http.Handle } // Create authentication handlers - authHandlers := make(map[provider.ID]*oidcproxy.Handler, len(app.AuthProviders)) - for _, auth := range app.AuthProviders { - switch p := auth.(type) { - case provider.OIDC: - authHandlers[auth.ID()] = m.oidcProxyManager.NewHandler(app, p, appUpstream) - default: - panic("unknown auth provider type") - } - } + authHandlers := m.authProxyManager.NewHandlers(app, appUpstream) // Create root handler for application handler, err := newAppHandler(m, app, appUpstream, authHandlers) From 57c60060b96736d9be22387589a3dfc9b2779c96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Va=C5=A1ko?= Date: Mon, 24 Jun 2024 08:27:35 +0200 Subject: [PATCH 03/15] feat: Add simple form page --- .../appsproxy/proxy/pagewriter/form.go | 13 +++++++ .../proxy/pagewriter/template/form.gohtml | 37 +++++++++++++++++++ 2 files changed, 50 insertions(+) create mode 100644 internal/pkg/service/appsproxy/proxy/pagewriter/form.go create mode 100644 internal/pkg/service/appsproxy/proxy/pagewriter/template/form.gohtml diff --git a/internal/pkg/service/appsproxy/proxy/pagewriter/form.go b/internal/pkg/service/appsproxy/proxy/pagewriter/form.go new file mode 100644 index 0000000000..cd53f8d108 --- /dev/null +++ b/internal/pkg/service/appsproxy/proxy/pagewriter/form.go @@ -0,0 +1,13 @@ +package pagewriter + +import "net/http" + +type FormPageData struct { + App AppData +} + +func (pw *Writer) WriteFormPage(w http.ResponseWriter, req *http.Request, status int) { + w.Header().Set("Cache-Control", "no-cache, no-store, must-revalidate;") + w.Header().Set("pragma", "no-cache") + pw.writePage(w, req, "form.gohtml", status, nil) +} diff --git a/internal/pkg/service/appsproxy/proxy/pagewriter/template/form.gohtml b/internal/pkg/service/appsproxy/proxy/pagewriter/template/form.gohtml new file mode 100644 index 0000000000..66fce11790 --- /dev/null +++ b/internal/pkg/service/appsproxy/proxy/pagewriter/template/form.gohtml @@ -0,0 +1,37 @@ + + + + + + + + + Basic Authentication + + + + +
+

{{.App.Name}}

+
+
+ App ID: {{.App.ID}} +
+ , +
+ Project ID: {{.App.ProjectID}} +
+
+
+
+

Basic Authentication

+ {{.Name}} +
+
+

Powered By

+ + + +
+ + From e1f9d245c152201cb3acde9f74def814ab0bc5ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Va=C5=A1ko?= Date: Mon, 24 Jun 2024 14:38:15 +0200 Subject: [PATCH 04/15] feat: Mock up behaviour of login/logout in login page. Change name from form->login based on design. Adjust the css so the design is correct. --- .../apphandler/authproxy/basicauth/handler.go | 83 ++++++++++++++--- .../apphandler/authproxy/selector/selector.go | 4 +- .../proxy/pagewriter/assets/styles.css | 93 ++++++++++++++++++- .../appsproxy/proxy/pagewriter/form.go | 13 --- .../appsproxy/proxy/pagewriter/login.go | 18 ++++ .../proxy/pagewriter/template/form.gohtml | 37 -------- .../proxy/pagewriter/template/login.gohtml | 56 +++++++++++ 7 files changed, 239 insertions(+), 65 deletions(-) delete mode 100644 internal/pkg/service/appsproxy/proxy/pagewriter/form.go create mode 100644 internal/pkg/service/appsproxy/proxy/pagewriter/login.go delete mode 100644 internal/pkg/service/appsproxy/proxy/pagewriter/template/form.gohtml create mode 100644 internal/pkg/service/appsproxy/proxy/pagewriter/template/login.gohtml diff --git a/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/basicauth/handler.go b/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/basicauth/handler.go index fbf65ea36e..761efdf15a 100644 --- a/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/basicauth/handler.go +++ b/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/basicauth/handler.go @@ -1,7 +1,10 @@ package basicauth import ( + "crypto/sha256" + "io" "net/http" + "net/url" "time" "github.com/benbjohnson/clock" @@ -11,12 +14,14 @@ import ( "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/config" "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/dataapps/api" "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/dataapps/auth/provider" + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/selector" "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/proxy/apphandler/chain" "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/proxy/pagewriter" "github.com/keboola/keboola-as-code/internal/pkg/utils/errors" ) const ( + providerCookie = "_oauth2_provider" basicAuthCookie = "proxyBasicAuth" formPagePath = config.InternalPrefix + "/form" ) @@ -57,31 +62,86 @@ func (h *Handler) SignInPath() string { } func (h *Handler) CookieExpiration() time.Duration { - return 5 * time.Minute + return 1 * time.Hour } func (h *Handler) ServeHTTPOrError(w http.ResponseWriter, req *http.Request) error { - cookie, _ := req.Cookie(basicAuthCookie) - if _, ok := req.Form["password"]; !ok || cookie == nil { - h.pageWriter.WriteFormPage(w, req, http.StatusOK) + host, _ := util.SplitHostPort(req.Host) + if host == "" { + panic(errors.New("host cannot be empty")) + } + + requestCookie, _ := req.Cookie(basicAuthCookie) + + // req.Form / req.PostForm does not work + b, err := io.ReadAll(req.Body) + if err != nil { + return err + } + + values, err := url.ParseQuery(string(b)) + if err != nil { + return err + } + + // Unset cookie as /_proxy/sign_out was called and enforce login + if requestCookie != nil && req.URL.Path == selector.SignOutPath { + requestCookie.Value = "" + h.setCookie(w, host, -1, requestCookie) + requestCookie = nil + } + + // Login page + if !values.Has("password") && requestCookie == nil { + h.pageWriter.WriteLoginPage(w, req, &h.app, nil) return nil } - if req.URL.Path != formPagePath { + p := values.Get("password") + // Login page with unauthorized alert + if err := h.isAuthorized(p, requestCookie); err != nil { + h.logger.Warn(req.Context(), err.Error()) + h.pageWriter.WriteLoginPage(w, req, &h.app, err) return nil } - if !h.basicAuth.IsAuthorized("") { - return errors.New("wrong password prompted") + expires := h.CookieExpiration() + // Skip generating cookie value when already set and verified + if requestCookie != nil { + h.setCookie(w, host, expires, requestCookie) + return h.upstream.ServeHTTPOrError(w, req) } - host, _ := util.SplitHostPort(req.Host) - if host == "" { - panic(errors.New("host cannot be empty")) + hash := sha256.New() + hash.Write([]byte(p)) + hashedValue := hash.Sum(nil) + v := &http.Cookie{ + Value: string(hashedValue), + } + h.setCookie(w, host, expires, v) + return h.upstream.ServeHTTPOrError(w, req) +} + +func (h *Handler) isAuthorized(p string, cookie *http.Cookie) error { + if err := cookie.Valid(); cookie != nil { + if err != nil { + return err + } + + return nil + } + + if !h.basicAuth.IsAuthorized(p) { + return errors.New("wrong password was given") } + return nil +} + +func (h *Handler) setCookie(w http.ResponseWriter, host string, expires time.Duration, cookie *http.Cookie) { v := &http.Cookie{ Name: basicAuthCookie, + Value: cookie.Value, Path: "/", Domain: host, Secure: true, @@ -89,7 +149,6 @@ func (h *Handler) ServeHTTPOrError(w http.ResponseWriter, req *http.Request) err SameSite: http.SameSiteStrictMode, } - expires := h.CookieExpiration() if expires > 0 { // If there is an expiration, set it v.Expires = h.clock.Now().Add(expires) @@ -98,5 +157,5 @@ func (h *Handler) ServeHTTPOrError(w http.ResponseWriter, req *http.Request) err v.MaxAge = -1 } - return h.upstream.ServeHTTPOrError(w, req) + http.SetCookie(w, v) } diff --git a/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/selector/selector.go b/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/selector/selector.go index ea408064e1..1a4fe5575f 100644 --- a/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/selector/selector.go +++ b/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/selector/selector.go @@ -25,7 +25,7 @@ const ( continueAuthQueryParam = "continue_auth" callbackQueryParam = "rd" // value match OAuth2Proxy internals and shouldn't be modified (see AppDirector there) selectionPagePath = config.InternalPrefix + "/selection" - signOutPath = config.InternalPrefix + "/sign_out" + SignOutPath = config.InternalPrefix + "/sign_out" proxyCallbackPath = config.InternalPrefix + "/callback" ignoreProviderCookieCtxKey = ctxKey("ignoreProviderCookieCtxKey") selectorHandlerCtxKey = ctxKey("selectorHandlerCtxKey") @@ -93,7 +93,7 @@ func (s *SelectorForAppRule) ServeHTTPOrError(w http.ResponseWriter, req *http.R req = req.WithContext(context.WithValue(req.Context(), selectorHandlerCtxKey, s)) // Clear cookie on logout - if req.URL.Path == signOutPath { + if req.URL.Path == SignOutPath { // This clears provider selection cookie while oauth2-proxy clears the session cookie. // The user isn't logged out on the provider's side, but when redirected to the provider they're // forced to select their account again because of the "select_account" flag in LoginURLParameters. diff --git a/internal/pkg/service/appsproxy/proxy/pagewriter/assets/styles.css b/internal/pkg/service/appsproxy/proxy/pagewriter/assets/styles.css index ff9627b8aa..317f3135ff 100644 --- a/internal/pkg/service/appsproxy/proxy/pagewriter/assets/styles.css +++ b/internal/pkg/service/appsproxy/proxy/pagewriter/assets/styles.css @@ -1,5 +1,10 @@ :root { --primary: #1f8fff; + --secondary: #1EC71E; + --secondar-hover: #179b17; + --error: #FFE0E6; + --error-bold: #FFC2CC; + --input: #EDF0F5; --background: #f2f4f7; --white: #ffffff; --text: #222529; @@ -31,7 +36,7 @@ footer { } header { - max-width: 376px; + max-width: 410px; text-align: center; } @@ -199,3 +204,89 @@ a:hover { padding: 10px 16px; margin-bottom: 16px; } + +form { + display: flex; + flex-direction: column; + gap: 16px; + margin-top: 8px; +} + +form button[type='submit'] { + margin-top: 8px; +} + +.form-group { + display: flex; + flex-direction: column; + gap: 4px; +} + +label { + font-size: 14px; + line-height: 20px; + color: var(--text); + text-align: left; +} + +input { + display: block; + color: var(--text); + background: var(--input); + border-radius: 4px; + padding: 10px 16px; + border: 1px solid rgba(255, 255, 255, 0); +} + +input:focus { + outline: 0; + border: 1px solid var(--primary) !important; + box-shadow: 0 0 0 3px rgba(34, 141, 255, 0.25); +} + +button { + display: flex; + align-items: center; + justify-content: center; + gap: 8px; + font-weight: 600; + font-size: 12px; + line-height: 20px; + text-transform: uppercase; + letter-spacing: 1px; + padding: 10px 16px; + border-radius: 4px; + border: none; + cursor: pointer; + background: var(--secondary); + color: var(--white); +} + +button:is(:hover, :focus):not(:disabled) { + background: var(--secondar-hover); +} + +form:has(input:placeholder-shown) button[type='submit'] { + opacity: 0.4; + cursor: not-allowed; +} + +.alert { + display: flex; + align-items: flex-start; + gap: 8px; + padding: 12px 16px; + margin: 8px 0 16px 0; + background: var(--error); + border: 1px solid var(--error-bold); + border-radius: 4px; +} + +.alert p { + font-size: 14px; + line-height: 20px; +} + +.alert p:last-child { + margin: 0; +} diff --git a/internal/pkg/service/appsproxy/proxy/pagewriter/form.go b/internal/pkg/service/appsproxy/proxy/pagewriter/form.go deleted file mode 100644 index cd53f8d108..0000000000 --- a/internal/pkg/service/appsproxy/proxy/pagewriter/form.go +++ /dev/null @@ -1,13 +0,0 @@ -package pagewriter - -import "net/http" - -type FormPageData struct { - App AppData -} - -func (pw *Writer) WriteFormPage(w http.ResponseWriter, req *http.Request, status int) { - w.Header().Set("Cache-Control", "no-cache, no-store, must-revalidate;") - w.Header().Set("pragma", "no-cache") - pw.writePage(w, req, "form.gohtml", status, nil) -} diff --git a/internal/pkg/service/appsproxy/proxy/pagewriter/login.go b/internal/pkg/service/appsproxy/proxy/pagewriter/login.go new file mode 100644 index 0000000000..5c3ec8bb88 --- /dev/null +++ b/internal/pkg/service/appsproxy/proxy/pagewriter/login.go @@ -0,0 +1,18 @@ +package pagewriter + +import ( + "net/http" + + "github.com/keboola/keboola-as-code/internal/pkg/service/appsproxy/dataapps/api" +) + +type LoginPageData struct { + App AppData + Error error +} + +func (pw *Writer) WriteLoginPage(w http.ResponseWriter, req *http.Request, app *api.AppConfig, err error) { + w.Header().Set("Cache-Control", "no-cache, no-store, must-revalidate;") + w.Header().Set("pragma", "no-cache") + pw.writePage(w, req, "login.gohtml", http.StatusOK, &LoginPageData{App: NewAppData(app), Error: err}) +} diff --git a/internal/pkg/service/appsproxy/proxy/pagewriter/template/form.gohtml b/internal/pkg/service/appsproxy/proxy/pagewriter/template/form.gohtml deleted file mode 100644 index 66fce11790..0000000000 --- a/internal/pkg/service/appsproxy/proxy/pagewriter/template/form.gohtml +++ /dev/null @@ -1,37 +0,0 @@ - - - - - - - - - Basic Authentication - - - - -
-

{{.App.Name}}

-
-
- App ID: {{.App.ID}} -
- , -
- Project ID: {{.App.ProjectID}} -
-
-
-
-

Basic Authentication

- {{.Name}} -
-
-

Powered By

- - - -
- - diff --git a/internal/pkg/service/appsproxy/proxy/pagewriter/template/login.gohtml b/internal/pkg/service/appsproxy/proxy/pagewriter/template/login.gohtml new file mode 100644 index 0000000000..82e79bb330 --- /dev/null +++ b/internal/pkg/service/appsproxy/proxy/pagewriter/template/login.gohtml @@ -0,0 +1,56 @@ + + + + + + + + + Login + + + + +
+

{{.App.Name}}

+
+
+ App ID: {{.App.ID}} +
+ , +
+ Project ID: {{.App.ProjectID}} +
+
+
+
+

Basic Authentication

+ {{ if .Error }} +
+ + + +

Please enter a correct password.

+
+ {{ end }} +
+
+ + +
+ +
+
+
+

Powered By

+ + + +
+ + From 95ed37f0ccceaed7c5e8dee13e64d9d786c6cc12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Va=C5=A1ko?= Date: Wed, 26 Jun 2024 10:00:56 +0200 Subject: [PATCH 05/15] feat: Add new basic auth tests. --- .../apphandler/authproxy/basicauth/handler.go | 8 ++ .../pkg/service/appsproxy/proxy/proxy_test.go | 123 ++++++++++++++++++ 2 files changed, 131 insertions(+) diff --git a/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/basicauth/handler.go b/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/basicauth/handler.go index 761efdf15a..ca1b435af5 100644 --- a/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/basicauth/handler.go +++ b/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/basicauth/handler.go @@ -2,6 +2,7 @@ package basicauth import ( "crypto/sha256" + "fmt" "io" "net/http" "net/url" @@ -73,6 +74,13 @@ func (h *Handler) ServeHTTPOrError(w http.ResponseWriter, req *http.Request) err requestCookie, _ := req.Cookie(basicAuthCookie) + err := req.ParseForm() + if err != nil { + return err + } + + fmt.Println(req.FormValue("password")) + // req.Form / req.PostForm does not work b, err := io.ReadAll(req.Body) if err != nil { diff --git a/internal/pkg/service/appsproxy/proxy/proxy_test.go b/internal/pkg/service/appsproxy/proxy/proxy_test.go index b902fd67bb..f18f98dcca 100644 --- a/internal/pkg/service/appsproxy/proxy/proxy_test.go +++ b/internal/pkg/service/appsproxy/proxy/proxy_test.go @@ -2,6 +2,7 @@ package proxy_test import ( + "bytes" "context" "crypto/rand" "fmt" @@ -1980,6 +1981,104 @@ func TestAppProxyRouter(t *testing.T) { expectedNotifications: map[string]int{}, expectedWakeUps: map[string]int{}, }, + { + name: "public-basic-auth-wrong-login", + run: func(t *testing.T, client *http.Client, m []*mockoidc.MockOIDC, appServer *testutil.AppServer, service *testutil.DataAppsAPI, dnsServer *dnsmock.Server) { + // Request public basic auth app + request, err := http.NewRequestWithContext(context.Background(), http.MethodGet, "https://basic-auth.hub.keboola.local/", nil) + require.NoError(t, err) + response, err := client.Do(request) + require.NoError(t, err) + require.Equal(t, http.StatusOK, response.StatusCode) + body, err := io.ReadAll(response.Body) + require.NoError(t, err) + assert.Contains(t, string(body), "Basic Authentication") + + // Fill wrong password into form + request, err = http.NewRequestWithContext(context.Background(), http.MethodPost, "https://basic-auth.hub.keboola.local/", bytes.NewBuffer([]byte("password=def"))) + require.NoError(t, err) + response, err = client.Do(request) + require.NoError(t, err) + require.Equal(t, http.StatusOK, response.StatusCode) + body, err = io.ReadAll(response.Body) + require.NoError(t, err) + assert.Contains(t, string(body), "Please enter a correct password.") + }, + expectedNotifications: map[string]int{}, + expectedWakeUps: map[string]int{}, + }, + { + name: "public-basic-auth-correct-login", + run: func(t *testing.T, client *http.Client, m []*mockoidc.MockOIDC, appServer *testutil.AppServer, service *testutil.DataAppsAPI, dnsServer *dnsmock.Server) { + // Request public basic auth app + // request, err := http.NewRequestWithContext(context.Background(), http.MethodGet, "https://basic-auth.hub.keboola.local/", nil) + // require.NoError(t, err) + // response, err := client.Do(request) + // require.NoError(t, err) + // require.Equal(t, http.StatusOK, response.StatusCode) + // body, err := io.ReadAll(response.Body) + // require.NoError(t, err) + // assert.Contains(t, string(body), "Basic Authentication") + + // Fill wrong password into form + request, err := http.NewRequestWithContext(context.Background(), http.MethodPost, "https://basic-auth.hub.keboola.local/", bytes.NewBuffer([]byte("password=abc"))) + request.Header.Set("Content-Type", "application/x-www-form-urlencoded") + require.NoError(t, err) + response, err := client.Do(request) + require.NoError(t, err) + require.Equal(t, http.StatusOK, response.StatusCode) + body, err := io.ReadAll(response.Body) + require.NoError(t, err) + assert.Contains(t, string(body), "Hello, client") + // Check that cookies were set + cookies := response.Cookies() + if assert.Len(t, cookies, 1) { + assert.Equal(t, "proxyBasicAuth", cookies[0].Name) + assert.Equal(t, "xAA@]#aza", cookies[0].Value) + assert.Equal(t, "/", cookies[0].Path) + assert.Equal(t, "basic-auth.hub.keboola.local", cookies[0].Domain) + assert.True(t, cookies[0].HttpOnly) + assert.True(t, cookies[0].Secure) + assert.Equal(t, http.SameSiteStrictMode, cookies[0].SameSite) + } + }, + expectedNotifications: map[string]int{ + "auth": 1, + }, + expectedWakeUps: map[string]int{}, + }, + { + name: "public-basic-auth-expired-cookie", + run: func(t *testing.T, client *http.Client, m []*mockoidc.MockOIDC, appServer *testutil.AppServer, service *testutil.DataAppsAPI, dnsServer *dnsmock.Server) { + // Access with expired cookie + request, err := http.NewRequestWithContext(context.Background(), http.MethodPost, "https://basic-auth.hub.keboola.local/", nil) + client.Jar.SetCookies( + &url.URL{ + Scheme: "https", + Host: "oidc.hub.keboola.local", + }, + []*http.Cookie{ + { + Name: "_oauth2_proxy_csrf", + Value: "", + Path: "/", + Domain: "oidc.hub.keboola.local", + }, + }, + ) + require.NoError(t, err) + response, err := client.Do(request) + require.NoError(t, err) + require.Equal(t, http.StatusOK, response.StatusCode) + body, err := io.ReadAll(response.Body) + require.NoError(t, err) + assert.Contains(t, string(body), "Hello, client") + }, + expectedNotifications: map[string]int{ + "auth": 1, + }, + expectedWakeUps: map[string]int{}, + }, } publicAppTestCaseFactory := func(method string) testCase { @@ -2453,6 +2552,30 @@ func testDataApps(upstream *url.URL, m []*mockoidc.MockOIDC) []api.AppConfig { }, }, }, + { + ID: "auth", + ProjectID: "123", + UpstreamAppURL: upstream.String(), + AppSlug: ptr.Ptr("basic"), //basic-auth.hub.keboola.local + AuthProviders: provider.Providers{ + provider.Basic{ + Base: provider.Base{ + Info: provider.Info{ + ID: "basic", + Type: provider.TypeBasic, + }, + }, + Password: "abc", + }, + }, + AuthRules: []api.Rule{ + { + Type: api.RulePathPrefix, + Value: "/", + Auth: []provider.ID{"basic"}, + }, + }, + }, } } From 21c460e2538ea65d8c6aa55441fed30a8b587454 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Va=C5=A1ko?= Date: Wed, 26 Jun 2024 13:15:31 +0200 Subject: [PATCH 06/15] fix: Cookie value comparation with config. Add cookies tests and sign out one. Coverage 90% --- .../apphandler/authproxy/basicauth/handler.go | 50 ++++------ .../proxy/pagewriter/template/login.gohtml | 4 +- .../pkg/service/appsproxy/proxy/proxy_test.go | 93 +++++++++++++------ 3 files changed, 85 insertions(+), 62 deletions(-) diff --git a/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/basicauth/handler.go b/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/basicauth/handler.go index ca1b435af5..6b87b727e6 100644 --- a/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/basicauth/handler.go +++ b/internal/pkg/service/appsproxy/proxy/apphandler/authproxy/basicauth/handler.go @@ -2,10 +2,8 @@ package basicauth import ( "crypto/sha256" - "fmt" - "io" + "encoding/hex" "net/http" - "net/url" "time" "github.com/benbjohnson/clock" @@ -22,7 +20,6 @@ import ( ) const ( - providerCookie = "_oauth2_provider" basicAuthCookie = "proxyBasicAuth" formPagePath = config.InternalPrefix + "/form" ) @@ -72,27 +69,15 @@ func (h *Handler) ServeHTTPOrError(w http.ResponseWriter, req *http.Request) err panic(errors.New("host cannot be empty")) } - requestCookie, _ := req.Cookie(basicAuthCookie) - - err := req.ParseForm() - if err != nil { + if err := req.ParseForm(); err != nil { return err } - fmt.Println(req.FormValue("password")) - - // req.Form / req.PostForm does not work - b, err := io.ReadAll(req.Body) - if err != nil { - return err - } - - values, err := url.ParseQuery(string(b)) - if err != nil { - return err - } + // Unset content length as we do not want to push req.Body to upstream + req.ContentLength = 0 // Unset cookie as /_proxy/sign_out was called and enforce login + requestCookie, _ := req.Cookie(basicAuthCookie) if requestCookie != nil && req.URL.Path == selector.SignOutPath { requestCookie.Value = "" h.setCookie(w, host, -1, requestCookie) @@ -100,12 +85,12 @@ func (h *Handler) ServeHTTPOrError(w http.ResponseWriter, req *http.Request) err } // Login page - if !values.Has("password") && requestCookie == nil { + if !req.Form.Has("password") && requestCookie == nil { h.pageWriter.WriteLoginPage(w, req, &h.app, nil) return nil } - p := values.Get("password") + p := req.Form.Get("password") // Login page with unauthorized alert if err := h.isAuthorized(p, requestCookie); err != nil { h.logger.Warn(req.Context(), err.Error()) @@ -124,23 +109,28 @@ func (h *Handler) ServeHTTPOrError(w http.ResponseWriter, req *http.Request) err hash.Write([]byte(p)) hashedValue := hash.Sum(nil) v := &http.Cookie{ - Value: string(hashedValue), + Value: hex.EncodeToString(hashedValue), } h.setCookie(w, host, expires, v) return h.upstream.ServeHTTPOrError(w, req) } func (h *Handler) isAuthorized(p string, cookie *http.Cookie) error { - if err := cookie.Valid(); cookie != nil { - if err != nil { - return err - } + if p != "" && !h.basicAuth.IsAuthorized(p) { + return errors.New("Please enter a correct password.") + } - return nil + if err := cookie.Valid(); cookie != nil && err != nil { + return err } - if !h.basicAuth.IsAuthorized(p) { - return errors.New("wrong password was given") + if cookie != nil { + hash := sha256.New() + hash.Write([]byte(h.basicAuth.Password)) + hashedValue := hash.Sum(nil) + if hex.EncodeToString(hashedValue) != cookie.Value { + return errors.New("Cookie not set correctly.") + } } return nil diff --git a/internal/pkg/service/appsproxy/proxy/pagewriter/template/login.gohtml b/internal/pkg/service/appsproxy/proxy/pagewriter/template/login.gohtml index 82e79bb330..6d53468e6e 100644 --- a/internal/pkg/service/appsproxy/proxy/pagewriter/template/login.gohtml +++ b/internal/pkg/service/appsproxy/proxy/pagewriter/template/login.gohtml @@ -30,7 +30,7 @@ -

Please enter a correct password.

+

{{.Error}}

{{ end }}
@@ -38,7 +38,7 @@ -