Skip to content

Add basicauth authorizer migration #145

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ReneWerner87 merged 1 commit into from
Jul 27, 2025
Merged

Add basicauth authorizer migration #145

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cmd/internal/migrations/lists.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ var Migrations = []Migration{
v3migrations.MigrateEnvVarConfig,
v3migrations.MigrateSessionConfig,
v3migrations.MigrateTimeoutConfig,
v3migrations.MigrateBasicauthAuthorizer,
v3migrations.MigrateReqHeaderParser,
MigrateGoVersion("1.24"),
},
Expand Down
15 changes: 15 additions & 0 deletions cmd/internal/migrations/v3/common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -598,3 +598,18 @@ func MigrateReqHeaderParser(cmd *cobra.Command, cwd string, _, _ *semver.Version
cmd.Println("Migrating request header parser helper")
return nil
}

// MigrateBasicauthAuthorizer updates inline basicauth authorizer functions to include the context parameter
func MigrateBasicauthAuthorizer(cmd *cobra.Command, cwd string, _, _ *semver.Version) error {
re := regexp.MustCompile(`Authorizer:\s*func\(([^)]*)\)`)

err := internal.ChangeFileContent(cwd, func(content string) string {
return re.ReplaceAllString(content, `Authorizer: func($1, _ fiber.Ctx)`)
})
Comment on lines +604 to +608
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Jul 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The current implementation for migrating the basicauth.Authorizer signature is not fully robust. It correctly handles the common case where parameters are present, but it may produce invalid Go code if the authorizer function has no parameters (e.g., func() bool).

While the basicauth.Authorizer type requires parameters, making this an edge case, a migration script should ideally be resilient to such variations.

I suggest a more robust implementation using regexp.ReplaceAllStringFunc to handle cases with and without parameters, as well as with optional trailing commas, correctly. This will ensure the migration is safer across a wider range of user code.

	re := regexp.MustCompile(`(Authorizer:\s*func\()([^)]*)(\))\s*bool\s*\{\s*return true\s*\}`)

	err := internal.ChangeFileContent(cwd, func(content string) string {
		return re.ReplaceAllStringFunc(content, func(s string) string {
			parts := re.FindStringSubmatch(s)
			if len(parts) != 4 {
				return s // Return original string if no match
			}
			params := parts[2]

			if strings.TrimSpace(params) == "" {
				return parts[1] + "_ fiber.Ctx" + parts[3] + ` bool { return true }`
			}

			if !strings.HasSuffix(strings.TrimSpace(params), ",") {
				params += ","
			}

			return parts[1] + params + " _ fiber.Ctx" + parts[3] + ` bool { return true }`
		})
	})

if err != nil {
return fmt.Errorf("failed to migrate basicauth authorizer: %w", err)
}

cmd.Println("Migrating basicauth authorizer")
return nil
}
25 changes: 25 additions & 0 deletions cmd/internal/migrations/v3/common_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -802,3 +802,28 @@ var _ = timeout.New(func(c fiber.Ctx) error { return nil }, 2*time.Second)`)
assert.Contains(t, content, `timeout.New(func(c fiber.Ctx) error { return nil }, timeout.Config{Timeout: 2*time.Second})`)
assert.Contains(t, buf.String(), "Migrating timeout middleware configs")
}

func Test_MigrateBasicauthAuthorizer(t *testing.T) {
t.Parallel()

dir, err := os.MkdirTemp("", "mbauthorizer")
require.NoError(t, err)
defer func() { require.NoError(t, os.RemoveAll(dir)) }()

file := writeTempFile(t, dir, `package main
import (
"github.com/gofiber/fiber/v2"
"github.com/gofiber/fiber/v2/middleware/basicauth"
)
var _ = basicauth.New(basicauth.Config{
Authorizer: func(u, p string) bool { return true },
})`)

var buf bytes.Buffer
cmd := newCmd(&buf)
require.NoError(t, v3.MigrateBasicauthAuthorizer(cmd, dir, nil, nil))

content := readFile(t, file)
assert.Contains(t, content, `Authorizer: func(u, p string, _ fiber.Ctx) bool`)
assert.Contains(t, buf.String(), "Migrating basicauth authorizer")
}
Comment on lines +806 to +829
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Jul 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The test for MigrateBasicauthAuthorizer covers the primary success case, which is great. To improve the robustness of the migration and ensure it handles various code styles, I recommend converting this to a table-driven test.

This would allow you to easily add and test edge cases, such as:

  • An authorizer function with no parameters.
  • An authorizer function with a trailing comma in its parameter list.

This will provide greater confidence that the migration works correctly for all users, especially with a more robust implementation of MigrateBasicauthAuthorizer.

func Test_MigrateBasicauthAuthorizer(t *testing.T) {
	t.Parallel()

	testCases := []struct {
		name     string
		input    string
		expected string
	}{
		{
			name: "with parameters",
			input:    `var _ = basicauth.New(basicauth.Config{\n    Authorizer: func(u, p string) bool { return true },\n})`,
			expected: `Authorizer: func(u, p string, _ fiber.Ctx) bool { return true }`,
		},
		{
			name: "with parameters and trailing comma",
			input:    `var _ = basicauth.New(basicauth.Config{\n    Authorizer: func(u, p string,) bool { return true },\n})`,
			expected: `Authorizer: func(u, p string, _ fiber.Ctx) bool { return true }`,
		},
		{
			name: "without parameters",
			input:    `var _ = basicauth.New(basicauth.Config{\n    Authorizer: func() bool { return true },\n})`,
			expected: `Authorizer: func(_ fiber.Ctx) bool { return true }`,
		},
	}

	for _, tc := range testCases {
		t.Run(tc.name, func(t *testing.T) {
			t.Parallel()

			dir, err := os.MkdirTemp("", "mbauthorizer-"+tc.name)
			require.NoError(t, err)
			defer func() { require.NoError(t, os.RemoveAll(dir)) }()

			fileContent := "package main\nimport (\n    \"github.com/gofiber/fiber/v2\"\n    \"github.com/gofiber/fiber/v2/middleware/basicauth\"\n)\n" + tc.input
			file := writeTempFile(t, dir, fileContent)

			var buf bytes.Buffer
			cmd := newCmd(&buf)
			require.NoError(t, v3.MigrateBasicauthAuthorizer(cmd, dir, nil, nil))

			content := readFile(t, file)
			assert.Contains(t, content, tc.expected)
			assert.Contains(t, buf.String(), "Migrating basicauth authorizer")
		})
	}
}

Loading