-
Notifications
You must be signed in to change notification settings - Fork 1k
fix: xtoken discrepancies #4676
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.
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
base: main
Are you sure you want to change the base?
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,12 +5,14 @@ import ( | |
| "crypto/tls" | ||
| "encoding/json" | ||
| "errors" | ||
| "fmt" | ||
| "net" | ||
| "os" | ||
| "path/filepath" | ||
| "time" | ||
|
|
||
| grpc_retry "github.com/grpc-ecosystem/go-grpc-middleware/retry" | ||
| logging "github.com/ipfs/go-log/v2" | ||
| "go.uber.org/fx" | ||
| "google.golang.org/grpc" | ||
| "google.golang.org/grpc/codes" | ||
|
|
@@ -22,6 +24,8 @@ import ( | |
| "github.com/celestiaorg/celestia-node/libs/utils" | ||
| ) | ||
|
|
||
| var log = logging.Logger("core") | ||
|
|
||
| const ( | ||
| // gRPC client requires fetching a block on initialization that can be larger | ||
| // than the default message size set in gRPC. Increasing defaults up to 64MB | ||
|
|
@@ -32,7 +36,8 @@ const ( | |
| // TODO(@vgonkivs): Revisit this constant once the block size reaches 64MB. | ||
| defaultGRPCMessageSize = 64 * 1024 * 1024 // 64Mb | ||
|
|
||
| xtokenFileName = "xtoken.json" | ||
| xtokenFileName = "xtoken.json" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can u just array and loop? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why? there are only two file names: xtoken.json or x-token.json |
||
| xtokenFileNameAlt = "x-token.json" //nolint:gosec | ||
| ) | ||
|
|
||
| type AdditionalCoreConns []*grpc.ClientConn | ||
|
|
@@ -144,28 +149,68 @@ func authStreamInterceptor(xtoken string) grpc.StreamClientInterceptor { | |
| } | ||
|
|
||
| // parseTokenPath retrieves the authentication token from a JSON file at the specified path. | ||
| // It supports both "xtoken.json" and "x-token.json" filenames, and both "xtoken" and "x-token" JSON keys. | ||
| func parseTokenPath(xtokenPath string) (string, error) { | ||
| xtokenPath = filepath.Join(xtokenPath, xtokenFileName) | ||
| exist := utils.Exists(xtokenPath) | ||
| if !exist { | ||
| return "", os.ErrNotExist | ||
| // Try both filename variants: xtoken.json and x-token.json | ||
| var tokenFilePath string | ||
|
|
||
| primaryPath := filepath.Join(xtokenPath, xtokenFileName) | ||
| altPath := filepath.Join(xtokenPath, xtokenFileNameAlt) | ||
|
|
||
| switch { | ||
| case utils.Exists(primaryPath): | ||
| tokenFilePath = primaryPath | ||
| case utils.Exists(altPath): | ||
| tokenFilePath = altPath | ||
| log.Warnf("Using alternate filename '%s'. Consider using '%s' for consistency.", xtokenFileNameAlt, xtokenFileName) | ||
| default: | ||
| return "", fmt.Errorf("authentication token file not found. Expected '%s' or '%s' in directory: %s", | ||
| xtokenFileName, xtokenFileNameAlt, xtokenPath) | ||
| } | ||
|
|
||
| token, err := os.ReadFile(xtokenPath) | ||
| token, err := os.ReadFile(tokenFilePath) | ||
| if err != nil { | ||
| return "", err | ||
| return "", fmt.Errorf("failed to read token file '%s': %w", tokenFilePath, err) | ||
| } | ||
|
|
||
| // Support "x-token" (preferred), "xtoken", and "token" JSON keys for maximum compatibility | ||
| auth := struct { | ||
| Token string `json:"x-token"` | ||
| XToken string `json:"x-token"` | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this can be simplified Just construct the auth token when you successfully read from one of the expected file names. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. wdym? i am reading from the expected file names, but the token name can be either of these three (x-token | xtoken | token) hence the struct and unmarshal. can u clarify? |
||
| XTokenAlt string `json:"xtoken"` | ||
| Token string `json:"token"` | ||
| }{} | ||
|
|
||
| err = json.Unmarshal(token, &auth) | ||
| if err != nil { | ||
| return "", err | ||
| return "", fmt.Errorf( | ||
| "failed to parse token file '%s': %w. Expected JSON with 'x-token', 'xtoken', or 'token' key", | ||
| tokenFilePath, err) | ||
| } | ||
| if auth.Token == "" { | ||
| return "", errors.New("x-token is empty. Please setup a token or cleanup xtokenPath") | ||
|
|
||
| var tokenValue string | ||
|
|
||
| switch { | ||
| case auth.XToken != "": | ||
| tokenValue = auth.XToken | ||
| case auth.XTokenAlt != "": | ||
| tokenValue = auth.XTokenAlt | ||
| log.Warnf("Using alternate JSON key 'xtoken' in file '%s'. Consider using 'x-token' for consistency.", tokenFilePath) | ||
| case auth.Token != "": | ||
| tokenValue = auth.Token | ||
| log.Warnf("Using alternate JSON key 'token' in file '%s'. Consider using 'x-token' for consistency.", tokenFilePath) | ||
| default: | ||
| return "", fmt.Errorf( | ||
| "authentication token is empty or missing in file '%s'. "+ | ||
| "Please provide a JSON file with 'x-token' (preferred), 'xtoken', or 'token' key containing the token value", | ||
| tokenFilePath) | ||
| } | ||
| return auth.Token, nil | ||
|
|
||
| if tokenValue == "" { | ||
| return "", fmt.Errorf( | ||
| "authentication token is empty in file '%s'. "+ | ||
| "Please setup a valid token or remove the xtokenPath configuration", | ||
| tokenFilePath) | ||
| } | ||
|
|
||
| return tokenValue, nil | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,168 @@ | ||
| package core | ||
|
|
||
| import ( | ||
| "os" | ||
| "path/filepath" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestParseTokenPath(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| filename string | ||
| jsonContent string | ||
| expectedToken string | ||
| expectError bool | ||
| errorContains string | ||
| }{ | ||
| { | ||
| name: "x-token key with xtoken.json filename", | ||
| filename: "xtoken.json", | ||
| jsonContent: `{"x-token": "test-token-123"}`, | ||
| expectedToken: "test-token-123", | ||
| expectError: false, | ||
| }, | ||
| { | ||
| name: "x-token key with x-token.json filename", | ||
| filename: "x-token.json", | ||
| jsonContent: `{"x-token": "test-token-456"}`, | ||
| expectedToken: "test-token-456", | ||
| expectError: false, | ||
| }, | ||
| { | ||
| name: "xtoken key with xtoken.json filename", | ||
| filename: "xtoken.json", | ||
| jsonContent: `{"xtoken": "test-token-789"}`, | ||
| expectedToken: "test-token-789", | ||
| expectError: false, | ||
| }, | ||
| { | ||
| name: "xtoken key with x-token.json filename", | ||
| filename: "x-token.json", | ||
| jsonContent: `{"xtoken": "test-token-abc"}`, | ||
| expectedToken: "test-token-abc", | ||
| expectError: false, | ||
| }, | ||
| { | ||
| name: "token key with xtoken.json filename (QuickNode style)", | ||
| filename: "xtoken.json", | ||
| jsonContent: `{"token": "hunter2"}`, | ||
| expectedToken: "hunter2", | ||
| expectError: false, | ||
| }, | ||
| { | ||
| name: "token key with x-token.json filename", | ||
| filename: "x-token.json", | ||
| jsonContent: `{"token": "hunter3"}`, | ||
| expectedToken: "hunter3", | ||
| expectError: false, | ||
| }, | ||
| { | ||
| name: "x-token key takes precedence over xtoken", | ||
| filename: "xtoken.json", | ||
| jsonContent: `{"x-token": "priority-token", "xtoken": "should-be-ignored"}`, | ||
| expectedToken: "priority-token", | ||
| expectError: false, | ||
| }, | ||
| { | ||
| name: "x-token key takes precedence over token", | ||
| filename: "xtoken.json", | ||
| jsonContent: `{"x-token": "priority-token", "token": "should-be-ignored"}`, | ||
| expectedToken: "priority-token", | ||
| expectError: false, | ||
| }, | ||
| { | ||
| name: "xtoken key takes precedence over token", | ||
| filename: "xtoken.json", | ||
| jsonContent: `{"xtoken": "priority-token", "token": "should-be-ignored"}`, | ||
| expectedToken: "priority-token", | ||
| expectError: false, | ||
| }, | ||
| { | ||
| name: "empty token value", | ||
| filename: "xtoken.json", | ||
| jsonContent: `{"x-token": ""}`, | ||
| expectedToken: "", | ||
| expectError: true, | ||
| errorContains: "authentication token is empty", | ||
| }, | ||
| { | ||
| name: "missing token key", | ||
| filename: "xtoken.json", | ||
| jsonContent: `{"other-key": "value"}`, | ||
| expectedToken: "", | ||
| expectError: true, | ||
| errorContains: "authentication token is empty or missing", | ||
| }, | ||
| { | ||
| name: "invalid JSON", | ||
| filename: "xtoken.json", | ||
| jsonContent: `invalid json`, | ||
| expectedToken: "", | ||
| expectError: true, | ||
| errorContains: "failed to parse token file", | ||
| }, | ||
| { | ||
| name: "file not found", | ||
| filename: "nonexistent.json", | ||
| jsonContent: ``, | ||
| expectedToken: "", | ||
| expectError: true, | ||
| errorContains: "authentication token file not found", | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| testDir := t.TempDir() | ||
|
|
||
| if tt.jsonContent != "" { | ||
| tokenFile := filepath.Join(testDir, tt.filename) | ||
| err := os.WriteFile(tokenFile, []byte(tt.jsonContent), 0o644) | ||
| require.NoError(t, err) | ||
| } | ||
|
|
||
| token, err := parseTokenPath(testDir) | ||
|
|
||
| if tt.expectError { | ||
| require.Error(t, err) | ||
| if tt.errorContains != "" { | ||
| assert.Contains(t, err.Error(), tt.errorContains) | ||
| } | ||
| } else { | ||
| require.NoError(t, err) | ||
| assert.Equal(t, tt.expectedToken, token) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestParseTokenPathFilenamePreference(t *testing.T) { | ||
| tmpDir := t.TempDir() | ||
|
|
||
| // Create both files - xtoken.json should be preferred | ||
| xtokenFile := filepath.Join(tmpDir, "xtoken.json") | ||
| xTokenFile := filepath.Join(tmpDir, "x-token.json") | ||
|
|
||
| err := os.WriteFile(xtokenFile, []byte(`{"x-token": "from-xtoken.json"}`), 0o644) | ||
| require.NoError(t, err) | ||
|
|
||
| err = os.WriteFile(xTokenFile, []byte(`{"x-token": "from-x-token.json"}`), 0o644) | ||
| require.NoError(t, err) | ||
|
|
||
| // Should prefer xtoken.json | ||
| token, err := parseTokenPath(tmpDir) | ||
| require.NoError(t, err) | ||
| assert.Equal(t, "from-xtoken.json", token) | ||
|
|
||
| // Remove xtoken.json, should use x-token.json | ||
| err = os.Remove(xtokenFile) | ||
| require.NoError(t, err) | ||
|
|
||
| token, err = parseTokenPath(tmpDir) | ||
| require.NoError(t, err) | ||
| assert.Equal(t, "from-x-token.json", token) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.