From 96f5aaf497c407013a115f211c9befb2b180bba1 Mon Sep 17 00:00:00 2001 From: mo khan Date: Wed, 28 May 2025 10:30:39 -0600 Subject: refactor: extract RequestParser type --- app/middleware/request_parser.go | 9 +++++++++ app/middleware/token_parser.go | 8 +------- 2 files changed, 10 insertions(+), 7 deletions(-) create mode 100644 app/middleware/request_parser.go diff --git a/app/middleware/request_parser.go b/app/middleware/request_parser.go new file mode 100644 index 0000000..dfc5d3a --- /dev/null +++ b/app/middleware/request_parser.go @@ -0,0 +1,9 @@ +package middleware + +import ( + "net/http" + + "github.com/xlgmokha/x/pkg/x" +) + +type RequestParser[T any] x.Mapper[*http.Request, T] diff --git a/app/middleware/token_parser.go b/app/middleware/token_parser.go index 48034f0..1a92760 100644 --- a/app/middleware/token_parser.go +++ b/app/middleware/token_parser.go @@ -1,9 +1,3 @@ package middleware -import ( - "net/http" - - "github.com/xlgmokha/x/pkg/x" -) - -type TokenParser x.Mapper[*http.Request, RawToken] +type TokenParser RequestParser[RawToken] -- cgit v1.2.3 From f76542bc846bc77e825055a1a6ea7cd0cb178844 Mon Sep 17 00:00:00 2001 From: mo khan Date: Wed, 28 May 2025 10:38:24 -0600 Subject: refactor: extract type to parse user from http.Request --- app/middleware/user.go | 32 +++++--------------------------- app/middleware/user_parser.go | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 27 deletions(-) create mode 100644 app/middleware/user_parser.go diff --git a/app/middleware/user.go b/app/middleware/user.go index 2a6bf71..0916cfa 100644 --- a/app/middleware/user.go +++ b/app/middleware/user.go @@ -3,42 +3,20 @@ 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 User(db domain.Repository[*domain.User]) func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - subject := r.Header.Get("x-jwt-claim-sub") - log.WithFields(r.Context(), log.Fields{"sub": subject}) - user := db.Find(r.Context(), domain.ID(subject)) - - if !x.IsPresent(user) { - idToken := cfg.IDToken.From(r.Context()) - - if x.IsZero(idToken) { - next.ServeHTTP(w, r) - return - } - - 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) - next.ServeHTTP(w, r) - return - } - } + user := UserParser(db)(r) + if x.IsPresent(user) { + next.ServeHTTP(w, r.WithContext(cfg.CurrentUser.With(r.Context(), user))) + } else { + next.ServeHTTP(w, r) } - - next.ServeHTTP(w, r.WithContext(cfg.CurrentUser.With(r.Context(), user))) }) } } diff --git a/app/middleware/user_parser.go b/app/middleware/user_parser.go new file mode 100644 index 0000000..cd6f36f --- /dev/null +++ b/app/middleware/user_parser.go @@ -0,0 +1,36 @@ +package middleware + +import ( + "net/http" + + "github.com/coreos/go-oidc/v3/oidc" + "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] { + 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 + } +} -- cgit v1.2.3 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 +++++++----------------- etc/envoy/envoy.yaml | 6 ++ pkg/authz/id_token.go | 38 +++++++--- pkg/authz/id_token_test.go | 1 + vendor/github.com/coreos/go-oidc/v3/oidc/oidc.go | 2 +- 15 files changed, 135 insertions(+), 263 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 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) }) } diff --git a/etc/envoy/envoy.yaml b/etc/envoy/envoy.yaml index b18a0ac..4378647 100644 --- a/etc/envoy/envoy.yaml +++ b/etc/envoy/envoy.yaml @@ -178,6 +178,12 @@ static_resources: claim_to_headers: - header_name: x-jwt-claim-sub claim_name: sub + - header_name: x-jwt-claim-username + claim_name: nickname + - header_name: x-jwt-claim-profile-url + claim_name: profile + - header_name: x-jwt-claim-picture-url + claim_name: picture forward: true forward_payload_header: x-jwt-payload from_cookies: diff --git a/pkg/authz/id_token.go b/pkg/authz/id_token.go index ccc96de..3271af8 100644 --- a/pkg/authz/id_token.go +++ b/pkg/authz/id_token.go @@ -5,21 +5,35 @@ import ( "encoding/json" "errors" "strings" - "time" ) +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"` +} + type IDToken struct { - // Audience []string `json:"aud"` - Email string `json:"email"` - EmailVerified bool `json:"email_verified"` - ExpiredAt int64 `json:"exp"` - IssuedAt int64 `json:"iat"` - Issuer string `json:"iss"` - Name string `json:"name"` - Nickname string `json:"nickname"` - Picture string `json:"picture"` - Subject string `json:"sub"` - UpdatedAt time.Time `json:"updated_at"` + Issuer string `json:"iss"` + Subject string `json:"sub"` + Audience any `json:"aud"` + Expiry any `json:"exp"` + IssuedAt any `json:"iat"` + NotBefore any `json:"nbf"` + Nonce string `json:"nonce"` + AtHash string `json:"at_hash"` + ClaimNames map[string]string `json:"_claim_names"` + ClaimSources map[string]ClaimSource `json:"_claim_sources"` + + CustomClaims +} + +type ClaimSource struct { + Endpoint string `json:"endpoint"` + AccessToken string `json:"access_token"` } func NewIDToken(raw string) (*IDToken, error) { diff --git a/pkg/authz/id_token_test.go b/pkg/authz/id_token_test.go index 22aabc4..054c48b 100644 --- a/pkg/authz/id_token_test.go +++ b/pkg/authz/id_token_test.go @@ -15,6 +15,7 @@ func TestIDToken(t *testing.T) { t.Run("when the token is valid", func(t *testing.T) { user := mockoidc.DefaultUser() _, rawIDToken := idp.CreateTokensFor(user) + t.Logf("id_token: %v\n", rawIDToken) token, err := NewIDToken(rawIDToken) require.NoError(t, err) diff --git a/vendor/github.com/coreos/go-oidc/v3/oidc/oidc.go b/vendor/github.com/coreos/go-oidc/v3/oidc/oidc.go index f6a7ea8..e06286c 100644 --- a/vendor/github.com/coreos/go-oidc/v3/oidc/oidc.go +++ b/vendor/github.com/coreos/go-oidc/v3/oidc/oidc.go @@ -162,7 +162,7 @@ var supportedAlgorithms = map[string]bool{ // parsing. // // // Directly fetch the metadata document. -// resp, err := http.Get("https://login.example.com/custom-metadata-path") +// resp, err := http.Get("https://login.example.com/custom-metadata-path") // if err != nil { // // ... // } -- cgit v1.2.3 From 5b6ed074bfb9c99d24d17dd9ba720d69fadf91b1 Mon Sep 17 00:00:00 2001 From: mo khan Date: Wed, 28 May 2025 12:34:58 -0600 Subject: refactor: delete jwt verification code --- app/init.go | 22 ---------------------- app/middleware/from_cookie.go | 15 --------------- app/middleware/from_custom_header.go | 9 --------- app/middleware/init.go | 2 +- app/middleware/raw_token.go | 7 ------- app/middleware/token_parser.go | 3 --- app/middleware/user.go | 23 +++++++++++++++-------- app/middleware/user_parser.go | 16 ---------------- app/middleware/user_parser_test.go | 36 ------------------------------------ app/middleware/user_test.go | 2 ++ bin/envoy.sh | 2 +- pkg/web/cookie.go | 35 ----------------------------------- pkg/web/cookie_test.go | 33 --------------------------------- pkg/web/oidc.go | 27 --------------------------- 14 files changed, 19 insertions(+), 213 deletions(-) delete mode 100644 app/middleware/from_cookie.go delete mode 100644 app/middleware/from_custom_header.go delete mode 100644 app/middleware/raw_token.go delete mode 100644 app/middleware/token_parser.go delete mode 100644 app/middleware/user_parser.go delete mode 100644 app/middleware/user_parser_test.go delete mode 100644 pkg/web/cookie.go delete mode 100644 pkg/web/cookie_test.go delete mode 100644 pkg/web/oidc.go diff --git a/app/init.go b/app/init.go index 935c962..5057fe4 100644 --- a/app/init.go +++ b/app/init.go @@ -1,24 +1,20 @@ package app import ( - "context" "net/http" "os" - "github.com/coreos/go-oidc/v3/oidc" "github.com/rs/zerolog" "github.com/xlgmokha/x/pkg/env" "github.com/xlgmokha/x/pkg/ioc" "github.com/xlgmokha/x/pkg/log" "github.com/xlgmokha/x/pkg/mapper" "gitlab.com/gitlab-org/software-supply-chain-security/authorization/authzd.git/pkg/rpc" - "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/db" "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/app/domain" "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/pkg/web" - "golang.org/x/oauth2" ) func init() { @@ -28,9 +24,6 @@ func init() { ioc.RegisterSingleton[domain.Repository[*domain.Sparkle]](ioc.Default, func() domain.Repository[*domain.Sparkle] { return db.NewRepository[*domain.Sparkle]() }) - ioc.RegisterSingleton[domain.Repository[*domain.User]](ioc.Default, func() domain.Repository[*domain.User] { - return db.NewRepository[*domain.User]() - }) ioc.RegisterSingleton[*http.ServeMux](ioc.Default, func() *http.ServeMux { return http.NewServeMux() }) @@ -47,21 +40,6 @@ func init() { }, } }) - ioc.RegisterSingleton[*oidc.Provider](ioc.Default, func() *oidc.Provider { - ctx := context.WithValue( - context.Background(), - oauth2.HTTPClient, - ioc.MustResolve[*http.Client](ioc.Default), - ) - return web.NewOIDCProvider(ctx, cfg.OIDCIssuer, func(err error) { - ioc.MustResolve[*zerolog.Logger](ioc.Default).Err(err).Send() - }) - }) - ioc.Register[*oidc.Config](ioc.Default, func() *oidc.Config { - return &oidc.Config{ - ClientID: cfg.OAuthClientID, - } - }) ioc.Register[rpc.Ability](ioc.Default, func() rpc.Ability { return rpc.NewAbilityProtobufClient( env.Fetch("AUTHZD_HOST", ""), diff --git a/app/middleware/from_cookie.go b/app/middleware/from_cookie.go deleted file mode 100644 index 316d6e4..0000000 --- a/app/middleware/from_cookie.go +++ /dev/null @@ -1,15 +0,0 @@ -package middleware - -import "net/http" - -func FromCookie(name string) TokenParser { - return func(r *http.Request) RawToken { - cookies := r.CookiesNamed(name) - - if len(cookies) != 1 { - return "" - } - - return RawToken(cookies[0].Value) - } -} diff --git a/app/middleware/from_custom_header.go b/app/middleware/from_custom_header.go deleted file mode 100644 index f385911..0000000 --- a/app/middleware/from_custom_header.go +++ /dev/null @@ -1,9 +0,0 @@ -package middleware - -import "net/http" - -func FromCustomHeader(name string) TokenParser { - return func(r *http.Request) RawToken { - return RawToken(r.Header.Get(name)) - } -} diff --git a/app/middleware/init.go b/app/middleware/init.go index 5bf84f6..23c524d 100644 --- a/app/middleware/init.go +++ b/app/middleware/init.go @@ -13,7 +13,7 @@ func init() { subject := h.Get("x-jwt-claim-sub") if x.IsPresent(subject) { return &domain.User{ - ID: domain.ID(subject), + ID: domain.ID(h.Get("x-jwt-claim-sub")), Username: h.Get("x-jwt-claim-username"), ProfileURL: h.Get("x-jwt-claim-profile-url"), Picture: h.Get("x-jwt-claim-picture-url"), diff --git a/app/middleware/raw_token.go b/app/middleware/raw_token.go deleted file mode 100644 index f7aa264..0000000 --- a/app/middleware/raw_token.go +++ /dev/null @@ -1,7 +0,0 @@ -package middleware - -type RawToken string - -func (r RawToken) String() string { - return string(r) -} diff --git a/app/middleware/token_parser.go b/app/middleware/token_parser.go deleted file mode 100644 index 1a92760..0000000 --- a/app/middleware/token_parser.go +++ /dev/null @@ -1,3 +0,0 @@ -package middleware - -type TokenParser RequestParser[RawToken] diff --git a/app/middleware/user.go b/app/middleware/user.go index 90bf6aa..2b2dd17 100644 --- a/app/middleware/user.go +++ b/app/middleware/user.go @@ -3,20 +3,27 @@ package middleware import ( "net/http" - "github.com/xlgmokha/x/pkg/x" + "github.com/xlgmokha/x/pkg/log" + "github.com/xlgmokha/x/pkg/mapper" "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() 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 := parser(r) - if x.IsPresent(user) { - next.ServeHTTP(w, r.WithContext(cfg.CurrentUser.With(r.Context(), user))) - } else { - next.ServeHTTP(w, r) - } + log.WithFields(r.Context(), log.Fields{ + "payload": r.Header.Get("x-jwt-payload"), + "photo": r.Header.Get("x-jwt-claim-picture-url"), + "profile": r.Header.Get("x-jwt-claim-profile-url"), + "sub": r.Header.Get("x-jwt-claim-sub"), + "username": r.Header.Get("x-jwt-claim-username"), + }) + + next.ServeHTTP(w, r.WithContext(cfg.CurrentUser.With( + r.Context(), + mapper.MapFrom[http.Header, *domain.User](r.Header), + ))) }) } } diff --git a/app/middleware/user_parser.go b/app/middleware/user_parser.go deleted file mode 100644 index dfa0cce..0000000 --- a/app/middleware/user_parser.go +++ /dev/null @@ -1,16 +0,0 @@ -package middleware - -import ( - "net/http" - - "github.com/xlgmokha/x/pkg/log" - "github.com/xlgmokha/x/pkg/mapper" - "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/app/domain" -) - -func UserParser() RequestParser[*domain.User] { - return func(r *http.Request) *domain.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 deleted file mode 100644 index 2127a10..0000000 --- a/app/middleware/user_parser_test.go +++ /dev/null @@ -1,36 +0,0 @@ -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 c5fa7ed..66ca121 100644 --- a/app/middleware/user_test.go +++ b/app/middleware/user_test.go @@ -29,6 +29,8 @@ func TestUser(t *testing.T) { t.Run("when x-jwt-claim-* headers are provided", func(t *testing.T) { server := middleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.True(t, IsLoggedIn(r)) + user := cfg.CurrentUser.From(r.Context()) require.NotNil(t, user) diff --git a/bin/envoy.sh b/bin/envoy.sh index 692167c..219228f 100755 --- a/bin/envoy.sh +++ b/bin/envoy.sh @@ -33,4 +33,4 @@ fi envoy \ --config-yaml "$yaml" \ --log-level warn \ - --component-log-level admin:warn,connection:warn,ext_authz:info,grpc:info,health_checker:warn,http:warn,http2:warn,jwt:warn,oauth2:warn,router:warn,secret:warn,upstream:warn + --component-log-level admin:warn,connection:warn,ext_authz:info,grpc:info,health_checker:warn,http:warn,http2:warn,jwt:trace,oauth2:warn,router:warn,secret:warn,upstream:warn diff --git a/pkg/web/cookie.go b/pkg/web/cookie.go deleted file mode 100644 index 11cc807..0000000 --- a/pkg/web/cookie.go +++ /dev/null @@ -1,35 +0,0 @@ -package web - -import ( - "net/http" - - "github.com/xlgmokha/x/pkg/cookie" - "github.com/xlgmokha/x/pkg/x" -) - -func NewCookie(name, value string, options ...x.Option[*http.Cookie]) *http.Cookie { - return x.New[*http.Cookie](x.Prepend[x.Option[*http.Cookie]]( - options, - cookie.WithName(name), - cookie.WithValue(value), - cookie.WithPath("/"), - cookie.WithHttpOnly(true), - cookie.WithSecure(true), - )...) -} - -func ExpireCookie(w http.ResponseWriter, name string) error { - return WriteCookie(w, cookie.Reset(name, - cookie.WithPath("/"), - cookie.WithHttpOnly(true), - cookie.WithSecure(true), - )) -} - -func WriteCookie(w http.ResponseWriter, c *http.Cookie) error { - if err := c.Valid(); err != nil { - return err - } - cookie.Write(w, c) - return nil -} diff --git a/pkg/web/cookie_test.go b/pkg/web/cookie_test.go deleted file mode 100644 index 1a3bfb0..0000000 --- a/pkg/web/cookie_test.go +++ /dev/null @@ -1,33 +0,0 @@ -package web - -import ( - "net/http" - "net/http/httptest" - "testing" - "time" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestNewCookie(t *testing.T) { - cookie := NewCookie("name", "value") - assert.True(t, cookie.HttpOnly) - assert.True(t, cookie.Secure) -} - -func TestExpireCookie(t *testing.T) { - w := httptest.NewRecorder() - - ExpireCookie(w, "example") - - result, err := http.ParseSetCookie(w.Header().Get("Set-Cookie")) - require.NoError(t, err) - - assert.Empty(t, result.Value) - assert.Equal(t, -1, result.MaxAge) - assert.Equal(t, time.Unix(0, 0).Unix(), result.Expires.Unix()) - assert.True(t, result.HttpOnly) - assert.True(t, result.Secure) - assert.Zero(t, result.SameSite) -} diff --git a/pkg/web/oidc.go b/pkg/web/oidc.go deleted file mode 100644 index 707a1b5..0000000 --- a/pkg/web/oidc.go +++ /dev/null @@ -1,27 +0,0 @@ -package web - -import ( - "context" - - "github.com/coreos/go-oidc/v3/oidc" -) - -func NewOIDCProvider(ctx context.Context, issuer string, report func(error)) *oidc.Provider { - provider, err := oidc.NewProvider(ctx, issuer) - if err == nil { - return provider - } - - report(err) - - config := &oidc.ProviderConfig{ - IssuerURL: issuer, - AuthURL: issuer + "/oauth/authorize", - TokenURL: issuer + "/oauth/token", - DeviceAuthURL: "", - UserInfoURL: issuer + "/oauth/userinfo", - JWKSURL: issuer + "/oauth/disovery/keys", - Algorithms: []string{"RS256"}, - } - return config.NewProvider(ctx) -} -- cgit v1.2.3 From df6f325e350f4407acd797136bd708cba20f197c Mon Sep 17 00:00:00 2001 From: mo khan Date: Wed, 28 May 2025 13:00:39 -0600 Subject: chore: specify the issuer of the jwt provider --- etc/envoy/envoy.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/etc/envoy/envoy.yaml b/etc/envoy/envoy.yaml index 4378647..6cf6c55 100644 --- a/etc/envoy/envoy.yaml +++ b/etc/envoy/envoy.yaml @@ -173,6 +173,7 @@ static_resources: "@type": type.googleapis.com/envoy.extensions.filters.http.jwt_authn.v3.JwtAuthentication providers: provider1: + issuer: https://example.com audiences: - OAUTH_CLIENT_ID claim_to_headers: -- cgit v1.2.3 From 1de6a34a55c2e8b7d50945984acb45e7809f6a37 Mon Sep 17 00:00:00 2001 From: mo khan Date: Wed, 28 May 2025 13:03:58 -0600 Subject: chore: read from id_token and bearer_token cookies --- etc/envoy/envoy.yaml | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/etc/envoy/envoy.yaml b/etc/envoy/envoy.yaml index 6cf6c55..ef676fb 100644 --- a/etc/envoy/envoy.yaml +++ b/etc/envoy/envoy.yaml @@ -177,18 +177,19 @@ static_resources: audiences: - OAUTH_CLIENT_ID claim_to_headers: - - header_name: x-jwt-claim-sub - claim_name: sub - - header_name: x-jwt-claim-username - claim_name: nickname - - header_name: x-jwt-claim-profile-url - claim_name: profile - - header_name: x-jwt-claim-picture-url - claim_name: picture + - claim_name: sub + header_name: x-jwt-claim-sub + - claim_name: nickname + header_name: x-jwt-claim-username + - claim_name: profile + header_name: x-jwt-claim-profile-url + - claim_name: picture + header_name: x-jwt-claim-picture-url forward: true forward_payload_header: x-jwt-payload from_cookies: - id_token + - bearer_token issuer: https://example.com remote_jwks: http_uri: -- cgit v1.2.3 From e9546b40c8befabda26c1598c124a6ee2a8d2b8f Mon Sep 17 00:00:00 2001 From: mo khan Date: Wed, 28 May 2025 14:26:19 -0600 Subject: refactor: always provide a user in the request context --- app/controllers/dashboard/controller.go | 6 ++---- app/controllers/dashboard/controller_test.go | 3 ++- app/controllers/sparkles/controller_test.go | 3 ++- app/domain/entity.go | 6 ++++++ app/domain/user.go | 6 +----- app/middleware/init.go | 15 +++++---------- app/middleware/is_logged_in.go | 3 ++- app/middleware/request_parser.go | 9 --------- app/middleware/require_user_test.go | 3 ++- app/middleware/user.go | 11 ++++++----- app/middleware/user_test.go | 14 +++++++------- bin/envoy.sh | 2 +- etc/envoy/envoy.yaml | 23 +++++++---------------- pkg/authz/check_service.go | 1 + 14 files changed, 44 insertions(+), 61 deletions(-) delete mode 100644 app/middleware/request_parser.go diff --git a/app/controllers/dashboard/controller.go b/app/controllers/dashboard/controller.go index 04a7ed1..d279930 100644 --- a/app/controllers/dashboard/controller.go +++ b/app/controllers/dashboard/controller.go @@ -41,14 +41,12 @@ func (c *Controller) Show(w http.ResponseWriter, r *http.Request) { } func (c *Controller) Navigation(w http.ResponseWriter, r *http.Request) { - currentUser := cfg.CurrentUser.From(r.Context()) - w.WriteHeader(http.StatusOK) w.Header().Add("Content-Type", "text/html") dto := &NavigationDTO{ - CurrentUser: currentUser, - IsLoggedIn: currentUser != nil, + CurrentUser: cfg.CurrentUser.From(r.Context()), + IsLoggedIn: middleware.IsLoggedIn(r), } if err := views.Render(w, "dashboard/nav", dto); err != nil { pls.LogError(r.Context(), err) diff --git a/app/controllers/dashboard/controller_test.go b/app/controllers/dashboard/controller_test.go index c717a74..ddbfd34 100644 --- a/app/controllers/dashboard/controller_test.go +++ b/app/controllers/dashboard/controller_test.go @@ -28,7 +28,7 @@ func TestController(t *testing.T) { }) t.Run("when authenticated", func(t *testing.T) { - ctx := cfg.CurrentUser.With(t.Context(), &domain.User{}) + ctx := cfg.CurrentUser.With(t.Context(), &domain.User{ID: domain.ID("1")}) r, w := test.RequestResponse("GET", "/dashboard", test.WithContext(ctx)) mux.ServeHTTP(w, r) @@ -55,6 +55,7 @@ func TestController(t *testing.T) { t.Run("when authenticated", func(t *testing.T) { ctx := cfg.CurrentUser.With(t.Context(), &domain.User{ + ID: domain.ID("1"), Username: "root", }) r, w := test.RequestResponse("GET", "/dashboard/nav", test.WithContext(ctx)) diff --git a/app/controllers/sparkles/controller_test.go b/app/controllers/sparkles/controller_test.go index 8a1717d..0619b99 100644 --- a/app/controllers/sparkles/controller_test.go +++ b/app/controllers/sparkles/controller_test.go @@ -44,9 +44,10 @@ func TestSparkles(t *testing.T) { t.Run("POST /sparkles", func(t *testing.T) { t.Run("when a user is logged in", func(t *testing.T) { - currentUser := &domain.User{} + currentUser := domain.NewUser(domain.WithID[*domain.User](domain.ID("1"))) t.Run("saves a new sparkle", func(t *testing.T) { + repository := db.NewRepository[*domain.Sparkle]() mux := http.NewServeMux() controller := New(repository) diff --git a/app/domain/entity.go b/app/domain/entity.go index 0377c51..b2c2166 100644 --- a/app/domain/entity.go +++ b/app/domain/entity.go @@ -1,6 +1,12 @@ package domain +import "github.com/xlgmokha/x/pkg/x" + type Entity interface { Identifiable Validate() error } + +func New[T Entity](options ...x.Configure[T]) T { + return x.New[T](x.Map[x.Configure[T], x.Option[T]](options, x.With[T])...) +} diff --git a/app/domain/user.go b/app/domain/user.go index a6adfa8..52cd780 100644 --- a/app/domain/user.go +++ b/app/domain/user.go @@ -10,11 +10,7 @@ type User struct { } func NewUser(options ...x.Configure[*User]) *User { - user := &User{} - for _, option := range options { - option(user) - } - return user + return New[*User](options...) } func (u *User) GetID() ID { diff --git a/app/middleware/init.go b/app/middleware/init.go index 23c524d..4ff10c4 100644 --- a/app/middleware/init.go +++ b/app/middleware/init.go @@ -4,21 +4,16 @@ import ( "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" ) func init() { 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(h.Get("x-jwt-claim-sub")), - Username: h.Get("x-jwt-claim-username"), - ProfileURL: h.Get("x-jwt-claim-profile-url"), - Picture: h.Get("x-jwt-claim-picture-url"), - } + return &domain.User{ + ID: domain.ID(h.Get("x-id-jwt-claim-sub")), + Username: h.Get("x-id-jwt-claim-username"), + ProfileURL: h.Get("x-id-jwt-claim-profile-url"), + Picture: h.Get("x-id-jwt-claim-picture-url"), } - return nil }) } diff --git a/app/middleware/is_logged_in.go b/app/middleware/is_logged_in.go index e2f0445..f70a03b 100644 --- a/app/middleware/is_logged_in.go +++ b/app/middleware/is_logged_in.go @@ -8,5 +8,6 @@ import ( ) var IsLoggedIn x.Predicate[*http.Request] = x.Predicate[*http.Request](func(r *http.Request) bool { - return x.IsPresent(cfg.CurrentUser.From(r.Context())) + user := cfg.CurrentUser.From(r.Context()) + return x.IsPresent(user) && x.IsPresent(user.ID) }) diff --git a/app/middleware/request_parser.go b/app/middleware/request_parser.go deleted file mode 100644 index dfc5d3a..0000000 --- a/app/middleware/request_parser.go +++ /dev/null @@ -1,9 +0,0 @@ -package middleware - -import ( - "net/http" - - "github.com/xlgmokha/x/pkg/x" -) - -type RequestParser[T any] x.Mapper[*http.Request, T] diff --git a/app/middleware/require_user_test.go b/app/middleware/require_user_test.go index 07cbf92..20b5f94 100644 --- a/app/middleware/require_user_test.go +++ b/app/middleware/require_user_test.go @@ -28,7 +28,8 @@ func TestRequireUser(t *testing.T) { t.Run("when a user is logged in", func(t *testing.T) { t.Run("forwards the request", func(t *testing.T) { - r, w := test.RequestResponse("GET", "/example", test.WithContextKeyValue(t.Context(), cfg.CurrentUser, &domain.User{})) + user := &domain.User{ID: domain.ID("1")} + r, w := test.RequestResponse("GET", "/example", test.WithContextKeyValue(t.Context(), cfg.CurrentUser, user)) server := middleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusTeapot) diff --git a/app/middleware/user.go b/app/middleware/user.go index 2b2dd17..317671e 100644 --- a/app/middleware/user.go +++ b/app/middleware/user.go @@ -13,11 +13,12 @@ func User() func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { log.WithFields(r.Context(), log.Fields{ - "payload": r.Header.Get("x-jwt-payload"), - "photo": r.Header.Get("x-jwt-claim-picture-url"), - "profile": r.Header.Get("x-jwt-claim-profile-url"), - "sub": r.Header.Get("x-jwt-claim-sub"), - "username": r.Header.Get("x-jwt-claim-username"), + "authorization": r.Header.Get("Authorization"), + "payload": r.Header.Get("x-id-jwt-payload"), + "photo": r.Header.Get("x-id-jwt-claim-picture-url"), + "profile": r.Header.Get("x-id-jwt-claim-profile-url"), + "sub": r.Header.Get("x-id-jwt-claim-sub"), + "username": r.Header.Get("x-id-jwt-claim-username"), }) next.ServeHTTP(w, r.WithContext(cfg.CurrentUser.With( diff --git a/app/middleware/user_test.go b/app/middleware/user_test.go index 66ca121..c778c98 100644 --- a/app/middleware/user_test.go +++ b/app/middleware/user_test.go @@ -14,9 +14,9 @@ import ( func TestUser(t *testing.T) { middleware := User() - t.Run("when x-jwt-claim-* headers are not provided", func(t *testing.T) { + t.Run("when x-id-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())) + require.False(t, IsLoggedIn(r)) w.WriteHeader(http.StatusTeapot) })) @@ -27,7 +27,7 @@ func TestUser(t *testing.T) { assert.Equal(t, http.StatusTeapot, w.Code) }) - t.Run("when x-jwt-claim-* headers are provided", func(t *testing.T) { + t.Run("when x-id-jwt-claim-* headers are provided", func(t *testing.T) { server := middleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { require.True(t, IsLoggedIn(r)) @@ -43,10 +43,10 @@ func TestUser(t *testing.T) { })) 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"), + test.WithRequestHeader("x-id-jwt-claim-sub", "1"), + test.WithRequestHeader("x-id-jwt-claim-username", "root"), + test.WithRequestHeader("x-id-jwt-claim-profile-url", "https://gitlab.com/tanuki"), + test.WithRequestHeader("x-id-jwt-claim-picture-url", "https://example.com/profile.png"), ) server.ServeHTTP(w, r) diff --git a/bin/envoy.sh b/bin/envoy.sh index 219228f..1af16aa 100755 --- a/bin/envoy.sh +++ b/bin/envoy.sh @@ -33,4 +33,4 @@ fi envoy \ --config-yaml "$yaml" \ --log-level warn \ - --component-log-level admin:warn,connection:warn,ext_authz:info,grpc:info,health_checker:warn,http:warn,http2:warn,jwt:trace,oauth2:warn,router:warn,secret:warn,upstream:warn + --component-log-level admin:warn,connection:warn,ext_authz:info,grpc:info,health_checker:warn,http:warn,http2:warn,jwt:trace,oauth2:trace,router:warn,secret:warn,upstream:warn diff --git a/etc/envoy/envoy.yaml b/etc/envoy/envoy.yaml index ef676fb..a7d20be 100644 --- a/etc/envoy/envoy.yaml +++ b/etc/envoy/envoy.yaml @@ -172,24 +172,23 @@ static_resources: typed_config: "@type": type.googleapis.com/envoy.extensions.filters.http.jwt_authn.v3.JwtAuthentication providers: - provider1: + id_token_provider: issuer: https://example.com audiences: - OAUTH_CLIENT_ID claim_to_headers: - claim_name: sub - header_name: x-jwt-claim-sub + header_name: x-id-jwt-claim-sub - claim_name: nickname - header_name: x-jwt-claim-username + header_name: x-id-jwt-claim-username - claim_name: profile - header_name: x-jwt-claim-profile-url + header_name: x-id-jwt-claim-profile-url - claim_name: picture - header_name: x-jwt-claim-picture-url + header_name: x-id-jwt-claim-picture-url forward: true - forward_payload_header: x-jwt-payload + forward_payload_header: x-id-jwt-payload from_cookies: - id_token - - bearer_token issuer: https://example.com remote_jwks: http_uri: @@ -197,21 +196,13 @@ static_resources: cluster: oidc timeout: 5s rules: - - match: - path: /health - - match: - prefix: /sparkles - - match: - prefix: /dashboard/nav - match: safe_regex: regex: .*\\.(css|js|png|html|ico)$ - match: path: / - - match: - path: /dashboard requires: - provider_name: provider1 + provider_name: id_token_provider - name: envoy.filters.http.ext_authz typed_config: "@type": type.googleapis.com/envoy.extensions.filters.http.ext_authz.v3.ExtAuthz diff --git a/pkg/authz/check_service.go b/pkg/authz/check_service.go index 641ba92..9db32d0 100644 --- a/pkg/authz/check_service.go +++ b/pkg/authz/check_service.go @@ -40,6 +40,7 @@ func NewCheckService() *CheckService { } func (svc *CheckService) Check(ctx context.Context, request *auth.CheckRequest) (*auth.CheckResponse, error) { + log.WithFields(ctx, log.Fields{"headers": request.Attributes.Request.Http.Headers}) if svc.isAllowed(ctx, request) { return svc.OK(ctx), nil } -- cgit v1.2.3 From 1ba0b855feb4af5ce723066e5a48d2aed09fc03a Mon Sep 17 00:00:00 2001 From: mo khan Date: Wed, 28 May 2025 14:49:18 -0600 Subject: chore: inject jwt headers for all requests --- etc/envoy/envoy.yaml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/etc/envoy/envoy.yaml b/etc/envoy/envoy.yaml index a7d20be..a8cdc59 100644 --- a/etc/envoy/envoy.yaml +++ b/etc/envoy/envoy.yaml @@ -200,9 +200,12 @@ static_resources: safe_regex: regex: .*\\.(css|js|png|html|ico)$ - match: - path: / + prefix: / requires: - provider_name: id_token_provider + requires_any: + requirements: + - provider_name: id_token_provider + - allow_missing: {} - name: envoy.filters.http.ext_authz typed_config: "@type": type.googleapis.com/envoy.extensions.filters.http.ext_authz.v3.ExtAuthz -- cgit v1.2.3 From c7fe2ec85cf77ede757675f9f068c85d304d9f61 Mon Sep 17 00:00:00 2001 From: mo khan Date: Wed, 28 May 2025 14:54:14 -0600 Subject: chore: remove logging of sensitive fields --- app/middleware/user.go | 11 +++++------ pkg/authz/check_service.go | 1 - 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/app/middleware/user.go b/app/middleware/user.go index 317671e..0ffc8cf 100644 --- a/app/middleware/user.go +++ b/app/middleware/user.go @@ -13,12 +13,11 @@ func User() func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { log.WithFields(r.Context(), log.Fields{ - "authorization": r.Header.Get("Authorization"), - "payload": r.Header.Get("x-id-jwt-payload"), - "photo": r.Header.Get("x-id-jwt-claim-picture-url"), - "profile": r.Header.Get("x-id-jwt-claim-profile-url"), - "sub": r.Header.Get("x-id-jwt-claim-sub"), - "username": r.Header.Get("x-id-jwt-claim-username"), + "payload": r.Header.Get("x-id-jwt-payload"), + "photo": r.Header.Get("x-id-jwt-claim-picture-url"), + "profile": r.Header.Get("x-id-jwt-claim-profile-url"), + "sub": r.Header.Get("x-id-jwt-claim-sub"), + "username": r.Header.Get("x-id-jwt-claim-username"), }) next.ServeHTTP(w, r.WithContext(cfg.CurrentUser.With( diff --git a/pkg/authz/check_service.go b/pkg/authz/check_service.go index 9db32d0..641ba92 100644 --- a/pkg/authz/check_service.go +++ b/pkg/authz/check_service.go @@ -40,7 +40,6 @@ func NewCheckService() *CheckService { } func (svc *CheckService) Check(ctx context.Context, request *auth.CheckRequest) (*auth.CheckResponse, error) { - log.WithFields(ctx, log.Fields{"headers": request.Attributes.Request.Http.Headers}) if svc.isAllowed(ctx, request) { return svc.OK(ctx), nil } -- cgit v1.2.3 From c8b86d928d2c109b0d762e5dd8ad4f5a96386410 Mon Sep 17 00:00:00 2001 From: mo khan Date: Wed, 28 May 2025 16:13:23 -0600 Subject: chore: reset log-level to warn --- bin/envoy.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/envoy.sh b/bin/envoy.sh index 1af16aa..692167c 100755 --- a/bin/envoy.sh +++ b/bin/envoy.sh @@ -33,4 +33,4 @@ fi envoy \ --config-yaml "$yaml" \ --log-level warn \ - --component-log-level admin:warn,connection:warn,ext_authz:info,grpc:info,health_checker:warn,http:warn,http2:warn,jwt:trace,oauth2:trace,router:warn,secret:warn,upstream:warn + --component-log-level admin:warn,connection:warn,ext_authz:info,grpc:info,health_checker:warn,http:warn,http2:warn,jwt:warn,oauth2:warn,router:warn,secret:warn,upstream:warn -- cgit v1.2.3 From 60fbfa7411109d0d26f1c8e619205311bb24f62d Mon Sep 17 00:00:00 2001 From: mo khan Date: Wed, 28 May 2025 16:23:36 -0600 Subject: chore: rename headers from x-id-jwt to x-jwt --- app/middleware/init.go | 8 ++++---- app/middleware/user.go | 8 +------- app/middleware/user_test.go | 12 ++++++------ etc/envoy/envoy.yaml | 10 +++++----- 4 files changed, 16 insertions(+), 22 deletions(-) diff --git a/app/middleware/init.go b/app/middleware/init.go index 4ff10c4..770bd19 100644 --- a/app/middleware/init.go +++ b/app/middleware/init.go @@ -10,10 +10,10 @@ import ( func init() { mapper.Register(func(h http.Header) *domain.User { return &domain.User{ - ID: domain.ID(h.Get("x-id-jwt-claim-sub")), - Username: h.Get("x-id-jwt-claim-username"), - ProfileURL: h.Get("x-id-jwt-claim-profile-url"), - Picture: h.Get("x-id-jwt-claim-picture-url"), + ID: domain.ID(h.Get("x-jwt-claim-sub")), + Username: h.Get("x-jwt-claim-username"), + ProfileURL: h.Get("x-jwt-claim-profile-url"), + Picture: h.Get("x-jwt-claim-picture-url"), } }) } diff --git a/app/middleware/user.go b/app/middleware/user.go index 0ffc8cf..184bf1a 100644 --- a/app/middleware/user.go +++ b/app/middleware/user.go @@ -12,13 +12,7 @@ import ( func User() func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - log.WithFields(r.Context(), log.Fields{ - "payload": r.Header.Get("x-id-jwt-payload"), - "photo": r.Header.Get("x-id-jwt-claim-picture-url"), - "profile": r.Header.Get("x-id-jwt-claim-profile-url"), - "sub": r.Header.Get("x-id-jwt-claim-sub"), - "username": r.Header.Get("x-id-jwt-claim-username"), - }) + log.WithFields(r.Context(), log.Fields{"sub": r.Header.Get("x-jwt-claim-sub")}) next.ServeHTTP(w, r.WithContext(cfg.CurrentUser.With( r.Context(), diff --git a/app/middleware/user_test.go b/app/middleware/user_test.go index c778c98..371605c 100644 --- a/app/middleware/user_test.go +++ b/app/middleware/user_test.go @@ -14,7 +14,7 @@ import ( func TestUser(t *testing.T) { middleware := User() - t.Run("when x-id-jwt-claim-* headers are not provided", func(t *testing.T) { + 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.False(t, IsLoggedIn(r)) @@ -27,7 +27,7 @@ func TestUser(t *testing.T) { assert.Equal(t, http.StatusTeapot, w.Code) }) - t.Run("when x-id-jwt-claim-* headers are provided", func(t *testing.T) { + t.Run("when x-jwt-claim-* headers are provided", func(t *testing.T) { server := middleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { require.True(t, IsLoggedIn(r)) @@ -43,10 +43,10 @@ func TestUser(t *testing.T) { })) r, w := test.RequestResponse("GET", "/", - test.WithRequestHeader("x-id-jwt-claim-sub", "1"), - test.WithRequestHeader("x-id-jwt-claim-username", "root"), - test.WithRequestHeader("x-id-jwt-claim-profile-url", "https://gitlab.com/tanuki"), - test.WithRequestHeader("x-id-jwt-claim-picture-url", "https://example.com/profile.png"), + 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) diff --git a/etc/envoy/envoy.yaml b/etc/envoy/envoy.yaml index a8cdc59..b483fe9 100644 --- a/etc/envoy/envoy.yaml +++ b/etc/envoy/envoy.yaml @@ -178,15 +178,15 @@ static_resources: - OAUTH_CLIENT_ID claim_to_headers: - claim_name: sub - header_name: x-id-jwt-claim-sub + header_name: x-jwt-claim-sub - claim_name: nickname - header_name: x-id-jwt-claim-username + header_name: x-jwt-claim-username - claim_name: profile - header_name: x-id-jwt-claim-profile-url + header_name: x-jwt-claim-profile-url - claim_name: picture - header_name: x-id-jwt-claim-picture-url + header_name: x-jwt-claim-picture-url forward: true - forward_payload_header: x-id-jwt-payload + forward_payload_header: x-jwt-payload from_cookies: - id_token issuer: https://example.com -- cgit v1.2.3 From 7edfed201bfbfb477f8cf3a936878fce8a55b25c Mon Sep 17 00:00:00 2001 From: mo khan Date: Wed, 28 May 2025 16:48:57 -0600 Subject: chore: do not forward sensitive headers to Sparkle --- app/middleware/user.go | 3 --- etc/envoy/envoy.yaml | 6 +++++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/app/middleware/user.go b/app/middleware/user.go index 184bf1a..2865477 100644 --- a/app/middleware/user.go +++ b/app/middleware/user.go @@ -3,7 +3,6 @@ package middleware import ( "net/http" - "github.com/xlgmokha/x/pkg/log" "github.com/xlgmokha/x/pkg/mapper" "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/app/cfg" "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/app/domain" @@ -12,8 +11,6 @@ import ( func User() func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - log.WithFields(r.Context(), log.Fields{"sub": r.Header.Get("x-jwt-claim-sub")}) - next.ServeHTTP(w, r.WithContext(cfg.CurrentUser.With( r.Context(), mapper.MapFrom[http.Header, *domain.User](r.Header), diff --git a/etc/envoy/envoy.yaml b/etc/envoy/envoy.yaml index b483fe9..eb4901a 100644 --- a/etc/envoy/envoy.yaml +++ b/etc/envoy/envoy.yaml @@ -185,7 +185,7 @@ static_resources: header_name: x-jwt-claim-profile-url - claim_name: picture header_name: x-jwt-claim-picture-url - forward: true + forward: false forward_payload_header: x-jwt-payload from_cookies: - id_token @@ -219,6 +219,10 @@ static_resources: "@type": type.googleapis.com/envoy.extensions.filters.http.router.v3.Router suppress_envoy_headers: true route_config: + request_headers_to_remove: + - authorization + - cookie + - user-agent virtual_hosts: - name: local domains: ["*"] -- cgit v1.2.3 From 2f3c9a1012b1b8334cb9bcb13dff2a08e9c52660 Mon Sep 17 00:00:00 2001 From: mo khan Date: Wed, 28 May 2025 16:52:35 -0600 Subject: docs: update envoy documentation --- etc/envoy/envoy.yaml | 1 - share/man/ENVOY.md | 12 +++++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/etc/envoy/envoy.yaml b/etc/envoy/envoy.yaml index eb4901a..a6977d1 100644 --- a/etc/envoy/envoy.yaml +++ b/etc/envoy/envoy.yaml @@ -173,7 +173,6 @@ static_resources: "@type": type.googleapis.com/envoy.extensions.filters.http.jwt_authn.v3.JwtAuthentication providers: id_token_provider: - issuer: https://example.com audiences: - OAUTH_CLIENT_ID claim_to_headers: diff --git a/share/man/ENVOY.md b/share/man/ENVOY.md index 7ad8b64..4b5d765 100644 --- a/share/man/ENVOY.md +++ b/share/man/ENVOY.md @@ -775,9 +775,8 @@ and will immediately reject tokens that are invalid. audiences: - OAUTH_CLIENT_ID claim_to_headers: - - header_name: x-jwt-claim-sub - claim_name: sub - forward: true + - claim_name: sub + header_name: x-jwt-claim-sub forward_payload_header: x-jwt-payload from_cookies: - id_token @@ -787,9 +786,12 @@ and will immediately reject tokens that are invalid. uri: https://gitlab.com/oauth/discovery/keys rules: - match: - path: / + prefix: / requires: - provider_name: gitlab_provider + requires_any: + requirements: + - provider_name: gitlab_provider + - allow_missing: {} - name: envoy.filters.http.router # ... ``` -- cgit v1.2.3 From e5bc1eb2b71d46958088f1c62e69e1074e9f8026 Mon Sep 17 00:00:00 2001 From: mo khan Date: Wed, 28 May 2025 17:01:00 -0600 Subject: test: remove logging from test --- pkg/authz/id_token_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/authz/id_token_test.go b/pkg/authz/id_token_test.go index 054c48b..22aabc4 100644 --- a/pkg/authz/id_token_test.go +++ b/pkg/authz/id_token_test.go @@ -15,7 +15,6 @@ func TestIDToken(t *testing.T) { t.Run("when the token is valid", func(t *testing.T) { user := mockoidc.DefaultUser() _, rawIDToken := idp.CreateTokensFor(user) - t.Logf("id_token: %v\n", rawIDToken) token, err := NewIDToken(rawIDToken) require.NoError(t, err) -- cgit v1.2.3