From 591f293c8bcf464ed62701321d3f27de31ceb621 Mon Sep 17 00:00:00 2001 From: mo khan Date: Wed, 28 May 2025 12:14:11 -0600 Subject: refactor: parse headers injected by envoy --- app/app.go | 12 +---- app/cfg/cfg.go | 3 +- app/domain/identifiable.go | 8 ++++ app/domain/user.go | 11 +++-- app/middleware/id_token.go | 38 --------------- app/middleware/id_token_test.go | 80 -------------------------------- app/middleware/init.go | 35 ++++++-------- app/middleware/user.go | 6 +-- app/middleware/user_parser.go | 28 ++---------- app/middleware/user_parser_test.go | 36 +++++++++++++++ app/middleware/user_test.go | 94 +++++++++++--------------------------- 11 files changed, 101 insertions(+), 250 deletions(-) delete mode 100644 app/middleware/id_token.go delete mode 100644 app/middleware/id_token_test.go create mode 100644 app/middleware/user_parser_test.go (limited to 'app') diff --git a/app/app.go b/app/app.go index 94b3e7c..9ccdaba 100644 --- a/app/app.go +++ b/app/app.go @@ -4,15 +4,12 @@ import ( "net/http" "path/filepath" - "github.com/coreos/go-oidc/v3/oidc" "github.com/rs/zerolog" "github.com/xlgmokha/x/pkg/ioc" "github.com/xlgmokha/x/pkg/log" "github.com/xlgmokha/x/pkg/x" - "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/app/cfg" "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/app/controllers/dashboard" "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/app/controllers/sparkles" - "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/app/domain" "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/app/middleware" ) @@ -35,17 +32,10 @@ func New(rootDir string) http.Handler { mux.Handle("GET /", http.FileServer(dir)) logger := ioc.MustResolve[*zerolog.Logger](ioc.Default) - users := ioc.MustResolve[domain.Repository[*domain.User]](ioc.Default) return x.Middleware[http.Handler]( mux, log.HTTP(logger), - middleware.IDToken( - ioc.MustResolve[*oidc.Provider](ioc.Default), - ioc.MustResolve[*oidc.Config](ioc.Default), - middleware.FromCustomHeader("x-jwt-payload"), - middleware.FromCookie(cfg.IDTokenCookie), - ), - middleware.User(users), + middleware.User(), ) } diff --git a/app/cfg/cfg.go b/app/cfg/cfg.go index b423413..7c5e717 100644 --- a/app/cfg/cfg.go +++ b/app/cfg/cfg.go @@ -1,14 +1,13 @@ package cfg import ( - "github.com/coreos/go-oidc/v3/oidc" "github.com/xlgmokha/x/pkg/context" "github.com/xlgmokha/x/pkg/env" "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/app/domain" ) var CurrentUser context.Key[*domain.User] = context.Key[*domain.User]("current_user") -var IDToken context.Key[*oidc.IDToken] = context.Key[*oidc.IDToken]("id_token") + var OIDCIssuer string = env.Fetch("OIDC_ISSUER", "https://gitlab.com") var OAuthClientID string = env.Fetch("OAUTH_CLIENT_ID", "client_id") diff --git a/app/domain/identifiable.go b/app/domain/identifiable.go index 8fbc1e4..06bec07 100644 --- a/app/domain/identifiable.go +++ b/app/domain/identifiable.go @@ -1,7 +1,15 @@ package domain +import "github.com/xlgmokha/x/pkg/x" + type Identifiable interface { GetID() ID SetID(id ID) error ToGID() string } + +func WithID[T Identifiable](id ID) x.Configure[T] { + return func(item T) { + item.SetID(id) + } +} diff --git a/app/domain/user.go b/app/domain/user.go index 02ddd26..a6adfa8 100644 --- a/app/domain/user.go +++ b/app/domain/user.go @@ -1,15 +1,20 @@ package domain +import "github.com/xlgmokha/x/pkg/x" + type User struct { ID ID `json:"id" jsonapi:"primary,users"` Username string `json:"username" jsonapi:"attr,username"` - Email string `json:"email" jsonapi:"attr,email"` ProfileURL string `json:"profile" jsonapi:"attr,profile"` Picture string `json:"picture" jsonapi:"attr,picture"` } -func NewUser() *User { - return &User{} +func NewUser(options ...x.Configure[*User]) *User { + user := &User{} + for _, option := range options { + option(user) + } + return user } func (u *User) GetID() ID { diff --git a/app/middleware/id_token.go b/app/middleware/id_token.go deleted file mode 100644 index 0c1503e..0000000 --- a/app/middleware/id_token.go +++ /dev/null @@ -1,38 +0,0 @@ -package middleware - -import ( - "net/http" - - "github.com/coreos/go-oidc/v3/oidc" - "github.com/xlgmokha/x/pkg/log" - "github.com/xlgmokha/x/pkg/x" - xcfg "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/app/cfg" - "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/pkg/pls" -) - -func IDToken(provider *oidc.Provider, config *oidc.Config, parsers ...TokenParser) func(http.Handler) http.Handler { - return func(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - for _, parser := range parsers { - rawIDToken := parser(r) - if x.IsPresent(rawIDToken) { - verifier := provider.VerifierContext(r.Context(), config) - idToken, err := verifier.Verify(r.Context(), rawIDToken.String()) - - if err != nil { - pls.LogError(r.Context(), err) - } else { - log.WithFields(r.Context(), log.Fields{"id_token": idToken}) - next.ServeHTTP( - w, - r.WithContext(xcfg.IDToken.With(r.Context(), idToken)), - ) - return - } - } - } - - next.ServeHTTP(w, r) - }) - } -} diff --git a/app/middleware/id_token_test.go b/app/middleware/id_token_test.go deleted file mode 100644 index 9d8521a..0000000 --- a/app/middleware/id_token_test.go +++ /dev/null @@ -1,80 +0,0 @@ -package middleware - -import ( - "net/http" - "testing" - - "github.com/coreos/go-oidc/v3/oidc" - "github.com/oauth2-proxy/mockoidc" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "github.com/xlgmokha/x/pkg/test" - "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/app/cfg" - xcfg "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/app/cfg" - "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/pkg/web" -) - -func TestIDToken(t *testing.T) { - srv := web.NewOIDCServer(t) - defer srv.Close() - - middleware := IDToken(srv.Provider, &oidc.Config{ClientID: srv.MockOIDC.ClientID}, FromCookie(cfg.IDTokenCookie)) - - t.Run("when an active id_token cookie is provided", func(t *testing.T) { - t.Run("attaches the token to the request context", func(t *testing.T) { - user := mockoidc.DefaultUser() - _, rawIDToken := srv.CreateTokensFor(user) - - server := middleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - token := xcfg.IDToken.From(r.Context()) - require.NotNil(t, token) - assert.Equal(t, user.Subject, token.Subject) - - w.WriteHeader(http.StatusTeapot) - })) - - r, w := test.RequestResponse( - "GET", - "/example", - test.WithCookie(web.NewCookie(xcfg.IDTokenCookie, rawIDToken)), - ) - server.ServeHTTP(w, r) - - assert.Equal(t, http.StatusTeapot, w.Code) - }) - }) - - t.Run("when an invalid id_token cookie is provided", func(t *testing.T) { - t.Run("forwards the request", func(t *testing.T) { - server := middleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - require.Nil(t, xcfg.IDToken.From(r.Context())) - - w.WriteHeader(http.StatusTeapot) - })) - - r, w := test.RequestResponse( - "GET", - "/example", - test.WithCookie(web.NewCookie(xcfg.IDTokenCookie, "invalid")), - ) - server.ServeHTTP(w, r) - - assert.Equal(t, http.StatusTeapot, w.Code) - }) - }) - - t.Run("when no cookies are provided", func(t *testing.T) { - t.Run("forwards the request", func(t *testing.T) { - server := middleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - require.Nil(t, xcfg.IDToken.From(r.Context())) - - w.WriteHeader(http.StatusTeapot) - })) - - r, w := test.RequestResponse("GET", "/example") - server.ServeHTTP(w, r) - - assert.Equal(t, http.StatusTeapot, w.Code) - }) - }) -} diff --git a/app/middleware/init.go b/app/middleware/init.go index 874ca52..5bf84f6 100644 --- a/app/middleware/init.go +++ b/app/middleware/init.go @@ -1,33 +1,24 @@ package middleware import ( - "github.com/coreos/go-oidc/v3/oidc" + "net/http" + "github.com/xlgmokha/x/pkg/mapper" + "github.com/xlgmokha/x/pkg/x" "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/app/domain" ) -type CustomClaims struct { - Name string `json:"name"` - Nickname string `json:"nickname"` - Email string `json:"email"` - ProfileURL string `json:"profile"` - Picture string `json:"picture"` - Groups []string `json:"groups_direct"` -} - func init() { - mapper.Register(func(idToken *oidc.IDToken) *domain.User { - customClaims := &CustomClaims{} - if err := idToken.Claims(customClaims); err != nil { - return &domain.User{ID: domain.ID(idToken.Subject)} - } - - return &domain.User{ - ID: domain.ID(idToken.Subject), - Username: customClaims.Nickname, - Email: customClaims.Email, - ProfileURL: customClaims.ProfileURL, - Picture: customClaims.Picture, + mapper.Register(func(h http.Header) *domain.User { + subject := h.Get("x-jwt-claim-sub") + if x.IsPresent(subject) { + return &domain.User{ + ID: domain.ID(subject), + Username: h.Get("x-jwt-claim-username"), + ProfileURL: h.Get("x-jwt-claim-profile-url"), + Picture: h.Get("x-jwt-claim-picture-url"), + } } + return nil }) } diff --git a/app/middleware/user.go b/app/middleware/user.go index 0916cfa..90bf6aa 100644 --- a/app/middleware/user.go +++ b/app/middleware/user.go @@ -5,13 +5,13 @@ import ( "github.com/xlgmokha/x/pkg/x" "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/app/cfg" - "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/app/domain" ) -func User(db domain.Repository[*domain.User]) func(http.Handler) http.Handler { +func User() func(http.Handler) http.Handler { + parser := UserParser() return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - user := UserParser(db)(r) + user := parser(r) if x.IsPresent(user) { next.ServeHTTP(w, r.WithContext(cfg.CurrentUser.With(r.Context(), user))) } else { diff --git a/app/middleware/user_parser.go b/app/middleware/user_parser.go index cd6f36f..dfa0cce 100644 --- a/app/middleware/user_parser.go +++ b/app/middleware/user_parser.go @@ -3,34 +3,14 @@ package middleware import ( "net/http" - "github.com/coreos/go-oidc/v3/oidc" + "github.com/xlgmokha/x/pkg/log" "github.com/xlgmokha/x/pkg/mapper" - "github.com/xlgmokha/x/pkg/x" - "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/app/cfg" "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/app/domain" - "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/pkg/pls" ) -func UserParser(db domain.Repository[*domain.User]) RequestParser[*domain.User] { +func UserParser() RequestParser[*domain.User] { return func(r *http.Request) *domain.User { - subject := r.Header.Get("x-jwt-claim-sub") - user := db.Find(r.Context(), domain.ID(subject)) - - if !x.IsPresent(user) { - idToken := cfg.IDToken.From(r.Context()) - - if x.IsZero(idToken) { - return nil - } - - user = db.Find(r.Context(), domain.ID(idToken.Subject)) - if !x.IsPresent(user) { - user = mapper.MapFrom[*oidc.IDToken, *domain.User](idToken) - if err := db.Save(r.Context(), user); err != nil { - pls.LogError(r.Context(), err) - } - } - } - return user + log.WithFields(r.Context(), log.Fields{"header": r.Header}) + return mapper.MapFrom[http.Header, *domain.User](r.Header) } } diff --git a/app/middleware/user_parser_test.go b/app/middleware/user_parser_test.go new file mode 100644 index 0000000..2127a10 --- /dev/null +++ b/app/middleware/user_parser_test.go @@ -0,0 +1,36 @@ +package middleware + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/xlgmokha/x/pkg/test" + "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/app/domain" +) + +func TestUserParser(t *testing.T) { + parser := UserParser() + + t.Run("when x-jwt-claim-* headers are not provided", func(t *testing.T) { + t.Run("forwards the request without a current user attached to the request", func(t *testing.T) { + assert.Nil(t, parser(test.Request("GET", "/"))) + }) + }) + + t.Run("when x-jwt-claim-* headers are provided", func(t *testing.T) { + r := test.Request("GET", "/", + test.WithRequestHeader("x-jwt-claim-sub", "1"), + test.WithRequestHeader("x-jwt-claim-username", "root"), + test.WithRequestHeader("x-jwt-claim-profile-url", "https://gitlab.com/tanuki"), + test.WithRequestHeader("x-jwt-claim-picture-url", "https://example.com/profile.png"), + ) + + result := parser(r) + require.NotNil(t, result) + assert.Equal(t, domain.ID("1"), result.ID) + assert.Equal(t, "root", result.Username) + assert.Equal(t, "https://gitlab.com/tanuki", result.ProfileURL) + assert.Equal(t, "https://example.com/profile.png", result.Picture) + }) +} diff --git a/app/middleware/user_test.go b/app/middleware/user_test.go index 7653684..c5fa7ed 100644 --- a/app/middleware/user_test.go +++ b/app/middleware/user_test.go @@ -4,90 +4,50 @@ import ( "net/http" "testing" - "github.com/coreos/go-oidc/v3/oidc" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/xlgmokha/x/pkg/test" "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/app/cfg" - "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/app/db" "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/app/domain" - "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/pkg/pls" ) func TestUser(t *testing.T) { - repository := db.NewRepository[*domain.User]() - middleware := User(repository) + middleware := User() - knownUser := &domain.User{ID: domain.ID(pls.GenerateULID())} - require.NoError(t, repository.Save(t.Context(), knownUser)) + t.Run("when x-jwt-claim-* headers are not provided", func(t *testing.T) { + server := middleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Nil(t, cfg.CurrentUser.From(r.Context())) - t.Run("when ID Token is provided", func(t *testing.T) { - t.Run("when user is known", func(t *testing.T) { - server := middleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - user := cfg.CurrentUser.From(r.Context()) - require.NotNil(t, user) - assert.Equal(t, knownUser.ID, user.ID) + w.WriteHeader(http.StatusTeapot) + })) - w.WriteHeader(http.StatusTeapot) - })) + r, w := test.RequestResponse("GET", "/example") + server.ServeHTTP(w, r) - ctx := cfg.IDToken.With(t.Context(), &oidc.IDToken{Subject: knownUser.ID.String()}) - - r, w := test.RequestResponse("GET", "/example", test.WithContext(ctx)) - server.ServeHTTP(w, r) - - assert.Equal(t, http.StatusTeapot, w.Code) - }) - - t.Run("when user is unknown", func(t *testing.T) { - unknownID := pls.GenerateULID() - - server := middleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - user := cfg.CurrentUser.From(r.Context()) - require.NotNil(t, user) - assert.Equal(t, domain.ID(unknownID), user.ID) - - w.WriteHeader(http.StatusTeapot) - })) - - ctx := cfg.IDToken.With(t.Context(), &oidc.IDToken{Subject: unknownID}) - - r, w := test.RequestResponse("GET", "/example", test.WithContext(ctx)) - server.ServeHTTP(w, r) - - assert.Equal(t, http.StatusTeapot, w.Code) - require.NotNil(t, repository.Find(t.Context(), domain.ID(unknownID))) - }) + assert.Equal(t, http.StatusTeapot, w.Code) }) - t.Run("when ID Token is not provided", func(t *testing.T) { - t.Run("without custom headers", func(t *testing.T) { - server := middleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - user := cfg.CurrentUser.From(r.Context()) - require.Nil(t, user) - - w.WriteHeader(http.StatusTeapot) - })) - - r, w := test.RequestResponse("GET", "/example") - server.ServeHTTP(w, r) - - assert.Equal(t, http.StatusTeapot, w.Code) - }) + t.Run("when x-jwt-claim-* headers are provided", func(t *testing.T) { + server := middleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + user := cfg.CurrentUser.From(r.Context()) + require.NotNil(t, user) - t.Run("with x-jwt-claim-sub header", func(t *testing.T) { - server := middleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - user := cfg.CurrentUser.From(r.Context()) - require.NotNil(t, user) - require.Equal(t, knownUser.ID, user.ID) + assert.Equal(t, domain.ID("1"), user.ID) + assert.Equal(t, "root", user.Username) + assert.Equal(t, "https://gitlab.com/tanuki", user.ProfileURL) + assert.Equal(t, "https://example.com/profile.png", user.Picture) - w.WriteHeader(http.StatusTeapot) - })) + w.WriteHeader(http.StatusTeapot) + })) - r, w := test.RequestResponse("GET", "/example", test.WithRequestHeader("x-jwt-claim-sub", knownUser.ID.String())) - server.ServeHTTP(w, r) + r, w := test.RequestResponse("GET", "/", + test.WithRequestHeader("x-jwt-claim-sub", "1"), + test.WithRequestHeader("x-jwt-claim-username", "root"), + test.WithRequestHeader("x-jwt-claim-profile-url", "https://gitlab.com/tanuki"), + test.WithRequestHeader("x-jwt-claim-picture-url", "https://example.com/profile.png"), + ) + server.ServeHTTP(w, r) - assert.Equal(t, http.StatusTeapot, w.Code) - }) + assert.Equal(t, http.StatusTeapot, w.Code) }) } -- cgit v1.2.3