From ac2d04686ffaf8229c921c4c39631d14e7412d08 Mon Sep 17 00:00:00 2001 From: Erikson Bahr Date: Wed, 31 Jan 2024 16:59:52 -0300 Subject: [PATCH 01/10] implement oidc singl sign on cookie refresher --- .gitignore | 2 + oauthproxy.go | 3 ++ pkg/apis/sessions/session_state.go | 5 ++- pkg/middleware/cookie_refresh.go | 60 ++++++++++++++++++++++++++++++ pkg/middleware/stored_session.go | 3 ++ providers/oidc.go | 1 + 6 files changed, 72 insertions(+), 2 deletions(-) create mode 100644 pkg/middleware/cookie_refresh.go diff --git a/.gitignore b/.gitignore index 57f3044462..393f27d53d 100644 --- a/.gitignore +++ b/.gitignore @@ -41,3 +41,5 @@ _testmain.go # vi Dockerfile.dev # docker build -f Dockerfile.dev . Dockerfile.dev + +obj \ No newline at end of file diff --git a/oauthproxy.go b/oauthproxy.go index d11040c380..d5f79cb779 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -393,6 +393,9 @@ func buildSessionChain(opts *options.Options, provider providers.Provider, sessi ValidateSession: provider.ValidateSession, })) + x := opts.Providers[0] + chain = chain.Append(middleware.NewCookieRefresh(&middleware.CookieRefreshOptions{IssuerURL: x.OIDCConfig.IssuerURL})) + return chain } diff --git a/pkg/apis/sessions/session_state.go b/pkg/apis/sessions/session_state.go index 2fd5161347..43075adcd7 100644 --- a/pkg/apis/sessions/session_state.go +++ b/pkg/apis/sessions/session_state.go @@ -31,8 +31,9 @@ type SessionState struct { IntrospectClaims string `msgpack:"ic,omitempty"` // Internal helpers, not serialized - Clock clock.Clock `msgpack:"-"` - Lock Lock `msgpack:"-"` + Clock clock.Clock `msgpack:"-"` + Lock Lock `msgpack:"-"` + SessionJustRefreshed bool `msgpack:"-"` } func (s *SessionState) ObtainLock(ctx context.Context, expiration time.Duration) error { diff --git a/pkg/middleware/cookie_refresh.go b/pkg/middleware/cookie_refresh.go new file mode 100644 index 0000000000..e5fc9104b3 --- /dev/null +++ b/pkg/middleware/cookie_refresh.go @@ -0,0 +1,60 @@ +package middleware + +import ( + "fmt" + "net/http" + + "github.com/justinas/alice" + middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/requests" +) + +type CookieRefreshOptions struct { + IssuerURL string +} + +func NewCookieRefresh(opts *CookieRefreshOptions) alice.Constructor { + cr := &cookieRefresh{ + HttpClient: &http.Client{}, + IssuerURL: opts.IssuerURL, + } + return cr.refreshCookie +} + +type cookieRefresh struct { + HttpClient *http.Client + IssuerURL string +} + +func (cr *cookieRefresh) refreshCookie(next http.Handler) http.Handler { + return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + scope := middlewareapi.GetRequestScope(req) + if scope.Session == nil || !scope.Session.SessionJustRefreshed { + next.ServeHTTP(rw, req) + return + } + + cookie, err := req.Cookie("hsdpamcookie") + if err != nil { + logger.Errorf("SSO Cookie Refresher - Could find 'hsdpamcookie' cookie in the request: %v", err) + return + } + resp := requests.New(fmt.Sprintf("%s/session/refresh", cr.IssuerURL)). + WithContext(req.Context()). + WithMethod("GET"). + SetHeader("api-version", "1"). + SetHeader("Cookie", fmt.Sprintf("hsdpamcookie=%s", cookie.Value)). + Do() + + if resp.StatusCode() != http.StatusNoContent { + bodyString := string(resp.Body()) + logger.Errorf("SSO Cookie Refresher - Could not refresh the 'hsdpamcookie' cookie due to status and content: %v - %v", resp.StatusCode(), bodyString) + return + } else { + logger.Print("SSO Cookie Refresher - Cookie 'hsdpamcookie' refreshed") + } + + next.ServeHTTP(rw, req) + }) +} diff --git a/pkg/middleware/stored_session.go b/pkg/middleware/stored_session.go index 1afe6d0cf0..75501ac4a6 100644 --- a/pkg/middleware/stored_session.go +++ b/pkg/middleware/stored_session.go @@ -48,6 +48,9 @@ type StoredSessionLoaderOptions struct { // If the sesssion is older than `RefreshPeriod` but the provider doesn't // refresh it, we must re-validate using this validation. ValidateSession func(context.Context, *sessionsapi.SessionState) bool + + // Callback that is called when a session is refreshed + OnSessionRefreshed *func(context.Context, *http.Request, *sessionsapi.SessionState) } // NewStoredSessionLoader creates a new storedSessionLoader which loads diff --git a/providers/oidc.go b/providers/oidc.go index de7b827754..f55ec0e4fc 100644 --- a/providers/oidc.go +++ b/providers/oidc.go @@ -159,6 +159,7 @@ func (p *OIDCProvider) RefreshSession(ctx context.Context, s *sessions.SessionSt if err != nil { return false, fmt.Errorf("unable to redeem refresh token: %v", err) } + s.SessionJustRefreshed = true return true, nil } From 0c8b7f9e330041a9e1ad3ab108747da00652689a Mon Sep 17 00:00:00 2001 From: Erikson Bahr Date: Thu, 1 Feb 2024 09:56:38 -0300 Subject: [PATCH 02/10] fix lint --- pkg/middleware/cookie_refresh.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/pkg/middleware/cookie_refresh.go b/pkg/middleware/cookie_refresh.go index e5fc9104b3..c5b803bbac 100644 --- a/pkg/middleware/cookie_refresh.go +++ b/pkg/middleware/cookie_refresh.go @@ -16,14 +16,14 @@ type CookieRefreshOptions struct { func NewCookieRefresh(opts *CookieRefreshOptions) alice.Constructor { cr := &cookieRefresh{ - HttpClient: &http.Client{}, + HTTPClient: &http.Client{}, IssuerURL: opts.IssuerURL, } return cr.refreshCookie } type cookieRefresh struct { - HttpClient *http.Client + HTTPClient *http.Client IssuerURL string } @@ -51,10 +51,9 @@ func (cr *cookieRefresh) refreshCookie(next http.Handler) http.Handler { bodyString := string(resp.Body()) logger.Errorf("SSO Cookie Refresher - Could not refresh the 'hsdpamcookie' cookie due to status and content: %v - %v", resp.StatusCode(), bodyString) return - } else { - logger.Print("SSO Cookie Refresher - Cookie 'hsdpamcookie' refreshed") } + logger.Print("SSO Cookie Refresher - Cookie 'hsdpamcookie' refreshed") next.ServeHTTP(rw, req) }) } From df015b5123d504eb18b9b0350737b43eb96fd3ad Mon Sep 17 00:00:00 2001 From: Erikson Bahr Date: Thu, 1 Feb 2024 10:38:06 -0300 Subject: [PATCH 03/10] add 'OAUTH2_PROXY_OIDC_ENABLE_COOKIE_REFRESH' and 'OAUTH2_PROXY_OIDC_COOKIE_REFRESH_NAME' options --- oauthproxy.go | 7 +++++-- pkg/apis/options/legacy_options.go | 6 ++++++ pkg/apis/options/providers.go | 4 ++++ pkg/middleware/cookie_refresh.go | 23 +++++++++++++---------- 4 files changed, 28 insertions(+), 12 deletions(-) diff --git a/oauthproxy.go b/oauthproxy.go index d5f79cb779..4eb58e0ccc 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -393,8 +393,11 @@ func buildSessionChain(opts *options.Options, provider providers.Provider, sessi ValidateSession: provider.ValidateSession, })) - x := opts.Providers[0] - chain = chain.Append(middleware.NewCookieRefresh(&middleware.CookieRefreshOptions{IssuerURL: x.OIDCConfig.IssuerURL})) + oidcProviderSettings := opts.Providers[0].OIDCConfig + if oidcProviderSettings.EnableCookieRefresh { + chain = chain.Append(middleware.NewCookieRefresh(&middleware.CookieRefreshOptions{IssuerURL: oidcProviderSettings.IssuerURL, CookieRefreshName: oidcProviderSettings.CookieRefreshName})) + logger.Printf("Enabling OIDC cookie refresh for the cookie '%s' functionality because OIDCEnableCookieRefresh is enabled", oidcProviderSettings.CookieRefreshName) + } return chain } diff --git a/pkg/apis/options/legacy_options.go b/pkg/apis/options/legacy_options.go index 616b33abcb..c3c010a289 100644 --- a/pkg/apis/options/legacy_options.go +++ b/pkg/apis/options/legacy_options.go @@ -543,6 +543,8 @@ type LegacyProvider struct { OIDCGroupsClaim string `flag:"oidc-groups-claim" cfg:"oidc_groups_claim"` OIDCAudienceClaims []string `flag:"oidc-audience-claim" cfg:"oidc_audience_claims"` OIDCExtraAudiences []string `flag:"oidc-extra-audience" cfg:"oidc_extra_audiences"` + OIDCEnableCookieRefresh bool `flag:"oidc-enable-cookie-refresh" cfg:"oidc_enable_cookie_refresh"` + OIDCCookieRefreshName string `flag:"oidc-cookie-refresh-name" cfg:"oidc_cookie_refresh_name"` LoginURL string `flag:"login-url" cfg:"login_url"` RedeemURL string `flag:"redeem-url" cfg:"redeem_url"` ProfileURL string `flag:"profile-url" cfg:"profile_url"` @@ -601,6 +603,8 @@ func legacyProviderFlagSet() *pflag.FlagSet { flagSet.String("oidc-email-claim", OIDCEmailClaim, "which OIDC claim contains the user's email") flagSet.StringSlice("oidc-audience-claim", OIDCAudienceClaims, "which OIDC claims are used as audience to verify against client id") flagSet.StringSlice("oidc-extra-audience", []string{}, "additional audiences allowed to pass audience verification") + flagSet.Bool("oidc-enable-cookie-refresh", false, "Refresh the OIDC provider cookies to enable SSO in an extended period of time") + flagSet.String("oidc-cookie-refresh-name", "hsdpamcookie", "The name of the cookie that the OIDC provider uses to keep its session fresh") flagSet.String("login-url", "", "Authentication endpoint") flagSet.String("redeem-url", "", "Token redemption endpoint") flagSet.String("profile-url", "", "Profile access endpoint") @@ -702,6 +706,8 @@ func (l *LegacyProvider) convert() (Providers, error) { GroupsClaim: l.OIDCGroupsClaim, AudienceClaims: l.OIDCAudienceClaims, ExtraAudiences: l.OIDCExtraAudiences, + EnableCookieRefresh: l.OIDCEnableCookieRefresh, + CookieRefreshName: l.OIDCCookieRefreshName, } // Support for legacy configuration option diff --git a/pkg/apis/options/providers.go b/pkg/apis/options/providers.go index 7b9934051c..a7e02b7d17 100644 --- a/pkg/apis/options/providers.go +++ b/pkg/apis/options/providers.go @@ -230,6 +230,10 @@ type OIDCOptions struct { // ExtraAudiences is a list of additional audiences that are allowed // to pass verification in addition to the client id. ExtraAudiences []string `json:"extraAudiences,omitempty"` + // Enable cookie refresh functionality that is going to be triggered every time the session is updated + EnableCookieRefresh bool `json:"enableCookieRefresh,omitempty"` + // Name of the cookie that is going to be extracted from the request and refreshed + CookieRefreshName string `json:"cookieRefreshName,omitempty"` } type LoginGovOptions struct { diff --git a/pkg/middleware/cookie_refresh.go b/pkg/middleware/cookie_refresh.go index c5b803bbac..a64c161f15 100644 --- a/pkg/middleware/cookie_refresh.go +++ b/pkg/middleware/cookie_refresh.go @@ -11,20 +11,23 @@ import ( ) type CookieRefreshOptions struct { - IssuerURL string + IssuerURL string + CookieRefreshName string } func NewCookieRefresh(opts *CookieRefreshOptions) alice.Constructor { cr := &cookieRefresh{ - HTTPClient: &http.Client{}, - IssuerURL: opts.IssuerURL, + HTTPClient: &http.Client{}, + IssuerURL: opts.IssuerURL, + CookieRefreshName: opts.CookieRefreshName, } return cr.refreshCookie } type cookieRefresh struct { - HTTPClient *http.Client - IssuerURL string + HTTPClient *http.Client + IssuerURL string + CookieRefreshName string } func (cr *cookieRefresh) refreshCookie(next http.Handler) http.Handler { @@ -35,25 +38,25 @@ func (cr *cookieRefresh) refreshCookie(next http.Handler) http.Handler { return } - cookie, err := req.Cookie("hsdpamcookie") + cookie, err := req.Cookie(cr.CookieRefreshName) if err != nil { - logger.Errorf("SSO Cookie Refresher - Could find 'hsdpamcookie' cookie in the request: %v", err) + logger.Errorf("SSO Cookie Refresher - Could find '%s' cookie in the request: %v", cr.CookieRefreshName, err) return } resp := requests.New(fmt.Sprintf("%s/session/refresh", cr.IssuerURL)). WithContext(req.Context()). WithMethod("GET"). SetHeader("api-version", "1"). - SetHeader("Cookie", fmt.Sprintf("hsdpamcookie=%s", cookie.Value)). + SetHeader("Cookie", fmt.Sprintf("%s=%s", cr.CookieRefreshName, cookie.Value)). Do() if resp.StatusCode() != http.StatusNoContent { bodyString := string(resp.Body()) - logger.Errorf("SSO Cookie Refresher - Could not refresh the 'hsdpamcookie' cookie due to status and content: %v - %v", resp.StatusCode(), bodyString) + logger.Errorf("SSO Cookie Refresher - Could not refresh the '%s' cookie due to status and content: %v - %v", cr.CookieRefreshName, resp.StatusCode(), bodyString) return } - logger.Print("SSO Cookie Refresher - Cookie 'hsdpamcookie' refreshed") + logger.Printf("SSO Cookie Refresher - Cookie '%s' refreshed", cr.CookieRefreshName) next.ServeHTTP(rw, req) }) } From 1140950b9c034329e19637762f62203fc53565bd Mon Sep 17 00:00:00 2001 From: Erikson Bahr Date: Thu, 1 Feb 2024 11:00:15 -0300 Subject: [PATCH 04/10] update docs --- docs/docs/configuration/alpha_config.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/docs/configuration/alpha_config.md b/docs/docs/configuration/alpha_config.md index 24e6a429a1..03f10faa17 100644 --- a/docs/docs/configuration/alpha_config.md +++ b/docs/docs/configuration/alpha_config.md @@ -388,6 +388,8 @@ character. | `userIDClaim` | _string_ | UserIDClaim indicates which claim contains the user ID
default set to 'email' | | `audienceClaims` | _[]string_ | AudienceClaim allows to define any claim that is verified against the client id
By default `aud` claim is used for verification. | | `extraAudiences` | _[]string_ | ExtraAudiences is a list of additional audiences that are allowed
to pass verification in addition to the client id. | +| `enableCookieRefresh` | _bool_ | Enable cookie refresh functionality that is going to be triggered every time the session is updated | +| `cookieRefreshName` | _string_ | Name of the cookie that is going to be extracted from the request and refreshed | ### Provider From 7ca98f24c8e412049d643e107728fe0828f858a7 Mon Sep 17 00:00:00 2001 From: Erikson Bahr Date: Thu, 1 Feb 2024 11:15:02 -0300 Subject: [PATCH 05/10] fix unit tests --- pkg/apis/options/load_test.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/pkg/apis/options/load_test.go b/pkg/apis/options/load_test.go index cface5140b..a43c9b84c9 100644 --- a/pkg/apis/options/load_test.go +++ b/pkg/apis/options/load_test.go @@ -36,14 +36,16 @@ var _ = Describe("Load", func() { }, LegacyProvider: LegacyProvider{ - ProviderType: "google", - AzureTenant: "common", - ApprovalPrompt: "force", - UserIDClaim: "email", - OIDCEmailClaim: "email", - OIDCGroupsClaim: "groups", - OIDCAudienceClaims: []string{"aud"}, - InsecureOIDCSkipNonce: true, + ProviderType: "google", + AzureTenant: "common", + ApprovalPrompt: "force", + UserIDClaim: "email", + OIDCEmailClaim: "email", + OIDCGroupsClaim: "groups", + OIDCAudienceClaims: []string{"aud"}, + InsecureOIDCSkipNonce: true, + OIDCEnableCookieRefresh: false, + OIDCCookieRefreshName: "hsdpamcookie", }, Options: Options{ From e18e301a86b27352912f33c30a4851edee30da0a Mon Sep 17 00:00:00 2001 From: Erikson Bahr Date: Thu, 1 Feb 2024 11:19:47 -0300 Subject: [PATCH 06/10] add local-debug-build to make --- Makefile | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Makefile b/Makefile index fdfda8eafd..e373766e95 100644 --- a/Makefile +++ b/Makefile @@ -116,3 +116,7 @@ validate-go-version: .PHONY: local-env-% local-env-%: make -C contrib/local-environment $* + +.PHONY: local-debug-build +local-debug-build: + go build -gcflags="all=-N -l" -o app.exe \ No newline at end of file From 71512442de797e5fb7b04bc123c39ef71c7b5422 Mon Sep 17 00:00:00 2001 From: Erikson Bahr Date: Thu, 1 Feb 2024 11:26:05 -0300 Subject: [PATCH 07/10] fix unit tests --- main_test.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/main_test.go b/main_test.go index 6a640817b7..7ecf419b22 100644 --- a/main_test.go +++ b/main_test.go @@ -151,12 +151,14 @@ redirect_url="http://localhost:4180/oauth2/callback" Tenant: "common", }, OIDCConfig: options.OIDCOptions{ - GroupsClaim: "groups", - EmailClaim: "email", - UserIDClaim: "email", - AudienceClaims: []string{"aud"}, - ExtraAudiences: []string{}, - InsecureSkipNonce: true, + GroupsClaim: "groups", + EmailClaim: "email", + UserIDClaim: "email", + AudienceClaims: []string{"aud"}, + ExtraAudiences: []string{}, + InsecureSkipNonce: true, + EnableCookieRefresh: false, + CookieRefreshName: "hsdpamcookie", }, LoginURLParameters: []options.LoginURLParameter{ {Name: "approval_prompt", Default: []string{"force"}}, From 4b00a44fcfe928bb367208f7c50a97a6d6bb58fa Mon Sep 17 00:00:00 2001 From: Erikson Bahr Date: Thu, 1 Feb 2024 11:55:09 -0300 Subject: [PATCH 08/10] fix unit tests --- main_test.go | 16 ++++++++-------- pkg/apis/options/load_test.go | 18 ++++++++---------- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/main_test.go b/main_test.go index 7ecf419b22..5bce633546 100644 --- a/main_test.go +++ b/main_test.go @@ -77,6 +77,7 @@ providers: insecureSkipNonce: true audienceClaims: [aud] extraAudiences: [] + cookieRefreshName: 'hsdpamcookie' loginURLParameters: - name: approval_prompt default: @@ -151,14 +152,13 @@ redirect_url="http://localhost:4180/oauth2/callback" Tenant: "common", }, OIDCConfig: options.OIDCOptions{ - GroupsClaim: "groups", - EmailClaim: "email", - UserIDClaim: "email", - AudienceClaims: []string{"aud"}, - ExtraAudiences: []string{}, - InsecureSkipNonce: true, - EnableCookieRefresh: false, - CookieRefreshName: "hsdpamcookie", + GroupsClaim: "groups", + EmailClaim: "email", + UserIDClaim: "email", + AudienceClaims: []string{"aud"}, + ExtraAudiences: []string{}, + InsecureSkipNonce: true, + CookieRefreshName: "hsdpamcookie", }, LoginURLParameters: []options.LoginURLParameter{ {Name: "approval_prompt", Default: []string{"force"}}, diff --git a/pkg/apis/options/load_test.go b/pkg/apis/options/load_test.go index a43c9b84c9..cface5140b 100644 --- a/pkg/apis/options/load_test.go +++ b/pkg/apis/options/load_test.go @@ -36,16 +36,14 @@ var _ = Describe("Load", func() { }, LegacyProvider: LegacyProvider{ - ProviderType: "google", - AzureTenant: "common", - ApprovalPrompt: "force", - UserIDClaim: "email", - OIDCEmailClaim: "email", - OIDCGroupsClaim: "groups", - OIDCAudienceClaims: []string{"aud"}, - InsecureOIDCSkipNonce: true, - OIDCEnableCookieRefresh: false, - OIDCCookieRefreshName: "hsdpamcookie", + ProviderType: "google", + AzureTenant: "common", + ApprovalPrompt: "force", + UserIDClaim: "email", + OIDCEmailClaim: "email", + OIDCGroupsClaim: "groups", + OIDCAudienceClaims: []string{"aud"}, + InsecureOIDCSkipNonce: true, }, Options: Options{ From d6734336dc3c19a34d87afe196092b8cd9ba0b95 Mon Sep 17 00:00:00 2001 From: Erikson Bahr Date: Thu, 1 Feb 2024 13:38:54 -0300 Subject: [PATCH 09/10] remove deadcode --- pkg/middleware/stored_session.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pkg/middleware/stored_session.go b/pkg/middleware/stored_session.go index 75501ac4a6..1afe6d0cf0 100644 --- a/pkg/middleware/stored_session.go +++ b/pkg/middleware/stored_session.go @@ -48,9 +48,6 @@ type StoredSessionLoaderOptions struct { // If the sesssion is older than `RefreshPeriod` but the provider doesn't // refresh it, we must re-validate using this validation. ValidateSession func(context.Context, *sessionsapi.SessionState) bool - - // Callback that is called when a session is refreshed - OnSessionRefreshed *func(context.Context, *http.Request, *sessionsapi.SessionState) } // NewStoredSessionLoader creates a new storedSessionLoader which loads From e09053e3c3e09d88fb2e7d7c8ba8d93e6e9220ae Mon Sep 17 00:00:00 2001 From: Erikson Bahr Date: Thu, 1 Feb 2024 13:59:16 -0300 Subject: [PATCH 10/10] fix unit tests --- pkg/apis/options/load_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/apis/options/load_test.go b/pkg/apis/options/load_test.go index cface5140b..e5d9097816 100644 --- a/pkg/apis/options/load_test.go +++ b/pkg/apis/options/load_test.go @@ -44,6 +44,7 @@ var _ = Describe("Load", func() { OIDCGroupsClaim: "groups", OIDCAudienceClaims: []string{"aud"}, InsecureOIDCSkipNonce: true, + OIDCCookieRefreshName: "hsdpamcookie", }, Options: Options{