From 21a84d9809f3f57873f8e09edd485817308b0c40 Mon Sep 17 00:00:00 2001 From: Maxim Lebedev Date: Tue, 7 May 2024 00:42:52 +0500 Subject: [PATCH] :truck: Moved variuos utils into util package --- internal/auth/delivery/http/auth_http.go | 8 +- .../auth/delivery/http/auth_http_schema.go | 2 - internal/client/delivery/http/client_http.go | 4 +- .../client/repository/http/http_client.go | 18 +++-- internal/domain/challenge/method_test.go | 2 - .../metadata/repository/http/http_metadata.go | 20 ++++- .../sqlite3/sqlite3_session_test.go | 2 +- internal/token/delivery/http/token_http.go | 6 +- .../repository/sqlite3/sqlite3_token_test.go | 2 +- internal/urlutil/shift_path.go | 22 ------ internal/user/delivery/http/user_http.go | 4 +- internal/util/http/parse_link.go | 57 ++++++++++++++ internal/util/http/parse_link_test.go | 47 ++++++++++++ internal/util/path/shift.go | 22 ++++++ .../path/shift_test.go} | 9 +-- .../{ => util}/testing/bolttest/bolttest.go | 0 internal/util/testing/golden_equal.go | 76 +++++++++++++++++++ internal/util/testing/golden_equal_test.go | 36 +++++++++ .../{ => util}/testing/sqltest/sqltest.go | 0 .../testing/testdata/TestGoldenEqual.golden | 12 +++ main.go | 6 +- 21 files changed, 297 insertions(+), 58 deletions(-) delete mode 100644 internal/urlutil/shift_path.go create mode 100644 internal/util/http/parse_link.go create mode 100644 internal/util/http/parse_link_test.go create mode 100644 internal/util/path/shift.go rename internal/{urlutil/shift_path_test.go => util/path/shift_test.go} (75%) rename internal/{ => util}/testing/bolttest/bolttest.go (100%) create mode 100644 internal/util/testing/golden_equal.go create mode 100644 internal/util/testing/golden_equal_test.go rename internal/{ => util}/testing/sqltest/sqltest.go (100%) create mode 100755 internal/util/testing/testdata/TestGoldenEqual.golden diff --git a/internal/auth/delivery/http/auth_http.go b/internal/auth/delivery/http/auth_http.go index 4a2af7b..ff0bda9 100644 --- a/internal/auth/delivery/http/auth_http.go +++ b/internal/auth/delivery/http/auth_http.go @@ -16,7 +16,7 @@ import ( "source.toby3d.me/toby3d/auth/internal/domain" "source.toby3d.me/toby3d/auth/internal/middleware" "source.toby3d.me/toby3d/auth/internal/profile" - "source.toby3d.me/toby3d/auth/internal/urlutil" + pathutil "source.toby3d.me/toby3d/auth/internal/util/path" "source.toby3d.me/toby3d/auth/web/template" "source.toby3d.me/toby3d/auth/web/template/layout" ) @@ -51,7 +51,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { chain := middleware.Chain{ middleware.CSRFWithConfig(middleware.CSRFConfig{ Skipper: func(_ http.ResponseWriter, r *http.Request) bool { - head, _ := urlutil.ShiftPath(r.URL.Path) + head, _ := pathutil.Shift(r.URL.Path) return head == "" }, @@ -68,7 +68,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { }), middleware.BasicAuthWithConfig(middleware.BasicAuthConfig{ Skipper: func(_ http.ResponseWriter, r *http.Request) bool { - head, _ := urlutil.ShiftPath(r.URL.Path) + head, _ := pathutil.Shift(r.URL.Path) return r.Method != http.MethodPost || head != "verify" || r.PostFormValue("authorize") == "deny" @@ -85,7 +85,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { }), } - head, _ := urlutil.ShiftPath(r.URL.Path) + head, _ := pathutil.Shift(r.URL.Path) switch r.Method { default: diff --git a/internal/auth/delivery/http/auth_http_schema.go b/internal/auth/delivery/http/auth_http_schema.go index aec3bd9..1f4af75 100644 --- a/internal/auth/delivery/http/auth_http_schema.go +++ b/internal/auth/delivery/http/auth_http_schema.go @@ -107,7 +107,6 @@ func NewAuthAuthorizationRequest() *AuthAuthorizationRequest { } } -//nolint:cyclop func (r *AuthAuthorizationRequest) bind(req *http.Request) error { indieAuthError := new(domain.Error) @@ -142,7 +141,6 @@ func NewAuthVerifyRequest() *AuthVerifyRequest { } } -//nolint:cyclop func (r *AuthVerifyRequest) bind(req *http.Request) error { indieAuthError := new(domain.Error) diff --git a/internal/client/delivery/http/client_http.go b/internal/client/delivery/http/client_http.go index 78404c5..58bb0bf 100644 --- a/internal/client/delivery/http/client_http.go +++ b/internal/client/delivery/http/client_http.go @@ -10,7 +10,7 @@ import ( "source.toby3d.me/toby3d/auth/internal/common" "source.toby3d.me/toby3d/auth/internal/domain" "source.toby3d.me/toby3d/auth/internal/token" - "source.toby3d.me/toby3d/auth/internal/urlutil" + pathutil "source.toby3d.me/toby3d/auth/internal/util/path" "source.toby3d.me/toby3d/auth/web/template" "source.toby3d.me/toby3d/auth/web/template/layout" ) @@ -48,7 +48,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } var head string - head, r.URL.Path = urlutil.ShiftPath(r.URL.Path) + head, r.URL.Path = pathutil.Shift(r.URL.Path) switch head { default: diff --git a/internal/client/repository/http/http_client.go b/internal/client/repository/http/http_client.go index 3803c32..5f60028 100644 --- a/internal/client/repository/http/http_client.go +++ b/internal/client/repository/http/http_client.go @@ -8,8 +8,8 @@ import ( "net/http" "net/url" "slices" + "strings" - "github.com/tomnomnom/linkheader" "willnorris.com/go/microformats" "source.toby3d.me/toby3d/auth/internal/client" @@ -19,6 +19,7 @@ import ( "source.toby3d.me/toby3d/auth/internal/domain/grant" "source.toby3d.me/toby3d/auth/internal/domain/response" "source.toby3d.me/toby3d/auth/internal/domain/scope" + httputil "source.toby3d.me/toby3d/auth/internal/util/http" ) type ( @@ -106,15 +107,18 @@ func (repo httpClientRepository) Get(ctx context.Context, cid domain.ClientID) ( } // NOTE(toby3d): fetch redirect uri's from Link header - for _, link := range linkheader.Parse(resp.Header.Get(common.HeaderLink)) { - if link.Rel != common.RelRedirectURI { + links, err := httputil.ParseLink(resp.Header.Get(common.HeaderLink)) + if err != nil { + return out, fmt.Errorf("cannot parse Link header value '%s': %w", resp.Header.Get(common.HeaderLink), + err) + } + + for _, link := range links { + if !slices.Contains(strings.Fields(link.Params.Get("rel")), common.RelRedirectURI) { continue } - var u *url.URL - if u, err = url.Parse(link.URL); err == nil { - out.RedirectURI = append(out.RedirectURI, u) - } + out.RedirectURI = append(out.RedirectURI, link.URL) } return out, nil diff --git a/internal/domain/challenge/method_test.go b/internal/domain/challenge/method_test.go index bbebb85..1a21ebb 100644 --- a/internal/domain/challenge/method_test.go +++ b/internal/domain/challenge/method_test.go @@ -1,7 +1,5 @@ package challenge_test -//nolint:gosec // support old clients - import ( "crypto/md5" "crypto/sha1" diff --git a/internal/metadata/repository/http/http_metadata.go b/internal/metadata/repository/http/http_metadata.go index 3ca3b01..118c83b 100644 --- a/internal/metadata/repository/http/http_metadata.go +++ b/internal/metadata/repository/http/http_metadata.go @@ -7,7 +7,6 @@ import ( "net/url" "github.com/goccy/go-json" - "github.com/tomnomnom/linkheader" "willnorris.com/go/microformats" "source.toby3d.me/toby3d/auth/internal/common" @@ -17,6 +16,7 @@ import ( "source.toby3d.me/toby3d/auth/internal/domain/response" "source.toby3d.me/toby3d/auth/internal/domain/scope" "source.toby3d.me/toby3d/auth/internal/metadata" + httputil "source.toby3d.me/toby3d/auth/internal/util/http" ) type ( @@ -64,8 +64,22 @@ func (repo *httpMetadataRepository) Get(_ context.Context, u *url.URL) (*domain. } relVals := make(map[string][]string) - for _, link := range linkheader.Parse(resp.Header.Get(common.HeaderLink)) { - populateBuffer(relVals, link.Rel, link.URL) + + links, err := httputil.ParseLink(resp.Header.Get(common.HeaderLink)) + if err != nil { + return nil, fmt.Errorf("cannot parse Links header value '%s': %w", resp.Header.Get(common.HeaderLink), + err) + } + + for _, link := range links { + rels, ok := link.Params["rel"] + if !ok { + continue + } + + for _, rel := range rels { + populateBuffer(relVals, rel, link.URL.String()) + } } if mf2 := microformats.Parse(resp.Body, resp.Request.URL); mf2 != nil { diff --git a/internal/session/repository/sqlite3/sqlite3_session_test.go b/internal/session/repository/sqlite3/sqlite3_session_test.go index 7e2201c..36e816c 100644 --- a/internal/session/repository/sqlite3/sqlite3_session_test.go +++ b/internal/session/repository/sqlite3/sqlite3_session_test.go @@ -9,7 +9,7 @@ import ( "source.toby3d.me/toby3d/auth/internal/domain" repository "source.toby3d.me/toby3d/auth/internal/session/repository/sqlite3" - "source.toby3d.me/toby3d/auth/internal/testing/sqltest" + "source.toby3d.me/toby3d/auth/internal/util/testing/sqltest" ) //nolint:gochecknoglobals // slices cannot be contants diff --git a/internal/token/delivery/http/token_http.go b/internal/token/delivery/http/token_http.go index 1074946..34e0649 100644 --- a/internal/token/delivery/http/token_http.go +++ b/internal/token/delivery/http/token_http.go @@ -11,7 +11,7 @@ import ( "source.toby3d.me/toby3d/auth/internal/domain/action" "source.toby3d.me/toby3d/auth/internal/middleware" "source.toby3d.me/toby3d/auth/internal/token" - "source.toby3d.me/toby3d/auth/internal/urlutil" + pathutil "source.toby3d.me/toby3d/auth/internal/util/path" ) type Handler struct { @@ -31,7 +31,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { //nolint:exhaustivestruct middleware.JWTWithConfig(middleware.JWTConfig{ Skipper: func(_ http.ResponseWriter, r *http.Request) bool { - head, _ := urlutil.ShiftPath(r.URL.Path) + head, _ := pathutil.Shift(r.URL.Path) return head == "token" }, @@ -50,7 +50,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } var head string - head, r.URL.Path = urlutil.ShiftPath(r.URL.Path) + head, r.URL.Path = pathutil.Shift(r.URL.Path) switch head { default: diff --git a/internal/token/repository/sqlite3/sqlite3_token_test.go b/internal/token/repository/sqlite3/sqlite3_token_test.go index 1f181cc..043cda9 100644 --- a/internal/token/repository/sqlite3/sqlite3_token_test.go +++ b/internal/token/repository/sqlite3/sqlite3_token_test.go @@ -8,8 +8,8 @@ import ( "github.com/DATA-DOG/go-sqlmock" "source.toby3d.me/toby3d/auth/internal/domain" - "source.toby3d.me/toby3d/auth/internal/testing/sqltest" repository "source.toby3d.me/toby3d/auth/internal/token/repository/sqlite3" + "source.toby3d.me/toby3d/auth/internal/util/testing/sqltest" ) //nolint:gochecknoglobals // slices cannot be contants diff --git a/internal/urlutil/shift_path.go b/internal/urlutil/shift_path.go deleted file mode 100644 index 39dbf71..0000000 --- a/internal/urlutil/shift_path.go +++ /dev/null @@ -1,22 +0,0 @@ -package urlutil - -import ( - "path" - "strings" -) - -// ShiftPath splits off the first component of p, which will be cleaned of -// relative components before processing. head will never contain a slash and -// tail will always be a rooted path without trailing slash. -// -// See: https://blog.merovius.de/posts/2017-06-18-how-not-to-use-an-http-router/ -func ShiftPath(p string) (head, tail string) { - p = path.Clean("/" + p) - - i := strings.Index(p[1:], "/") + 1 - if i <= 0 { - return p[1:], "/" - } - - return p[1:i], p[i:] -} diff --git a/internal/user/delivery/http/user_http.go b/internal/user/delivery/http/user_http.go index 116cbe3..8a1d6e1 100644 --- a/internal/user/delivery/http/user_http.go +++ b/internal/user/delivery/http/user_http.go @@ -58,7 +58,7 @@ func (h *Handler) handleFunc(w http.ResponseWriter, r *http.Request) { if err != nil || tkn == nil { // WARN(toby3d): If the token is not valid, the endpoint still // MUST return a 200 Response. - _ = encoder.Encode(err) //nolint:errchkjson + _ = encoder.Encode(err) w.WriteHeader(http.StatusOK) @@ -66,7 +66,6 @@ func (h *Handler) handleFunc(w http.ResponseWriter, r *http.Request) { } if !tkn.Scope.Has(scope.Profile) { - //nolint:errchkjson _ = encoder.Encode(domain.NewError( domain.ErrorCodeInsufficientScope, "token with 'profile' scope is required to view profile data", @@ -78,7 +77,6 @@ func (h *Handler) handleFunc(w http.ResponseWriter, r *http.Request) { return } - //nolint:errchkjson _ = encoder.Encode(NewUserInformationResponse(userInfo, tkn.Scope.Has(scope.Email))) w.WriteHeader(http.StatusOK) diff --git a/internal/util/http/parse_link.go b/internal/util/http/parse_link.go new file mode 100644 index 0000000..2479caf --- /dev/null +++ b/internal/util/http/parse_link.go @@ -0,0 +1,57 @@ +package http + +import ( + "fmt" + "net/url" + "strconv" + "strings" +) + +type Link struct { + URL *url.URL + Params url.Values +} + +// ParseLink parse Link HTTP header value into URL and it's params collection. +func ParseLink(raw string) ([]Link, error) { + links := strings.Split(raw, ",") + result := make([]Link, len(links)) + + for i := range links { + parts := strings.Split(links[i], ";") + start := strings.Index(parts[0], "<") + end := strings.Index(parts[0], ">") + + if start == -1 || end == -1 { + continue + } + + var err error + if result[i].URL, err = url.Parse(parts[0][start+1 : end]); err != nil { + return nil, fmt.Errorf("cannot parse Link header URL: %w", err) + } + + result[i].Params = make(url.Values) + + for _, param := range parts[1:] { + paramParts := strings.SplitN(strings.TrimSpace(param), "=", 2) + if unquotted, err := strconv.Unquote(paramParts[1]); err == nil { + result[i].Params.Add(paramParts[0], unquotted) + } else { + result[i].Params.Add(paramParts[0], paramParts[1]) + } + } + } + + return result, nil +} + +func (l Link) String() string { + result := "<" + l.URL.String() + ">" + + if len(l.Params) != 0 { + result += "; " + strings.ReplaceAll(l.Params.Encode(), "&", "; ") + } + + return result +} diff --git a/internal/util/http/parse_link_test.go b/internal/util/http/parse_link_test.go new file mode 100644 index 0000000..01224e1 --- /dev/null +++ b/internal/util/http/parse_link_test.go @@ -0,0 +1,47 @@ +package http_test + +import ( + "net/url" + "testing" + + "github.com/google/go-cmp/cmp" + + httputil "source.toby3d.me/toby3d/auth/internal/util/http" +) + +func TestParseLink(t *testing.T) { + t.Parallel() + + for name, tc := range map[string]struct { + input string + expect []httputil.Link + }{ + "param": { + input: `; rel="preconnect"`, + expect: []httputil.Link{{ + URL: &url.URL{Scheme: "https", Host: "example.com"}, + Params: url.Values{"rel": []string{"preconnect"}}, + }}, + }, + "params": { + input: `; rel="preconnect"; priority=high`, + expect: []httputil.Link{{ + URL: &url.URL{Scheme: "https", Host: "example.com", Path: "/苗条"}, + Params: url.Values{"rel": []string{"preconnect"}, "priority": []string{"high"}}, + }}, + }, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + + actual, err := httputil.ParseLink(tc.input) + if err != nil { + t.Fatal(err) + } + + if diff := cmp.Diff(actual, tc.expect, cmp.AllowUnexported(url.URL{})); diff != "" { + t.Error(diff) + } + }) + } +} diff --git a/internal/util/path/shift.go b/internal/util/path/shift.go new file mode 100644 index 0000000..1bf2022 --- /dev/null +++ b/internal/util/path/shift.go @@ -0,0 +1,22 @@ +package path + +import ( + "path" + "strings" +) + +// Shift splits off the first component of p, which will be cleaned of relative +// components before processing. head will never contain a slash and tail will +// always be a rooted path without trailing slash. +// +// See: https://blog.merovius.de/posts/2017-06-18-how-not-to-use-an-http-router/ +func Shift(p string) (head, tail string) { + p = path.Clean("/" + p) + + i := strings.Index(p[1:], "/") + 1 + if i <= 0 { + return p[1:], "/" + } + + return p[1:i], p[i:] +} diff --git a/internal/urlutil/shift_path_test.go b/internal/util/path/shift_test.go similarity index 75% rename from internal/urlutil/shift_path_test.go rename to internal/util/path/shift_test.go index 4b20f36..c40cc34 100644 --- a/internal/urlutil/shift_path_test.go +++ b/internal/util/path/shift_test.go @@ -1,12 +1,12 @@ -package urlutil_test +package path_test import ( "testing" - "source.toby3d.me/toby3d/auth/internal/urlutil" + pathutil "source.toby3d.me/toby3d/auth/internal/util/path" ) -func TestShiftPath(t *testing.T) { +func TestShift(t *testing.T) { t.Parallel() for in, out := range map[string][2]string{ @@ -21,8 +21,7 @@ func TestShiftPath(t *testing.T) { t.Run(in, func(t *testing.T) { t.Parallel() - head, path := urlutil.ShiftPath(in) - + head, path := pathutil.Shift(in) if out[0] != head || out[1] != path { t.Errorf("ShiftPath(%s) = '%s', '%s', want '%s', '%s'", in, head, path, out[0], out[1]) } diff --git a/internal/testing/bolttest/bolttest.go b/internal/util/testing/bolttest/bolttest.go similarity index 100% rename from internal/testing/bolttest/bolttest.go rename to internal/util/testing/bolttest/bolttest.go diff --git a/internal/util/testing/golden_equal.go b/internal/util/testing/golden_equal.go new file mode 100644 index 0000000..ef7b326 --- /dev/null +++ b/internal/util/testing/golden_equal.go @@ -0,0 +1,76 @@ +package testing + +import ( + "errors" + "flag" + "io" + "os" + "path/filepath" + "testing" + + "github.com/google/go-cmp/cmp" +) + +//nolint:gochecknoglobals // сompiler global flags cannot be read from within tests +var update = flag.Bool("update", false, "save current tests results as golden files") + +// GoldenEqual compares the bytes of the provided output with the contents of +// the golden file for a exact match. +// +// When running go test with the -update flag, the contents of golden-files will +// be overwritten with the provided contents of output, creating the testdata/ +// directory if it does not exist. +// +// Check TestGoldenEqual in testing_test.go and testdata/TestGoldenEqual.golden +// for example usage. +// +// See: https://youtu.be/8hQG7QlcLBk?t=749 +// +//nolint:cyclop // no need for splitting +func GoldenEqual(tb testing.TB, output io.Reader) { + tb.Helper() + + workDir, err := os.Getwd() + if err != nil { + tb.Fatal("cannot get current working directory path:", err) + } + + actual, err := io.ReadAll(output) + if err != nil { + tb.Fatal("cannot read provided data:", err) + } + + file := filepath.Join(workDir, "testdata", tb.Name()+".golden") + dir := filepath.Dir(file) + + //nolint:nestif // errchecks for testdata folder first, then for output + if *update { + _, err = os.Stat(dir) + if err != nil && !errors.Is(err, os.ErrExist) && !errors.Is(err, os.ErrNotExist) { + tb.Fatal("cannot create testdata folder for golden files:", err) + } + + if errors.Is(err, os.ErrNotExist) { + if err = os.MkdirAll(dir, os.ModePerm); err != nil { + tb.Fatal("cannot create testdata folder for golden files:", err) + } + } + + if err = os.WriteFile(file, actual, os.ModePerm); err != nil { + tb.Fatal("cannot write data into golden file:", err) + } + + tb.Skip("skipped due force updating golden file") + + return + } + + expect, err := os.ReadFile(file) + if err != nil { + tb.Fatal("cannot read golden file data:", err) + } + + if diff := cmp.Diff(actual, expect); diff != "" { + tb.Error(diff) + } +} diff --git a/internal/util/testing/golden_equal_test.go b/internal/util/testing/golden_equal_test.go new file mode 100644 index 0000000..d1c067a --- /dev/null +++ b/internal/util/testing/golden_equal_test.go @@ -0,0 +1,36 @@ +package testing_test + +import ( + "fmt" + "net/http" + "net/http/httptest" + "testing" + + testutil "source.toby3d.me/toby3d/auth/internal/util/testing" +) + +func TestGoldenEqual(t *testing.T) { + t.Parallel() + + req := httptest.NewRequest(http.MethodGet, "https://example.com/", nil) + w := httptest.NewRecorder() + testHandler := func(w http.ResponseWriter, r *http.Request) { + fmt.Fprintf(w, ` + + + + + Testing + + +

Hello, World!

+

This is a testing HTML page of %s website.

+ +`, r.Host) // NOTE(toby3d): Host must be 'example.com' + } + + testHandler(w, req) + + // NOTE(toby3d): compare recorded response body against saved golden file + testutil.GoldenEqual(t, w.Result().Body) +} diff --git a/internal/testing/sqltest/sqltest.go b/internal/util/testing/sqltest/sqltest.go similarity index 100% rename from internal/testing/sqltest/sqltest.go rename to internal/util/testing/sqltest/sqltest.go diff --git a/internal/util/testing/testdata/TestGoldenEqual.golden b/internal/util/testing/testdata/TestGoldenEqual.golden new file mode 100755 index 0000000..cf6b273 --- /dev/null +++ b/internal/util/testing/testdata/TestGoldenEqual.golden @@ -0,0 +1,12 @@ + + + + + + Testing + + +

Hello, World!

+

This is a testing HTML page of example.com website.

+ + \ No newline at end of file diff --git a/main.go b/main.go index 6925043..2ef925f 100644 --- a/main.go +++ b/main.go @@ -56,8 +56,8 @@ import ( tokenmemoryrepo "source.toby3d.me/toby3d/auth/internal/token/repository/memory" tokensqlite3repo "source.toby3d.me/toby3d/auth/internal/token/repository/sqlite3" tokenucase "source.toby3d.me/toby3d/auth/internal/token/usecase" - "source.toby3d.me/toby3d/auth/internal/urlutil" userhttpdelivery "source.toby3d.me/toby3d/auth/internal/user/delivery/http" + pathutil "source.toby3d.me/toby3d/auth/internal/util/path" ) type ( @@ -325,7 +325,7 @@ func (app *App) Handler() http.Handler { staticHandler := http.FileServer(http.FS(app.static)) return http.HandlerFunc(middleware.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - head, tail := urlutil.ShiftPath(r.URL.Path) + head, tail := pathutil.Shift(r.URL.Path) switch head { default: // NOTE(toby3d): static or 404 @@ -339,7 +339,7 @@ func (app *App) Handler() http.Handler { switch head { case ".well-known": // NOTE(toby3d): public server config - if head, _ = urlutil.ShiftPath(r.URL.Path); head == "oauth-authorization-server" { + if head, _ = pathutil.Shift(r.URL.Path); head == "oauth-authorization-server" { metadata.ServeHTTP(w, r) return