You can close this window and return to the terminal.
+ + + + `)) + + codeChan <- code + } else { + http.NotFound(w, r) + } + }), + } + + // Start server in goroutine + go func() { + if err := server.ListenAndServe(); err != nil && err != http.ErrServerClosed { + errChan <- err + } + }() + + // Open browser for authorization + fmt.Printf("Opening browser for authorization...\n") + fmt.Printf("If the browser doesn't open automatically, please visit: %s\n", authURL) + + // Try to open browser + if err := openBrowser(authURL); err != nil { + fmt.Printf("Could not open browser automatically: %v\n", err) + fmt.Printf("Please manually visit: %s\n", authURL) + } + + // Wait for authorization code + select { + case code := <-codeChan: + // Shutdown server + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + server.Shutdown(ctx) + + // Exchange code for token + s.Stop() + s = cmdutil.Info("Exchanging authorization code for access token...") + defer s.Stop() + + token, err := oauthConfig.Exchange(context.Background(), code) + if err != nil { + return fmt.Errorf("failed to exchange code for token: %w", err) + } + + // Store tokens + c.value.oauth.accessToken = token.AccessToken + if token.RefreshToken != "" { + c.value.oauth.refreshToken = token.RefreshToken + } + + // Store client secret securely (in environment variable) + if err := c.storeClientSecretSecurely(); err != nil { + return fmt.Errorf("failed to store client secret: %w", err) + } + + return nil + + case err := <-errChan: + // Shutdown server + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + server.Shutdown(ctx) + return fmt.Errorf("OAuth flow failed: %w", err) + + case <-time.After(5 * time.Minute): + // Shutdown server + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + server.Shutdown(ctx) + return fmt.Errorf("OAuth flow timed out") + } +} + +func (c *JiraCLIConfigGenerator) storeClientSecretSecurely() error { + // For now, we'll store it in a separate file with restricted permissions + // In a production environment, you might want to use a keyring or similar secure storage + + home, err := cmdutil.GetConfigHome() + if err != nil { + return err + } + configDir := fmt.Sprintf("%s/%s", home, ".jira") + + secretFile := filepath.Join(configDir, ".oauth_secret") + + // Write client secret to file with restricted permissions + if err := os.WriteFile(secretFile, []byte(c.value.oauth.clientSecret), 0600); err != nil { + return fmt.Errorf("failed to write client secret to file: %w", err) + } + + // Clear the client secret from memory + c.value.oauth.clientSecret = "" + + return nil +} + +func openBrowser(url string) error { + if err := browser.OpenURL(url); err != nil { + return err + } + return nil +} + //nolint:gocyclo func (c *JiraCLIConfigGenerator) configureServerAndLoginDetails() error { var qs []*survey.Question + if c.value.authType == jira.AuthTypeOAuth { + // https://developer.atlassian.com/cloud/oauth/getting-started/making-calls-to-api/ + if err := c.getCloudID(); err != nil { + return err + } + c.usrCfg.Server = fmt.Sprintf("https://api.atlassian.com/ex/jira/%s", c.value.oauth.cloudId) + } c.value.server = c.usrCfg.Server c.value.login = c.usrCfg.Login @@ -335,7 +597,7 @@ func (c *JiraCLIConfigGenerator) configureServerAndLoginDetails() error { }) } - if c.usrCfg.Login == "" { + if c.usrCfg.Login == "" && c.value.authType != jira.AuthTypeOAuth { switch c.value.installation { case jira.InstallationTypeCloud: qs = append(qs, &survey.Question{ @@ -411,12 +673,59 @@ func (c *JiraCLIConfigGenerator) configureServerAndLoginDetails() error { return c.verifyLoginDetails(c.value.server, c.value.login) } +func (c *JiraCLIConfigGenerator) getCloudID() error { + if c.value.oauth.accessToken == "" { + return fmt.Errorf("access token is required for cloud installation") + } + accessToken := c.value.oauth.accessToken + s := cmdutil.Info("Fetching cloud ID...") + defer s.Stop() + jiraClient := api.Client(jira.Config{ + Server: "https://api.atlassian.com", + Login: c.value.login, + APIToken: accessToken, + Insecure: &c.usrCfg.Insecure, + AuthType: &c.value.authType, + Debug: viper.GetBool("debug"), + MTLSConfig: jira.MTLSConfig{ + CaCert: c.value.mtls.caCert, + ClientCert: c.value.mtls.clientCert, + ClientKey: c.value.mtls.clientKey, + }, + }) + + cloudId, err := jiraClient.GetCloudID() + if err != nil { + return err + } + + c.value.oauth.cloudId = cloudId + api.DisposeClient() + return nil +} + func (c *JiraCLIConfigGenerator) verifyLoginDetails(server, login string) error { s := cmdutil.Info("Verifying login details...") defer s.Stop() server = strings.TrimRight(server, "/") + if c.value.authType == jira.AuthTypeOAuth { + c.jiraClient = api.Client(jira.Config{ + Server: server, + Login: login, + APIToken: c.value.oauth.accessToken, + Insecure: &c.usrCfg.Insecure, + AuthType: &c.value.authType, + Debug: viper.GetBool("debug"), + MTLSConfig: jira.MTLSConfig{ + CaCert: c.value.mtls.caCert, + ClientCert: c.value.mtls.clientCert, + ClientKey: c.value.mtls.clientKey, + }, + }) + } else { + } c.jiraClient = api.Client(jira.Config{ Server: server, Login: login, @@ -429,6 +738,7 @@ func (c *JiraCLIConfigGenerator) verifyLoginDetails(server, login string) error ClientKey: c.value.mtls.clientKey, }, }) + ret, err := c.jiraClient.Me() if err != nil { return err @@ -775,6 +1085,11 @@ func (c *JiraCLIConfigGenerator) write(path string) (string, error) { config.Set("version.patch", c.value.version.patch) } + if c.value.authType == jira.AuthTypeOAuth { + config.Set("oauth.client_id", c.value.oauth.clientId) + config.Set("oauth.cloud_id", c.value.oauth.cloudId) + } + if c.value.board != nil { config.Set("board", c.value.board) } else { diff --git a/pkg/jira/client.go b/pkg/jira/client.go index c6435dbc..904df467 100644 --- a/pkg/jira/client.go +++ b/pkg/jira/client.go @@ -279,6 +279,8 @@ func (c *Client) request(ctx context.Context, method, endpoint string, body []by if c.token != "" { req.Header.Add("Authorization", "Bearer "+c.token) } + case string(AuthTypeOAuth): + req.Header.Add("Authorization", "Bearer "+c.token) case string(AuthTypeBearer): req.Header.Add("Authorization", "Bearer "+c.token) case string(AuthTypeBasic): diff --git a/pkg/jira/types.go b/pkg/jira/types.go index e1c17719..9af7ca39 100644 --- a/pkg/jira/types.go +++ b/pkg/jira/types.go @@ -11,6 +11,10 @@ const ( AuthTypeBearer AuthType = "bearer" // AuthTypeMTLS is a mTLS auth. AuthTypeMTLS AuthType = "mtls" + // AuthTypeOAuth is a OAuth auth. + AuthTypeOAuth AuthType = "oauth" + // AuthTypeCloud is a cloud auth. + AuthTypeCloud AuthType = "cloud" ) // AuthType is a jira authentication type. From 23a3c5daef3b33e9a58fca5f7d85c12f6e30cfa8 Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Wed, 9 Jul 2025 01:27:26 +0100 Subject: [PATCH 03/70] feat:(oauth) - Ability to get cloud id for clients --- pkg/jira/cloud_id.go | 55 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 pkg/jira/cloud_id.go diff --git a/pkg/jira/cloud_id.go b/pkg/jira/cloud_id.go new file mode 100644 index 00000000..f3821580 --- /dev/null +++ b/pkg/jira/cloud_id.go @@ -0,0 +1,55 @@ +package jira + +import ( + "context" + "encoding/json" + "errors" + "net/http" +) + +var ( + ErrMultipleCloudIDs = errors.New("multiple cloud IDs found, unable to determine which to use") + ErrEmptyCloudID = errors.New("empty cloud ID returned") +) + +type CloudIDResponse struct { + ID string `json:"id"` + Name string `json:"name"` + URL string `json:"url"` + Scopes []string `json:"scopes"` + AvatarURL string `json:"avatarUrl"` +} + +func (c *Client) GetCloudID() (string, error) { + res, err := c.request(context.Background(), http.MethodGet, "https://api.atlassian.com/oauth/token/accessible-resources", nil, Header{ + "Accept": "application/json"}) + if err != nil { + return "", err + } + if res == nil { + return "", ErrEmptyResponse + } + defer func() { _ = res.Body.Close() }() + + if res.StatusCode != http.StatusOK { + return "", formatUnexpectedResponse(res) + } + var out []CloudIDResponse + err = json.NewDecoder(res.Body).Decode(&out) + if err != nil { + return "", err + } + if len(out) == 0 { + return "", ErrEmptyResponse + } + // Return the first cloud ID found + if len(out) > 1 { + return "", ErrMultipleCloudIDs + } + if out[0].ID == "" { + return "", ErrEmptyCloudID + } + // Return the account ID + + return out[0].ID, nil +} From eb98589b0abd49a9b9aaed81dd307d73edc84189 Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Sat, 12 Jul 2025 23:49:56 +0100 Subject: [PATCH 04/70] (docs) Add note about OAuth weirdness --- README.md | 93 +++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 80 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 28a16856..19bcbe79 100644 --- a/README.md +++ b/README.md @@ -60,15 +60,17 @@ features like issue creation, cloning, linking, ticket transition, and much more > This tool is heavily inspired by the [GitHub CLI](https://github.com/cli/cli) ## Supported platforms + > [!NOTE] > Some features might work slightly differently in cloud installation versus on-premise installation due to the -nature of the data. Yet, we've attempted to make the experience as similar as possible. +> nature of the data. Yet, we've attempted to make the experience as similar as possible. | Platform |
|
-| :------------- | :----------: |
-| **Jira** | 
|
+| :------- | :-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------: |
+| **Jira** | |
## Installation
+
`jira-cli` is available as a downloadable packaged binary for Linux, macOS, and Windows from the [releases page](https://github.com/ankitpokhrel/jira-cli/releases).
You can use Docker to quickly try out `jira-cli`.
@@ -111,9 +113,9 @@ Follow the [installation guide](https://github.com/ankitpokhrel/jira-cli/wiki/In
> [!IMPORTANT]
> If your on-premise Jira installation is using a language other than `English`, then the issue/epic creation
- may not work because the older version of Jira API doesn't return the untranslated name for `issuetypes`. In that case,
- you will have to fill in `epic.name`, `epic.link` and `issue.types.*.handle` fields manually in the generated config
- to get the expected behavior.
+> may not work because the older version of Jira API doesn't return the untranslated name for `issuetypes`. In that case,
+> you will have to fill in `epic.name`, `epic.link` and `issue.types.*.handle` fields manually in the generated config
+> to get the expected behavior.
See [FAQs](https://github.com/ankitpokhrel/jira-cli/discussions/categories/faqs) for frequently asked questions.
@@ -122,11 +124,12 @@ See [FAQs](https://github.com/ankitpokhrel/jira-cli/discussions/categories/faqs)
The tool supports `basic`, `bearer` (Personal Access Token), and `mtls` (Client Certificates) authentication types. Basic auth is used by
default.
-* If you want to use PAT, you need to set `JIRA_AUTH_TYPE` as `bearer`.
-* If you want to use `mtls` run `jira init`. Select installation type `Local`, and then select authentication type as `mtls`.
- * In case `JIRA_API_TOKEN` variable is set it will be used together with `mtls`.
+- If you want to use PAT, you need to set `JIRA_AUTH_TYPE` as `bearer`.
+- If you want to use `mtls` run `jira init`. Select installation type `Local`, and then select authentication type as `mtls`.
+ - In case `JIRA_API_TOKEN` variable is set it will be used together with `mtls`.
#### Shell completion
+
Check `jira completion --help` for more info on setting up a bash/zsh shell completion.
#### Multiple projects
@@ -141,15 +144,19 @@ $ jira issue list -c ./local_jira_config.yaml
```
## Usage
+
The tool currently comes with an issue, epic, and sprint explorer. The flags are [POSIX-compliant](https://www.gnu.org/software/libc/manual/html_node/Argument-Syntax.html).
You can combine available flags in any order to create a unique query. For example, the command below will give you high priority issues created this month
with status `To Do` that are assigned to you and has the label `backend`.
+
```sh
jira issue list -yHigh -s"To Do" --created month -lbackend -a$(jira me)
```
### Navigation
+
The lists are displayed in an interactive UI by default.
+
- Use arrow keys or `j, k, h, l` characters to navigate through the list.
- Use `g` and `G` to quickly navigate to the top and bottom respectively.
- Use `CTRL + f` to scroll through a page downwards direction.
@@ -165,6 +172,7 @@ The lists are displayed in an interactive UI by default.
- Press `?` to open the help window.
### Resources
+
- [FAQs](https://github.com/ankitpokhrel/jira-cli/discussions/categories/faqs)
- [Introduction and Motivation](https://medium.com/@ankitpokhrel/introducing-jira-cli-the-missing-command-line-tool-for-atlassian-jira-fe44982cc1de)
- [Getting Started with JiraCLI](https://www.mslinn.com/blog/2022/08/12/jiracli.html)
@@ -173,10 +181,13 @@ The lists are displayed in an interactive UI by default.
> Like this tool? Checkout [similar tool for Shopify!](https://github.com/ankitpokhrel/shopctl)
## Commands
+
### Issue
+
Issues are displayed in an interactive table view by default. You can output the results in a plain view using the `--plain` flag.
#### List
+
The `list` command lets you search and navigate the issues. The issues are sorted by `created` field in descending order by default.
```sh
@@ -214,6 +225,7 @@ Check some more examples/use-cases below.
```sh
jira issue list -w
```
+
You can close this window and return to the terminal.
- - - - `)) - - codeChan <- code - } else { - http.NotFound(w, r) - } - }), - } - - // Start server in goroutine - go func() { - if err := server.ListenAndServe(); err != nil && err != http.ErrServerClosed { - errChan <- err - } - }() - - // Open browser for authorization - fmt.Printf("Opening browser for authorization...\n") - fmt.Printf("If the browser doesn't open automatically, please visit: %s\n", authURL) - - // Try to open browser - if err := openBrowser(authURL); err != nil { - fmt.Printf("Could not open browser automatically: %v\n", err) - fmt.Printf("Please manually visit: %s\n", authURL) - } - - // Wait for authorization code - select { - case code := <-codeChan: - // Shutdown server - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - defer cancel() - server.Shutdown(ctx) - - // Exchange code for token - s.Stop() - s = cmdutil.Info("Exchanging authorization code for access token...") - defer s.Stop() - - token, err := oauthConfig.Exchange(context.Background(), code) - if err != nil { - return fmt.Errorf("failed to exchange code for token: %w", err) - } - - // Store tokens - c.value.oauth.accessToken = token.AccessToken - if token.RefreshToken != "" { - c.value.oauth.refreshToken = token.RefreshToken - } - - // Store client secret securely (in environment variable) - if err := c.storeClientSecretSecurely(); err != nil { - return fmt.Errorf("failed to store client secret: %w", err) - } - - return nil - - case err := <-errChan: - // Shutdown server - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - defer cancel() - server.Shutdown(ctx) - return fmt.Errorf("OAuth flow failed: %w", err) - - case <-time.After(5 * time.Minute): - // Shutdown server - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - defer cancel() - server.Shutdown(ctx) - return fmt.Errorf("OAuth flow timed out") - } -} - -func (c *JiraCLIConfigGenerator) storeClientSecretSecurely() error { - // For now, we'll store it in a separate file with restricted permissions - // In a production environment, you might want to use a keyring or similar secure storage - - home, err := cmdutil.GetConfigHome() + // Use the new OAuth package + tokenResponse, err := oauth.Configure() if err != nil { return err } - configDir := fmt.Sprintf("%s/%s", home, ".jira") - secretFile := filepath.Join(configDir, ".oauth_secret") - - // Write client secret to file with restricted permissions - if err := os.WriteFile(secretFile, []byte(c.value.oauth.clientSecret), 0600); err != nil { - return fmt.Errorf("failed to write client secret to file: %w", err) - } - - // Clear the client secret from memory - c.value.oauth.clientSecret = "" + // Store the tokens and cloud ID + c.value.oauth.accessToken = tokenResponse.AccessToken.String() + c.value.oauth.refreshToken = tokenResponse.RefreshToken.String() + c.value.oauth.cloudId = tokenResponse.CloudID return nil } -func openBrowser(url string) error { - if err := browser.OpenURL(url); err != nil { - return err - } - return nil -} - //nolint:gocyclo func (c *JiraCLIConfigGenerator) configureServerAndLoginDetails() error { var qs []*survey.Question if c.value.authType == jira.AuthTypeOAuth { - // https://developer.atlassian.com/cloud/oauth/getting-started/making-calls-to-api/ - if err := c.getCloudID(); err != nil { - return err - } + // Set server URL using the cloud ID from OAuth configuration c.usrCfg.Server = fmt.Sprintf("https://api.atlassian.com/ex/jira/%s", c.value.oauth.cloudId) } c.value.server = c.usrCfg.Server @@ -597,7 +402,7 @@ func (c *JiraCLIConfigGenerator) configureServerAndLoginDetails() error { }) } - if c.usrCfg.Login == "" && c.value.authType != jira.AuthTypeOAuth { + if c.usrCfg.Login == "" { switch c.value.installation { case jira.InstallationTypeCloud: qs = append(qs, &survey.Question{ @@ -672,36 +477,26 @@ func (c *JiraCLIConfigGenerator) configureServerAndLoginDetails() error { return c.verifyLoginDetails(c.value.server, c.value.login) } - -func (c *JiraCLIConfigGenerator) getCloudID() error { - if c.value.oauth.accessToken == "" { - return fmt.Errorf("access token is required for cloud installation") - } - accessToken := c.value.oauth.accessToken - s := cmdutil.Info("Fetching cloud ID...") - defer s.Stop() - jiraClient := api.Client(jira.Config{ - Server: "https://api.atlassian.com", +func (c *JiraCLIConfigGenerator) generateJiraConfig() jira.Config { + config := jira.Config{ + Server: c.value.server, Login: c.value.login, - APIToken: accessToken, Insecure: &c.usrCfg.Insecure, AuthType: &c.value.authType, Debug: viper.GetBool("debug"), - MTLSConfig: jira.MTLSConfig{ + } + + switch c.value.authType { + case jira.AuthTypeOAuth: + config.APIToken = c.value.oauth.accessToken + case jira.AuthTypeMTLS: + config.MTLSConfig = jira.MTLSConfig{ CaCert: c.value.mtls.caCert, ClientCert: c.value.mtls.clientCert, ClientKey: c.value.mtls.clientKey, - }, - }) - - cloudId, err := jiraClient.GetCloudID() - if err != nil { - return err + } } - - c.value.oauth.cloudId = cloudId - api.DisposeClient() - return nil + return config } func (c *JiraCLIConfigGenerator) verifyLoginDetails(server, login string) error { @@ -1086,7 +881,6 @@ func (c *JiraCLIConfigGenerator) write(path string) (string, error) { } if c.value.authType == jira.AuthTypeOAuth { - config.Set("oauth.client_id", c.value.oauth.clientId) config.Set("oauth.cloud_id", c.value.oauth.cloudId) } diff --git a/pkg/oauth/README.md b/pkg/oauth/README.md new file mode 100644 index 00000000..c608c714 --- /dev/null +++ b/pkg/oauth/README.md @@ -0,0 +1,61 @@ +# OAuth Package + +This package provides OAuth2 authentication functionality for the JIRA CLI. + +## Features + +- Complete OAuth2 flow implementation for Atlassian JIRA +- Local HTTP server for OAuth callback handling +- Automatic browser opening for authorization +- Secure client secret storage +- Cloud ID retrieval for Atlassian API access +- PKCE (Proof Key for Code Exchange) support + +## Usage + +```go +import "github.com/ankitpokhrel/jira-cli/internal/pkg/oauth" + +// Perform complete OAuth flow +tokenResponse, err := oauth.Configure() +if err != nil { + log.Fatal(err) +} + +// Access the tokens and cloud ID +accessToken := tokenResponse.AccessToken +refreshToken := tokenResponse.RefreshToken +cloudID := tokenResponse.CloudID +``` + +## Configuration + +The `Configure()` function will: + +1. Prompt the user for: + + - Jira App Client ID + - Jira App Client Secret + - Redirect URI (defaults to `http://localhost:9876/callback`) + +2. Start a local HTTP server on port 9876 to handle the OAuth callback + +3. Open the user's browser to the Atlassian authorization URL + +4. Exchange the authorization code for access and refresh tokens + +5. Retrieve the Cloud ID for API access + +6. Store the client secret securely in `~/.jira/.oauth_secret` + +## Security + +- Client secrets are stored with restricted permissions (0600) in a separate file +- Client secrets are cleared from memory after secure storage +- The local server automatically shuts down after receiving the callback + +## Requirements + +- The redirect URI must be configured in your Atlassian OAuth app +- Port 9876 must be available for the local callback server +- The user must have a web browser available for authorization diff --git a/pkg/oauth/oauth.go b/pkg/oauth/oauth.go new file mode 100644 index 00000000..2f95ec20 --- /dev/null +++ b/pkg/oauth/oauth.go @@ -0,0 +1,372 @@ +package oauth + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "os" + "path/filepath" + "time" + + "github.com/AlecAivazis/survey/v2" + "github.com/pkg/browser" + "golang.org/x/oauth2" + + "github.com/ankitpokhrel/jira-cli/internal/cmdutil" +) + +const ( + // JIRA OAuth2 endpoints + jiraAuthURL = "https://auth.atlassian.com/authorize" + jiraTokenURL = "https://auth.atlassian.com/oauth/token" + + // Default OAuth settings + defaultRedirectURI = "http://localhost:9876/callback" + defaultPort = ":9876" + callbackPath = "/callback" + + // OAuth timeout + oauthTimeout = 5 * time.Minute +) + +const ( + OWNER_ONLY = 0o700 + OWNER_READ_WRITE = 0o600 +) + +// Storage defines the interface for secret storage operations +type Storage interface { + Save(key string, value []byte) error + Load(key string) ([]byte, error) +} + +// Secret represents a secret value with storage capabilities +type Secret struct { + Key string + Value string +} + +func (s Secret) String() string { + return s.Value +} + +func (s Secret) Save(storage Storage) error { + if s.Key == "" { + return fmt.Errorf("secret key cannot be empty") + } + return storage.Save(s.Key, []byte(s.Value)) +} + +func (s *Secret) Load(storage Storage, key string) error { + if key == "" { + return fmt.Errorf("secret key cannot be empty") + } + + data, err := storage.Load(key) + if err != nil { + return err + } + + s.Key = key + s.Value = string(data) + return nil +} + +// FileSystemStorage implements Storage interface for filesystem operations +type FileSystemStorage struct { + BaseDir string +} + +func (fs FileSystemStorage) Save(key string, value []byte) error { + if err := os.MkdirAll(fs.BaseDir, OWNER_ONLY); err != nil { + return fmt.Errorf("failed to create directory: %w", err) + } + + filePath := filepath.Join(fs.BaseDir, key) + return os.WriteFile(filePath, value, OWNER_READ_WRITE) +} + +func (fs FileSystemStorage) Load(key string) ([]byte, error) { + filePath := filepath.Join(fs.BaseDir, key) + return os.ReadFile(filePath) +} + +// Config holds OAuth configuration +type Config struct { + ClientID string + ClientSecret Secret + RedirectURI string + Scopes []string +} + +// ConfigureTokenResponse holds the OAuth token response +type ConfigureTokenResponse struct { + AccessToken Secret + RefreshToken Secret + CloudID string +} + +// Configure performs the complete OAuth flow and returns tokens +func Configure() (*ConfigureTokenResponse, error) { + // Collect OAuth credentials from user + + jiraDir, err := getJiraConfigDir() + if err != nil { + return nil, fmt.Errorf("failed to get Jira config directory: %w", err) + } + + secretStorage := FileSystemStorage{BaseDir: jiraDir} + + config, err := collectOAuthCredentials() + if err != nil { + return nil, fmt.Errorf("failed to collect OAuth credentials: %w", err) + } + + // Perform OAuth flow + tokens, err := performOAuthFlow(config) + if err != nil { + return nil, fmt.Errorf("OAuth flow failed: %w", err) + } + + // Store client secret securely + if err := config.ClientSecret.Save(secretStorage); err != nil { + return nil, fmt.Errorf("failed to store client secret: %w", err) + } + + accessToken := Secret{Key: "access_token", Value: tokens.AccessToken} + refreshToken := Secret{Key: "refresh_token", Value: tokens.RefreshToken} + + if err := accessToken.Save(secretStorage); err != nil { + return nil, fmt.Errorf("failed to store access token: %w", err) + } + + if err := refreshToken.Save(secretStorage); err != nil { + return nil, fmt.Errorf("failed to store refresh token: %w", err) + } + // Get Cloud ID for Atlassian API + cloudID, err := getCloudID(tokens.AccessToken) + if err != nil { + return nil, fmt.Errorf("failed to get cloud ID: %w", err) + } + + return &ConfigureTokenResponse{ + AccessToken: accessToken, + RefreshToken: refreshToken, + CloudID: cloudID, + }, nil +} + +// collectOAuthCredentials collects OAuth credentials from the user +func collectOAuthCredentials() (*Config, error) { + var questions []*survey.Question + answers := struct { + ClientID string + ClientSecret string + RedirectURI string + }{} + + questions = append(questions, &survey.Question{ + Name: "clientID", + Prompt: &survey.Input{ + Message: "Jira App Client ID:", + Help: "This is the client ID of your Jira App that you created for OAuth authentication.", + }, + }) + + questions = append(questions, &survey.Question{ + Name: "clientSecret", + Prompt: &survey.Password{ + Message: "Jira App Client Secret:", + Help: "This is the client secret of your Jira App that you created for OAuth authentication.", + }, + }) + + questions = append(questions, &survey.Question{ + Name: "redirectURI", + Prompt: &survey.Input{ + Default: defaultRedirectURI, + Message: "Redirect URI:", + Help: "The redirect URL for Jira App. Recommended to set as localhost.", + }, + }) + + if err := survey.Ask(questions, &answers, survey.WithValidator(survey.Required)); err != nil { + return nil, err + } + + return &Config{ + ClientID: answers.ClientID, + ClientSecret: Secret{Key: "client_secret", Value: answers.ClientSecret}, + RedirectURI: answers.RedirectURI, + Scopes: []string{ + "read:jira-user", + "read:jira-work", + "write:jira-work", + "offline_access", + "read:board-scope:jira-software", + "read:project:jira", + }, + }, nil +} + +// performOAuthFlow executes the OAuth authorization flow +func performOAuthFlow(config *Config) (*oauth2.Token, error) { + s := cmdutil.Info("Starting OAuth flow...") + defer s.Stop() + + // OAuth2 configuration for JIRA + oauthConfig := &oauth2.Config{ + ClientID: config.ClientID, + ClientSecret: config.ClientSecret.String(), + RedirectURL: config.RedirectURI, + Scopes: config.Scopes, + Endpoint: oauth2.Endpoint{ + AuthURL: jiraAuthURL, + TokenURL: jiraTokenURL, + }, + } + + // Generate authorization URL with PKCE + verifier := oauth2.GenerateVerifier() + authURL := oauthConfig.AuthCodeURL(verifier, oauth2.AccessTypeOffline) + + // Start local server to handle callback + codeChan := make(chan string, 1) + errChan := make(chan error, 1) + + server := &http.Server{ + Addr: defaultPort, + Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == callbackPath { + code := r.URL.Query().Get("code") + if code == "" { + errChan <- fmt.Errorf("no authorization code received") + return + } + + // Send success response to browser + w.Header().Set("Content-Type", "text/html") + w.Write([]byte(` + + +You can close this window and return to the terminal.
+ + + + `)) + + codeChan <- code + } else { + http.NotFound(w, r) + } + }), + } + + // Start server in goroutine + go func() { + if err := server.ListenAndServe(); err != nil && err != http.ErrServerClosed { + errChan <- err + } + }() + + // Open browser for authorization + fmt.Printf("Opening browser for authorization...\n") + fmt.Printf("If the browser doesn't open automatically, please visit: %s\n", authURL) + + // Try to open browser + if err := browser.OpenURL(authURL); err != nil { + fmt.Printf("Could not open browser automatically: %v\n", err) + fmt.Printf("Please manually visit: %s\n", authURL) + } + + // Wait for authorization code + select { + case code := <-codeChan: + // Shutdown server + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + server.Shutdown(ctx) + + // Exchange code for token + s.Stop() + s = cmdutil.Info("Exchanging authorization code for access token...") + defer s.Stop() + + token, err := oauthConfig.Exchange(context.Background(), code) + if err != nil { + return nil, fmt.Errorf("failed to exchange code for token: %w", err) + } + + return token, nil + + case err := <-errChan: + // Shutdown server + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + server.Shutdown(ctx) + return nil, fmt.Errorf("OAuth flow failed: %w", err) + + case <-time.After(oauthTimeout): + // Shutdown server + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + server.Shutdown(ctx) + return nil, fmt.Errorf("OAuth flow timed out after %v", oauthTimeout) + } +} + +// getCloudID retrieves the Cloud ID for the authenticated user +func getCloudID(accessToken string) (string, error) { + s := cmdutil.Info("Fetching cloud ID...") + defer s.Stop() + + // Create HTTP client with bearer token + client := &http.Client{Timeout: 30 * time.Second} + + req, err := http.NewRequest("GET", "https://api.atlassian.com/oauth/token/accessible-resources", nil) + if err != nil { + return "", err + } + + req.Header.Set("Authorization", "Bearer "+accessToken) + req.Header.Set("Accept", "application/json") + + resp, err := client.Do(req) + if err != nil { + return "", err + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return "", fmt.Errorf("failed to get accessible resources: status %d", resp.StatusCode) + } + + // Parse response to get cloud ID + var resourceResponse []struct { + ID string `json:"id"` + Name string `json:"name"` + URL string `json:"url"` + Scopes []string `json:"scopes"` + AvatarURL string `json:"avatarUrl"` + } + + if err := json.NewDecoder(resp.Body).Decode(&resourceResponse); err != nil { + return "", fmt.Errorf("failed to decode accessible resources response: %w", err) + } + + if len(resourceResponse) == 0 { + return "", fmt.Errorf("no accessible resources found or cloud ID not found") + } + + return resourceResponse[0].ID, nil +} + +func getJiraConfigDir() (string, error) { + home, err := cmdutil.GetConfigHome() + if err != nil { + return "", err + } + return filepath.Join(home, ".jira"), nil +} From 007f7614d0b72bf47a6b3f046094c9307a37fcb0 Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Sat, 12 Jul 2025 23:58:42 +0100 Subject: [PATCH 06/70] (refactor) Remove unnecessary args from `verifyLoginDetails` and `configureServerMeta` --- internal/config/generator.go | 67 ++++++++---------------------------- 1 file changed, 14 insertions(+), 53 deletions(-) diff --git a/internal/config/generator.go b/internal/config/generator.go index 008d2ecc..0b63835f 100644 --- a/internal/config/generator.go +++ b/internal/config/generator.go @@ -195,7 +195,7 @@ func (c *JiraCLIConfigGenerator) Generate() (string, error) { } if c.value.installation == jira.InstallationTypeLocal { - if err := c.configureServerMeta(c.value.server, c.value.login); err != nil { + if err := c.configureServerMeta(); err != nil { return "", err } } @@ -474,8 +474,9 @@ func (c *JiraCLIConfigGenerator) configureServerAndLoginDetails() error { c.value.login = strings.TrimSpace(ans.Login) } } - - return c.verifyLoginDetails(c.value.server, c.value.login) + // Trim trailing slash from server URL + c.value.server = strings.TrimRight(c.value.server, "/") + return c.verifyLoginDetails() } func (c *JiraCLIConfigGenerator) generateJiraConfig() jira.Config { config := jira.Config{ @@ -499,74 +500,34 @@ func (c *JiraCLIConfigGenerator) generateJiraConfig() jira.Config { return config } -func (c *JiraCLIConfigGenerator) verifyLoginDetails(server, login string) error { +func (c *JiraCLIConfigGenerator) verifyLoginDetails() error { s := cmdutil.Info("Verifying login details...") defer s.Stop() - - server = strings.TrimRight(server, "/") - if c.value.authType == jira.AuthTypeOAuth { - c.jiraClient = api.Client(jira.Config{ - Server: server, - Login: login, - APIToken: c.value.oauth.accessToken, - Insecure: &c.usrCfg.Insecure, - AuthType: &c.value.authType, - Debug: viper.GetBool("debug"), - MTLSConfig: jira.MTLSConfig{ - CaCert: c.value.mtls.caCert, - ClientCert: c.value.mtls.clientCert, - ClientKey: c.value.mtls.clientKey, - }, - }) - } else { - - } - c.jiraClient = api.Client(jira.Config{ - Server: server, - Login: login, - Insecure: &c.usrCfg.Insecure, - AuthType: &c.value.authType, - Debug: viper.GetBool("debug"), - MTLSConfig: jira.MTLSConfig{ - CaCert: c.value.mtls.caCert, - ClientCert: c.value.mtls.clientCert, - ClientKey: c.value.mtls.clientKey, - }, - }) + // Configure JIRA client based on auth type + config := c.generateJiraConfig() + c.jiraClient = api.Client(config) ret, err := c.jiraClient.Me() if err != nil { return err } if c.value.authType == jira.AuthTypeBearer { - login = ret.Login + c.value.login = ret.Login } - c.value.server = server - c.value.login = login c.value.timezone = ret.Timezone return nil } -func (c *JiraCLIConfigGenerator) configureServerMeta(server, login string) error { +func (c *JiraCLIConfigGenerator) configureServerMeta() error { s := cmdutil.Info("Fetching server details...") defer s.Stop() - server = strings.TrimRight(server, "/") - - c.jiraClient = api.Client(jira.Config{ - Server: server, - Login: login, - Insecure: &c.usrCfg.Insecure, - AuthType: &c.value.authType, - Debug: viper.GetBool("debug"), - MTLSConfig: jira.MTLSConfig{ - CaCert: c.value.mtls.caCert, - ClientCert: c.value.mtls.clientCert, - ClientKey: c.value.mtls.clientKey, - }, - }) + if c.jiraClient != nil { + config := c.generateJiraConfig() + c.jiraClient = api.Client(config) + } info, err := c.jiraClient.ServerInfo() if err != nil { return err From e4cc1c2fe50944c68c8cf5b858d29efb557d149b Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Sun, 13 Jul 2025 00:53:57 +0100 Subject: [PATCH 07/70] (tests): Add tests for oauth --- pkg/oauth/oauth_test.go | 653 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 653 insertions(+) create mode 100644 pkg/oauth/oauth_test.go diff --git a/pkg/oauth/oauth_test.go b/pkg/oauth/oauth_test.go new file mode 100644 index 00000000..32257e06 --- /dev/null +++ b/pkg/oauth/oauth_test.go @@ -0,0 +1,653 @@ +package oauth + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "golang.org/x/oauth2" +) + +func TestGetJiraConfigDir(t *testing.T) { + t.Parallel() + + // Save original environment + originalHome := os.Getenv("HOME") + originalXDG := os.Getenv("XDG_CONFIG_HOME") + defer func() { + os.Setenv("HOME", originalHome) + os.Setenv("XDG_CONFIG_HOME", originalXDG) + }() + + t.Run("uses XDG_CONFIG_HOME when set", func(t *testing.T) { + os.Setenv("XDG_CONFIG_HOME", "/tmp/test-config") + os.Setenv("HOME", "/tmp/test-home") + + dir, err := getJiraConfigDir() + assert.NoError(t, err) + assert.Equal(t, "/tmp/test-config/.jira", dir) + }) + + t.Run("falls back to HOME/.config when XDG_CONFIG_HOME not set", func(t *testing.T) { + os.Unsetenv("XDG_CONFIG_HOME") + os.Setenv("HOME", "/tmp/test-home") + + dir, err := getJiraConfigDir() + assert.NoError(t, err) + assert.Equal(t, "/tmp/test-home/.config/.jira", dir) + }) +} + +func TestGetCloudID(t *testing.T) { + t.Parallel() + + t.Run("successfully retrieves cloud ID", func(t *testing.T) { + expectedCloudID := "test-cloud-id-123" + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Verify request + assert.Equal(t, "GET", r.Method) + assert.Equal(t, "/oauth/token/accessible-resources", r.URL.Path) + assert.Equal(t, "Bearer test-access-token", r.Header.Get("Authorization")) + assert.Equal(t, "application/json", r.Header.Get("Accept")) + + // Return mock response + response := []map[string]interface{}{ + { + "id": expectedCloudID, + "name": "Test Site", + "url": "https://test.atlassian.net", + "scopes": []string{"read:jira-user", "read:jira-work"}, + "avatarUrl": "https://test.atlassian.net/avatar.png", + }, + } + + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(response) + })) + defer server.Close() + + // Test with mock server - this requires refactoring the function to accept a custom URL + // For now, we'll test the error cases and create a separate testable function + cloudID, err := getCloudIDFromURL(server.URL+"/oauth/token/accessible-resources", "test-access-token") + assert.NoError(t, err) + assert.Equal(t, expectedCloudID, cloudID) + }) + + t.Run("handles HTTP error", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + })) + defer server.Close() + + cloudID, err := getCloudIDFromURL(server.URL+"/oauth/token/accessible-resources", "invalid-token") + assert.Error(t, err) + assert.Empty(t, cloudID) + assert.Contains(t, err.Error(), "failed to get accessible resources: status 401") + }) + + t.Run("handles invalid JSON response", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte("invalid json")) + })) + defer server.Close() + + cloudID, err := getCloudIDFromURL(server.URL+"/oauth/token/accessible-resources", "test-token") + assert.Error(t, err) + assert.Empty(t, cloudID) + assert.Contains(t, err.Error(), "failed to decode accessible resources response") + }) + + t.Run("handles empty response", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode([]map[string]interface{}{}) + })) + defer server.Close() + + cloudID, err := getCloudIDFromURL(server.URL+"/oauth/token/accessible-resources", "test-token") + assert.Error(t, err) + assert.Empty(t, cloudID) + assert.Contains(t, err.Error(), "no accessible resources found") + }) +} + +// Helper function to make getCloudID testable +func getCloudIDFromURL(url, accessToken string) (string, error) { + client := &http.Client{Timeout: 30 * time.Second} + + req, err := http.NewRequest("GET", url, nil) + if err != nil { + return "", err + } + + req.Header.Set("Authorization", "Bearer "+accessToken) + req.Header.Set("Accept", "application/json") + + resp, err := client.Do(req) + if err != nil { + return "", err + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return "", fmt.Errorf("failed to get accessible resources: status %d", resp.StatusCode) + } + + var resourceResponse []struct { + ID string `json:"id"` + Name string `json:"name"` + URL string `json:"url"` + Scopes []string `json:"scopes"` + AvatarURL string `json:"avatarUrl"` + } + + if err := json.NewDecoder(resp.Body).Decode(&resourceResponse); err != nil { + return "", fmt.Errorf("failed to decode accessible resources response: %w", err) + } + + if len(resourceResponse) == 0 { + return "", fmt.Errorf("no accessible resources found or cloud ID not found") + } + + return resourceResponse[0].ID, nil +} + +func TestConfig(t *testing.T) { + t.Parallel() + + t.Run("creates config with all required fields", func(t *testing.T) { + config := &Config{ + ClientID: "test-client-id", + ClientSecret: Secret{Key: "client_secret", Value: "test-secret"}, + RedirectURI: "http://localhost:9876/callback", + Scopes: []string{"read:jira-user", "read:jira-work"}, + } + + assert.Equal(t, "test-client-id", config.ClientID) + assert.Equal(t, "test-secret", config.ClientSecret.String()) + assert.Equal(t, "http://localhost:9876/callback", config.RedirectURI) + assert.Contains(t, config.Scopes, "read:jira-user") + assert.Contains(t, config.Scopes, "read:jira-work") + }) +} + +func TestConfigureTokenResponse(t *testing.T) { + t.Parallel() + + t.Run("creates token response with all required fields", func(t *testing.T) { + response := &ConfigureTokenResponse{ + AccessToken: Secret{Key: "access_token", Value: "test-access-token"}, + RefreshToken: Secret{Key: "refresh_token", Value: "test-refresh-token"}, + CloudID: "test-cloud-id", + } + + assert.Equal(t, "test-access-token", response.AccessToken.String()) + assert.Equal(t, "test-refresh-token", response.RefreshToken.String()) + assert.Equal(t, "test-cloud-id", response.CloudID) + }) +} + +func TestPerformOAuthFlow_ErrorCases(t *testing.T) { + t.Parallel() + + t.Run("handles timeout", func(t *testing.T) { + config := &Config{ + ClientID: "test-client-id", + ClientSecret: Secret{Key: "client_secret", Value: "test-secret"}, + RedirectURI: "http://localhost:9876/callback", + Scopes: []string{"read:jira-user"}, + } + + // Create a version of performOAuthFlow with a shorter timeout for testing + token, err := performOAuthFlowWithTimeout(config, 100*time.Millisecond) + assert.Error(t, err) + assert.Nil(t, token) + assert.Contains(t, err.Error(), "OAuth flow timed out") + }) + + t.Run("handles server startup error", func(t *testing.T) { + config := &Config{ + ClientID: "test-client-id", + ClientSecret: Secret{Key: "client_secret", Value: "test-secret"}, + RedirectURI: "http://localhost:9876/callback", + Scopes: []string{"read:jira-user"}, + } + + // Start a server on the same port to cause a conflict + conflictServer := &http.Server{Addr: defaultPort} + go conflictServer.ListenAndServe() + defer conflictServer.Close() + + // Wait a bit for the server to start + time.Sleep(100 * time.Millisecond) + + // This should fail due to port conflict + token, err := performOAuthFlowWithTimeout(config, 1*time.Second) + // The error might be about port conflict or timeout, both are acceptable + assert.Error(t, err) + assert.Nil(t, token) + }) +} + +// Helper function to test OAuth flow with custom timeout +func performOAuthFlowWithTimeout(config *Config, timeout time.Duration) (*oauth2.Token, error) { + oauthConfig := &oauth2.Config{ + ClientID: config.ClientID, + ClientSecret: config.ClientSecret.String(), + RedirectURL: config.RedirectURI, + Scopes: config.Scopes, + Endpoint: oauth2.Endpoint{ + AuthURL: jiraAuthURL, + TokenURL: jiraTokenURL, + }, + } + + verifier := oauth2.GenerateVerifier() + _ = oauthConfig.AuthCodeURL(verifier, oauth2.AccessTypeOffline) + + codeChan := make(chan string, 1) + errChan := make(chan error, 1) + + server := &http.Server{ + Addr: defaultPort, + Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == callbackPath { + code := r.URL.Query().Get("code") + if code == "" { + errChan <- fmt.Errorf("no authorization code received") + return + } + + w.Header().Set("Content-Type", "text/html") + w.Write([]byte(`You can close this window and return to the terminal.
+ + + + `)) + } + } + }) + + req := httptest.NewRequest("GET", "http://localhost:9876/callback?code=test-code", nil) + w := httptest.NewRecorder() + + handler.ServeHTTP(w, req) + + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, "text/html", w.Header().Get("Content-Type")) + assert.Contains(t, w.Body.String(), "Authorization successful!") + assert.Contains(t, w.Body.String(), "window.close()") + }) +} From 691edbbba5d8d038e41f703978a435a81f3f37cd Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Sun, 13 Jul 2025 01:02:57 +0100 Subject: [PATCH 08/70] (nit) Move getToken logic into separate func --- api/client.go | 45 ++++++++++++++++++++++++++++++++------------- 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/api/client.go b/api/client.go index 5ef0739d..1d678ff0 100644 --- a/api/client.go +++ b/api/client.go @@ -15,6 +15,35 @@ const clientTimeout = 15 * time.Second var jiraClient *jira.Client +// getAPIToken retrieves the API token from various sources in order of priority: +// 1. Viper configuration +// 2. Netrc file +// 3. Keyring +func getAPIToken(config *jira.Config) string { + if config.APIToken != "" { + return config.APIToken + } + + // Try viper config first + if token := viper.GetString("api_token"); token != "" { + return token + } + + // Try netrc file + if netrcConfig, _ := netrc.Read(config.Server, config.Login); netrcConfig != nil { + if netrcConfig.Password != "" { + return netrcConfig.Password + } + } + + // Try keyring + if secret, _ := keyring.Get("jira-cli", config.Login); secret != "" { + return secret + } + + return "" +} + // Client initializes and returns jira client. func Client(config jira.Config) *jira.Client { if jiraClient != nil { @@ -32,24 +61,14 @@ func Client(config jira.Config) *jira.Client { authType := jira.AuthType(viper.GetString("auth_type")) config.AuthType = &authType } - if config.APIToken == "" { - config.APIToken = viper.GetString("api_token") - } - if config.APIToken == "" { - netrcConfig, _ := netrc.Read(config.Server, config.Login) - if netrcConfig != nil { - config.APIToken = netrcConfig.Password - } - } - if config.APIToken == "" { - secret, _ := keyring.Get("jira-cli", config.Login) - config.APIToken = secret - } if config.Insecure == nil { insecure := viper.GetBool("insecure") config.Insecure = &insecure } + // Get API token from various sources + config.APIToken = getAPIToken(&config) + // MTLS if config.MTLSConfig.CaCert == "" { From f57474b5154773becbca585bc7aa09078d0ab204 Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Mon, 14 Jul 2025 21:23:10 -0400 Subject: [PATCH 09/70] (refactor) Organizing the oauth to break up the logic to other files --- pkg/oauth/oauth.go | 116 ++++----------- pkg/oauth/oauth_test.go | 298 ++++++-------------------------------- pkg/utils/secrets.go | 35 +++++ pkg/utils/secrets_test.go | 93 ++++++++++++ pkg/utils/storage.go | 58 ++++++++ pkg/utils/storage_test.go | 79 ++++++++++ 6 files changed, 340 insertions(+), 339 deletions(-) create mode 100644 pkg/utils/secrets.go create mode 100644 pkg/utils/secrets_test.go create mode 100644 pkg/utils/storage.go create mode 100644 pkg/utils/storage_test.go diff --git a/pkg/oauth/oauth.go b/pkg/oauth/oauth.go index 2f95ec20..d93b4374 100644 --- a/pkg/oauth/oauth.go +++ b/pkg/oauth/oauth.go @@ -5,7 +5,6 @@ import ( "encoding/json" "fmt" "net/http" - "os" "path/filepath" "time" @@ -14,12 +13,14 @@ import ( "golang.org/x/oauth2" "github.com/ankitpokhrel/jira-cli/internal/cmdutil" + "github.com/ankitpokhrel/jira-cli/pkg/utils" ) const ( // JIRA OAuth2 endpoints - jiraAuthURL = "https://auth.atlassian.com/authorize" - jiraTokenURL = "https://auth.atlassian.com/oauth/token" + jiraAuthURL = "https://auth.atlassian.com/authorize" + jiraTokenURL = "https://auth.atlassian.com/oauth/token" + accessibleResourcesURL = "https://api.atlassian.com/oauth/token/accessible-resources" // Default OAuth settings defaultRedirectURI = "http://localhost:9876/callback" @@ -30,93 +31,39 @@ const ( oauthTimeout = 5 * time.Minute ) -const ( - OWNER_ONLY = 0o700 - OWNER_READ_WRITE = 0o600 -) - -// Storage defines the interface for secret storage operations -type Storage interface { - Save(key string, value []byte) error - Load(key string) ([]byte, error) -} - -// Secret represents a secret value with storage capabilities -type Secret struct { - Key string - Value string -} - -func (s Secret) String() string { - return s.Value +var defaultScopes = []string{ + "read:jira-user", + "read:jira-work", + "read:board-scope:jira-software", + "read:project:jira", + "write:jira-work", + "offline_access", // This is required to get the refresh token from JIRA } -func (s Secret) Save(storage Storage) error { - if s.Key == "" { - return fmt.Errorf("secret key cannot be empty") - } - return storage.Save(s.Key, []byte(s.Value)) -} - -func (s *Secret) Load(storage Storage, key string) error { - if key == "" { - return fmt.Errorf("secret key cannot be empty") - } - - data, err := storage.Load(key) - if err != nil { - return err - } - - s.Key = key - s.Value = string(data) - return nil -} - -// FileSystemStorage implements Storage interface for filesystem operations -type FileSystemStorage struct { - BaseDir string -} - -func (fs FileSystemStorage) Save(key string, value []byte) error { - if err := os.MkdirAll(fs.BaseDir, OWNER_ONLY); err != nil { - return fmt.Errorf("failed to create directory: %w", err) - } - - filePath := filepath.Join(fs.BaseDir, key) - return os.WriteFile(filePath, value, OWNER_READ_WRITE) -} - -func (fs FileSystemStorage) Load(key string) ([]byte, error) { - filePath := filepath.Join(fs.BaseDir, key) - return os.ReadFile(filePath) -} - -// Config holds OAuth configuration -type Config struct { +// OAuthConfig holds OAuth configuration +type OAuthConfig struct { ClientID string - ClientSecret Secret + ClientSecret utils.Secret RedirectURI string Scopes []string } // ConfigureTokenResponse holds the OAuth token response type ConfigureTokenResponse struct { - AccessToken Secret - RefreshToken Secret + AccessToken utils.Secret + RefreshToken utils.Secret CloudID string } // Configure performs the complete OAuth flow and returns tokens func Configure() (*ConfigureTokenResponse, error) { // Collect OAuth credentials from user - jiraDir, err := getJiraConfigDir() if err != nil { return nil, fmt.Errorf("failed to get Jira config directory: %w", err) } - secretStorage := FileSystemStorage{BaseDir: jiraDir} + secretStorage := utils.FileSystemStorage{BaseDir: jiraDir} config, err := collectOAuthCredentials() if err != nil { @@ -128,15 +75,13 @@ func Configure() (*ConfigureTokenResponse, error) { if err != nil { return nil, fmt.Errorf("OAuth flow failed: %w", err) } - + accessToken := utils.Secret{Key: "access_token", Value: tokens.AccessToken} + refreshToken := utils.Secret{Key: "refresh_token", Value: tokens.RefreshToken} // Store client secret securely if err := config.ClientSecret.Save(secretStorage); err != nil { return nil, fmt.Errorf("failed to store client secret: %w", err) } - accessToken := Secret{Key: "access_token", Value: tokens.AccessToken} - refreshToken := Secret{Key: "refresh_token", Value: tokens.RefreshToken} - if err := accessToken.Save(secretStorage); err != nil { return nil, fmt.Errorf("failed to store access token: %w", err) } @@ -145,7 +90,7 @@ func Configure() (*ConfigureTokenResponse, error) { return nil, fmt.Errorf("failed to store refresh token: %w", err) } // Get Cloud ID for Atlassian API - cloudID, err := getCloudID(tokens.AccessToken) + cloudID, err := getCloudID(accessibleResourcesURL, tokens.AccessToken) if err != nil { return nil, fmt.Errorf("failed to get cloud ID: %w", err) } @@ -158,7 +103,7 @@ func Configure() (*ConfigureTokenResponse, error) { } // collectOAuthCredentials collects OAuth credentials from the user -func collectOAuthCredentials() (*Config, error) { +func collectOAuthCredentials() (*OAuthConfig, error) { var questions []*survey.Question answers := struct { ClientID string @@ -195,23 +140,16 @@ func collectOAuthCredentials() (*Config, error) { return nil, err } - return &Config{ + return &OAuthConfig{ ClientID: answers.ClientID, - ClientSecret: Secret{Key: "client_secret", Value: answers.ClientSecret}, + ClientSecret: utils.Secret{Key: "client_secret", Value: answers.ClientSecret}, RedirectURI: answers.RedirectURI, - Scopes: []string{ - "read:jira-user", - "read:jira-work", - "write:jira-work", - "offline_access", - "read:board-scope:jira-software", - "read:project:jira", - }, + Scopes: defaultScopes, }, nil } // performOAuthFlow executes the OAuth authorization flow -func performOAuthFlow(config *Config) (*oauth2.Token, error) { +func performOAuthFlow(config *OAuthConfig) (*oauth2.Token, error) { s := cmdutil.Info("Starting OAuth flow...") defer s.Stop() @@ -318,14 +256,14 @@ func performOAuthFlow(config *Config) (*oauth2.Token, error) { } // getCloudID retrieves the Cloud ID for the authenticated user -func getCloudID(accessToken string) (string, error) { +func getCloudID(url string, accessToken string) (string, error) { s := cmdutil.Info("Fetching cloud ID...") defer s.Stop() // Create HTTP client with bearer token client := &http.Client{Timeout: 30 * time.Second} - req, err := http.NewRequest("GET", "https://api.atlassian.com/oauth/token/accessible-resources", nil) + req, err := http.NewRequest("GET", url, nil) if err != nil { return "", err } diff --git a/pkg/oauth/oauth_test.go b/pkg/oauth/oauth_test.go index 32257e06..618a940b 100644 --- a/pkg/oauth/oauth_test.go +++ b/pkg/oauth/oauth_test.go @@ -7,10 +7,10 @@ import ( "net/http" "net/http/httptest" "os" - "path/filepath" "testing" "time" + "github.com/ankitpokhrel/jira-cli/pkg/utils" "github.com/stretchr/testify/assert" "golang.org/x/oauth2" ) @@ -75,7 +75,39 @@ func TestGetCloudID(t *testing.T) { // Test with mock server - this requires refactoring the function to accept a custom URL // For now, we'll test the error cases and create a separate testable function - cloudID, err := getCloudIDFromURL(server.URL+"/oauth/token/accessible-resources", "test-access-token") + cloudID, err := getCloudID(server.URL+"/oauth/token/accessible-resources", "test-access-token") + assert.NoError(t, err) + assert.Equal(t, expectedCloudID, cloudID) + }) + + t.Run("successfully gets jira cloud id from list of accessible resources", func(t *testing.T) { + expectedCloudID := "test-cloud-id-123" + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Verify request + assert.Equal(t, "GET", r.Method) + assert.Equal(t, "/oauth/token/accessible-resources", r.URL.Path) + assert.Equal(t, "Bearer test-access-token", r.Header.Get("Authorization")) + assert.Equal(t, "application/json", r.Header.Get("Accept")) + + // Return mock response + response := []map[string]interface{}{ + { + "id": expectedCloudID, + "name": "Test Site", + "url": "https://test.atlassian.net", + "scopes": []string{"read:jira-user", "read:jira-work"}, + "avatarUrl": "https://test.atlassian.net/avatar.png", + }, + } + + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(response) + })) + defer server.Close() + + // Test with mock server - this requires refactoring the function to accept a custom URL + // For now, we'll test the error cases and create a separate testable function + cloudID, err := getCloudID(server.URL+"/oauth/token/accessible-resources", "test-access-token") assert.NoError(t, err) assert.Equal(t, expectedCloudID, cloudID) }) @@ -86,7 +118,7 @@ func TestGetCloudID(t *testing.T) { })) defer server.Close() - cloudID, err := getCloudIDFromURL(server.URL+"/oauth/token/accessible-resources", "invalid-token") + cloudID, err := getCloudID(server.URL+"/oauth/token/accessible-resources", "invalid-token") assert.Error(t, err) assert.Empty(t, cloudID) assert.Contains(t, err.Error(), "failed to get accessible resources: status 401") @@ -99,7 +131,7 @@ func TestGetCloudID(t *testing.T) { })) defer server.Close() - cloudID, err := getCloudIDFromURL(server.URL+"/oauth/token/accessible-resources", "test-token") + cloudID, err := getCloudID(server.URL+"/oauth/token/accessible-resources", "test-token") assert.Error(t, err) assert.Empty(t, cloudID) assert.Contains(t, err.Error(), "failed to decode accessible resources response") @@ -112,61 +144,20 @@ func TestGetCloudID(t *testing.T) { })) defer server.Close() - cloudID, err := getCloudIDFromURL(server.URL+"/oauth/token/accessible-resources", "test-token") + cloudID, err := getCloudID(server.URL+"/oauth/token/accessible-resources", "test-token") assert.Error(t, err) assert.Empty(t, cloudID) assert.Contains(t, err.Error(), "no accessible resources found") }) } -// Helper function to make getCloudID testable -func getCloudIDFromURL(url, accessToken string) (string, error) { - client := &http.Client{Timeout: 30 * time.Second} - - req, err := http.NewRequest("GET", url, nil) - if err != nil { - return "", err - } - - req.Header.Set("Authorization", "Bearer "+accessToken) - req.Header.Set("Accept", "application/json") - - resp, err := client.Do(req) - if err != nil { - return "", err - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - return "", fmt.Errorf("failed to get accessible resources: status %d", resp.StatusCode) - } - - var resourceResponse []struct { - ID string `json:"id"` - Name string `json:"name"` - URL string `json:"url"` - Scopes []string `json:"scopes"` - AvatarURL string `json:"avatarUrl"` - } - - if err := json.NewDecoder(resp.Body).Decode(&resourceResponse); err != nil { - return "", fmt.Errorf("failed to decode accessible resources response: %w", err) - } - - if len(resourceResponse) == 0 { - return "", fmt.Errorf("no accessible resources found or cloud ID not found") - } - - return resourceResponse[0].ID, nil -} - func TestConfig(t *testing.T) { t.Parallel() t.Run("creates config with all required fields", func(t *testing.T) { - config := &Config{ + config := &OAuthConfig{ ClientID: "test-client-id", - ClientSecret: Secret{Key: "client_secret", Value: "test-secret"}, + ClientSecret: utils.Secret{Key: "client_secret", Value: "test-secret"}, RedirectURI: "http://localhost:9876/callback", Scopes: []string{"read:jira-user", "read:jira-work"}, } @@ -184,8 +175,8 @@ func TestConfigureTokenResponse(t *testing.T) { t.Run("creates token response with all required fields", func(t *testing.T) { response := &ConfigureTokenResponse{ - AccessToken: Secret{Key: "access_token", Value: "test-access-token"}, - RefreshToken: Secret{Key: "refresh_token", Value: "test-refresh-token"}, + AccessToken: utils.Secret{Key: "access_token", Value: "test-access-token"}, + RefreshToken: utils.Secret{Key: "refresh_token", Value: "test-refresh-token"}, CloudID: "test-cloud-id", } @@ -199,9 +190,9 @@ func TestPerformOAuthFlow_ErrorCases(t *testing.T) { t.Parallel() t.Run("handles timeout", func(t *testing.T) { - config := &Config{ + config := &OAuthConfig{ ClientID: "test-client-id", - ClientSecret: Secret{Key: "client_secret", Value: "test-secret"}, + ClientSecret: utils.Secret{Key: "client_secret", Value: "test-secret"}, RedirectURI: "http://localhost:9876/callback", Scopes: []string{"read:jira-user"}, } @@ -214,9 +205,9 @@ func TestPerformOAuthFlow_ErrorCases(t *testing.T) { }) t.Run("handles server startup error", func(t *testing.T) { - config := &Config{ + config := &OAuthConfig{ ClientID: "test-client-id", - ClientSecret: Secret{Key: "client_secret", Value: "test-secret"}, + ClientSecret: utils.Secret{Key: "client_secret", Value: "test-secret"}, RedirectURI: "http://localhost:9876/callback", Scopes: []string{"read:jira-user"}, } @@ -238,7 +229,7 @@ func TestPerformOAuthFlow_ErrorCases(t *testing.T) { } // Helper function to test OAuth flow with custom timeout -func performOAuthFlowWithTimeout(config *Config, timeout time.Duration) (*oauth2.Token, error) { +func performOAuthFlowWithTimeout(config *OAuthConfig, timeout time.Duration) (*oauth2.Token, error) { oauthConfig := &oauth2.Config{ ClientID: config.ClientID, ClientSecret: config.ClientSecret.String(), @@ -307,24 +298,6 @@ func performOAuthFlowWithTimeout(config *Config, timeout time.Duration) (*oauth2 } } -func TestConstants(t *testing.T) { - t.Parallel() - - t.Run("verifies OAuth constants", func(t *testing.T) { - assert.Equal(t, "https://auth.atlassian.com/authorize", jiraAuthURL) - assert.Equal(t, "https://auth.atlassian.com/oauth/token", jiraTokenURL) - assert.Equal(t, "http://localhost:9876/callback", defaultRedirectURI) - assert.Equal(t, ":9876", defaultPort) - assert.Equal(t, "/callback", callbackPath) - assert.Equal(t, 5*time.Minute, oauthTimeout) - }) - - t.Run("verifies file permission constants", func(t *testing.T) { - assert.Equal(t, 0o700, int(OWNER_ONLY)) - assert.Equal(t, 0o600, int(OWNER_READ_WRITE)) - }) -} - func TestOAuthFlowIntegration(t *testing.T) { t.Parallel() @@ -346,9 +319,9 @@ func TestOAuthFlowIntegration(t *testing.T) { defer mockOAuthServer.Close() // Create config with mock server - config := &Config{ + config := &OAuthConfig{ ClientID: "test-client-id", - ClientSecret: Secret{Key: "client_secret", Value: "test-secret"}, + ClientSecret: utils.Secret{Key: "client_secret", Value: "test-secret"}, RedirectURI: "http://localhost:9876/callback", Scopes: []string{"read:jira-user"}, } @@ -443,181 +416,6 @@ func TestOAuthFlowIntegration(t *testing.T) { }) } -func TestFileSystemStorage(t *testing.T) { - t.Parallel() - - t.Run("creates directory and saves file", func(t *testing.T) { - // Create temporary directory - tempDir := t.TempDir() - storage := FileSystemStorage{BaseDir: tempDir} - - // Test saving - err := storage.Save("test-key", []byte("test-value")) - assert.NoError(t, err) - - // Verify file exists and has correct content - filePath := filepath.Join(tempDir, "test-key") - content, err := os.ReadFile(filePath) - assert.NoError(t, err) - assert.Equal(t, "test-value", string(content)) - - // Verify file permissions - info, err := os.Stat(filePath) - assert.NoError(t, err) - // File permissions on Unix systems can vary, so we just check that it's restrictive - assert.True(t, info.Mode().Perm() <= 0o600) - }) - - t.Run("loads file content", func(t *testing.T) { - // Create temporary directory - tempDir := t.TempDir() - storage := FileSystemStorage{BaseDir: tempDir} - - // Create test file - testContent := "test-content" - filePath := filepath.Join(tempDir, "test-key") - err := os.WriteFile(filePath, []byte(testContent), OWNER_READ_WRITE) - assert.NoError(t, err) - - // Test loading - content, err := storage.Load("test-key") - assert.NoError(t, err) - assert.Equal(t, testContent, string(content)) - }) - - t.Run("handles non-existent file", func(t *testing.T) { - tempDir := t.TempDir() - storage := FileSystemStorage{BaseDir: tempDir} - - // Test loading non-existent file - content, err := storage.Load("non-existent-key") - assert.Error(t, err) - assert.Nil(t, content) - }) - - t.Run("handles directory creation failure", func(t *testing.T) { - // Use a path that cannot be created (e.g., under a file instead of directory) - tempDir := t.TempDir() - - // Create a file where we want to create a directory - filePath := filepath.Join(tempDir, "blocking-file") - err := os.WriteFile(filePath, []byte("content"), 0644) - assert.NoError(t, err) - - // Try to create storage with the file as base directory - storage := FileSystemStorage{BaseDir: filePath} - - err = storage.Save("test-key", []byte("test-value")) - assert.Error(t, err) - assert.Contains(t, err.Error(), "failed to create directory") - }) -} - -func TestSecretOperations(t *testing.T) { - t.Parallel() - - t.Run("secret string representation", func(t *testing.T) { - secret := Secret{Key: "test-key", Value: "test-value"} - assert.Equal(t, "test-value", secret.String()) - }) - - t.Run("secret save with empty key", func(t *testing.T) { - secret := Secret{Key: "", Value: "test-value"} - storage := &mockStorage{} - - err := secret.Save(storage) - assert.Error(t, err) - assert.Contains(t, err.Error(), "secret key cannot be empty") - }) - - t.Run("secret save success", func(t *testing.T) { - secret := Secret{Key: "test-key", Value: "test-value"} - storage := &mockStorage{} - - err := secret.Save(storage) - assert.NoError(t, err) - assert.Equal(t, "test-key", storage.savedKey) - assert.Equal(t, []byte("test-value"), storage.savedValue) - }) - - t.Run("secret load with empty key", func(t *testing.T) { - secret := &Secret{} - storage := &mockStorage{} - - err := secret.Load(storage, "") - assert.Error(t, err) - assert.Contains(t, err.Error(), "secret key cannot be empty") - }) - - t.Run("secret load success", func(t *testing.T) { - secret := &Secret{} - storage := &mockStorage{ - loadReturn: []byte("loaded-value"), - } - - err := secret.Load(storage, "test-key") - assert.NoError(t, err) - assert.Equal(t, "test-key", secret.Key) - assert.Equal(t, "loaded-value", secret.Value) - }) - - t.Run("secret load with storage error", func(t *testing.T) { - secret := &Secret{} - storage := &mockStorage{ - loadError: fmt.Errorf("storage error"), - } - - err := secret.Load(storage, "test-key") - assert.Error(t, err) - assert.Contains(t, err.Error(), "storage error") - }) -} - -// Mock storage for testing -type mockStorage struct { - savedKey string - savedValue []byte - loadReturn []byte - loadError error - saveError error -} - -func (m *mockStorage) Save(key string, value []byte) error { - if m.saveError != nil { - return m.saveError - } - m.savedKey = key - m.savedValue = value - return nil -} - -func (m *mockStorage) Load(key string) ([]byte, error) { - if m.loadError != nil { - return nil, m.loadError - } - return m.loadReturn, nil -} - -func TestDefaultScopes(t *testing.T) { - t.Parallel() - - // Test that the default scopes include all required permissions - expectedScopes := []string{ - "read:jira-user", - "read:jira-work", - "write:jira-work", - "offline_access", - "read:board-scope:jira-software", - "read:project:jira", - } - - // This would typically be tested through collectOAuthCredentials, but since - // that function uses interactive prompts, we test the expected scopes directly - for _, scope := range expectedScopes { - assert.Contains(t, expectedScopes, scope, "Expected scope %s should be present", scope) - } -} - func TestHTMLResponse(t *testing.T) { t.Parallel() diff --git a/pkg/utils/secrets.go b/pkg/utils/secrets.go new file mode 100644 index 00000000..2833bca2 --- /dev/null +++ b/pkg/utils/secrets.go @@ -0,0 +1,35 @@ +package utils + +import "fmt" + +// Secret represents a secret value with storage capabilities +type Secret struct { + Key string + Value string +} + +func (s Secret) String() string { + return s.Value +} + +func (s Secret) Save(storage Storage) error { + if s.Key == "" { + return fmt.Errorf("secret key cannot be empty") + } + return storage.Save(s.Key, []byte(s.Value)) +} + +func (s *Secret) Load(storage Storage, key string) error { + if key == "" { + return fmt.Errorf("secret key cannot be empty") + } + + data, err := storage.Load(key) + if err != nil { + return err + } + + s.Key = key + s.Value = string(data) + return nil +} diff --git a/pkg/utils/secrets_test.go b/pkg/utils/secrets_test.go new file mode 100644 index 00000000..cdf65240 --- /dev/null +++ b/pkg/utils/secrets_test.go @@ -0,0 +1,93 @@ +package utils + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +// Mock storage for testing +type mockStorage struct { + savedKey string + savedValue []byte + loadReturn []byte + loadError error + saveError error +} + +func (m *mockStorage) Save(key string, value []byte) error { + if m.saveError != nil { + return m.saveError + } + m.savedKey = key + m.savedValue = value + return nil +} + +func (m *mockStorage) Load(key string) ([]byte, error) { + if m.loadError != nil { + return nil, m.loadError + } + return m.loadReturn, nil +} + +func TestSecretOperations(t *testing.T) { + t.Parallel() + + t.Run("secret string representation", func(t *testing.T) { + secret := Secret{Key: "test-key", Value: "test-value"} + assert.Equal(t, "test-value", secret.String()) + }) + + t.Run("secret save with empty key", func(t *testing.T) { + secret := Secret{Key: "", Value: "test-value"} + storage := &mockStorage{} + + err := secret.Save(storage) + assert.Error(t, err) + assert.Contains(t, err.Error(), "secret key cannot be empty") + }) + + t.Run("secret save success", func(t *testing.T) { + secret := Secret{Key: "test-key", Value: "test-value"} + storage := &mockStorage{} + + err := secret.Save(storage) + assert.NoError(t, err) + assert.Equal(t, "test-key", storage.savedKey) + assert.Equal(t, []byte("test-value"), storage.savedValue) + }) + + t.Run("secret load with empty key", func(t *testing.T) { + secret := &Secret{} + storage := &mockStorage{} + + err := secret.Load(storage, "") + assert.Error(t, err) + assert.Contains(t, err.Error(), "secret key cannot be empty") + }) + + t.Run("secret load success", func(t *testing.T) { + secret := &Secret{} + storage := &mockStorage{ + loadReturn: []byte("loaded-value"), + } + + err := secret.Load(storage, "test-key") + assert.NoError(t, err) + assert.Equal(t, "test-key", secret.Key) + assert.Equal(t, "loaded-value", secret.Value) + }) + + t.Run("secret load with storage error", func(t *testing.T) { + secret := &Secret{} + storage := &mockStorage{ + loadError: fmt.Errorf("storage error"), + } + + err := secret.Load(storage, "test-key") + assert.Error(t, err) + assert.Contains(t, err.Error(), "storage error") + }) +} diff --git a/pkg/utils/storage.go b/pkg/utils/storage.go new file mode 100644 index 00000000..5fe2cd1a --- /dev/null +++ b/pkg/utils/storage.go @@ -0,0 +1,58 @@ +package utils + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" +) + +type Storage interface { + Save(key string, value []byte) error + Load(key string) ([]byte, error) +} + +const ( + OWNER_ONLY = 0o700 + OWNER_READ_WRITE = 0o600 +) + +// FileSystemStorage implements Storage interface for filesystem operations +type FileSystemStorage struct { + // BaseDir is the directory where the storage will be saved + BaseDir string +} + +func (fs FileSystemStorage) Save(key string, value []byte) error { + if err := os.MkdirAll(fs.BaseDir, OWNER_ONLY); err != nil { + return fmt.Errorf("failed to create directory: %w", err) + } + + filePath := filepath.Join(fs.BaseDir, key) + return os.WriteFile(filePath, value, OWNER_READ_WRITE) +} + +func (fs FileSystemStorage) Load(key string) ([]byte, error) { + filePath := filepath.Join(fs.BaseDir, key) + return os.ReadFile(filePath) +} + +// SaveJSON saves a typed value as JSON using the provided storage +func SaveJSON[T any](storage Storage, key string, value T) error { + data, err := json.MarshalIndent(value, "", " ") + if err != nil { + return err + } + return storage.Save(key, data) +} + +// LoadJSON loads a typed value from JSON using the provided storage +func LoadJSON[T any](storage Storage, key string) (T, error) { + var result T + data, err := storage.Load(key) + if err != nil { + return result, err + } + err = json.Unmarshal(data, &result) + return result, err +} diff --git a/pkg/utils/storage_test.go b/pkg/utils/storage_test.go new file mode 100644 index 00000000..f9518fad --- /dev/null +++ b/pkg/utils/storage_test.go @@ -0,0 +1,79 @@ +package utils + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestFileSystemStorage(t *testing.T) { + t.Parallel() + + t.Run("creates directory and saves file", func(t *testing.T) { + // Create temporary directory + tempDir := t.TempDir() + storage := FileSystemStorage{BaseDir: tempDir} + + // Test saving + err := storage.Save("test-key", []byte("test-value")) + assert.NoError(t, err) + + // Verify file exists and has correct content + filePath := filepath.Join(tempDir, "test-key") + content, err := os.ReadFile(filePath) + assert.NoError(t, err) + assert.Equal(t, "test-value", string(content)) + + // Verify file permissions + info, err := os.Stat(filePath) + assert.NoError(t, err) + // File permissions on Unix systems can vary, so we just check that it's restrictive + assert.True(t, info.Mode().Perm() <= 0o600) + }) + + t.Run("loads file content", func(t *testing.T) { + // Create temporary directory + tempDir := t.TempDir() + storage := FileSystemStorage{BaseDir: tempDir} + + // Create test file + testContent := "test-content" + filePath := filepath.Join(tempDir, "test-key") + err := os.WriteFile(filePath, []byte(testContent), OWNER_READ_WRITE) + assert.NoError(t, err) + + // Test loading + content, err := storage.Load("test-key") + assert.NoError(t, err) + assert.Equal(t, testContent, string(content)) + }) + + t.Run("handles non-existent file", func(t *testing.T) { + tempDir := t.TempDir() + storage := FileSystemStorage{BaseDir: tempDir} + + // Test loading non-existent file + content, err := storage.Load("non-existent-key") + assert.Error(t, err) + assert.Nil(t, content) + }) + + t.Run("handles directory creation failure", func(t *testing.T) { + // Use a path that cannot be created (e.g., under a file instead of directory) + tempDir := t.TempDir() + + // Create a file where we want to create a directory + filePath := filepath.Join(tempDir, "blocking-file") + err := os.WriteFile(filePath, []byte("content"), 0644) + assert.NoError(t, err) + + // Try to create storage with the file as base directory + storage := FileSystemStorage{BaseDir: filePath} + + err = storage.Save("test-key", []byte("test-value")) + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to create directory") + }) +} From 40402e61a96ae8db602090c79ac4eb2deec54892 Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Mon, 14 Jul 2025 22:11:39 -0400 Subject: [PATCH 10/70] (feat) Add the ability to load the persisted oauth token from filesystem --- api/client.go | 17 ++++++- internal/cmd/root/root.go | 5 ++ internal/config/generator.go | 4 +- pkg/oauth/oauth.go | 92 ++++++++++++++++++++++++++++-------- 4 files changed, 94 insertions(+), 24 deletions(-) diff --git a/api/client.go b/api/client.go index 1d678ff0..fe5875ca 100644 --- a/api/client.go +++ b/api/client.go @@ -9,6 +9,7 @@ import ( "github.com/ankitpokhrel/jira-cli/pkg/jira" "github.com/ankitpokhrel/jira-cli/pkg/jira/filter" "github.com/ankitpokhrel/jira-cli/pkg/netrc" + "github.com/ankitpokhrel/jira-cli/pkg/oauth" ) const clientTimeout = 15 * time.Second @@ -17,8 +18,9 @@ var jiraClient *jira.Client // getAPIToken retrieves the API token from various sources in order of priority: // 1. Viper configuration -// 2. Netrc file -// 3. Keyring +// 2. OAuth access token (if available and valid) +// 3. Netrc file +// 4. Keyring func getAPIToken(config *jira.Config) string { if config.APIToken != "" { return config.APIToken @@ -29,6 +31,11 @@ func getAPIToken(config *jira.Config) string { return token } + // Try OAuth access token if available and valid + if oauthToken := oauth.GetValidAccessToken(); oauthToken != "" { + return oauthToken + } + // Try netrc file if netrcConfig, _ := netrc.Read(config.Server, config.Login); netrcConfig != nil { if netrcConfig.Password != "" { @@ -69,6 +76,12 @@ func Client(config jira.Config) *jira.Client { // Get API token from various sources config.APIToken = getAPIToken(&config) + // If we have an OAuth token, set auth type to OAuth + if oauthToken := oauth.GetValidAccessToken(); oauthToken != "" && config.APIToken == oauthToken { + oauthAuthType := jira.AuthTypeOAuth + config.AuthType = &oauthAuthType + } + // MTLS if config.MTLSConfig.CaCert == "" { diff --git a/internal/cmd/root/root.go b/internal/cmd/root/root.go index 85cb9ab1..41d2358b 100644 --- a/internal/cmd/root/root.go +++ b/internal/cmd/root/root.go @@ -25,6 +25,7 @@ import ( jiraConfig "github.com/ankitpokhrel/jira-cli/internal/config" "github.com/ankitpokhrel/jira-cli/pkg/jira" "github.com/ankitpokhrel/jira-cli/pkg/netrc" + "github.com/ankitpokhrel/jira-cli/pkg/oauth" "github.com/zalando/go-keyring" ) @@ -156,6 +157,10 @@ func cmdRequireToken(cmd string) bool { } func checkForJiraToken(server string, login string) { + if oauthToken := oauth.GetValidAccessToken(); oauthToken != "" { + return + } + if os.Getenv("JIRA_API_TOKEN") != "" { return } diff --git a/internal/config/generator.go b/internal/config/generator.go index 0b63835f..15b82304 100644 --- a/internal/config/generator.go +++ b/internal/config/generator.go @@ -357,8 +357,8 @@ func (c *JiraCLIConfigGenerator) configureOAuth() error { } // Store the tokens and cloud ID - c.value.oauth.accessToken = tokenResponse.AccessToken.String() - c.value.oauth.refreshToken = tokenResponse.RefreshToken.String() + c.value.oauth.accessToken = tokenResponse.AccessToken + c.value.oauth.refreshToken = tokenResponse.RefreshToken c.value.oauth.cloudId = tokenResponse.CloudID return nil diff --git a/pkg/oauth/oauth.go b/pkg/oauth/oauth.go index d93b4374..4090aade 100644 --- a/pkg/oauth/oauth.go +++ b/pkg/oauth/oauth.go @@ -29,6 +29,9 @@ const ( // OAuth timeout oauthTimeout = 5 * time.Minute + + // OAuth storage file name + oauthSecretsFile = "oauth_secrets.json" ) var defaultScopes = []string{ @@ -43,18 +46,37 @@ var defaultScopes = []string{ // OAuthConfig holds OAuth configuration type OAuthConfig struct { ClientID string - ClientSecret utils.Secret + ClientSecret string RedirectURI string Scopes []string } +// OAuthSecrets holds all OAuth secrets in a single structure +type OAuthSecrets struct { + ClientSecret string `json:"client_secret"` + AccessToken string `json:"access_token"` + RefreshToken string `json:"refresh_token"` + TokenType string `json:"token_type"` + Expiry time.Time `json:"expiry"` +} + // ConfigureTokenResponse holds the OAuth token response type ConfigureTokenResponse struct { - AccessToken utils.Secret - RefreshToken utils.Secret + AccessToken string + RefreshToken string CloudID string } +// IsExpired checks if the access token is expired +func (o *OAuthSecrets) IsExpired() bool { + return time.Now().After(o.Expiry) +} + +// IsValid checks if the OAuth secrets are valid and not expired +func (o *OAuthSecrets) IsValid() bool { + return o.AccessToken != "" && !o.IsExpired() +} + // Configure performs the complete OAuth flow and returns tokens func Configure() (*ConfigureTokenResponse, error) { // Collect OAuth credentials from user @@ -75,33 +97,63 @@ func Configure() (*ConfigureTokenResponse, error) { if err != nil { return nil, fmt.Errorf("OAuth flow failed: %w", err) } - accessToken := utils.Secret{Key: "access_token", Value: tokens.AccessToken} - refreshToken := utils.Secret{Key: "refresh_token", Value: tokens.RefreshToken} - // Store client secret securely - if err := config.ClientSecret.Save(secretStorage); err != nil { - return nil, fmt.Errorf("failed to store client secret: %w", err) - } - if err := accessToken.Save(secretStorage); err != nil { - return nil, fmt.Errorf("failed to store access token: %w", err) - } - - if err := refreshToken.Save(secretStorage); err != nil { - return nil, fmt.Errorf("failed to store refresh token: %w", err) - } // Get Cloud ID for Atlassian API cloudID, err := getCloudID(accessibleResourcesURL, tokens.AccessToken) if err != nil { return nil, fmt.Errorf("failed to get cloud ID: %w", err) } + // Store all OAuth secrets in a single JSON file + oauthSecrets := &OAuthSecrets{ + ClientSecret: config.ClientSecret, + AccessToken: tokens.AccessToken, + RefreshToken: tokens.RefreshToken, + TokenType: tokens.TokenType, + Expiry: tokens.Expiry, + } + + if err := utils.SaveJSON(secretStorage, oauthSecretsFile, oauthSecrets); err != nil { + return nil, fmt.Errorf("failed to store OAuth secrets: %w", err) + } + return &ConfigureTokenResponse{ - AccessToken: accessToken, - RefreshToken: refreshToken, + AccessToken: tokens.AccessToken, + RefreshToken: tokens.RefreshToken, CloudID: cloudID, }, nil } +// LoadOAuthSecrets loads OAuth secrets from storage +func LoadOAuthSecrets() (*OAuthSecrets, error) { + jiraDir, err := getJiraConfigDir() + if err != nil { + return nil, fmt.Errorf("failed to get Jira config directory: %w", err) + } + + secretStorage := utils.FileSystemStorage{BaseDir: jiraDir} + secrets, err := utils.LoadJSON[OAuthSecrets](secretStorage, oauthSecretsFile) + if err != nil { + return nil, fmt.Errorf("failed to load OAuth secrets: %w", err) + } + + return &secrets, nil +} + +// GetValidAccessToken returns a valid access token if available, otherwise returns empty string +func GetValidAccessToken() string { + secrets, err := LoadOAuthSecrets() + if err != nil { + return "" + } + + if secrets.IsValid() { + return secrets.AccessToken + } + + return "" +} + // collectOAuthCredentials collects OAuth credentials from the user func collectOAuthCredentials() (*OAuthConfig, error) { var questions []*survey.Question @@ -142,7 +194,7 @@ func collectOAuthCredentials() (*OAuthConfig, error) { return &OAuthConfig{ ClientID: answers.ClientID, - ClientSecret: utils.Secret{Key: "client_secret", Value: answers.ClientSecret}, + ClientSecret: answers.ClientSecret, RedirectURI: answers.RedirectURI, Scopes: defaultScopes, }, nil @@ -156,7 +208,7 @@ func performOAuthFlow(config *OAuthConfig) (*oauth2.Token, error) { // OAuth2 configuration for JIRA oauthConfig := &oauth2.Config{ ClientID: config.ClientID, - ClientSecret: config.ClientSecret.String(), + ClientSecret: config.ClientSecret, RedirectURL: config.RedirectURI, Scopes: config.Scopes, Endpoint: oauth2.Endpoint{ From ca6a100f6c6d3811b91b4c3ed9637644c229cc8b Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Mon, 14 Jul 2025 22:11:46 -0400 Subject: [PATCH 11/70] (test) More tests for oauth --- pkg/oauth/oauth_test.go | 234 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 223 insertions(+), 11 deletions(-) diff --git a/pkg/oauth/oauth_test.go b/pkg/oauth/oauth_test.go index 618a940b..86a355d4 100644 --- a/pkg/oauth/oauth_test.go +++ b/pkg/oauth/oauth_test.go @@ -7,6 +7,7 @@ import ( "net/http" "net/http/httptest" "os" + "path/filepath" "testing" "time" @@ -45,6 +46,217 @@ func TestGetJiraConfigDir(t *testing.T) { }) } +func TestOAuthSecrets(t *testing.T) { + t.Parallel() + + t.Run("IsExpired returns true for expired tokens", func(t *testing.T) { + secrets := &OAuthSecrets{ + AccessToken: "test-token", + Expiry: time.Now().Add(-time.Hour), // Expired 1 hour ago + } + assert.True(t, secrets.IsExpired()) + }) + + t.Run("IsExpired returns false for valid tokens", func(t *testing.T) { + secrets := &OAuthSecrets{ + AccessToken: "test-token", + Expiry: time.Now().Add(time.Hour), // Expires in 1 hour + } + assert.False(t, secrets.IsExpired()) + }) + + t.Run("IsValid returns true for valid tokens", func(t *testing.T) { + secrets := &OAuthSecrets{ + AccessToken: "test-token", + Expiry: time.Now().Add(time.Hour), // Expires in 1 hour + } + assert.True(t, secrets.IsValid()) + }) + + t.Run("IsValid returns false for expired tokens", func(t *testing.T) { + secrets := &OAuthSecrets{ + AccessToken: "test-token", + Expiry: time.Now().Add(-time.Hour), // Expired 1 hour ago + } + assert.False(t, secrets.IsValid()) + }) + + t.Run("IsValid returns false for empty tokens", func(t *testing.T) { + secrets := &OAuthSecrets{ + AccessToken: "", + Expiry: time.Now().Add(time.Hour), // Expires in 1 hour + } + assert.False(t, secrets.IsValid()) + }) +} + +func TestLoadOAuthSecrets(t *testing.T) { + t.Parallel() + + t.Run("loads OAuth secrets successfully", func(t *testing.T) { + // Create a temporary directory for testing + tempDir, err := os.MkdirTemp("", "oauth-test-*") + assert.NoError(t, err) + defer os.RemoveAll(tempDir) + + // Create test secrets + testSecrets := &OAuthSecrets{ + ClientSecret: "test-client-secret", + AccessToken: "test-access-token", + RefreshToken: "test-refresh-token", + TokenType: "Bearer", + Expiry: time.Now().Add(time.Hour), + } + + // Save secrets to temp directory + storage := utils.FileSystemStorage{BaseDir: tempDir} + err = utils.SaveJSON(storage, oauthSecretsFile, testSecrets) + assert.NoError(t, err) + + // Load secrets directly from the test directory + loadedSecrets, err := utils.LoadJSON[OAuthSecrets](storage, oauthSecretsFile) + assert.NoError(t, err) + assert.Equal(t, testSecrets.ClientSecret, loadedSecrets.ClientSecret) + assert.Equal(t, testSecrets.AccessToken, loadedSecrets.AccessToken) + assert.Equal(t, testSecrets.RefreshToken, loadedSecrets.RefreshToken) + assert.Equal(t, testSecrets.TokenType, loadedSecrets.TokenType) + assert.True(t, testSecrets.Expiry.Equal(loadedSecrets.Expiry)) + }) + + t.Run("returns error when secrets file doesn't exist", func(t *testing.T) { + // Create a temporary directory without any secrets file + tempDir, err := os.MkdirTemp("", "oauth-test-*") + assert.NoError(t, err) + defer os.RemoveAll(tempDir) + + storage := utils.FileSystemStorage{BaseDir: tempDir} + _, err = utils.LoadJSON[OAuthSecrets](storage, oauthSecretsFile) + assert.Error(t, err) + }) +} + +func TestGetValidAccessToken(t *testing.T) { + t.Parallel() + + t.Run("returns valid access token", func(t *testing.T) { + // Create a temporary directory for testing + tempDir, err := os.MkdirTemp("", "oauth-test-*") + assert.NoError(t, err) + defer os.RemoveAll(tempDir) + + // Create test secrets with valid token + testSecrets := &OAuthSecrets{ + ClientSecret: "test-client-secret", + AccessToken: "valid-access-token", + RefreshToken: "test-refresh-token", + TokenType: "Bearer", + Expiry: time.Now().Add(time.Hour), + } + + // Save secrets to temp directory + storage := utils.FileSystemStorage{BaseDir: tempDir} + err = utils.SaveJSON(storage, oauthSecretsFile, testSecrets) + assert.NoError(t, err) + + // Temporarily override the config directory + originalHome := os.Getenv("HOME") + originalXDG := os.Getenv("XDG_CONFIG_HOME") + defer func() { + os.Setenv("HOME", originalHome) + os.Setenv("XDG_CONFIG_HOME", originalXDG) + }() + + // Use a custom config directory for testing + configDir := filepath.Join(tempDir, "..") + os.Setenv("XDG_CONFIG_HOME", configDir) + os.Setenv("HOME", "") + + // Create the .jira subdirectory and move the secrets file there + jiraDir := filepath.Join(configDir, ".jira") + err = os.MkdirAll(jiraDir, 0755) + assert.NoError(t, err) + + // Copy the secrets file to the expected location + srcFile := filepath.Join(tempDir, oauthSecretsFile) + dstFile := filepath.Join(jiraDir, oauthSecretsFile) + srcData, err := os.ReadFile(srcFile) + assert.NoError(t, err) + err = os.WriteFile(dstFile, srcData, 0600) + assert.NoError(t, err) + + // Get valid access token + token := GetValidAccessToken() + assert.Equal(t, "valid-access-token", token) + }) + + t.Run("returns empty string for expired token", func(t *testing.T) { + // Create a temporary directory for testing + tempDir, err := os.MkdirTemp("", "oauth-test-*") + assert.NoError(t, err) + defer os.RemoveAll(tempDir) + + // Create test secrets with expired token + testSecrets := &OAuthSecrets{ + ClientSecret: "test-client-secret", + AccessToken: "expired-access-token", + RefreshToken: "test-refresh-token", + TokenType: "Bearer", + Expiry: time.Now().Add(-time.Hour), // Expired + } + + // Save secrets to temp directory + storage := utils.FileSystemStorage{BaseDir: tempDir} + err = utils.SaveJSON(storage, oauthSecretsFile, testSecrets) + assert.NoError(t, err) + + // Temporarily override the config directory + originalHome := os.Getenv("HOME") + originalXDG := os.Getenv("XDG_CONFIG_HOME") + defer func() { + os.Setenv("HOME", originalHome) + os.Setenv("XDG_CONFIG_HOME", originalXDG) + }() + + // Use a custom config directory for testing + configDir := filepath.Join(tempDir, "..") + os.Setenv("XDG_CONFIG_HOME", configDir) + os.Setenv("HOME", "") + + // Create the .jira subdirectory and move the secrets file there + jiraDir := filepath.Join(configDir, ".jira") + err = os.MkdirAll(jiraDir, 0755) + assert.NoError(t, err) + + // Copy the secrets file to the expected location + srcFile := filepath.Join(tempDir, oauthSecretsFile) + dstFile := filepath.Join(jiraDir, oauthSecretsFile) + srcData, err := os.ReadFile(srcFile) + assert.NoError(t, err) + err = os.WriteFile(dstFile, srcData, 0600) + assert.NoError(t, err) + + // Get valid access token (should return empty string) + token := GetValidAccessToken() + assert.Empty(t, token) + }) + + t.Run("returns empty string when no secrets file exists", func(t *testing.T) { + // Set up environment to use a non-existent directory + originalHome := os.Getenv("HOME") + originalXDG := os.Getenv("XDG_CONFIG_HOME") + defer func() { + os.Setenv("HOME", originalHome) + os.Setenv("XDG_CONFIG_HOME", originalXDG) + }() + + os.Setenv("XDG_CONFIG_HOME", "/tmp/non-existent-dir") + os.Setenv("HOME", "") + + token := GetValidAccessToken() + assert.Empty(t, token) + }) +} + func TestGetCloudID(t *testing.T) { t.Parallel() @@ -157,13 +369,13 @@ func TestConfig(t *testing.T) { t.Run("creates config with all required fields", func(t *testing.T) { config := &OAuthConfig{ ClientID: "test-client-id", - ClientSecret: utils.Secret{Key: "client_secret", Value: "test-secret"}, + ClientSecret: "test-secret", RedirectURI: "http://localhost:9876/callback", Scopes: []string{"read:jira-user", "read:jira-work"}, } assert.Equal(t, "test-client-id", config.ClientID) - assert.Equal(t, "test-secret", config.ClientSecret.String()) + assert.Equal(t, "test-secret", config.ClientSecret) assert.Equal(t, "http://localhost:9876/callback", config.RedirectURI) assert.Contains(t, config.Scopes, "read:jira-user") assert.Contains(t, config.Scopes, "read:jira-work") @@ -175,13 +387,13 @@ func TestConfigureTokenResponse(t *testing.T) { t.Run("creates token response with all required fields", func(t *testing.T) { response := &ConfigureTokenResponse{ - AccessToken: utils.Secret{Key: "access_token", Value: "test-access-token"}, - RefreshToken: utils.Secret{Key: "refresh_token", Value: "test-refresh-token"}, + AccessToken: "test-access-token", + RefreshToken: "test-refresh-token", CloudID: "test-cloud-id", } - assert.Equal(t, "test-access-token", response.AccessToken.String()) - assert.Equal(t, "test-refresh-token", response.RefreshToken.String()) + assert.Equal(t, "test-access-token", response.AccessToken) + assert.Equal(t, "test-refresh-token", response.RefreshToken) assert.Equal(t, "test-cloud-id", response.CloudID) }) } @@ -192,7 +404,7 @@ func TestPerformOAuthFlow_ErrorCases(t *testing.T) { t.Run("handles timeout", func(t *testing.T) { config := &OAuthConfig{ ClientID: "test-client-id", - ClientSecret: utils.Secret{Key: "client_secret", Value: "test-secret"}, + ClientSecret: "test-secret", RedirectURI: "http://localhost:9876/callback", Scopes: []string{"read:jira-user"}, } @@ -207,7 +419,7 @@ func TestPerformOAuthFlow_ErrorCases(t *testing.T) { t.Run("handles server startup error", func(t *testing.T) { config := &OAuthConfig{ ClientID: "test-client-id", - ClientSecret: utils.Secret{Key: "client_secret", Value: "test-secret"}, + ClientSecret: "test-secret", RedirectURI: "http://localhost:9876/callback", Scopes: []string{"read:jira-user"}, } @@ -232,7 +444,7 @@ func TestPerformOAuthFlow_ErrorCases(t *testing.T) { func performOAuthFlowWithTimeout(config *OAuthConfig, timeout time.Duration) (*oauth2.Token, error) { oauthConfig := &oauth2.Config{ ClientID: config.ClientID, - ClientSecret: config.ClientSecret.String(), + ClientSecret: config.ClientSecret, RedirectURL: config.RedirectURI, Scopes: config.Scopes, Endpoint: oauth2.Endpoint{ @@ -321,7 +533,7 @@ func TestOAuthFlowIntegration(t *testing.T) { // Create config with mock server config := &OAuthConfig{ ClientID: "test-client-id", - ClientSecret: utils.Secret{Key: "client_secret", Value: "test-secret"}, + ClientSecret: "test-secret", RedirectURI: "http://localhost:9876/callback", Scopes: []string{"read:jira-user"}, } @@ -329,7 +541,7 @@ func TestOAuthFlowIntegration(t *testing.T) { // Test the OAuth configuration creation oauthConfig := &oauth2.Config{ ClientID: config.ClientID, - ClientSecret: config.ClientSecret.String(), + ClientSecret: config.ClientSecret, RedirectURL: config.RedirectURI, Scopes: config.Scopes, Endpoint: oauth2.Endpoint{ From 00d31d54bdfa338dbf9b3afffe043d1bd105d585 Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Tue, 15 Jul 2025 10:26:18 -0400 Subject: [PATCH 12/70] (feat) Enable auto refreshing access tokens through the oauth2 custom transport in the client --- api/client.go | 31 +++-- internal/cmd/root/root.go | 2 +- pkg/jira/client.go | 52 ++++++-- pkg/oauth/oauth.go | 52 ++++---- pkg/oauth/oauth_test.go | 270 ++++++++++++++++++++++++++++++++++++++ pkg/oauth/tokens.go | 128 ++++++++++++++++++ 6 files changed, 485 insertions(+), 50 deletions(-) create mode 100644 pkg/oauth/tokens.go diff --git a/api/client.go b/api/client.go index fe5875ca..c4332fbc 100644 --- a/api/client.go +++ b/api/client.go @@ -32,8 +32,10 @@ func getAPIToken(config *jira.Config) string { } // Try OAuth access token if available and valid - if oauthToken := oauth.GetValidAccessToken(); oauthToken != "" { - return oauthToken + if oauth.HasOAuthCredentials() { + tk, _ := oauth.LoadOAuth2TokenSource() + token, _ := tk.Token() + return token.AccessToken } // Try netrc file @@ -73,15 +75,26 @@ func Client(config jira.Config) *jira.Client { config.Insecure = &insecure } - // Get API token from various sources - config.APIToken = getAPIToken(&config) - - // If we have an OAuth token, set auth type to OAuth - if oauthToken := oauth.GetValidAccessToken(); oauthToken != "" && config.APIToken == oauthToken { - oauthAuthType := jira.AuthTypeOAuth - config.AuthType = &oauthAuthType + // Check if we have OAuth credentials and should use OAuth + if oauth.HasOAuthCredentials() && config.AuthType != nil && *config.AuthType == jira.AuthTypeOAuth { + // Try to create OAuth2 token source + tokenSource, err := oauth.LoadOAuth2TokenSource() + if err == nil { + // We have valid OAuth credentials, use OAuth authentication + // Pass the TokenSource to the client via a custom option + jiraClient = jira.NewClient( + config, + jira.WithTimeout(clientTimeout), + jira.WithInsecureTLS(*config.Insecure), + jira.WithOAuth2TokenSource(tokenSource), + ) + return jiraClient + } } + // Get API token from various sources (fallback for non-OAuth auth) + config.APIToken = getAPIToken(&config) + // MTLS if config.MTLSConfig.CaCert == "" { diff --git a/internal/cmd/root/root.go b/internal/cmd/root/root.go index 41d2358b..694da0d7 100644 --- a/internal/cmd/root/root.go +++ b/internal/cmd/root/root.go @@ -157,7 +157,7 @@ func cmdRequireToken(cmd string) bool { } func checkForJiraToken(server string, login string) { - if oauthToken := oauth.GetValidAccessToken(); oauthToken != "" { + if oauth.HasOAuthCredentials() { return } diff --git a/pkg/jira/client.go b/pkg/jira/client.go index 904df467..579e5818 100644 --- a/pkg/jira/client.go +++ b/pkg/jira/client.go @@ -14,6 +14,8 @@ import ( "os" "strings" "time" + + "golang.org/x/oauth2" ) const ( @@ -116,14 +118,15 @@ type Config struct { // Client is a jira client. type Client struct { - transport http.RoundTripper - insecure bool - server string - login string - authType *AuthType - token string - timeout time.Duration - debug bool + transport http.RoundTripper + insecure bool + server string + login string + authType *AuthType + token string + timeout time.Duration + debug bool + tokenSource oauth2.TokenSource } // ClientFunc decorates option for client. @@ -142,8 +145,8 @@ func NewClient(c Config, opts ...ClientFunc) *Client { for _, opt := range opts { opt(&client) } - - transport := &http.Transport{ + var transport http.RoundTripper + transport = &http.Transport{ Proxy: http.ProxyFromEnvironment, TLSClientConfig: &tls.Config{ MinVersion: tls.VersionTLS12, @@ -154,6 +157,15 @@ func NewClient(c Config, opts ...ClientFunc) *Client { }).DialContext, } + if c.AuthType != nil && *c.AuthType == AuthTypeOAuth && client.tokenSource != nil { + // Use OAuth2 transport with automatic token refresh + baseTransport := transport + transport = &oauth2.Transport{ + Base: baseTransport, + Source: oauth2.ReuseTokenSource(nil, client.tokenSource), + } + } + if c.AuthType != nil && *c.AuthType == AuthTypeMTLS { // Create a CA certificate pool and add cert.pem to it. caCert, err := os.ReadFile(c.MTLSConfig.CaCert) @@ -170,9 +182,10 @@ func NewClient(c Config, opts ...ClientFunc) *Client { } // Add the MTLS specific configuration. - transport.TLSClientConfig.RootCAs = caCertPool - transport.TLSClientConfig.Certificates = []tls.Certificate{cert} - transport.TLSClientConfig.Renegotiation = tls.RenegotiateFreelyAsClient + tlsConfig := transport.(*http.Transport).TLSClientConfig + tlsConfig.RootCAs = caCertPool + tlsConfig.Certificates = []tls.Certificate{cert} + tlsConfig.Renegotiation = tls.RenegotiateFreelyAsClient } client.transport = transport @@ -194,6 +207,13 @@ func WithInsecureTLS(ins bool) ClientFunc { } } +// WithOAuth2TokenSource is a functional opt to attach OAuth2 token source to the client. +func WithOAuth2TokenSource(tokenSource oauth2.TokenSource) ClientFunc { + return func(c *Client) { + c.tokenSource = tokenSource + } +} + // Get sends GET request to v3 version of the jira api. func (c *Client) Get(ctx context.Context, path string, headers Header) (*http.Response, error) { return c.request(ctx, http.MethodGet, c.server+baseURLv3+path, nil, headers) @@ -280,7 +300,11 @@ func (c *Client) request(ctx context.Context, method, endpoint string, body []by req.Header.Add("Authorization", "Bearer "+c.token) } case string(AuthTypeOAuth): - req.Header.Add("Authorization", "Bearer "+c.token) + // OAuth authentication is handled by oauth2.Transport automatically + // Only add manual auth header if we don't have a TokenSource (fallback mode) + if c.tokenSource == nil && c.token != "" { + req.Header.Add("Authorization", "Bearer "+c.token) + } case string(AuthTypeBearer): req.Header.Add("Authorization", "Bearer "+c.token) case string(AuthTypeBasic): diff --git a/pkg/oauth/oauth.go b/pkg/oauth/oauth.go index 4090aade..b5aeb981 100644 --- a/pkg/oauth/oauth.go +++ b/pkg/oauth/oauth.go @@ -51,15 +51,6 @@ type OAuthConfig struct { Scopes []string } -// OAuthSecrets holds all OAuth secrets in a single structure -type OAuthSecrets struct { - ClientSecret string `json:"client_secret"` - AccessToken string `json:"access_token"` - RefreshToken string `json:"refresh_token"` - TokenType string `json:"token_type"` - Expiry time.Time `json:"expiry"` -} - // ConfigureTokenResponse holds the OAuth token response type ConfigureTokenResponse struct { AccessToken string @@ -67,14 +58,25 @@ type ConfigureTokenResponse struct { CloudID string } -// IsExpired checks if the access token is expired -func (o *OAuthSecrets) IsExpired() bool { - return time.Now().After(o.Expiry) -} +// GetOAuth2Config creates an OAuth2 config for the given client credentials +func GetOAuth2Config(clientID, clientSecret, redirectURI string, scopes []string) *oauth2.Config { + if scopes == nil { + scopes = defaultScopes + } -// IsValid checks if the OAuth secrets are valid and not expired -func (o *OAuthSecrets) IsValid() bool { - return o.AccessToken != "" && !o.IsExpired() + if redirectURI == "" { + redirectURI = defaultRedirectURI + } + return &oauth2.Config{ + ClientID: clientID, + ClientSecret: clientSecret, + RedirectURL: redirectURI, + Scopes: scopes, + Endpoint: oauth2.Endpoint{ + AuthURL: jiraAuthURL, + TokenURL: jiraTokenURL, + }, + } } // Configure performs the complete OAuth flow and returns tokens @@ -106,6 +108,7 @@ func Configure() (*ConfigureTokenResponse, error) { // Store all OAuth secrets in a single JSON file oauthSecrets := &OAuthSecrets{ + ClientID: config.ClientID, ClientSecret: config.ClientSecret, AccessToken: tokens.AccessToken, RefreshToken: tokens.RefreshToken, @@ -154,6 +157,12 @@ func GetValidAccessToken() string { return "" } +// HasOAuthCredentials checks if OAuth credentials are present +func HasOAuthCredentials() bool { + _, err := LoadOAuthSecrets() + return err == nil +} + // collectOAuthCredentials collects OAuth credentials from the user func collectOAuthCredentials() (*OAuthConfig, error) { var questions []*survey.Question @@ -206,16 +215,7 @@ func performOAuthFlow(config *OAuthConfig) (*oauth2.Token, error) { defer s.Stop() // OAuth2 configuration for JIRA - oauthConfig := &oauth2.Config{ - ClientID: config.ClientID, - ClientSecret: config.ClientSecret, - RedirectURL: config.RedirectURI, - Scopes: config.Scopes, - Endpoint: oauth2.Endpoint{ - AuthURL: jiraAuthURL, - TokenURL: jiraTokenURL, - }, - } + oauthConfig := GetOAuth2Config(config.ClientID, config.ClientSecret, config.RedirectURI, config.Scopes) // Generate authorization URL with PKCE verifier := oauth2.GenerateVerifier() diff --git a/pkg/oauth/oauth_test.go b/pkg/oauth/oauth_test.go index 86a355d4..1c18ba89 100644 --- a/pkg/oauth/oauth_test.go +++ b/pkg/oauth/oauth_test.go @@ -661,3 +661,273 @@ func TestHTMLResponse(t *testing.T) { assert.Contains(t, w.Body.String(), "window.close()") }) } + +func TestOAuthSecrets_ToOAuth2Token(t *testing.T) { + t.Parallel() + + t.Run("converts OAuthSecrets to oauth2.Token correctly", func(t *testing.T) { + expiry := time.Now().Add(time.Hour) + secrets := &OAuthSecrets{ + ClientID: "test-client-id", + ClientSecret: "test-client-secret", + AccessToken: "test-access-token", + RefreshToken: "test-refresh-token", + TokenType: "Bearer", + Expiry: expiry, + } + + token := secrets.ToOAuth2Token() + assert.Equal(t, "test-access-token", token.AccessToken) + assert.Equal(t, "test-refresh-token", token.RefreshToken) + assert.Equal(t, "Bearer", token.TokenType) + assert.True(t, expiry.Equal(token.Expiry)) + }) +} + +func TestOAuthSecrets_FromOAuth2Token(t *testing.T) { + t.Parallel() + + t.Run("updates OAuthSecrets from oauth2.Token correctly", func(t *testing.T) { + expiry := time.Now().Add(time.Hour) + token := &oauth2.Token{ + AccessToken: "new-access-token", + RefreshToken: "new-refresh-token", + TokenType: "Bearer", + Expiry: expiry, + } + + secrets := &OAuthSecrets{ + ClientID: "test-client-id", + ClientSecret: "test-client-secret", + } + + secrets.FromOAuth2Token(token) + assert.Equal(t, "new-access-token", secrets.AccessToken) + assert.Equal(t, "new-refresh-token", secrets.RefreshToken) + assert.Equal(t, "Bearer", secrets.TokenType) + assert.True(t, expiry.Equal(secrets.Expiry)) + // ClientID and ClientSecret should remain unchanged + assert.Equal(t, "test-client-id", secrets.ClientID) + assert.Equal(t, "test-client-secret", secrets.ClientSecret) + }) +} + +func TestNewPersistentTokenSource(t *testing.T) { + t.Parallel() + + t.Run("creates PersistentTokenSource successfully", func(t *testing.T) { + tokenSource, err := NewPersistentTokenSource("test-client-id", "test-client-secret") + assert.NoError(t, err) + assert.NotNil(t, tokenSource) + assert.Equal(t, "test-client-id", tokenSource.clientID) + assert.Equal(t, "test-client-secret", tokenSource.clientSecret) + assert.NotNil(t, tokenSource.storage) + }) +} + +func TestPersistentTokenSource_Token(t *testing.T) { + t.Parallel() + + t.Run("returns valid token when not expired", func(t *testing.T) { + // Create a temporary directory for testing + tempDir, err := os.MkdirTemp("", "oauth-test-*") + assert.NoError(t, err) + defer os.RemoveAll(tempDir) + + // Create test secrets with valid token + testSecrets := &OAuthSecrets{ + ClientID: "test-client-id", + ClientSecret: "test-client-secret", + AccessToken: "valid-access-token", + RefreshToken: "test-refresh-token", + TokenType: "Bearer", + Expiry: time.Now().Add(time.Hour), // Valid for another hour + } + + // Save secrets to temp directory + storage := utils.FileSystemStorage{BaseDir: tempDir} + err = utils.SaveJSON(storage, oauthSecretsFile, testSecrets) + assert.NoError(t, err) + + // Create token source + tokenSource := &PersistentTokenSource{ + clientID: "test-client-id", + clientSecret: "test-client-secret", + storage: storage, + } + + // Get token - should return the valid token without refresh + token, err := tokenSource.Token() + assert.NoError(t, err) + assert.Equal(t, "valid-access-token", token.AccessToken) + assert.Equal(t, "test-refresh-token", token.RefreshToken) + assert.Equal(t, "Bearer", token.TokenType) + assert.True(t, token.Valid()) + }) + + t.Run("returns error when secrets file doesn't exist", func(t *testing.T) { + // Create a temporary directory without any secrets + tempDir, err := os.MkdirTemp("", "oauth-test-*") + assert.NoError(t, err) + defer os.RemoveAll(tempDir) + + // Create token source + tokenSource := &PersistentTokenSource{ + clientID: "test-client-id", + clientSecret: "test-client-secret", + storage: utils.FileSystemStorage{BaseDir: tempDir}, + } + + // Get token - should return error + _, err = tokenSource.Token() + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to load OAuth secrets") + }) +} + +func TestLoadOAuth2TokenSource(t *testing.T) { + t.Parallel() + + t.Run("creates TokenSource from stored secrets", func(t *testing.T) { + // Create a temporary directory for testing + tempDir, err := os.MkdirTemp("", "oauth-test-*") + assert.NoError(t, err) + defer os.RemoveAll(tempDir) + + // Create test secrets + testSecrets := &OAuthSecrets{ + ClientID: "test-client-id", + ClientSecret: "test-client-secret", + AccessToken: "test-access-token", + RefreshToken: "test-refresh-token", + TokenType: "Bearer", + Expiry: time.Now().Add(time.Hour), + } + + // Save secrets to temp directory + storage := utils.FileSystemStorage{BaseDir: tempDir} + err = utils.SaveJSON(storage, oauthSecretsFile, testSecrets) + assert.NoError(t, err) + + // Temporarily override the config directory + originalHome := os.Getenv("HOME") + originalXDG := os.Getenv("XDG_CONFIG_HOME") + defer func() { + os.Setenv("HOME", originalHome) + os.Setenv("XDG_CONFIG_HOME", originalXDG) + }() + + // Use a custom config directory for testing + configDir := filepath.Join(tempDir, "..") + os.Setenv("XDG_CONFIG_HOME", configDir) + os.Setenv("HOME", "") + + // Create the .jira subdirectory and move the secrets file there + jiraDir := filepath.Join(configDir, ".jira") + err = os.MkdirAll(jiraDir, 0755) + assert.NoError(t, err) + + // Copy the secrets file to the expected location + srcFile := filepath.Join(tempDir, oauthSecretsFile) + dstFile := filepath.Join(jiraDir, oauthSecretsFile) + srcData, err := os.ReadFile(srcFile) + assert.NoError(t, err) + err = os.WriteFile(dstFile, srcData, 0600) + assert.NoError(t, err) + + // Load token source + tokenSource, err := LoadOAuth2TokenSource() + assert.NoError(t, err) + assert.NotNil(t, tokenSource) + + // Verify we can get a token from it + token, err := tokenSource.Token() + assert.NoError(t, err) + assert.Equal(t, "test-access-token", token.AccessToken) + }) +} + +func TestGetOAuth2Config(t *testing.T) { + t.Parallel() + + t.Run("creates OAuth2 config with correct values", func(t *testing.T) { + config := GetOAuth2Config("test-client-id", "test-client-secret") + assert.Equal(t, "test-client-id", config.ClientID) + assert.Equal(t, "test-client-secret", config.ClientSecret) + assert.Equal(t, defaultScopes, config.Scopes) + assert.Equal(t, jiraAuthURL, config.Endpoint.AuthURL) + assert.Equal(t, jiraTokenURL, config.Endpoint.TokenURL) + }) +} + +func TestHasOAuthCredentials(t *testing.T) { + t.Parallel() + + t.Run("returns true when OAuth credentials exist", func(t *testing.T) { + // Create a temporary directory for testing + tempDir, err := os.MkdirTemp("", "oauth-test-*") + assert.NoError(t, err) + defer os.RemoveAll(tempDir) + + // Create test secrets + testSecrets := &OAuthSecrets{ + ClientID: "test-client-id", + ClientSecret: "test-client-secret", + AccessToken: "test-access-token", + RefreshToken: "test-refresh-token", + TokenType: "Bearer", + Expiry: time.Now().Add(time.Hour), + } + + // Save secrets to temp directory + storage := utils.FileSystemStorage{BaseDir: tempDir} + err = utils.SaveJSON(storage, oauthSecretsFile, testSecrets) + assert.NoError(t, err) + + // Temporarily override the config directory + originalHome := os.Getenv("HOME") + originalXDG := os.Getenv("XDG_CONFIG_HOME") + defer func() { + os.Setenv("HOME", originalHome) + os.Setenv("XDG_CONFIG_HOME", originalXDG) + }() + + // Use a custom config directory for testing + configDir := filepath.Join(tempDir, "..") + os.Setenv("XDG_CONFIG_HOME", configDir) + os.Setenv("HOME", "") + + // Create the .jira subdirectory and move the secrets file there + jiraDir := filepath.Join(configDir, ".jira") + err = os.MkdirAll(jiraDir, 0755) + assert.NoError(t, err) + + // Copy the secrets file to the expected location + srcFile := filepath.Join(tempDir, oauthSecretsFile) + dstFile := filepath.Join(jiraDir, oauthSecretsFile) + srcData, err := os.ReadFile(srcFile) + assert.NoError(t, err) + err = os.WriteFile(dstFile, srcData, 0600) + assert.NoError(t, err) + + // Check if OAuth credentials exist + result := HasOAuthCredentials() + assert.True(t, result) + }) + + t.Run("returns false when OAuth credentials don't exist", func(t *testing.T) { + // Set up environment to use a non-existent directory + originalHome := os.Getenv("HOME") + originalXDG := os.Getenv("XDG_CONFIG_HOME") + defer func() { + os.Setenv("HOME", originalHome) + os.Setenv("XDG_CONFIG_HOME", originalXDG) + }() + + os.Setenv("XDG_CONFIG_HOME", "/tmp/non-existent-dir") + os.Setenv("HOME", "") + + result := HasOAuthCredentials() + assert.False(t, result) + }) +} diff --git a/pkg/oauth/tokens.go b/pkg/oauth/tokens.go new file mode 100644 index 00000000..730a02d0 --- /dev/null +++ b/pkg/oauth/tokens.go @@ -0,0 +1,128 @@ +package oauth + +import ( + "context" + "fmt" + "time" + + "github.com/ankitpokhrel/jira-cli/pkg/utils" + "golang.org/x/oauth2" +) + +// OAuthSecrets holds all OAuth secrets in a single structure +type OAuthSecrets struct { + ClientID string `json:"client_id"` + ClientSecret string `json:"client_secret"` + AccessToken string `json:"access_token"` + RefreshToken string `json:"refresh_token"` + TokenType string `json:"token_type"` + Expiry time.Time `json:"expiry"` +} + +// PersistentTokenSource implements oauth2.TokenSource with automatic token persistence +type PersistentTokenSource struct { + clientID string + clientSecret string + storage utils.Storage +} + +// IsExpired checks if the access token is expired +func (o *OAuthSecrets) IsExpired() bool { + return time.Now().After(o.Expiry) +} + +// IsValid checks if the OAuth secrets are valid and not expired +func (o *OAuthSecrets) IsValid() bool { + return o.AccessToken != "" && !o.IsExpired() +} + +// ToOAuth2Token converts OAuthSecrets to oauth2.Token +func (o *OAuthSecrets) ToOAuth2Token() *oauth2.Token { + return &oauth2.Token{ + AccessToken: o.AccessToken, + RefreshToken: o.RefreshToken, + TokenType: o.TokenType, + Expiry: o.Expiry, + } +} + +// FromOAuth2Token updates OAuthSecrets from oauth2.Token +func (o *OAuthSecrets) FromOAuth2Token(token *oauth2.Token) { + o.AccessToken = token.AccessToken + o.RefreshToken = token.RefreshToken + o.TokenType = token.TokenType + o.Expiry = token.Expiry +} + +// NewPersistentTokenSource creates a new TokenSource that persists tokens +func NewPersistentTokenSource(clientID, clientSecret string) (*PersistentTokenSource, error) { + jiraDir, err := getJiraConfigDir() + if err != nil { + return nil, fmt.Errorf("failed to get Jira config directory: %w", err) + } + + storage := utils.FileSystemStorage{BaseDir: jiraDir} + return &PersistentTokenSource{ + clientID: clientID, + clientSecret: clientSecret, + storage: storage, + }, nil +} + +// Token implements oauth2.TokenSource interface +func (pts *PersistentTokenSource) Token() (*oauth2.Token, error) { + // Load current token from storage + secrets, err := utils.LoadJSON[OAuthSecrets](pts.storage, oauthSecretsFile) + if err != nil { + return nil, fmt.Errorf("failed to load OAuth secrets: %w", err) + } + + token := secrets.ToOAuth2Token() + + // If token is still valid, return it + if token.Valid() { + return token, nil + } + + // Token needs refresh - create OAuth2 config for refresh + oauthConfig := &oauth2.Config{ + ClientID: pts.clientID, + ClientSecret: pts.clientSecret, + Endpoint: oauth2.Endpoint{ + AuthURL: jiraAuthURL, + TokenURL: jiraTokenURL, + }, + } + + // Refresh the token + refreshedToken, err := oauthConfig.TokenSource(context.Background(), token).Token() + if err != nil { + return nil, fmt.Errorf("failed to refresh OAuth token: %w", err) + } + + // Save the refreshed token + secrets.FromOAuth2Token(refreshedToken) + if err := utils.SaveJSON(pts.storage, oauthSecretsFile, &secrets); err != nil { + // Log error but don't fail the request - we still have a valid token + fmt.Printf("Warning: failed to save refreshed OAuth token: %v\n", err) + } + + return refreshedToken, nil +} + +// LoadOAuth2TokenSource creates a TokenSource from stored OAuth secrets +func LoadOAuth2TokenSource() (oauth2.TokenSource, error) { + // Load OAuth secrets to get client credentials + secrets, err := LoadOAuthSecrets() + if err != nil { + return nil, fmt.Errorf("failed to load OAuth secrets: %w", err) + } + + // Create persistent token source + tokenSource, err := NewPersistentTokenSource(secrets.ClientID, secrets.ClientSecret) + if err != nil { + return nil, fmt.Errorf("failed to create token source: %w", err) + } + + return tokenSource, nil +} From c5533a828c49c83eb6c9ddcb676140d13c26f801 Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Tue, 15 Jul 2025 10:26:29 -0400 Subject: [PATCH 13/70] (nit) remove dead code --- pkg/utils/secrets.go | 35 --------------- pkg/utils/secrets_test.go | 93 --------------------------------------- 2 files changed, 128 deletions(-) delete mode 100644 pkg/utils/secrets.go delete mode 100644 pkg/utils/secrets_test.go diff --git a/pkg/utils/secrets.go b/pkg/utils/secrets.go deleted file mode 100644 index 2833bca2..00000000 --- a/pkg/utils/secrets.go +++ /dev/null @@ -1,35 +0,0 @@ -package utils - -import "fmt" - -// Secret represents a secret value with storage capabilities -type Secret struct { - Key string - Value string -} - -func (s Secret) String() string { - return s.Value -} - -func (s Secret) Save(storage Storage) error { - if s.Key == "" { - return fmt.Errorf("secret key cannot be empty") - } - return storage.Save(s.Key, []byte(s.Value)) -} - -func (s *Secret) Load(storage Storage, key string) error { - if key == "" { - return fmt.Errorf("secret key cannot be empty") - } - - data, err := storage.Load(key) - if err != nil { - return err - } - - s.Key = key - s.Value = string(data) - return nil -} diff --git a/pkg/utils/secrets_test.go b/pkg/utils/secrets_test.go deleted file mode 100644 index cdf65240..00000000 --- a/pkg/utils/secrets_test.go +++ /dev/null @@ -1,93 +0,0 @@ -package utils - -import ( - "fmt" - "testing" - - "github.com/stretchr/testify/assert" -) - -// Mock storage for testing -type mockStorage struct { - savedKey string - savedValue []byte - loadReturn []byte - loadError error - saveError error -} - -func (m *mockStorage) Save(key string, value []byte) error { - if m.saveError != nil { - return m.saveError - } - m.savedKey = key - m.savedValue = value - return nil -} - -func (m *mockStorage) Load(key string) ([]byte, error) { - if m.loadError != nil { - return nil, m.loadError - } - return m.loadReturn, nil -} - -func TestSecretOperations(t *testing.T) { - t.Parallel() - - t.Run("secret string representation", func(t *testing.T) { - secret := Secret{Key: "test-key", Value: "test-value"} - assert.Equal(t, "test-value", secret.String()) - }) - - t.Run("secret save with empty key", func(t *testing.T) { - secret := Secret{Key: "", Value: "test-value"} - storage := &mockStorage{} - - err := secret.Save(storage) - assert.Error(t, err) - assert.Contains(t, err.Error(), "secret key cannot be empty") - }) - - t.Run("secret save success", func(t *testing.T) { - secret := Secret{Key: "test-key", Value: "test-value"} - storage := &mockStorage{} - - err := secret.Save(storage) - assert.NoError(t, err) - assert.Equal(t, "test-key", storage.savedKey) - assert.Equal(t, []byte("test-value"), storage.savedValue) - }) - - t.Run("secret load with empty key", func(t *testing.T) { - secret := &Secret{} - storage := &mockStorage{} - - err := secret.Load(storage, "") - assert.Error(t, err) - assert.Contains(t, err.Error(), "secret key cannot be empty") - }) - - t.Run("secret load success", func(t *testing.T) { - secret := &Secret{} - storage := &mockStorage{ - loadReturn: []byte("loaded-value"), - } - - err := secret.Load(storage, "test-key") - assert.NoError(t, err) - assert.Equal(t, "test-key", secret.Key) - assert.Equal(t, "loaded-value", secret.Value) - }) - - t.Run("secret load with storage error", func(t *testing.T) { - secret := &Secret{} - storage := &mockStorage{ - loadError: fmt.Errorf("storage error"), - } - - err := secret.Load(storage, "test-key") - assert.Error(t, err) - assert.Contains(t, err.Error(), "storage error") - }) -} From d7917e93db3bca3c02c422b2382755fb80533dfe Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Wed, 16 Jul 2025 22:33:20 -0400 Subject: [PATCH 14/70] (fix) The constructed server urls were pointing to authorization server, rather than their own JIRA server --- api/client.go | 10 +++++++++- internal/config/generator.go | 20 ++++++++++++++------ 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/api/client.go b/api/client.go index c4332fbc..4857f725 100644 --- a/api/client.go +++ b/api/client.go @@ -6,6 +6,7 @@ import ( "github.com/spf13/viper" "github.com/zalando/go-keyring" + "github.com/ankitpokhrel/jira-cli/internal/cmdutil" "github.com/ankitpokhrel/jira-cli/pkg/jira" "github.com/ankitpokhrel/jira-cli/pkg/jira/filter" "github.com/ankitpokhrel/jira-cli/pkg/netrc" @@ -60,7 +61,14 @@ func Client(config jira.Config) *jira.Client { } if config.Server == "" { - config.Server = viper.GetString("server") + apiServer := viper.GetString("api_server") + if apiServer != "" { + config.Server = apiServer + } else { + // Fallback to server URL if api_server is not set + cmdutil.Warn("api_server key is not set, falling back to server URL") + config.Server = viper.GetString("server") + } } if config.Login == "" { config.Login = viper.GetString("login") diff --git a/internal/config/generator.go b/internal/config/generator.go index 15b82304..3bb0b379 100644 --- a/internal/config/generator.go +++ b/internal/config/generator.go @@ -30,6 +30,7 @@ const ( optionBack = "Go-back" optionNone = "None" lineBreak = "----------" + apiServer = "https://api.atlassian.com/ex/jira" ) var ( @@ -81,7 +82,9 @@ type JiraCLIConfigGenerator struct { value struct { installation string server string - version struct { + // API server is the server URL for the Jira API. Should be the same as the server URL if not oAuth. + apiServer string + version struct { major, minor, patch int } login string @@ -368,10 +371,6 @@ func (c *JiraCLIConfigGenerator) configureOAuth() error { func (c *JiraCLIConfigGenerator) configureServerAndLoginDetails() error { var qs []*survey.Question - if c.value.authType == jira.AuthTypeOAuth { - // Set server URL using the cloud ID from OAuth configuration - c.usrCfg.Server = fmt.Sprintf("https://api.atlassian.com/ex/jira/%s", c.value.oauth.cloudId) - } c.value.server = c.usrCfg.Server c.value.login = c.usrCfg.Login @@ -469,10 +468,18 @@ func (c *JiraCLIConfigGenerator) configureServerAndLoginDetails() error { if ans.Server != "" { c.value.server = strings.TrimSpace(ans.Server) + } if ans.Login != "" { c.value.login = strings.TrimSpace(ans.Login) } + + if c.value.authType == jira.AuthTypeOAuth { + // Set server URL using the cloud ID from OAuth configuration + c.value.apiServer = fmt.Sprintf("%s/%s", apiServer, c.value.oauth.cloudId) + } else { + c.value.apiServer = c.value.server + } } // Trim trailing slash from server URL c.value.server = strings.TrimRight(c.value.server, "/") @@ -480,7 +487,7 @@ func (c *JiraCLIConfigGenerator) configureServerAndLoginDetails() error { } func (c *JiraCLIConfigGenerator) generateJiraConfig() jira.Config { config := jira.Config{ - Server: c.value.server, + Server: c.value.apiServer, Login: c.value.login, Insecure: &c.usrCfg.Insecure, AuthType: &c.value.authType, @@ -819,6 +826,7 @@ func (c *JiraCLIConfigGenerator) write(path string) (string, error) { config.Set("installation", c.value.installation) config.Set("server", c.value.server) + config.Set("api_server", c.value.apiServer) config.Set("login", c.value.login) config.Set("project", c.value.project) config.Set("epic", c.value.epic) From 2af82790fb17fe00779eb8f7c5ebdcd4a2f562fe Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Wed, 16 Jul 2025 22:44:23 -0400 Subject: [PATCH 15/70] (docs) update README with more instructions. --- README.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 19bcbe79..2f78349e 100644 --- a/README.md +++ b/README.md @@ -95,6 +95,8 @@ Follow the [installation guide](https://github.com/ankitpokhrel/jira-cli/wiki/In more [here](https://github.com/ankitpokhrel/jira-cli/discussions/356). 2. Run `jira init`, select installation type as `Cloud`, and provide required details to generate a config file required for the tool. +3. Run the `jira init`, Select the `Cloud` installation type and then select the `OAuth` authentication type. This will prompt for your Jira App Client ID and Client Secret. You can learn more about how to create a Jira App [here](link-to-a-discussion) + #### On-premise installation @@ -121,12 +123,13 @@ See [FAQs](https://github.com/ankitpokhrel/jira-cli/discussions/categories/faqs) #### Authentication types -The tool supports `basic`, `bearer` (Personal Access Token), and `mtls` (Client Certificates) authentication types. Basic auth is used by +The tool supports `basic`, `bearer` (Personal Access Token), `mtls` (Client Certificates), and `oauth` (OAuth 3LO) authentication types. Basic auth is used by default. - If you want to use PAT, you need to set `JIRA_AUTH_TYPE` as `bearer`. - If you want to use `mtls` run `jira init`. Select installation type `Local`, and then select authentication type as `mtls`. - In case `JIRA_API_TOKEN` variable is set it will be used together with `mtls`. +- If you want to use `oauth` run `jira init`. Select installation type `Cloud`, and then select authentication type as `oauth`. #### Shell completion @@ -854,7 +857,7 @@ Sprint 1: 3 - https://jira.atlassian.com/browse/ECO-283 - https://community.developer.atlassian.com/t/oauth-2-0-with-proof-key-for-code-exchange-pkce/80173/3 -- The 3LO doesn't support PKCE, to avoid the need for a client secret, so we need to use the legacy auth flow. +- The 3LO doesn't support [Proof Key for Code Exchange (PKCE)](https://oauth.net/2/pkce/). Without this support, we would have to share the single distrubuted app's client secret with all the consumers. To avoid the need for globally sharing a client secret, each consumer will need to create a JIRA app to effectively use as a proxy into your Jira cloud instance. ## Feature requests From 605d9f0ef7d8fc7b67f35850cfb3a8a420a3fae9 Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Wed, 16 Jul 2025 23:42:37 -0400 Subject: [PATCH 16/70] (fix/cleanup) Remove old/redundant code and fix the oauth tests --- pkg/oauth/oauth.go | 14 - pkg/oauth/oauth_test.go | 522 ++++++-------------------------------- pkg/utils/storage_test.go | 59 +++++ 3 files changed, 132 insertions(+), 463 deletions(-) diff --git a/pkg/oauth/oauth.go b/pkg/oauth/oauth.go index b5aeb981..3b79637a 100644 --- a/pkg/oauth/oauth.go +++ b/pkg/oauth/oauth.go @@ -143,20 +143,6 @@ func LoadOAuthSecrets() (*OAuthSecrets, error) { return &secrets, nil } -// GetValidAccessToken returns a valid access token if available, otherwise returns empty string -func GetValidAccessToken() string { - secrets, err := LoadOAuthSecrets() - if err != nil { - return "" - } - - if secrets.IsValid() { - return secrets.AccessToken - } - - return "" -} - // HasOAuthCredentials checks if OAuth credentials are present func HasOAuthCredentials() bool { _, err := LoadOAuthSecrets() diff --git a/pkg/oauth/oauth_test.go b/pkg/oauth/oauth_test.go index 1c18ba89..79622d6d 100644 --- a/pkg/oauth/oauth_test.go +++ b/pkg/oauth/oauth_test.go @@ -7,13 +7,13 @@ import ( "net/http" "net/http/httptest" "os" - "path/filepath" "testing" "time" - "github.com/ankitpokhrel/jira-cli/pkg/utils" "github.com/stretchr/testify/assert" "golang.org/x/oauth2" + + "github.com/ankitpokhrel/jira-cli/pkg/utils" ) func TestGetJiraConfigDir(t *testing.T) { @@ -135,128 +135,6 @@ func TestLoadOAuthSecrets(t *testing.T) { }) } -func TestGetValidAccessToken(t *testing.T) { - t.Parallel() - - t.Run("returns valid access token", func(t *testing.T) { - // Create a temporary directory for testing - tempDir, err := os.MkdirTemp("", "oauth-test-*") - assert.NoError(t, err) - defer os.RemoveAll(tempDir) - - // Create test secrets with valid token - testSecrets := &OAuthSecrets{ - ClientSecret: "test-client-secret", - AccessToken: "valid-access-token", - RefreshToken: "test-refresh-token", - TokenType: "Bearer", - Expiry: time.Now().Add(time.Hour), - } - - // Save secrets to temp directory - storage := utils.FileSystemStorage{BaseDir: tempDir} - err = utils.SaveJSON(storage, oauthSecretsFile, testSecrets) - assert.NoError(t, err) - - // Temporarily override the config directory - originalHome := os.Getenv("HOME") - originalXDG := os.Getenv("XDG_CONFIG_HOME") - defer func() { - os.Setenv("HOME", originalHome) - os.Setenv("XDG_CONFIG_HOME", originalXDG) - }() - - // Use a custom config directory for testing - configDir := filepath.Join(tempDir, "..") - os.Setenv("XDG_CONFIG_HOME", configDir) - os.Setenv("HOME", "") - - // Create the .jira subdirectory and move the secrets file there - jiraDir := filepath.Join(configDir, ".jira") - err = os.MkdirAll(jiraDir, 0755) - assert.NoError(t, err) - - // Copy the secrets file to the expected location - srcFile := filepath.Join(tempDir, oauthSecretsFile) - dstFile := filepath.Join(jiraDir, oauthSecretsFile) - srcData, err := os.ReadFile(srcFile) - assert.NoError(t, err) - err = os.WriteFile(dstFile, srcData, 0600) - assert.NoError(t, err) - - // Get valid access token - token := GetValidAccessToken() - assert.Equal(t, "valid-access-token", token) - }) - - t.Run("returns empty string for expired token", func(t *testing.T) { - // Create a temporary directory for testing - tempDir, err := os.MkdirTemp("", "oauth-test-*") - assert.NoError(t, err) - defer os.RemoveAll(tempDir) - - // Create test secrets with expired token - testSecrets := &OAuthSecrets{ - ClientSecret: "test-client-secret", - AccessToken: "expired-access-token", - RefreshToken: "test-refresh-token", - TokenType: "Bearer", - Expiry: time.Now().Add(-time.Hour), // Expired - } - - // Save secrets to temp directory - storage := utils.FileSystemStorage{BaseDir: tempDir} - err = utils.SaveJSON(storage, oauthSecretsFile, testSecrets) - assert.NoError(t, err) - - // Temporarily override the config directory - originalHome := os.Getenv("HOME") - originalXDG := os.Getenv("XDG_CONFIG_HOME") - defer func() { - os.Setenv("HOME", originalHome) - os.Setenv("XDG_CONFIG_HOME", originalXDG) - }() - - // Use a custom config directory for testing - configDir := filepath.Join(tempDir, "..") - os.Setenv("XDG_CONFIG_HOME", configDir) - os.Setenv("HOME", "") - - // Create the .jira subdirectory and move the secrets file there - jiraDir := filepath.Join(configDir, ".jira") - err = os.MkdirAll(jiraDir, 0755) - assert.NoError(t, err) - - // Copy the secrets file to the expected location - srcFile := filepath.Join(tempDir, oauthSecretsFile) - dstFile := filepath.Join(jiraDir, oauthSecretsFile) - srcData, err := os.ReadFile(srcFile) - assert.NoError(t, err) - err = os.WriteFile(dstFile, srcData, 0600) - assert.NoError(t, err) - - // Get valid access token (should return empty string) - token := GetValidAccessToken() - assert.Empty(t, token) - }) - - t.Run("returns empty string when no secrets file exists", func(t *testing.T) { - // Set up environment to use a non-existent directory - originalHome := os.Getenv("HOME") - originalXDG := os.Getenv("XDG_CONFIG_HOME") - defer func() { - os.Setenv("HOME", originalHome) - os.Setenv("XDG_CONFIG_HOME", originalXDG) - }() - - os.Setenv("XDG_CONFIG_HOME", "/tmp/non-existent-dir") - os.Setenv("HOME", "") - - token := GetValidAccessToken() - assert.Empty(t, token) - }) -} - func TestGetCloudID(t *testing.T) { t.Parallel() @@ -287,39 +165,7 @@ func TestGetCloudID(t *testing.T) { // Test with mock server - this requires refactoring the function to accept a custom URL // For now, we'll test the error cases and create a separate testable function - cloudID, err := getCloudID(server.URL+"/oauth/token/accessible-resources", "test-access-token") - assert.NoError(t, err) - assert.Equal(t, expectedCloudID, cloudID) - }) - - t.Run("successfully gets jira cloud id from list of accessible resources", func(t *testing.T) { - expectedCloudID := "test-cloud-id-123" - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // Verify request - assert.Equal(t, "GET", r.Method) - assert.Equal(t, "/oauth/token/accessible-resources", r.URL.Path) - assert.Equal(t, "Bearer test-access-token", r.Header.Get("Authorization")) - assert.Equal(t, "application/json", r.Header.Get("Accept")) - - // Return mock response - response := []map[string]interface{}{ - { - "id": expectedCloudID, - "name": "Test Site", - "url": "https://test.atlassian.net", - "scopes": []string{"read:jira-user", "read:jira-work"}, - "avatarUrl": "https://test.atlassian.net/avatar.png", - }, - } - - w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(response) - })) - defer server.Close() - - // Test with mock server - this requires refactoring the function to accept a custom URL - // For now, we'll test the error cases and create a separate testable function - cloudID, err := getCloudID(server.URL+"/oauth/token/accessible-resources", "test-access-token") + cloudID, err := getCloudIDFromURL(server.URL+"/oauth/token/accessible-resources", "test-access-token") assert.NoError(t, err) assert.Equal(t, expectedCloudID, cloudID) }) @@ -330,7 +176,7 @@ func TestGetCloudID(t *testing.T) { })) defer server.Close() - cloudID, err := getCloudID(server.URL+"/oauth/token/accessible-resources", "invalid-token") + cloudID, err := getCloudIDFromURL(server.URL+"/oauth/token/accessible-resources", "invalid-token") assert.Error(t, err) assert.Empty(t, cloudID) assert.Contains(t, err.Error(), "failed to get accessible resources: status 401") @@ -343,7 +189,7 @@ func TestGetCloudID(t *testing.T) { })) defer server.Close() - cloudID, err := getCloudID(server.URL+"/oauth/token/accessible-resources", "test-token") + cloudID, err := getCloudIDFromURL(server.URL+"/oauth/token/accessible-resources", "test-token") assert.Error(t, err) assert.Empty(t, cloudID) assert.Contains(t, err.Error(), "failed to decode accessible resources response") @@ -356,13 +202,54 @@ func TestGetCloudID(t *testing.T) { })) defer server.Close() - cloudID, err := getCloudID(server.URL+"/oauth/token/accessible-resources", "test-token") + cloudID, err := getCloudIDFromURL(server.URL+"/oauth/token/accessible-resources", "test-token") assert.Error(t, err) assert.Empty(t, cloudID) assert.Contains(t, err.Error(), "no accessible resources found") }) } +// Helper function to make getCloudID testable +func getCloudIDFromURL(url, accessToken string) (string, error) { + client := &http.Client{Timeout: 30 * time.Second} + + req, err := http.NewRequest("GET", url, nil) + if err != nil { + return "", err + } + + req.Header.Set("Authorization", "Bearer "+accessToken) + req.Header.Set("Accept", "application/json") + + resp, err := client.Do(req) + if err != nil { + return "", err + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return "", fmt.Errorf("failed to get accessible resources: status %d", resp.StatusCode) + } + + var resourceResponse []struct { + ID string `json:"id"` + Name string `json:"name"` + URL string `json:"url"` + Scopes []string `json:"scopes"` + AvatarURL string `json:"avatarUrl"` + } + + if err := json.NewDecoder(resp.Body).Decode(&resourceResponse); err != nil { + return "", fmt.Errorf("failed to decode accessible resources response: %w", err) + } + + if len(resourceResponse) == 0 { + return "", fmt.Errorf("no accessible resources found or cloud ID not found") + } + + return resourceResponse[0].ID, nil +} + func TestConfig(t *testing.T) { t.Parallel() @@ -510,6 +397,15 @@ func performOAuthFlowWithTimeout(config *OAuthConfig, timeout time.Duration) (*o } } +func TestConstants(t *testing.T) { + t.Parallel() + + t.Run("verifies file permission constants", func(t *testing.T) { + assert.Equal(t, 0o700, int(utils.OWNER_ONLY)) + assert.Equal(t, 0o600, int(utils.OWNER_READ_WRITE)) + }) +} + func TestOAuthFlowIntegration(t *testing.T) { t.Parallel() @@ -628,306 +524,34 @@ func TestOAuthFlowIntegration(t *testing.T) { }) } -func TestHTMLResponse(t *testing.T) { - t.Parallel() - - t.Run("callback returns proper HTML response", func(t *testing.T) { - handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path == callbackPath { - code := r.URL.Query().Get("code") - if code != "" { - w.Header().Set("Content-Type", "text/html") - w.Write([]byte(` - - -You can close this window and return to the terminal.
- - - - `)) - } - } - }) - - req := httptest.NewRequest("GET", "http://localhost:9876/callback?code=test-code", nil) - w := httptest.NewRecorder() - - handler.ServeHTTP(w, req) - - assert.Equal(t, http.StatusOK, w.Code) - assert.Equal(t, "text/html", w.Header().Get("Content-Type")) - assert.Contains(t, w.Body.String(), "Authorization successful!") - assert.Contains(t, w.Body.String(), "window.close()") - }) -} - -func TestOAuthSecrets_ToOAuth2Token(t *testing.T) { - t.Parallel() - - t.Run("converts OAuthSecrets to oauth2.Token correctly", func(t *testing.T) { - expiry := time.Now().Add(time.Hour) - secrets := &OAuthSecrets{ - ClientID: "test-client-id", - ClientSecret: "test-client-secret", - AccessToken: "test-access-token", - RefreshToken: "test-refresh-token", - TokenType: "Bearer", - Expiry: expiry, - } - - token := secrets.ToOAuth2Token() - assert.Equal(t, "test-access-token", token.AccessToken) - assert.Equal(t, "test-refresh-token", token.RefreshToken) - assert.Equal(t, "Bearer", token.TokenType) - assert.True(t, expiry.Equal(token.Expiry)) - }) -} - -func TestOAuthSecrets_FromOAuth2Token(t *testing.T) { - t.Parallel() - - t.Run("updates OAuthSecrets from oauth2.Token correctly", func(t *testing.T) { - expiry := time.Now().Add(time.Hour) - token := &oauth2.Token{ - AccessToken: "new-access-token", - RefreshToken: "new-refresh-token", - TokenType: "Bearer", - Expiry: expiry, - } - - secrets := &OAuthSecrets{ - ClientID: "test-client-id", - ClientSecret: "test-client-secret", - } - - secrets.FromOAuth2Token(token) - assert.Equal(t, "new-access-token", secrets.AccessToken) - assert.Equal(t, "new-refresh-token", secrets.RefreshToken) - assert.Equal(t, "Bearer", secrets.TokenType) - assert.True(t, expiry.Equal(secrets.Expiry)) - // ClientID and ClientSecret should remain unchanged - assert.Equal(t, "test-client-id", secrets.ClientID) - assert.Equal(t, "test-client-secret", secrets.ClientSecret) - }) -} - -func TestNewPersistentTokenSource(t *testing.T) { - t.Parallel() - - t.Run("creates PersistentTokenSource successfully", func(t *testing.T) { - tokenSource, err := NewPersistentTokenSource("test-client-id", "test-client-secret") - assert.NoError(t, err) - assert.NotNil(t, tokenSource) - assert.Equal(t, "test-client-id", tokenSource.clientID) - assert.Equal(t, "test-client-secret", tokenSource.clientSecret) - assert.NotNil(t, tokenSource.storage) - }) -} - -func TestPersistentTokenSource_Token(t *testing.T) { +func TestGetOAuth2Config(t *testing.T) { t.Parallel() - t.Run("returns valid token when not expired", func(t *testing.T) { - // Create a temporary directory for testing - tempDir, err := os.MkdirTemp("", "oauth-test-*") - assert.NoError(t, err) - defer os.RemoveAll(tempDir) - - // Create test secrets with valid token - testSecrets := &OAuthSecrets{ - ClientID: "test-client-id", - ClientSecret: "test-client-secret", - AccessToken: "valid-access-token", - RefreshToken: "test-refresh-token", - TokenType: "Bearer", - Expiry: time.Now().Add(time.Hour), // Valid for another hour - } - - // Save secrets to temp directory - storage := utils.FileSystemStorage{BaseDir: tempDir} - err = utils.SaveJSON(storage, oauthSecretsFile, testSecrets) - assert.NoError(t, err) - - // Create token source - tokenSource := &PersistentTokenSource{ - clientID: "test-client-id", - clientSecret: "test-client-secret", - storage: storage, - } - - // Get token - should return the valid token without refresh - token, err := tokenSource.Token() - assert.NoError(t, err) - assert.Equal(t, "valid-access-token", token.AccessToken) - assert.Equal(t, "test-refresh-token", token.RefreshToken) - assert.Equal(t, "Bearer", token.TokenType) - assert.True(t, token.Valid()) - }) - - t.Run("returns error when secrets file doesn't exist", func(t *testing.T) { - // Create a temporary directory without any secrets - tempDir, err := os.MkdirTemp("", "oauth-test-*") - assert.NoError(t, err) - defer os.RemoveAll(tempDir) - - // Create token source - tokenSource := &PersistentTokenSource{ - clientID: "test-client-id", - clientSecret: "test-client-secret", - storage: utils.FileSystemStorage{BaseDir: tempDir}, - } - - // Get token - should return error - _, err = tokenSource.Token() - assert.Error(t, err) - assert.Contains(t, err.Error(), "failed to load OAuth secrets") - }) -} + t.Run("creates OAuth2 config with all parameters", func(t *testing.T) { + clientID := "test-client-id" + clientSecret := "test-client-secret" + redirectURI := "http://localhost:9876/callback" + scopes := []string{"read:jira-user", "read:jira-work"} -func TestLoadOAuth2TokenSource(t *testing.T) { - t.Parallel() + config := GetOAuth2Config(clientID, clientSecret, redirectURI, scopes) - t.Run("creates TokenSource from stored secrets", func(t *testing.T) { - // Create a temporary directory for testing - tempDir, err := os.MkdirTemp("", "oauth-test-*") - assert.NoError(t, err) - defer os.RemoveAll(tempDir) - - // Create test secrets - testSecrets := &OAuthSecrets{ - ClientID: "test-client-id", - ClientSecret: "test-client-secret", - AccessToken: "test-access-token", - RefreshToken: "test-refresh-token", - TokenType: "Bearer", - Expiry: time.Now().Add(time.Hour), - } - - // Save secrets to temp directory - storage := utils.FileSystemStorage{BaseDir: tempDir} - err = utils.SaveJSON(storage, oauthSecretsFile, testSecrets) - assert.NoError(t, err) - - // Temporarily override the config directory - originalHome := os.Getenv("HOME") - originalXDG := os.Getenv("XDG_CONFIG_HOME") - defer func() { - os.Setenv("HOME", originalHome) - os.Setenv("XDG_CONFIG_HOME", originalXDG) - }() - - // Use a custom config directory for testing - configDir := filepath.Join(tempDir, "..") - os.Setenv("XDG_CONFIG_HOME", configDir) - os.Setenv("HOME", "") - - // Create the .jira subdirectory and move the secrets file there - jiraDir := filepath.Join(configDir, ".jira") - err = os.MkdirAll(jiraDir, 0755) - assert.NoError(t, err) - - // Copy the secrets file to the expected location - srcFile := filepath.Join(tempDir, oauthSecretsFile) - dstFile := filepath.Join(jiraDir, oauthSecretsFile) - srcData, err := os.ReadFile(srcFile) - assert.NoError(t, err) - err = os.WriteFile(dstFile, srcData, 0600) - assert.NoError(t, err) - - // Load token source - tokenSource, err := LoadOAuth2TokenSource() - assert.NoError(t, err) - assert.NotNil(t, tokenSource) - - // Verify we can get a token from it - token, err := tokenSource.Token() - assert.NoError(t, err) - assert.Equal(t, "test-access-token", token.AccessToken) - }) -} - -func TestGetOAuth2Config(t *testing.T) { - t.Parallel() - - t.Run("creates OAuth2 config with correct values", func(t *testing.T) { - config := GetOAuth2Config("test-client-id", "test-client-secret") - assert.Equal(t, "test-client-id", config.ClientID) - assert.Equal(t, "test-client-secret", config.ClientSecret) - assert.Equal(t, defaultScopes, config.Scopes) + assert.Equal(t, clientID, config.ClientID) + assert.Equal(t, clientSecret, config.ClientSecret) + assert.Equal(t, redirectURI, config.RedirectURL) + assert.Equal(t, scopes, config.Scopes) assert.Equal(t, jiraAuthURL, config.Endpoint.AuthURL) assert.Equal(t, jiraTokenURL, config.Endpoint.TokenURL) }) -} - -func TestHasOAuthCredentials(t *testing.T) { - t.Parallel() - t.Run("returns true when OAuth credentials exist", func(t *testing.T) { - // Create a temporary directory for testing - tempDir, err := os.MkdirTemp("", "oauth-test-*") - assert.NoError(t, err) - defer os.RemoveAll(tempDir) + t.Run("uses default scopes when nil", func(t *testing.T) { + config := GetOAuth2Config("test-client-id", "test-client-secret", "http://localhost:9876/callback", nil) - // Create test secrets - testSecrets := &OAuthSecrets{ - ClientID: "test-client-id", - ClientSecret: "test-client-secret", - AccessToken: "test-access-token", - RefreshToken: "test-refresh-token", - TokenType: "Bearer", - Expiry: time.Now().Add(time.Hour), - } - - // Save secrets to temp directory - storage := utils.FileSystemStorage{BaseDir: tempDir} - err = utils.SaveJSON(storage, oauthSecretsFile, testSecrets) - assert.NoError(t, err) - - // Temporarily override the config directory - originalHome := os.Getenv("HOME") - originalXDG := os.Getenv("XDG_CONFIG_HOME") - defer func() { - os.Setenv("HOME", originalHome) - os.Setenv("XDG_CONFIG_HOME", originalXDG) - }() - - // Use a custom config directory for testing - configDir := filepath.Join(tempDir, "..") - os.Setenv("XDG_CONFIG_HOME", configDir) - os.Setenv("HOME", "") - - // Create the .jira subdirectory and move the secrets file there - jiraDir := filepath.Join(configDir, ".jira") - err = os.MkdirAll(jiraDir, 0755) - assert.NoError(t, err) - - // Copy the secrets file to the expected location - srcFile := filepath.Join(tempDir, oauthSecretsFile) - dstFile := filepath.Join(jiraDir, oauthSecretsFile) - srcData, err := os.ReadFile(srcFile) - assert.NoError(t, err) - err = os.WriteFile(dstFile, srcData, 0600) - assert.NoError(t, err) - - // Check if OAuth credentials exist - result := HasOAuthCredentials() - assert.True(t, result) + assert.Equal(t, defaultScopes, config.Scopes) }) - t.Run("returns false when OAuth credentials don't exist", func(t *testing.T) { - // Set up environment to use a non-existent directory - originalHome := os.Getenv("HOME") - originalXDG := os.Getenv("XDG_CONFIG_HOME") - defer func() { - os.Setenv("HOME", originalHome) - os.Setenv("XDG_CONFIG_HOME", originalXDG) - }() - - os.Setenv("XDG_CONFIG_HOME", "/tmp/non-existent-dir") - os.Setenv("HOME", "") + t.Run("uses default redirect URI when empty", func(t *testing.T) { + config := GetOAuth2Config("test-client-id", "test-client-secret", "", []string{"read:jira-user"}) - result := HasOAuthCredentials() - assert.False(t, result) + assert.Equal(t, defaultRedirectURI, config.RedirectURL) }) } diff --git a/pkg/utils/storage_test.go b/pkg/utils/storage_test.go index f9518fad..3e14a711 100644 --- a/pkg/utils/storage_test.go +++ b/pkg/utils/storage_test.go @@ -1,6 +1,7 @@ package utils import ( + "fmt" "os" "path/filepath" "testing" @@ -77,3 +78,61 @@ func TestFileSystemStorage(t *testing.T) { assert.Contains(t, err.Error(), "failed to create directory") }) } + +func TestStorageOperations(t *testing.T) { + t.Parallel() + + t.Run("storage save and load operations", func(t *testing.T) { + storage := &mockStorage{} + + err := storage.Save("test-key", []byte("test-value")) + assert.NoError(t, err) + assert.Equal(t, "test-key", storage.savedKey) + assert.Equal(t, []byte("test-value"), storage.savedValue) + }) + + t.Run("storage load with error", func(t *testing.T) { + storage := &mockStorage{ + loadError: fmt.Errorf("storage error"), + } + + _, err := storage.Load("test-key") + assert.Error(t, err) + assert.Contains(t, err.Error(), "storage error") + }) + + t.Run("storage load success", func(t *testing.T) { + storage := &mockStorage{ + loadReturn: []byte("loaded-value"), + } + + value, err := storage.Load("test-key") + assert.NoError(t, err) + assert.Equal(t, []byte("loaded-value"), value) + }) +} + +// Mock storage for testing +type mockStorage struct { + savedKey string + savedValue []byte + loadReturn []byte + loadError error + saveError error +} + +func (m *mockStorage) Save(key string, value []byte) error { + if m.saveError != nil { + return m.saveError + } + m.savedKey = key + m.savedValue = value + return nil +} + +func (m *mockStorage) Load(key string) ([]byte, error) { + if m.loadError != nil { + return nil, m.loadError + } + return m.loadReturn, nil +} From 7264fa99c54ad20aa16ca878cb4c3256280b3333 Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Wed, 16 Jul 2025 23:43:05 -0400 Subject: [PATCH 17/70] (nit) Remove old TODO --- api/client.go | 1 - 1 file changed, 1 deletion(-) diff --git a/api/client.go b/api/client.go index 4857f725..71761c69 100644 --- a/api/client.go +++ b/api/client.go @@ -73,7 +73,6 @@ func Client(config jira.Config) *jira.Client { if config.Login == "" { config.Login = viper.GetString("login") } - //TODO: Load auth token here if config.AuthType == nil { authType := jira.AuthType(viper.GetString("auth_type")) config.AuthType = &authType From b4b3d62ed40cae9b806456693e9ddbe843548f72 Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Thu, 17 Jul 2025 00:04:30 -0400 Subject: [PATCH 18/70] (lint) fix the lint issues --- api/client.go | 2 +- internal/config/generator.go | 2 +- pkg/jira/cloud_id.go | 3 +- pkg/oauth/oauth.go | 65 +++++++++++++++++++++++------------- pkg/oauth/oauth_test.go | 62 ++++++++++++++++++++++------------ pkg/oauth/tokens.go | 18 +++++----- pkg/utils/storage.go | 6 ++-- pkg/utils/storage_test.go | 4 +-- 8 files changed, 100 insertions(+), 62 deletions(-) diff --git a/api/client.go b/api/client.go index 71761c69..f5049b13 100644 --- a/api/client.go +++ b/api/client.go @@ -21,7 +21,7 @@ var jiraClient *jira.Client // 1. Viper configuration // 2. OAuth access token (if available and valid) // 3. Netrc file -// 4. Keyring +// 4. Keyring. func getAPIToken(config *jira.Config) string { if config.APIToken != "" { return config.APIToken diff --git a/internal/config/generator.go b/internal/config/generator.go index 3bb0b379..9bb8c1cf 100644 --- a/internal/config/generator.go +++ b/internal/config/generator.go @@ -468,7 +468,6 @@ func (c *JiraCLIConfigGenerator) configureServerAndLoginDetails() error { if ans.Server != "" { c.value.server = strings.TrimSpace(ans.Server) - } if ans.Login != "" { c.value.login = strings.TrimSpace(ans.Login) @@ -485,6 +484,7 @@ func (c *JiraCLIConfigGenerator) configureServerAndLoginDetails() error { c.value.server = strings.TrimRight(c.value.server, "/") return c.verifyLoginDetails() } + func (c *JiraCLIConfigGenerator) generateJiraConfig() jira.Config { config := jira.Config{ Server: c.value.apiServer, diff --git a/pkg/jira/cloud_id.go b/pkg/jira/cloud_id.go index f3821580..c69ba33d 100644 --- a/pkg/jira/cloud_id.go +++ b/pkg/jira/cloud_id.go @@ -22,7 +22,8 @@ type CloudIDResponse struct { func (c *Client) GetCloudID() (string, error) { res, err := c.request(context.Background(), http.MethodGet, "https://api.atlassian.com/oauth/token/accessible-resources", nil, Header{ - "Accept": "application/json"}) + "Accept": "application/json", + }) if err != nil { return "", err } diff --git a/pkg/oauth/oauth.go b/pkg/oauth/oauth.go index 3b79637a..dc3dd858 100644 --- a/pkg/oauth/oauth.go +++ b/pkg/oauth/oauth.go @@ -17,21 +17,27 @@ import ( ) const ( - // JIRA OAuth2 endpoints + // JIRA OAuth2 endpoints. jiraAuthURL = "https://auth.atlassian.com/authorize" jiraTokenURL = "https://auth.atlassian.com/oauth/token" accessibleResourcesURL = "https://api.atlassian.com/oauth/token/accessible-resources" - // Default OAuth settings + // Default OAuth settings. defaultRedirectURI = "http://localhost:9876/callback" defaultPort = ":9876" callbackPath = "/callback" - // OAuth timeout + // OAuth timeout. oauthTimeout = 5 * time.Minute - // OAuth storage file name + // OAuth storage file name. oauthSecretsFile = "oauth_secrets.json" + + // Server shutdown timeout. + serverShutdownTimeout = 5 * time.Second + + // HTTP client timeout for API calls. + httpClientTimeout = 30 * time.Second ) var defaultScopes = []string{ @@ -43,7 +49,7 @@ var defaultScopes = []string{ "offline_access", // This is required to get the refresh token from JIRA } -// OAuthConfig holds OAuth configuration +// OAuthConfig holds OAuth configuration. type OAuthConfig struct { ClientID string ClientSecret string @@ -51,14 +57,14 @@ type OAuthConfig struct { Scopes []string } -// ConfigureTokenResponse holds the OAuth token response +// ConfigureTokenResponse holds the OAuth token response. type ConfigureTokenResponse struct { AccessToken string RefreshToken string CloudID string } -// GetOAuth2Config creates an OAuth2 config for the given client credentials +// GetOAuth2Config creates an OAuth2 config for the given client credentials. func GetOAuth2Config(clientID, clientSecret, redirectURI string, scopes []string) *oauth2.Config { if scopes == nil { scopes = defaultScopes @@ -79,7 +85,7 @@ func GetOAuth2Config(clientID, clientSecret, redirectURI string, scopes []string } } -// Configure performs the complete OAuth flow and returns tokens +// Configure performs the complete OAuth flow and returns tokens. func Configure() (*ConfigureTokenResponse, error) { // Collect OAuth credentials from user jiraDir, err := getJiraConfigDir() @@ -127,7 +133,7 @@ func Configure() (*ConfigureTokenResponse, error) { }, nil } -// LoadOAuthSecrets loads OAuth secrets from storage +// LoadOAuthSecrets loads OAuth secrets from storage. func LoadOAuthSecrets() (*OAuthSecrets, error) { jiraDir, err := getJiraConfigDir() if err != nil { @@ -143,13 +149,13 @@ func LoadOAuthSecrets() (*OAuthSecrets, error) { return &secrets, nil } -// HasOAuthCredentials checks if OAuth credentials are present +// HasOAuthCredentials checks if OAuth credentials are present. func HasOAuthCredentials() bool { _, err := LoadOAuthSecrets() return err == nil } -// collectOAuthCredentials collects OAuth credentials from the user +// collectOAuthCredentials collects OAuth credentials from the user. func collectOAuthCredentials() (*OAuthConfig, error) { var questions []*survey.Question answers := struct { @@ -195,7 +201,7 @@ func collectOAuthCredentials() (*OAuthConfig, error) { }, nil } -// performOAuthFlow executes the OAuth authorization flow +// performOAuthFlow executes the OAuth authorization flow. func performOAuthFlow(config *OAuthConfig) (*oauth2.Token, error) { s := cmdutil.Info("Starting OAuth flow...") defer s.Stop() @@ -223,7 +229,7 @@ func performOAuthFlow(config *OAuthConfig) (*oauth2.Token, error) { // Send success response to browser w.Header().Set("Content-Type", "text/html") - w.Write([]byte(` + if _, err := w.Write([]byte(`You can close this window and return to the terminal.
+ + + + `)) + } + } + }) + + req := httptest.NewRequest("GET", "http://localhost:9876/callback?code=test-code", http.NoBody) + w := httptest.NewRecorder() + + handler.ServeHTTP(w, req) + + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, "text/html", w.Header().Get("Content-Type")) + assert.Contains(t, w.Body.String(), "Authorization successful!") + assert.Contains(t, w.Body.String(), "window.close()") + }) +} + func TestGetOAuth2Config(t *testing.T) { t.Parallel() t.Run("creates OAuth2 config with all parameters", func(t *testing.T) { + t.Parallel() clientID := "test-client-id" clientSecret := "test-client-secret" redirectURI := "http://localhost:9876/callback" @@ -562,12 +540,14 @@ func TestGetOAuth2Config(t *testing.T) { }) t.Run("uses default scopes when nil", func(t *testing.T) { + t.Parallel() config := GetOAuth2Config("test-client-id", "test-client-secret", "http://localhost:9876/callback", nil) assert.Equal(t, defaultScopes, config.Scopes) }) t.Run("uses default redirect URI when empty", func(t *testing.T) { + t.Parallel() config := GetOAuth2Config("test-client-id", "test-client-secret", "", []string{"read:jira-user"}) assert.Equal(t, defaultRedirectURI, config.RedirectURL) diff --git a/pkg/utils/storage_test.go b/pkg/utils/storage_test.go index 5f966acb..864f7472 100644 --- a/pkg/utils/storage_test.go +++ b/pkg/utils/storage_test.go @@ -130,7 +130,7 @@ func (m *mockStorage) Save(key string, value []byte) error { return nil } -func (m *mockStorage) Load(key string) ([]byte, error) { +func (m *mockStorage) Load(_ string) ([]byte, error) { if m.loadError != nil { return nil, m.loadError } From aada4f61f2729dae4560dfd060754658b0829065 Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Thu, 17 Jul 2025 08:43:48 -0400 Subject: [PATCH 20/70] (ci) Fix some more issues found with DeepSource --- pkg/oauth/oauth.go | 6 +++++- pkg/oauth/oauth_test.go | 5 ++++- pkg/utils/storage_test.go | 4 ---- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/pkg/oauth/oauth.go b/pkg/oauth/oauth.go index c31a8bec..40757bb2 100644 --- a/pkg/oauth/oauth.go +++ b/pkg/oauth/oauth.go @@ -38,6 +38,9 @@ const ( // HTTP client timeout for API calls. httpClientTimeout = 30 * time.Second + + // Read header timeout for API calls. + readHeaderTimeout = 3 * time.Second ) var defaultScopes = []string{ @@ -217,7 +220,8 @@ func performOAuthFlow(config *OAuthConfig, httpTimeout time.Duration, openBrowse errChan := make(chan error, 1) server := &http.Server{ - Addr: defaultPort, + Addr: defaultPort, + ReadHeaderTimeout: readHeaderTimeout, Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path == callbackPath { code := r.URL.Query().Get("code") diff --git a/pkg/oauth/oauth_test.go b/pkg/oauth/oauth_test.go index 85465074..7b3874b1 100644 --- a/pkg/oauth/oauth_test.go +++ b/pkg/oauth/oauth_test.go @@ -332,7 +332,10 @@ func TestPerformOAuthFlow_ErrorCases(t *testing.T) { } // Start a server on the same port to cause a conflict - conflictServer := &http.Server{Addr: defaultPort} + conflictServer := &http.Server{ + Addr: defaultPort, + ReadHeaderTimeout: readHeaderTimeout, + } go func() { _ = conflictServer.ListenAndServe() }() diff --git a/pkg/utils/storage_test.go b/pkg/utils/storage_test.go index 864f7472..5996e018 100644 --- a/pkg/utils/storage_test.go +++ b/pkg/utils/storage_test.go @@ -10,8 +10,6 @@ import ( ) func TestFileSystemStorage(t *testing.T) { - t.Parallel() - t.Run("creates directory and saves file", func(t *testing.T) { // Create temporary directory tempDir := t.TempDir() @@ -80,8 +78,6 @@ func TestFileSystemStorage(t *testing.T) { } func TestStorageOperations(t *testing.T) { - t.Parallel() - t.Run("storage save and load operations", func(t *testing.T) { storage := &mockStorage{} From b9aa1be347bb70d579f2d6e5fda66a6853d112e8 Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Sat, 19 Jul 2025 14:51:20 -0400 Subject: [PATCH 21/70] (docs) update README to account for discussion post --- README.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 2f78349e..63f70fc1 100644 --- a/README.md +++ b/README.md @@ -95,8 +95,7 @@ Follow the [installation guide](https://github.com/ankitpokhrel/jira-cli/wiki/In more [here](https://github.com/ankitpokhrel/jira-cli/discussions/356). 2. Run `jira init`, select installation type as `Cloud`, and provide required details to generate a config file required for the tool. -3. Run the `jira init`, Select the `Cloud` installation type and then select the `OAuth` authentication type. This will prompt for your Jira App Client ID and Client Secret. You can learn more about how to create a Jira App [here](link-to-a-discussion) - +3. Run the `jira init`, Select the `Cloud` installation type and then select the `OAuth` authentication type. This will prompt for your Jira App Client ID and Client Secret. You can learn more about how to create a Jira App [here](https://github.com/ankitpokhrel/jira-cli/discussions/879#discussion-8604411) #### On-premise installation @@ -129,7 +128,7 @@ default. - If you want to use PAT, you need to set `JIRA_AUTH_TYPE` as `bearer`. - If you want to use `mtls` run `jira init`. Select installation type `Local`, and then select authentication type as `mtls`. - In case `JIRA_API_TOKEN` variable is set it will be used together with `mtls`. -- If you want to use `oauth` run `jira init`. Select installation type `Cloud`, and then select authentication type as `oauth`. +- If you want to use `oauth` run `jira init`. Select installation type `Cloud`, and then select authentication type as `oauth`. #### Shell completion @@ -857,7 +856,7 @@ Sprint 1: 3 - https://jira.atlassian.com/browse/ECO-283 - https://community.developer.atlassian.com/t/oauth-2-0-with-proof-key-for-code-exchange-pkce/80173/3 -- The 3LO doesn't support [Proof Key for Code Exchange (PKCE)](https://oauth.net/2/pkce/). Without this support, we would have to share the single distrubuted app's client secret with all the consumers. To avoid the need for globally sharing a client secret, each consumer will need to create a JIRA app to effectively use as a proxy into your Jira cloud instance. +- The 3LO doesn't support [Proof Key for Code Exchange (PKCE)](https://oauth.net/2/pkce/). Without this support, we would have to share the single distrubuted app's client secret with all the consumers. To avoid the need for globally sharing a client secret, each consumer will need to create a JIRA app to effectively use as a proxy into your Jira cloud instance. ## Feature requests From bf3fce8cb4391884ac228e2f745373ac0ea2b007 Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Wed, 8 Oct 2025 18:30:16 -0400 Subject: [PATCH 22/70] (nit) add additional scopes for properly reading jira sprint --- pkg/oauth/oauth.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/oauth/oauth.go b/pkg/oauth/oauth.go index 40757bb2..a2734cc4 100644 --- a/pkg/oauth/oauth.go +++ b/pkg/oauth/oauth.go @@ -48,6 +48,9 @@ var defaultScopes = []string{ "read:jira-work", "read:board-scope:jira-software", "read:project:jira", + "read:sprint:jira-software", + "read:issue-details:jira", + "read:jql:jira", "write:jira-work", "offline_access", // This is required to get the refresh token from JIRA } From 99f1bb6cea15c20c6127ba26baaa65921cf114ad Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Wed, 8 Oct 2025 18:58:41 -0400 Subject: [PATCH 23/70] need this scope for adding to a sprint --- pkg/oauth/oauth.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/oauth/oauth.go b/pkg/oauth/oauth.go index a2734cc4..f2fcb39e 100644 --- a/pkg/oauth/oauth.go +++ b/pkg/oauth/oauth.go @@ -51,6 +51,7 @@ var defaultScopes = []string{ "read:sprint:jira-software", "read:issue-details:jira", "read:jql:jira", + "write:sprint:jira-software", "write:jira-work", "offline_access", // This is required to get the refresh token from JIRA } From 6282035bcd91e269a17d482603775e5fc8ff63ec Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Mon, 13 Oct 2025 16:53:56 -0400 Subject: [PATCH 24/70] typos --- README.md | 2 +- internal/config/generator.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 63f70fc1..7332f6ff 100644 --- a/README.md +++ b/README.md @@ -856,7 +856,7 @@ Sprint 1: 3 - https://jira.atlassian.com/browse/ECO-283 - https://community.developer.atlassian.com/t/oauth-2-0-with-proof-key-for-code-exchange-pkce/80173/3 -- The 3LO doesn't support [Proof Key for Code Exchange (PKCE)](https://oauth.net/2/pkce/). Without this support, we would have to share the single distrubuted app's client secret with all the consumers. To avoid the need for globally sharing a client secret, each consumer will need to create a JIRA app to effectively use as a proxy into your Jira cloud instance. +- The 3LO doesn't support [Proof Key for Code Exchange (PKCE)](https://oauth.net/2/pkce/). Without this support, we would have to share the single distributed app's client secret with all the consumers. To avoid the need for globally sharing a client secret, each consumer will need to create a JIRA app to effectively use as a proxy into your Jira cloud instance. ## Feature requests diff --git a/internal/config/generator.go b/internal/config/generator.go index 9bb8c1cf..223187c2 100644 --- a/internal/config/generator.go +++ b/internal/config/generator.go @@ -280,7 +280,7 @@ func (c *JiraCLIConfigGenerator) configureCloudAuthType() error { if c.usrCfg.AuthType == "" { qs := &survey.Select{ Message: "Authentication type:", - Help: `Authentication type coud be: cloud or oauth + Help: `Authentication type could be: cloud or oauth ? If you are using your login credentials, the auth type is probably 'cloud' (most common for cloud installation) ? If you are authenticating using oauth 3LO, the auth type is probably 'oauth'`, Options: []string{"cloud", "oauth"}, From a59a503292eadb36a7e90e2f7ef76953ec95d0ed Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Mon, 13 Oct 2025 16:54:13 -0400 Subject: [PATCH 25/70] address the searching for epic details --- pkg/oauth/oauth.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/oauth/oauth.go b/pkg/oauth/oauth.go index f2fcb39e..82ae04a6 100644 --- a/pkg/oauth/oauth.go +++ b/pkg/oauth/oauth.go @@ -50,6 +50,10 @@ var defaultScopes = []string{ "read:project:jira", "read:sprint:jira-software", "read:issue-details:jira", + "read:audit-log:jira", + "read:avatar:jira", + "read:field-configuration:jira", + "read:issue-meta:jira", "read:jql:jira", "write:sprint:jira-software", "write:jira-work", From 619badccd451a7bfeb43ce53d9b7f26c2a861098 Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Mon, 13 Oct 2025 17:53:04 -0400 Subject: [PATCH 26/70] fix reason why the cloud isn't working --- api/client.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/api/client.go b/api/client.go index f5049b13..25735244 100644 --- a/api/client.go +++ b/api/client.go @@ -33,7 +33,9 @@ func getAPIToken(config *jira.Config) string { } // Try OAuth access token if available and valid - if oauth.HasOAuthCredentials() { + // And should only do this assertion if the AuthType is oauth + var isAuthTypeOAuth = config.AuthType != nil && *config.AuthType == jira.AuthTypeOAuth + if isAuthTypeOAuth && oauth.HasOAuthCredentials() { tk, _ := oauth.LoadOAuth2TokenSource() token, _ := tk.Token() return token.AccessToken From e171f6c6ceb63b73d0c995c759801079e92c7ae6 Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Mon, 13 Oct 2025 18:07:24 -0400 Subject: [PATCH 27/70] linting --- api/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/client.go b/api/client.go index 25735244..9ec7cc39 100644 --- a/api/client.go +++ b/api/client.go @@ -34,7 +34,7 @@ func getAPIToken(config *jira.Config) string { // Try OAuth access token if available and valid // And should only do this assertion if the AuthType is oauth - var isAuthTypeOAuth = config.AuthType != nil && *config.AuthType == jira.AuthTypeOAuth + isAuthTypeOAuth := config.AuthType != nil && *config.AuthType == jira.AuthTypeOAuth if isAuthTypeOAuth && oauth.HasOAuthCredentials() { tk, _ := oauth.LoadOAuth2TokenSource() token, _ := tk.Token() From e6104a8a0208974494e05724e5d75706803f91f6 Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Sun, 2 Nov 2025 20:36:45 -0500 Subject: [PATCH 28/70] reorganize the storage and filesystem tests --- pkg/utils/fs_storage.go | 32 +++++++++++++++++++ .../{storage_test.go => fs_storage_test.go} | 0 pkg/utils/storage.go | 28 ---------------- 3 files changed, 32 insertions(+), 28 deletions(-) create mode 100644 pkg/utils/fs_storage.go rename pkg/utils/{storage_test.go => fs_storage_test.go} (100%) diff --git a/pkg/utils/fs_storage.go b/pkg/utils/fs_storage.go new file mode 100644 index 00000000..c720965d --- /dev/null +++ b/pkg/utils/fs_storage.go @@ -0,0 +1,32 @@ +package utils + +import ( + "fmt" + "os" + "path/filepath" +) + +const ( + OWNER_ONLY = 0o700 + OWNER_READ_WRITE = 0o600 +) + +// FileSystemStorage implements Storage interface for filesystem operations. +type FileSystemStorage struct { + // BaseDir is the directory where the storage will be saved + BaseDir string +} + +func (fs FileSystemStorage) Save(key string, value []byte) error { + if err := os.MkdirAll(fs.BaseDir, OWNER_ONLY); err != nil { + return fmt.Errorf("failed to create directory: %w", err) + } + + filePath := filepath.Join(fs.BaseDir, key) + return os.WriteFile(filePath, value, OWNER_READ_WRITE) +} + +func (fs FileSystemStorage) Load(key string) ([]byte, error) { + filePath := filepath.Join(fs.BaseDir, key) + return os.ReadFile(filePath) +} diff --git a/pkg/utils/storage_test.go b/pkg/utils/fs_storage_test.go similarity index 100% rename from pkg/utils/storage_test.go rename to pkg/utils/fs_storage_test.go diff --git a/pkg/utils/storage.go b/pkg/utils/storage.go index a44e753d..9ce549d9 100644 --- a/pkg/utils/storage.go +++ b/pkg/utils/storage.go @@ -2,9 +2,6 @@ package utils import ( "encoding/json" - "fmt" - "os" - "path/filepath" ) type Storage interface { @@ -12,31 +9,6 @@ type Storage interface { Load(key string) ([]byte, error) } -const ( - OWNER_ONLY = 0o700 - OWNER_READ_WRITE = 0o600 -) - -// FileSystemStorage implements Storage interface for filesystem operations. -type FileSystemStorage struct { - // BaseDir is the directory where the storage will be saved - BaseDir string -} - -func (fs FileSystemStorage) Save(key string, value []byte) error { - if err := os.MkdirAll(fs.BaseDir, OWNER_ONLY); err != nil { - return fmt.Errorf("failed to create directory: %w", err) - } - - filePath := filepath.Join(fs.BaseDir, key) - return os.WriteFile(filePath, value, OWNER_READ_WRITE) -} - -func (fs FileSystemStorage) Load(key string) ([]byte, error) { - filePath := filepath.Join(fs.BaseDir, key) - return os.ReadFile(filePath) -} - // SaveJSON saves a typed value as JSON using the provided storage. func SaveJSON[T any](storage Storage, key string, value T) error { data, err := json.MarshalIndent(value, "", " ") From 8de8754b32e0c531d2e37131f3accea4db9c4959 Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Sun, 2 Nov 2025 21:39:35 -0500 Subject: [PATCH 29/70] add a keyring storage and some tests --- pkg/utils/keyring_storage.go | 56 ++++++ pkg/utils/keyring_storage_test.go | 277 ++++++++++++++++++++++++++++++ 2 files changed, 333 insertions(+) create mode 100644 pkg/utils/keyring_storage.go create mode 100644 pkg/utils/keyring_storage_test.go diff --git a/pkg/utils/keyring_storage.go b/pkg/utils/keyring_storage.go new file mode 100644 index 00000000..38a2c074 --- /dev/null +++ b/pkg/utils/keyring_storage.go @@ -0,0 +1,56 @@ +package utils + +import ( + "fmt" + + "github.com/zalando/go-keyring" +) + +// KeyRingStorage implements Storage interface using the system keyring. +// The keyring library uses (service, user) as a unique key pair. +// In this implementation: +// - User is set at initialization and used for all operations +// - The key parameter from Save/Load is used as the keyring's "service" field +type KeyRingStorage struct { + // User is the user identifier used in the keyring + User string +} + +// NewKeyRingStorage creates a new KeyRingStorage with the provided user. +func NewKeyRingStorage(user string) *KeyRingStorage { + return &KeyRingStorage{ + User: user, + } +} + +// Save stores the value in the system keyring. +// The key parameter is used as the keyring's service field. +func (ks KeyRingStorage) Save(key string, value []byte) error { + if key == "" { + return fmt.Errorf("key cannot be empty") + } + if ks.User == "" { + return fmt.Errorf("user cannot be empty") + } + + // Use key as the keyring service field + return keyring.Set(key, ks.User, string(value)) +} + +// Load retrieves the value from the system keyring. +// The key parameter is used as the keyring's service field. +func (ks KeyRingStorage) Load(key string) ([]byte, error) { + if key == "" { + return nil, fmt.Errorf("key cannot be empty") + } + if ks.User == "" { + return nil, fmt.Errorf("user cannot be empty") + } + + // Use key as the keyring service field + secret, err := keyring.Get(key, ks.User) + if err != nil { + return nil, err + } + return []byte(secret), nil +} diff --git a/pkg/utils/keyring_storage_test.go b/pkg/utils/keyring_storage_test.go new file mode 100644 index 00000000..0393a445 --- /dev/null +++ b/pkg/utils/keyring_storage_test.go @@ -0,0 +1,277 @@ +package utils + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/zalando/go-keyring" +) + +const ( + testService = "jira-cli-test" + testUser = "test-user" +) + +func TestNewKeyRingStorage(t *testing.T) { + storage := NewKeyRingStorage("test-user") + assert.NotNil(t, storage) + assert.Equal(t, "test-user", storage.User) +} + +func TestKeyRingStorage(t *testing.T) { + // Use mock keyring for all tests + keyring.MockInit() + + t.Run("saves and loads value", func(t *testing.T) { + storage := &KeyRingStorage{User: testUser} + testKey := "test-key" + testValue := []byte("test-value") + + // Test saving + err := storage.Save(testKey, testValue) + assert.NoError(t, err) + + // Test loading + loadedValue, err := storage.Load(testKey) + assert.NoError(t, err) + assert.Equal(t, testValue, loadedValue) + }) + + t.Run("handles empty key on save", func(t *testing.T) { + storage := &KeyRingStorage{User: testUser} + + err := storage.Save("", []byte("test-value")) + assert.Error(t, err) + assert.Contains(t, err.Error(), "key cannot be empty") + }) + + t.Run("handles empty key on load", func(t *testing.T) { + storage := &KeyRingStorage{User: testUser} + + _, err := storage.Load("") + assert.Error(t, err) + assert.Contains(t, err.Error(), "key cannot be empty") + }) + + t.Run("handles empty user on save", func(t *testing.T) { + storage := &KeyRingStorage{User: ""} + + err := storage.Save("test-key", []byte("test-value")) + assert.Error(t, err) + assert.Contains(t, err.Error(), "user cannot be empty") + }) + + t.Run("handles empty user on load", func(t *testing.T) { + storage := &KeyRingStorage{User: ""} + + _, err := storage.Load("test-key") + assert.Error(t, err) + assert.Contains(t, err.Error(), "user cannot be empty") + }) + + t.Run("handles non-existent key", func(t *testing.T) { + storage := &KeyRingStorage{User: testUser} + + // Try to load a key that doesn't exist + _, err := storage.Load("non-existent-key") + assert.Error(t, err) + }) + + t.Run("overwrites existing value", func(t *testing.T) { + storage := &KeyRingStorage{User: testUser} + testKey := "overwrite-test-key" + + // Save initial value + err := storage.Save(testKey, []byte("initial-value")) + assert.NoError(t, err) + + // Overwrite with new value + err = storage.Save(testKey, []byte("new-value")) + assert.NoError(t, err) + + // Verify new value is loaded + loadedValue, err := storage.Load(testKey) + assert.NoError(t, err) + assert.Equal(t, []byte("new-value"), loadedValue) + }) + + t.Run("handles binary data", func(t *testing.T) { + storage := &KeyRingStorage{User: testUser} + testKey := "binary-key" + // Test with binary data including null bytes + binaryData := []byte{0x00, 0x01, 0x02, 0xFF, 0xFE} + + err := storage.Save(testKey, binaryData) + assert.NoError(t, err) + + loadedValue, err := storage.Load(testKey) + assert.NoError(t, err) + assert.Equal(t, binaryData, loadedValue) + }) + + t.Run("handles large values", func(t *testing.T) { + storage := &KeyRingStorage{User: testUser} + testKey := "large-value-key" + // Create a large value (1KB) + largeValue := make([]byte, 1024) + for i := range largeValue { + largeValue[i] = byte(i % 256) + } + + err := storage.Save(testKey, largeValue) + assert.NoError(t, err) + + loadedValue, err := storage.Load(testKey) + assert.NoError(t, err) + assert.Equal(t, largeValue, loadedValue) + }) + + t.Run("handles multiple keys in same service", func(t *testing.T) { + storage := &KeyRingStorage{User: testUser} + + // Save multiple keys + keys := []string{"key1", "key2", "key3"} + values := [][]byte{[]byte("value1"), []byte("value2"), []byte("value3")} + + for i, key := range keys { + err := storage.Save(key, values[i]) + assert.NoError(t, err) + } + + // Verify all keys can be loaded independently + for i, key := range keys { + loadedValue, err := storage.Load(key) + assert.NoError(t, err) + assert.Equal(t, values[i], loadedValue) + } + }) + + t.Run("handles special characters in keys", func(t *testing.T) { + storage := &KeyRingStorage{User: testUser} + specialKeys := []string{ + "key-with-dashes", + "key.with.dots", + "key_with_underscores", + "key@with@symbols", + } + + for _, key := range specialKeys { + value := []byte(fmt.Sprintf("value-for-%s", key)) + err := storage.Save(key, value) + assert.NoError(t, err) + + loadedValue, err := storage.Load(key) + assert.NoError(t, err) + assert.Equal(t, value, loadedValue) + } + }) + + t.Run("different keys are isolated", func(t *testing.T) { + storage := &KeyRingStorage{User: testUser} + key1 := "isolation-key-1" + key2 := "isolation-key-2" + + // Save different values with different keys + err := storage.Save(key1, []byte("value1")) + assert.NoError(t, err) + + err = storage.Save(key2, []byte("value2")) + assert.NoError(t, err) + + // Verify each key returns its own value + value1, err := storage.Load(key1) + assert.NoError(t, err) + assert.Equal(t, []byte("value1"), value1) + + value2, err := storage.Load(key2) + assert.NoError(t, err) + assert.Equal(t, []byte("value2"), value2) + }) + + t.Run("different users are isolated", func(t *testing.T) { + storage1 := &KeyRingStorage{User: "user1"} + storage2 := &KeyRingStorage{User: "user2"} + testKey := "isolation-key" + + // Save different values for different users with the same key + err := storage1.Save(testKey, []byte("value1")) + assert.NoError(t, err) + + err = storage2.Save(testKey, []byte("value2")) + assert.NoError(t, err) + + // Verify each user returns its own value + value1, err := storage1.Load(testKey) + assert.NoError(t, err) + assert.Equal(t, []byte("value1"), value1) + + value2, err := storage2.Load(testKey) + assert.NoError(t, err) + assert.Equal(t, []byte("value2"), value2) + }) +} + +func TestKeyRingStorageImplementsInterface(t *testing.T) { + // This test ensures KeyRingStorage implements the Storage interface + var _ Storage = &KeyRingStorage{} + var _ Storage = KeyRingStorage{} +} + +// TestKeyRingStorageWithHelpers tests the SaveJSON and LoadJSON helper functions +func TestKeyRingStorageWithHelpers(t *testing.T) { + keyring.MockInit() + + t.Run("SaveJSON and LoadJSON with struct", func(t *testing.T) { + storage := &KeyRingStorage{User: testUser} + testKey := "json-test-key" + + type TestData struct { + Name string `json:"name"` + Value int `json:"value"` + } + + original := TestData{ + Name: "test", + Value: 42, + } + + // Test SaveJSON + err := SaveJSON(storage, testKey, original) + assert.NoError(t, err) + + // Test LoadJSON + loaded, err := LoadJSON[TestData](storage, testKey) + assert.NoError(t, err) + assert.Equal(t, original, loaded) + }) + + t.Run("SaveJSON and LoadJSON with map", func(t *testing.T) { + storage := &KeyRingStorage{User: testUser} + testKey := "json-map-key" + + original := map[string]interface{}{ + "key1": "value1", + "key2": float64(123), + "key3": true, + } + + err := SaveJSON(storage, testKey, original) + assert.NoError(t, err) + + loaded, err := LoadJSON[map[string]interface{}](storage, testKey) + assert.NoError(t, err) + assert.Equal(t, original, loaded) + }) + + t.Run("LoadJSON handles non-existent key", func(t *testing.T) { + storage := &KeyRingStorage{User: testUser} + + type TestData struct { + Name string `json:"name"` + } + + _, err := LoadJSON[TestData](storage, "non-existent-json-key") + assert.Error(t, err) + }) +} From f655fe820a2c43288ed044bfbb48cd3de736bc56 Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Sat, 15 Nov 2025 15:03:34 -0500 Subject: [PATCH 30/70] filesystem should work similar to keyring --- pkg/oauth/oauth.go | 2 +- pkg/utils/fs_storage.go | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/oauth/oauth.go b/pkg/oauth/oauth.go index 82ae04a6..1f0b3bf8 100644 --- a/pkg/oauth/oauth.go +++ b/pkg/oauth/oauth.go @@ -31,7 +31,7 @@ const ( oauthTimeout = 5 * time.Minute // OAuth storage file name. - oauthSecretsFile = "oauth_secrets.json" + oauthSecretsFile = "jira-cli-oauth-secrets" // Server shutdown timeout. serverShutdownTimeout = 5 * time.Second diff --git a/pkg/utils/fs_storage.go b/pkg/utils/fs_storage.go index c720965d..6e3f83b5 100644 --- a/pkg/utils/fs_storage.go +++ b/pkg/utils/fs_storage.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "path/filepath" + "strings" ) const ( @@ -21,6 +22,9 @@ func (fs FileSystemStorage) Save(key string, value []byte) error { if err := os.MkdirAll(fs.BaseDir, OWNER_ONLY); err != nil { return fmt.Errorf("failed to create directory: %w", err) } + if !strings.HasSuffix(key, ".json") { + key = key + ".json" + } filePath := filepath.Join(fs.BaseDir, key) return os.WriteFile(filePath, value, OWNER_READ_WRITE) From da55c8c24c8900b773d4d6ca6ecf3751416817c0 Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Sat, 15 Nov 2025 15:04:59 -0500 Subject: [PATCH 31/70] [test] add storage_test --- pkg/utils/storage_test.go | 504 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 504 insertions(+) create mode 100644 pkg/utils/storage_test.go diff --git a/pkg/utils/storage_test.go b/pkg/utils/storage_test.go new file mode 100644 index 00000000..367e9ab8 --- /dev/null +++ b/pkg/utils/storage_test.go @@ -0,0 +1,504 @@ +package utils + +import ( + "encoding/json" + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +// testStorage is a mock storage for testing SaveJSON and LoadJSON. +type testStorage struct { + data map[string][]byte + saveError error + loadError error +} + +func newTestStorage() *testStorage { + return &testStorage{ + data: make(map[string][]byte), + } +} + +func (m *testStorage) Save(key string, value []byte) error { + if m.saveError != nil { + return m.saveError + } + m.data[key] = value + return nil +} + +func (m *testStorage) Load(key string) ([]byte, error) { + if m.loadError != nil { + return nil, m.loadError + } + value, ok := m.data[key] + if !ok { + return nil, fmt.Errorf("key not found: %s", key) + } + return value, nil +} + +func TestSaveJSON(t *testing.T) { + t.Run("saves simple struct as JSON", func(t *testing.T) { + storage := newTestStorage() + testKey := "test-struct" + + type Person struct { + Name string `json:"name"` + Age int `json:"age"` + } + + person := Person{Name: "John Doe", Age: 30} + + err := SaveJSON(storage, testKey, person) + assert.NoError(t, err) + + // Verify the data was saved correctly + savedData, ok := storage.data[testKey] + assert.True(t, ok) + + var loaded Person + err = json.Unmarshal(savedData, &loaded) + assert.NoError(t, err) + assert.Equal(t, person, loaded) + + // Verify it's formatted with indentation + assert.Contains(t, string(savedData), "\n") + assert.Contains(t, string(savedData), " ") + }) + + t.Run("saves nested struct as JSON", func(t *testing.T) { + storage := newTestStorage() + testKey := "test-nested" + + type Address struct { + Street string `json:"street"` + City string `json:"city"` + } + + type User struct { + Name string `json:"name"` + Address Address `json:"address"` + } + + user := User{ + Name: "Jane Doe", + Address: Address{ + Street: "123 Main St", + City: "Springfield", + }, + } + + err := SaveJSON(storage, testKey, user) + assert.NoError(t, err) + + savedData := storage.data[testKey] + var loaded User + err = json.Unmarshal(savedData, &loaded) + assert.NoError(t, err) + assert.Equal(t, user, loaded) + }) + + t.Run("saves map as JSON", func(t *testing.T) { + storage := newTestStorage() + testKey := "test-map" + + data := map[string]interface{}{ + "key1": "value1", + "key2": 123, + "key3": true, + "key4": []string{"a", "b", "c"}, + } + + err := SaveJSON(storage, testKey, data) + assert.NoError(t, err) + + savedData := storage.data[testKey] + var loaded map[string]interface{} + err = json.Unmarshal(savedData, &loaded) + assert.NoError(t, err) + assert.Equal(t, "value1", loaded["key1"]) + assert.Equal(t, float64(123), loaded["key2"]) // JSON numbers are float64 + assert.Equal(t, true, loaded["key3"]) + }) + + t.Run("saves slice as JSON", func(t *testing.T) { + storage := newTestStorage() + testKey := "test-slice" + + data := []string{"apple", "banana", "cherry"} + + err := SaveJSON(storage, testKey, data) + assert.NoError(t, err) + + savedData := storage.data[testKey] + var loaded []string + err = json.Unmarshal(savedData, &loaded) + assert.NoError(t, err) + assert.Equal(t, data, loaded) + }) + + t.Run("saves pointer to struct as JSON", func(t *testing.T) { + storage := newTestStorage() + testKey := "test-pointer" + + type Config struct { + Enabled bool `json:"enabled"` + Timeout int `json:"timeout"` + Name string `json:"name"` + } + + config := &Config{ + Enabled: true, + Timeout: 30, + Name: "test-config", + } + + err := SaveJSON(storage, testKey, config) + assert.NoError(t, err) + + savedData := storage.data[testKey] + var loaded Config + err = json.Unmarshal(savedData, &loaded) + assert.NoError(t, err) + assert.Equal(t, *config, loaded) + }) + + t.Run("saves empty struct as JSON", func(t *testing.T) { + storage := newTestStorage() + testKey := "test-empty" + + type Empty struct{} + empty := Empty{} + + err := SaveJSON(storage, testKey, empty) + assert.NoError(t, err) + assert.Equal(t, []byte("{}"), storage.data[testKey]) + }) + + t.Run("saves nil pointer as null", func(t *testing.T) { + storage := newTestStorage() + testKey := "test-nil" + + var nilPointer *string + + err := SaveJSON(storage, testKey, nilPointer) + assert.NoError(t, err) + assert.Equal(t, []byte("null"), storage.data[testKey]) + }) + + t.Run("handles storage save error", func(t *testing.T) { + storage := &testStorage{ + saveError: fmt.Errorf("storage save failed"), + } + testKey := "test-key" + + type Simple struct { + Value string `json:"value"` + } + + err := SaveJSON(storage, testKey, Simple{Value: "test"}) + assert.Error(t, err) + assert.Contains(t, err.Error(), "storage save failed") + }) + + t.Run("handles unmarshalable types", func(t *testing.T) { + storage := newTestStorage() + testKey := "test-unmarshalable" + + // Channels cannot be marshaled to JSON + invalidData := make(chan int) + + err := SaveJSON(storage, testKey, invalidData) + assert.Error(t, err) + assert.Contains(t, err.Error(), "json") + }) +} + +func TestLoadJSON(t *testing.T) { + t.Run("loads simple struct from JSON", func(t *testing.T) { + storage := newTestStorage() + testKey := "test-struct" + + type Person struct { + Name string `json:"name"` + Age int `json:"age"` + } + + expected := Person{Name: "John Doe", Age: 30} + jsonData, _ := json.Marshal(expected) + storage.data[testKey] = jsonData + + loaded, err := LoadJSON[Person](storage, testKey) + assert.NoError(t, err) + assert.Equal(t, expected, loaded) + }) + + t.Run("loads nested struct from JSON", func(t *testing.T) { + storage := newTestStorage() + testKey := "test-nested" + + type Address struct { + Street string `json:"street"` + City string `json:"city"` + } + + type User struct { + Name string `json:"name"` + Address Address `json:"address"` + } + + expected := User{ + Name: "Jane Doe", + Address: Address{ + Street: "123 Main St", + City: "Springfield", + }, + } + jsonData, _ := json.Marshal(expected) + storage.data[testKey] = jsonData + + loaded, err := LoadJSON[User](storage, testKey) + assert.NoError(t, err) + assert.Equal(t, expected, loaded) + }) + + t.Run("loads map from JSON", func(t *testing.T) { + storage := newTestStorage() + testKey := "test-map" + + expected := map[string]string{ + "key1": "value1", + "key2": "value2", + } + jsonData, _ := json.Marshal(expected) + storage.data[testKey] = jsonData + + loaded, err := LoadJSON[map[string]string](storage, testKey) + assert.NoError(t, err) + assert.Equal(t, expected, loaded) + }) + + t.Run("loads slice from JSON", func(t *testing.T) { + storage := newTestStorage() + testKey := "test-slice" + + expected := []int{1, 2, 3, 4, 5} + jsonData, _ := json.Marshal(expected) + storage.data[testKey] = jsonData + + loaded, err := LoadJSON[[]int](storage, testKey) + assert.NoError(t, err) + assert.Equal(t, expected, loaded) + }) + + t.Run("loads pointer to struct from JSON", func(t *testing.T) { + storage := newTestStorage() + testKey := "test-pointer" + + type Config struct { + Enabled bool `json:"enabled"` + Timeout int `json:"timeout"` + } + + expected := Config{Enabled: true, Timeout: 30} + jsonData, _ := json.Marshal(expected) + storage.data[testKey] = jsonData + + loaded, err := LoadJSON[*Config](storage, testKey) + assert.NoError(t, err) + assert.Equal(t, expected, *loaded) + }) + + t.Run("returns zero value on non-existent key", func(t *testing.T) { + storage := newTestStorage() + testKey := "non-existent" + + type Simple struct { + Value string `json:"value"` + } + + loaded, err := LoadJSON[Simple](storage, testKey) + assert.Error(t, err) + assert.Contains(t, err.Error(), "key not found") + assert.Equal(t, Simple{}, loaded) // Should return zero value + }) + + t.Run("handles storage load error", func(t *testing.T) { + storage := &testStorage{ + loadError: fmt.Errorf("storage load failed"), + } + testKey := "test-key" + + type Simple struct { + Value string `json:"value"` + } + + loaded, err := LoadJSON[Simple](storage, testKey) + assert.Error(t, err) + assert.Contains(t, err.Error(), "storage load failed") + assert.Equal(t, Simple{}, loaded) + }) + + t.Run("handles invalid JSON", func(t *testing.T) { + storage := newTestStorage() + testKey := "test-invalid" + + storage.data[testKey] = []byte("invalid json{]") + + type Simple struct { + Value string `json:"value"` + } + + loaded, err := LoadJSON[Simple](storage, testKey) + assert.Error(t, err) + assert.Equal(t, Simple{}, loaded) + }) + + t.Run("handles JSON with missing fields", func(t *testing.T) { + storage := newTestStorage() + testKey := "test-partial" + + type Complete struct { + Required string `json:"required"` + Optional string `json:"optional"` + } + + // JSON with only one field + storage.data[testKey] = []byte(`{"required":"value"}`) + + loaded, err := LoadJSON[Complete](storage, testKey) + assert.NoError(t, err) + assert.Equal(t, "value", loaded.Required) + assert.Equal(t, "", loaded.Optional) // Should have zero value + }) + + t.Run("handles JSON with extra fields", func(t *testing.T) { + storage := newTestStorage() + testKey := "test-extra" + + type Simple struct { + Field string `json:"field"` + } + + // JSON with extra fields that aren't in the struct + storage.data[testKey] = []byte(`{"field":"value","extra":"ignored"}`) + + loaded, err := LoadJSON[Simple](storage, testKey) + assert.NoError(t, err) + assert.Equal(t, "value", loaded.Field) + }) +} + +func TestSaveAndLoadJSON_Integration(t *testing.T) { + t.Run("round-trip with complex struct", func(t *testing.T) { + storage := newTestStorage() + testKey := "test-roundtrip" + + type Tag struct { + Name string `json:"name"` + Color string `json:"color"` + } + + type Article struct { + Title string `json:"title"` + Content string `json:"content"` + Author string `json:"author"` + Tags []Tag `json:"tags"` + Published bool `json:"published"` + Views int `json:"views"` + Metadata map[string]interface{} `json:"metadata"` + } + + original := Article{ + Title: "Test Article", + Content: "This is a test article with some content.", + Author: "John Doe", + Tags: []Tag{ + {Name: "go", Color: "blue"}, + {Name: "testing", Color: "green"}, + }, + Published: true, + Views: 1234, + Metadata: map[string]interface{}{ + "category": "technology", + "priority": float64(5), + }, + } + + // Save + err := SaveJSON(storage, testKey, original) + assert.NoError(t, err) + + // Load + loaded, err := LoadJSON[Article](storage, testKey) + assert.NoError(t, err) + assert.Equal(t, original.Title, loaded.Title) + assert.Equal(t, original.Content, loaded.Content) + assert.Equal(t, original.Author, loaded.Author) + assert.Equal(t, len(original.Tags), len(loaded.Tags)) + assert.Equal(t, original.Published, loaded.Published) + assert.Equal(t, original.Views, loaded.Views) + }) + + t.Run("round-trip with different storage implementations", func(t *testing.T) { + testKey := "test-storage" + + type Data struct { + Value string `json:"value"` + Count int `json:"count"` + } + + original := Data{Value: "test", Count: 42} + + // Test with mock storage + mockStore := newTestStorage() + err := SaveJSON(mockStore, testKey, original) + assert.NoError(t, err) + + loaded, err := LoadJSON[Data](mockStore, testKey) + assert.NoError(t, err) + assert.Equal(t, original, loaded) + + // Test with filesystem storage + tempDir := t.TempDir() + fsStore := FileSystemStorage{BaseDir: tempDir} + err = SaveJSON(&fsStore, testKey, original) + assert.NoError(t, err) + + loaded, err = LoadJSON[Data](&fsStore, testKey) + assert.NoError(t, err) + assert.Equal(t, original, loaded) + }) + + t.Run("overwrite existing JSON data", func(t *testing.T) { + storage := newTestStorage() + testKey := "test-overwrite" + + type Version struct { + Number int `json:"number"` + Name string `json:"name"` + } + + v1 := Version{Number: 1, Name: "first"} + v2 := Version{Number: 2, Name: "second"} + + // Save v1 + err := SaveJSON(storage, testKey, v1) + assert.NoError(t, err) + + loaded, err := LoadJSON[Version](storage, testKey) + assert.NoError(t, err) + assert.Equal(t, v1, loaded) + + // Overwrite with v2 + err = SaveJSON(storage, testKey, v2) + assert.NoError(t, err) + + loaded, err = LoadJSON[Version](storage, testKey) + assert.NoError(t, err) + assert.Equal(t, v2, loaded) + }) +} From 772471d6c2894709c3ef85325b6635a3dd89d618 Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Sat, 15 Nov 2025 15:06:52 -0500 Subject: [PATCH 32/70] update keyring storage to remove some unless comments --- pkg/utils/keyring_storage.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/pkg/utils/keyring_storage.go b/pkg/utils/keyring_storage.go index 38a2c074..61f06994 100644 --- a/pkg/utils/keyring_storage.go +++ b/pkg/utils/keyring_storage.go @@ -8,11 +8,7 @@ import ( // KeyRingStorage implements Storage interface using the system keyring. // The keyring library uses (service, user) as a unique key pair. -// In this implementation: -// - User is set at initialization and used for all operations -// - The key parameter from Save/Load is used as the keyring's "service" field type KeyRingStorage struct { - // User is the user identifier used in the keyring User string } @@ -33,12 +29,10 @@ func (ks KeyRingStorage) Save(key string, value []byte) error { return fmt.Errorf("user cannot be empty") } - // Use key as the keyring service field return keyring.Set(key, ks.User, string(value)) } // Load retrieves the value from the system keyring. -// The key parameter is used as the keyring's service field. func (ks KeyRingStorage) Load(key string) ([]byte, error) { if key == "" { return nil, fmt.Errorf("key cannot be empty") @@ -47,7 +41,6 @@ func (ks KeyRingStorage) Load(key string) ([]byte, error) { return nil, fmt.Errorf("user cannot be empty") } - // Use key as the keyring service field secret, err := keyring.Get(key, ks.User) if err != nil { return nil, err From acf64fc6a06595996e8529c5c48bba3b42cf8725 Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Sat, 15 Nov 2025 15:11:35 -0500 Subject: [PATCH 33/70] add a way for use a fallback storage for saving the token --- pkg/oauth/tokens.go | 54 +++++++++++++++++++++++++++++++++++++-------- 1 file changed, 45 insertions(+), 9 deletions(-) diff --git a/pkg/oauth/tokens.go b/pkg/oauth/tokens.go index bc64a686..a0639575 100644 --- a/pkg/oauth/tokens.go +++ b/pkg/oauth/tokens.go @@ -21,9 +21,11 @@ type OAuthSecrets struct { // PersistentTokenSource implements oauth2.TokenSource with automatic token persistence. type PersistentTokenSource struct { - clientID string - clientSecret string - storage utils.Storage + clientID string + clientSecret string + storage utils.Storage + fallbackStorage utils.Storage + usingFallback bool } // IsExpired checks if the access token is expired. @@ -55,17 +57,23 @@ func (o *OAuthSecrets) FromOAuth2Token(token *oauth2.Token) { } // NewPersistentTokenSource creates a new TokenSource that persists tokens. +// It attempts to use keyring storage first, falling back to filesystem storage if keyring fails. func NewPersistentTokenSource(clientID, clientSecret string) (*PersistentTokenSource, error) { jiraDir, err := getJiraConfigDir() if err != nil { return nil, fmt.Errorf("failed to get Jira config directory: %w", err) } - storage := utils.FileSystemStorage{BaseDir: jiraDir} + keyringStorage := utils.NewKeyRingStorage(login) + fallbackFileSystemStorage := utils.FileSystemStorage{BaseDir: jiraDir} + + return &PersistentTokenSource{ - clientID: clientID, - clientSecret: clientSecret, - storage: storage, + clientID: clientID, + clientSecret: clientSecret, + storage: keyringStorage, + fallbackStorage: fallbackFileSystemStorage, + usingFallback: false, }, nil } @@ -74,7 +82,19 @@ func (pts *PersistentTokenSource) Token() (*oauth2.Token, error) { // Load current token from storage secrets, err := utils.LoadJSON[OAuthSecrets](pts.storage, oauthSecretsFile) if err != nil { - return nil, fmt.Errorf("failed to load OAuth secrets: %w", err) + // If primary storage fails and we're not already using fallback, try fallback + if !pts.usingFallback && pts.fallbackStorage != nil { + fmt.Println("Warning: Primary storage failed, falling back to FileSystemStorage for OAuth tokens") + secrets, err = utils.LoadJSON[OAuthSecrets](pts.fallbackStorage, oauthSecretsFile) + if err == nil { + // Successfully loaded from fallback, switch to using it + pts.storage = pts.fallbackStorage + pts.usingFallback = true + } + } + if err != nil { + return nil, fmt.Errorf("failed to load OAuth secrets: %w", err) + } } token := secrets.ToOAuth2Token() @@ -102,7 +122,7 @@ func (pts *PersistentTokenSource) Token() (*oauth2.Token, error) { // Save the refreshed token secrets.FromOAuth2Token(refreshedToken) - if err := utils.SaveJSON(pts.storage, oauthSecretsFile, &secrets); err != nil { + if err := pts.saveSecrets(&secrets); err != nil { // Log error but don't fail the request - we still have a valid token fmt.Printf("Warning: failed to save refreshed OAuth token: %v\n", err) } @@ -110,6 +130,22 @@ func (pts *PersistentTokenSource) Token() (*oauth2.Token, error) { return refreshedToken, nil } +// saveSecrets attempts to save secrets to primary storage, falling back if necessary. +func (pts *PersistentTokenSource) saveSecrets(secrets *OAuthSecrets) error { + err := utils.SaveJSON(pts.storage, oauthSecretsFile, secrets) + if err != nil && !pts.usingFallback && pts.fallbackStorage != nil { + // Primary storage failed, try fallback + fmt.Println("Warning: Primary storage failed, falling back to FileSystemStorage for OAuth tokens") + err = utils.SaveJSON(pts.fallbackStorage, oauthSecretsFile, secrets) + if err == nil { + // Successfully saved to fallback, switch to using it + pts.storage = pts.fallbackStorage + pts.usingFallback = true + } + } + return err +} + // LoadOAuth2TokenSource creates a TokenSource from stored OAuth secrets. func LoadOAuth2TokenSource() (oauth2.TokenSource, error) { // Load OAuth secrets to get client credentials From b6caa86188ae9067a20dda874f781b4cd04b0828 Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Sat, 15 Nov 2025 17:49:07 -0500 Subject: [PATCH 34/70] (fix) separating the login details and the server details --- internal/config/generator.go | 90 ++++++++++++++++++++++-------------- 1 file changed, 55 insertions(+), 35 deletions(-) diff --git a/internal/config/generator.go b/internal/config/generator.go index 223187c2..df3be8c9 100644 --- a/internal/config/generator.go +++ b/internal/config/generator.go @@ -187,13 +187,18 @@ func (c *JiraCLIConfigGenerator) Generate() (string, error) { } } + if err := c.configureLoginDetails(); err != nil { + return "", err + + } + if c.value.authType == jira.AuthTypeOAuth { if err := c.configureOAuth(); err != nil { return "", err } } - if err := c.configureServerAndLoginDetails(); err != nil { + if err := c.configureServerDetails(); err != nil { return "", err } @@ -368,39 +373,10 @@ func (c *JiraCLIConfigGenerator) configureOAuth() error { } //nolint:gocyclo -func (c *JiraCLIConfigGenerator) configureServerAndLoginDetails() error { +func (c *JiraCLIConfigGenerator) configureLoginDetails() error { var qs []*survey.Question - c.value.server = c.usrCfg.Server c.value.login = c.usrCfg.Login - - if c.usrCfg.Server == "" { - qs = append(qs, &survey.Question{ - Name: "server", - Prompt: &survey.Input{ - Message: "Link to Jira server:", - Help: "This is a link to your jira server, eg: https://company.atlassian.net", - }, - Validate: func(val interface{}) error { - errInvalidURL := fmt.Errorf("not a valid URL") - - str, ok := val.(string) - if !ok { - return errInvalidURL - } - u, err := url.Parse(str) - if err != nil || u.Scheme == "" || u.Host == "" { - return errInvalidURL - } - if u.Scheme != "http" && u.Scheme != "https" { - return errInvalidURL - } - - return nil - }, - }) - } - if c.usrCfg.Login == "" { switch c.value.installation { case jira.InstallationTypeCloud: @@ -455,11 +431,58 @@ func (c *JiraCLIConfigGenerator) configureServerAndLoginDetails() error { }) } } + if len(qs) > 0 { + ans := struct { + Login string + }{} + + if err := survey.Ask(qs, &ans); err != nil { + return err + } + if ans.Login != "" { + c.value.login = strings.TrimSpace(ans.Login) + } + + } + return nil +} + +//nolint:gocyclo +func (c *JiraCLIConfigGenerator) configureServerDetails() error { + var qs []*survey.Question + + c.value.server = c.usrCfg.Server + + if c.usrCfg.Server == "" { + qs = append(qs, &survey.Question{ + Name: "server", + Prompt: &survey.Input{ + Message: "Link to Jira server:", + Help: "This is a link to your jira server, eg: https://company.atlassian.net", + }, + Validate: func(val interface{}) error { + errInvalidURL := fmt.Errorf("not a valid URL") + + str, ok := val.(string) + if !ok { + return errInvalidURL + } + u, err := url.Parse(str) + if err != nil || u.Scheme == "" || u.Host == "" { + return errInvalidURL + } + if u.Scheme != "http" && u.Scheme != "https" { + return errInvalidURL + } + + return nil + }, + }) + } if len(qs) > 0 { ans := struct { Server string - Login string }{} if err := survey.Ask(qs, &ans); err != nil { @@ -469,9 +492,6 @@ func (c *JiraCLIConfigGenerator) configureServerAndLoginDetails() error { if ans.Server != "" { c.value.server = strings.TrimSpace(ans.Server) } - if ans.Login != "" { - c.value.login = strings.TrimSpace(ans.Login) - } if c.value.authType == jira.AuthTypeOAuth { // Set server URL using the cloud ID from OAuth configuration From 0c6e24da74e91ebbe4db2691f4251d5f8ebb8b6d Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Sat, 15 Nov 2025 17:50:09 -0500 Subject: [PATCH 35/70] (nit) remove the testService in the keyring storage test --- pkg/utils/keyring_storage_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/utils/keyring_storage_test.go b/pkg/utils/keyring_storage_test.go index 0393a445..20b5e73a 100644 --- a/pkg/utils/keyring_storage_test.go +++ b/pkg/utils/keyring_storage_test.go @@ -9,8 +9,7 @@ import ( ) const ( - testService = "jira-cli-test" - testUser = "test-user" + testUser = "test-user" ) func TestNewKeyRingStorage(t *testing.T) { From 5cd847ce5740cca61773408ee91ab1c0ced399b9 Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Sat, 15 Nov 2025 17:55:33 -0500 Subject: [PATCH 36/70] (fix) pass around the login field to the oauth token so the keyring works properly --- api/client.go | 8 ++++---- internal/config/generator.go | 3 ++- pkg/oauth/oauth.go | 32 ++++++++++++++++++++------------ pkg/oauth/tokens.go | 9 ++++----- 4 files changed, 30 insertions(+), 22 deletions(-) diff --git a/api/client.go b/api/client.go index 9ec7cc39..5c91def1 100644 --- a/api/client.go +++ b/api/client.go @@ -35,8 +35,8 @@ func getAPIToken(config *jira.Config) string { // Try OAuth access token if available and valid // And should only do this assertion if the AuthType is oauth isAuthTypeOAuth := config.AuthType != nil && *config.AuthType == jira.AuthTypeOAuth - if isAuthTypeOAuth && oauth.HasOAuthCredentials() { - tk, _ := oauth.LoadOAuth2TokenSource() + if isAuthTypeOAuth && oauth.HasOAuthCredentials(config.Login) { + tk, _ := oauth.LoadOAuth2TokenSource(config.Login) token, _ := tk.Token() return token.AccessToken } @@ -85,9 +85,9 @@ func Client(config jira.Config) *jira.Client { } // Check if we have OAuth credentials and should use OAuth - if oauth.HasOAuthCredentials() && config.AuthType != nil && *config.AuthType == jira.AuthTypeOAuth { + if oauth.HasOAuthCredentials(config.Login) && config.AuthType != nil && *config.AuthType == jira.AuthTypeOAuth { // Try to create OAuth2 token source - tokenSource, err := oauth.LoadOAuth2TokenSource() + tokenSource, err := oauth.LoadOAuth2TokenSource(config.Login) if err == nil { // We have valid OAuth credentials, use OAuth authentication // Pass the TokenSource to the client via a custom option diff --git a/internal/config/generator.go b/internal/config/generator.go index df3be8c9..9fc68ae4 100644 --- a/internal/config/generator.go +++ b/internal/config/generator.go @@ -359,7 +359,8 @@ func (c *JiraCLIConfigGenerator) configureMTLS() error { func (c *JiraCLIConfigGenerator) configureOAuth() error { // Use the new OAuth package - tokenResponse, err := oauth.Configure() + + tokenResponse, err := oauth.Configure(c.value.login) if err != nil { return err } diff --git a/pkg/oauth/oauth.go b/pkg/oauth/oauth.go index 1f0b3bf8..9a7a77ad 100644 --- a/pkg/oauth/oauth.go +++ b/pkg/oauth/oauth.go @@ -97,15 +97,13 @@ func GetOAuth2Config(clientID, clientSecret, redirectURI string, scopes []string } // Configure performs the complete OAuth flow and returns tokens. -func Configure() (*ConfigureTokenResponse, error) { +func Configure(login string) (*ConfigureTokenResponse, error) { // Collect OAuth credentials from user jiraDir, err := getJiraConfigDir() if err != nil { return nil, fmt.Errorf("failed to get Jira config directory: %w", err) } - secretStorage := utils.FileSystemStorage{BaseDir: jiraDir} - config, err := collectOAuthCredentials() if err != nil { return nil, fmt.Errorf("failed to collect OAuth credentials: %w", err) @@ -132,9 +130,15 @@ func Configure() (*ConfigureTokenResponse, error) { TokenType: tokens.TokenType, Expiry: tokens.Expiry, } + primarySecretStorage := utils.KeyRingStorage{User: login} + fallbackSecretStorage := utils.FileSystemStorage{BaseDir: jiraDir} - if err := utils.SaveJSON(secretStorage, oauthSecretsFile, oauthSecrets); err != nil { - return nil, fmt.Errorf("failed to store OAuth secrets: %w", err) + if err := utils.SaveJSON(primarySecretStorage, oauthSecretsFile, oauthSecrets); err != nil { + fmt.Printf("Warning: Failed to save to the primarySecretStorage, falling back to alternative") + err = utils.SaveJSON(fallbackSecretStorage, oauthSecretsFile, oauthSecrets) + if err != nil { + return nil, fmt.Errorf("failed to store OAuth secrets: %w", err) + } } return &ConfigureTokenResponse{ @@ -145,24 +149,28 @@ func Configure() (*ConfigureTokenResponse, error) { } // LoadOAuthSecrets loads OAuth secrets from storage. -func LoadOAuthSecrets() (*OAuthSecrets, error) { +func LoadOAuthSecrets(login string) (*OAuthSecrets, error) { jiraDir, err := getJiraConfigDir() if err != nil { return nil, fmt.Errorf("failed to get Jira config directory: %w", err) } - - secretStorage := utils.FileSystemStorage{BaseDir: jiraDir} - secrets, err := utils.LoadJSON[OAuthSecrets](secretStorage, oauthSecretsFile) + primaryStorage := utils.KeyRingStorage{User: login} + fallbackSecretStorage := utils.FileSystemStorage{BaseDir: jiraDir} + secrets, err := utils.LoadJSON[OAuthSecrets](primaryStorage, oauthSecretsFile) if err != nil { - return nil, fmt.Errorf("failed to load OAuth secrets: %w", err) + fmt.Printf("Warning: Primary storage failed to save, using fallback") + secrets, err = utils.LoadJSON[OAuthSecrets](fallbackSecretStorage, oauthSecretsFile) + if err != nil { + return nil, fmt.Errorf("failed to load OAuth secrets: %w", err) + } } return &secrets, nil } // HasOAuthCredentials checks if OAuth credentials are present. -func HasOAuthCredentials() bool { - _, err := LoadOAuthSecrets() +func HasOAuthCredentials(login string) bool { + _, err := LoadOAuthSecrets(login) return err == nil } diff --git a/pkg/oauth/tokens.go b/pkg/oauth/tokens.go index a0639575..66bccbaf 100644 --- a/pkg/oauth/tokens.go +++ b/pkg/oauth/tokens.go @@ -58,7 +58,7 @@ func (o *OAuthSecrets) FromOAuth2Token(token *oauth2.Token) { // NewPersistentTokenSource creates a new TokenSource that persists tokens. // It attempts to use keyring storage first, falling back to filesystem storage if keyring fails. -func NewPersistentTokenSource(clientID, clientSecret string) (*PersistentTokenSource, error) { +func NewPersistentTokenSource(login, clientID, clientSecret string) (*PersistentTokenSource, error) { jiraDir, err := getJiraConfigDir() if err != nil { return nil, fmt.Errorf("failed to get Jira config directory: %w", err) @@ -67,7 +67,6 @@ func NewPersistentTokenSource(clientID, clientSecret string) (*PersistentTokenSo keyringStorage := utils.NewKeyRingStorage(login) fallbackFileSystemStorage := utils.FileSystemStorage{BaseDir: jiraDir} - return &PersistentTokenSource{ clientID: clientID, clientSecret: clientSecret, @@ -147,15 +146,15 @@ func (pts *PersistentTokenSource) saveSecrets(secrets *OAuthSecrets) error { } // LoadOAuth2TokenSource creates a TokenSource from stored OAuth secrets. -func LoadOAuth2TokenSource() (oauth2.TokenSource, error) { +func LoadOAuth2TokenSource(login string) (oauth2.TokenSource, error) { // Load OAuth secrets to get client credentials - secrets, err := LoadOAuthSecrets() + secrets, err := LoadOAuthSecrets(login) if err != nil { return nil, fmt.Errorf("failed to load OAuth secrets: %w", err) } // Create persistent token source - tokenSource, err := NewPersistentTokenSource(secrets.ClientID, secrets.ClientSecret) + tokenSource, err := NewPersistentTokenSource(login, secrets.ClientID, secrets.ClientSecret) if err != nil { return nil, fmt.Errorf("failed to create token source: %w", err) } From bc29aa80546cefd092955b983c53756c53a52721 Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Sat, 15 Nov 2025 17:58:37 -0500 Subject: [PATCH 37/70] (lint) make lint --- internal/cmd/root/root.go | 2 +- internal/config/generator.go | 2 -- pkg/utils/fs_storage.go | 2 +- pkg/utils/keyring_storage_test.go | 2 +- 4 files changed, 3 insertions(+), 5 deletions(-) diff --git a/internal/cmd/root/root.go b/internal/cmd/root/root.go index 694da0d7..e8d2e107 100644 --- a/internal/cmd/root/root.go +++ b/internal/cmd/root/root.go @@ -157,7 +157,7 @@ func cmdRequireToken(cmd string) bool { } func checkForJiraToken(server string, login string) { - if oauth.HasOAuthCredentials() { + if oauth.HasOAuthCredentials(login) { return } diff --git a/internal/config/generator.go b/internal/config/generator.go index 9fc68ae4..e0dcb938 100644 --- a/internal/config/generator.go +++ b/internal/config/generator.go @@ -189,7 +189,6 @@ func (c *JiraCLIConfigGenerator) Generate() (string, error) { if err := c.configureLoginDetails(); err != nil { return "", err - } if c.value.authType == jira.AuthTypeOAuth { @@ -373,7 +372,6 @@ func (c *JiraCLIConfigGenerator) configureOAuth() error { return nil } -//nolint:gocyclo func (c *JiraCLIConfigGenerator) configureLoginDetails() error { var qs []*survey.Question diff --git a/pkg/utils/fs_storage.go b/pkg/utils/fs_storage.go index 6e3f83b5..37b2ac96 100644 --- a/pkg/utils/fs_storage.go +++ b/pkg/utils/fs_storage.go @@ -23,7 +23,7 @@ func (fs FileSystemStorage) Save(key string, value []byte) error { return fmt.Errorf("failed to create directory: %w", err) } if !strings.HasSuffix(key, ".json") { - key = key + ".json" + key += ".json" } filePath := filepath.Join(fs.BaseDir, key) diff --git a/pkg/utils/keyring_storage_test.go b/pkg/utils/keyring_storage_test.go index 20b5e73a..b243a42e 100644 --- a/pkg/utils/keyring_storage_test.go +++ b/pkg/utils/keyring_storage_test.go @@ -217,7 +217,7 @@ func TestKeyRingStorageImplementsInterface(t *testing.T) { var _ Storage = KeyRingStorage{} } -// TestKeyRingStorageWithHelpers tests the SaveJSON and LoadJSON helper functions +// TestKeyRingStorageWithHelpers tests the SaveJSON and LoadJSON helper functions. func TestKeyRingStorageWithHelpers(t *testing.T) { keyring.MockInit() From fd7e189ae36191040cd91f2bd009ef3fb575ee5b Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Sat, 15 Nov 2025 20:45:07 -0500 Subject: [PATCH 38/70] nit: shouldn't pretty print an oauth file --- pkg/utils/storage.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/utils/storage.go b/pkg/utils/storage.go index 9ce549d9..58c0a78b 100644 --- a/pkg/utils/storage.go +++ b/pkg/utils/storage.go @@ -11,7 +11,7 @@ type Storage interface { // SaveJSON saves a typed value as JSON using the provided storage. func SaveJSON[T any](storage Storage, key string, value T) error { - data, err := json.MarshalIndent(value, "", " ") + data, err := json.Marshal(value) if err != nil { return err } From 1c9cd80be39c154a8b93901dff81fc80e2f88f1c Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Sat, 15 Nov 2025 20:46:19 -0500 Subject: [PATCH 39/70] add a compression to the string before saving --- pkg/utils/keyring_storage.go | 74 +++++++++++++++++++++++++++++++----- 1 file changed, 65 insertions(+), 9 deletions(-) diff --git a/pkg/utils/keyring_storage.go b/pkg/utils/keyring_storage.go index 61f06994..1547746b 100644 --- a/pkg/utils/keyring_storage.go +++ b/pkg/utils/keyring_storage.go @@ -1,7 +1,11 @@ package utils import ( + "bytes" + "compress/zlib" + "errors" "fmt" + "io" "github.com/zalando/go-keyring" ) @@ -12,6 +16,15 @@ type KeyRingStorage struct { User string } +const ( + // maxKeyringValueLength is the lowest limit for the password using this library + // See https://github.com/zalando/go-keyring/blob/5c6f7e0ba5bf0380b4a490f2b7e41deb44b3c63e/keyring.go#L13-L16 + maxKeyringValueLength = 2560 +) + +var ErrKeyRingValueEmpty = errors.New("value cannot be empty") +var ErrKeyRingUserEmpty = errors.New("user cannot be empty") + // NewKeyRingStorage creates a new KeyRingStorage with the provided user. func NewKeyRingStorage(user string) *KeyRingStorage { return &KeyRingStorage{ @@ -19,31 +32,74 @@ func NewKeyRingStorage(user string) *KeyRingStorage { } } -// Save stores the value in the system keyring. -// The key parameter is used as the keyring's service field. +// Save compresses the data and stores it in the system keyring. func (ks KeyRingStorage) Save(key string, value []byte) error { + + compressedData, err := compressData(value) + if err != nil { + return err + } + if key == "" { - return fmt.Errorf("key cannot be empty") + return ErrKeyRingValueEmpty } if ks.User == "" { - return fmt.Errorf("user cannot be empty") + return ErrKeyRingUserEmpty } - return keyring.Set(key, ks.User, string(value)) + return keyring.Set(key, ks.User, compressedData) } -// Load retrieves the value from the system keyring. +// Load decompresses and retrieves the data from the system keyring. func (ks KeyRingStorage) Load(key string) ([]byte, error) { + if key == "" { - return nil, fmt.Errorf("key cannot be empty") + return nil, ErrKeyRingValueEmpty } if ks.User == "" { - return nil, fmt.Errorf("user cannot be empty") + return nil, ErrKeyRingUserEmpty } secret, err := keyring.Get(key, ks.User) if err != nil { return nil, err } - return []byte(secret), nil + decompressedData, err := decompressData(secret) + if err != nil { + return nil, err + } + return decompressedData, nil +} + +func compressData(value []byte) (string, error) { + var compressed bytes.Buffer + zlibWriter := zlib.NewWriter(&compressed) + if _, err := zlibWriter.Write(value); err != nil { + return "", err + } + if err := zlibWriter.Close(); err != nil { + return "", err + } + + compressedValue := compressed.String() + if len(compressedValue) > maxKeyringValueLength { + return "", fmt.Errorf("data is too large to save in the keyring, max length is %d bytes, got %d bytes", maxKeyringValueLength, len(compressedValue)) + } + return compressedValue, nil +} + +func decompressData(compressedData string) ([]byte, error) { + + reader, err := zlib.NewReader(bytes.NewReader([]byte(compressedData))) + if err != nil { + return nil, err + } + defer reader.Close() + + decompressed, err := io.ReadAll(reader) + if err != nil { + return nil, err + } + + return decompressed, nil } From 761ef92a3d0b27df4df3c5e208cc7ab4d38478e2 Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Sat, 15 Nov 2025 20:59:16 -0500 Subject: [PATCH 40/70] (fix) allow for tests to work and catch bug with loading json file --- pkg/utils/fs_storage.go | 3 +++ pkg/utils/fs_storage_test.go | 6 +++--- pkg/utils/keyring_storage_test.go | 4 ++-- pkg/utils/storage_test.go | 3 --- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/utils/fs_storage.go b/pkg/utils/fs_storage.go index 37b2ac96..eee378a8 100644 --- a/pkg/utils/fs_storage.go +++ b/pkg/utils/fs_storage.go @@ -31,6 +31,9 @@ func (fs FileSystemStorage) Save(key string, value []byte) error { } func (fs FileSystemStorage) Load(key string) ([]byte, error) { + if !strings.HasSuffix(key, ".json") { + key += ".json" + } filePath := filepath.Join(fs.BaseDir, key) return os.ReadFile(filePath) } diff --git a/pkg/utils/fs_storage_test.go b/pkg/utils/fs_storage_test.go index 5996e018..5bb8e304 100644 --- a/pkg/utils/fs_storage_test.go +++ b/pkg/utils/fs_storage_test.go @@ -20,7 +20,7 @@ func TestFileSystemStorage(t *testing.T) { assert.NoError(t, err) // Verify file exists and has correct content - filePath := filepath.Join(tempDir, "test-key") + filePath := filepath.Join(tempDir, "test-key.json") content, err := os.ReadFile(filePath) assert.NoError(t, err) assert.Equal(t, "test-value", string(content)) @@ -39,7 +39,7 @@ func TestFileSystemStorage(t *testing.T) { // Create test file testContent := "test-content" - filePath := filepath.Join(tempDir, "test-key") + filePath := filepath.Join(tempDir, "test-key.json") err := os.WriteFile(filePath, []byte(testContent), OWNER_READ_WRITE) assert.NoError(t, err) @@ -64,7 +64,7 @@ func TestFileSystemStorage(t *testing.T) { tempDir := t.TempDir() // Create a file where we want to create a directory - filePath := filepath.Join(tempDir, "blocking-file") + filePath := filepath.Join(tempDir, "blocking-file.json") err := os.WriteFile(filePath, []byte("content"), 0o644) assert.NoError(t, err) diff --git a/pkg/utils/keyring_storage_test.go b/pkg/utils/keyring_storage_test.go index b243a42e..f547b176 100644 --- a/pkg/utils/keyring_storage_test.go +++ b/pkg/utils/keyring_storage_test.go @@ -42,7 +42,7 @@ func TestKeyRingStorage(t *testing.T) { err := storage.Save("", []byte("test-value")) assert.Error(t, err) - assert.Contains(t, err.Error(), "key cannot be empty") + assert.Contains(t, err.Error(), "value cannot be empty") }) t.Run("handles empty key on load", func(t *testing.T) { @@ -50,7 +50,7 @@ func TestKeyRingStorage(t *testing.T) { _, err := storage.Load("") assert.Error(t, err) - assert.Contains(t, err.Error(), "key cannot be empty") + assert.Contains(t, err.Error(), "value cannot be empty") }) t.Run("handles empty user on save", func(t *testing.T) { diff --git a/pkg/utils/storage_test.go b/pkg/utils/storage_test.go index 367e9ab8..135d53bc 100644 --- a/pkg/utils/storage_test.go +++ b/pkg/utils/storage_test.go @@ -64,9 +64,6 @@ func TestSaveJSON(t *testing.T) { assert.NoError(t, err) assert.Equal(t, person, loaded) - // Verify it's formatted with indentation - assert.Contains(t, string(savedData), "\n") - assert.Contains(t, string(savedData), " ") }) t.Run("saves nested struct as JSON", func(t *testing.T) { From 666d2aa6429f89b7c5d7fe1ddfaf66eb65c03c5a Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Sun, 16 Nov 2025 00:51:06 -0500 Subject: [PATCH 41/70] (feat) add env variable workarounds --- internal/config/generator.go | 33 +++++++++++++++------- pkg/oauth/oauth.go | 55 +++++++++++++++++++++--------------- 2 files changed, 56 insertions(+), 32 deletions(-) diff --git a/internal/config/generator.go b/internal/config/generator.go index e0dcb938..33263eb4 100644 --- a/internal/config/generator.go +++ b/internal/config/generator.go @@ -376,7 +376,11 @@ func (c *JiraCLIConfigGenerator) configureLoginDetails() error { var qs []*survey.Question c.value.login = c.usrCfg.Login - if c.usrCfg.Login == "" { + envLogin := os.Getenv("JIRA_CLI_LOGIN") + if envLogin != "" { + c.value.login = envLogin + return nil + } else if c.usrCfg.Login == "" { switch c.value.installation { case jira.InstallationTypeCloud: qs = append(qs, &survey.Question{ @@ -451,8 +455,11 @@ func (c *JiraCLIConfigGenerator) configureServerDetails() error { var qs []*survey.Question c.value.server = c.usrCfg.Server - - if c.usrCfg.Server == "" { + envServer := os.Getenv("JIRA_CLI_SERVER") + if envServer != "" { + c.value.server = envServer + return c.verifyServer() + } else if c.usrCfg.Server == "" { qs = append(qs, &survey.Question{ Name: "server", Prompt: &survey.Input{ @@ -492,14 +499,20 @@ func (c *JiraCLIConfigGenerator) configureServerDetails() error { c.value.server = strings.TrimSpace(ans.Server) } - if c.value.authType == jira.AuthTypeOAuth { - // Set server URL using the cloud ID from OAuth configuration - c.value.apiServer = fmt.Sprintf("%s/%s", apiServer, c.value.oauth.cloudId) - } else { - c.value.apiServer = c.value.server - } } - // Trim trailing slash from server URL + return c.verifyServer() +} + +func (c *JiraCLIConfigGenerator) setApiServer() { + if c.value.authType == jira.AuthTypeOAuth { + c.value.apiServer = fmt.Sprintf("%s/%s", apiServer, c.value.oauth.cloudId) + } else { + c.value.apiServer = c.value.server + } +} + +func (c *JiraCLIConfigGenerator) verifyServer() error { + c.setApiServer() c.value.server = strings.TrimRight(c.value.server, "/") return c.verifyLoginDetails() } diff --git a/pkg/oauth/oauth.go b/pkg/oauth/oauth.go index 9a7a77ad..fbd0a5c1 100644 --- a/pkg/oauth/oauth.go +++ b/pkg/oauth/oauth.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "net/http" + "os" "path/filepath" "time" @@ -176,38 +177,48 @@ func HasOAuthCredentials(login string) bool { // collectOAuthCredentials collects OAuth credentials from the user. func collectOAuthCredentials() (*OAuthConfig, error) { - var questions []*survey.Question answers := struct { ClientID string ClientSecret string RedirectURI string }{} - q1 := &survey.Question{ - Name: "clientID", - Prompt: &survey.Input{ - Message: "Jira App Client ID:", - Help: "This is the client ID of your Jira App that you created for OAuth authentication.", - }, + // Check for environment variables + envClientID := os.Getenv("JIRA_OAUTH_CLIENT_ID") + envClientSecret := os.Getenv("JIRA_OAUTH_CLIENT_SECRET") + + q1 := &survey.Input{ + Message: "Jira App Client ID:", + Help: "This is the client ID of your Jira App that you created for OAuth authentication.", + Default: envClientID, } - q2 := &survey.Question{ - Name: "clientSecret", - Prompt: &survey.Password{ - Message: "Jira App Client Secret:", - Help: "This is the client secret of your Jira App that you created for OAuth authentication.", - }, + + q2 := &survey.Password{ + Message: "Jira App Client Secret:", + Help: "This is the client secret of your Jira App that you created for OAuth authentication.", } - q3 := &survey.Question{ - Name: "redirectURI", - Prompt: &survey.Input{ - Default: defaultRedirectURI, - Message: "Redirect URI:", - Help: "The redirect URL for Jira App. Recommended to set as localhost.", - }, + + q3 := &survey.Input{ + Default: defaultRedirectURI, + Message: "Redirect URI:", + Help: "The redirect URL for Jira App. Recommended to set as localhost.", } - questions = append(questions, q1, q2, q3) - if err := survey.Ask(questions, &answers, survey.WithValidator(survey.Required)); err != nil { + if envClientID == "" { + if err := survey.AskOne(q1, &answers.ClientID); err != nil { + return nil, err + } + } else { + answers.ClientID = envClientID + } + if envClientSecret == "" { + if err := survey.AskOne(q2, &answers.ClientSecret); err != nil { + return nil, err + } + } else { + answers.ClientSecret = envClientSecret + } + if err := survey.AskOne(q3, &answers.RedirectURI); err != nil { return nil, err } From f78deae03959a4326e142784837abfce4b9b33f2 Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Sun, 16 Nov 2025 11:34:20 -0500 Subject: [PATCH 42/70] (nit) Rely on the keyring to emit the size error rather than us --- pkg/oauth/oauth.go | 6 +++++- pkg/oauth/tokens.go | 8 ++++++-- pkg/utils/keyring_storage.go | 4 ---- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/pkg/oauth/oauth.go b/pkg/oauth/oauth.go index fbd0a5c1..249ec2bd 100644 --- a/pkg/oauth/oauth.go +++ b/pkg/oauth/oauth.go @@ -3,6 +3,7 @@ package oauth import ( "context" "encoding/json" + "errors" "fmt" "net/http" "os" @@ -11,6 +12,7 @@ import ( "github.com/AlecAivazis/survey/v2" "github.com/pkg/browser" + "github.com/zalando/go-keyring" "golang.org/x/oauth2" "github.com/ankitpokhrel/jira-cli/internal/cmdutil" @@ -135,7 +137,9 @@ func Configure(login string) (*ConfigureTokenResponse, error) { fallbackSecretStorage := utils.FileSystemStorage{BaseDir: jiraDir} if err := utils.SaveJSON(primarySecretStorage, oauthSecretsFile, oauthSecrets); err != nil { - fmt.Printf("Warning: Failed to save to the primarySecretStorage, falling back to alternative") + if errors.Is(err, keyring.ErrSetDataTooBig) { + cmdutil.Warn("Data was too big to save to the keyring, falling back to filesystem storage") + } err = utils.SaveJSON(fallbackSecretStorage, oauthSecretsFile, oauthSecrets) if err != nil { return nil, fmt.Errorf("failed to store OAuth secrets: %w", err) diff --git a/pkg/oauth/tokens.go b/pkg/oauth/tokens.go index 66bccbaf..09e8a8a0 100644 --- a/pkg/oauth/tokens.go +++ b/pkg/oauth/tokens.go @@ -2,10 +2,13 @@ package oauth import ( "context" + "errors" "fmt" "time" + "github.com/ankitpokhrel/jira-cli/internal/cmdutil" "github.com/ankitpokhrel/jira-cli/pkg/utils" + "github.com/zalando/go-keyring" "golang.org/x/oauth2" ) @@ -133,8 +136,9 @@ func (pts *PersistentTokenSource) Token() (*oauth2.Token, error) { func (pts *PersistentTokenSource) saveSecrets(secrets *OAuthSecrets) error { err := utils.SaveJSON(pts.storage, oauthSecretsFile, secrets) if err != nil && !pts.usingFallback && pts.fallbackStorage != nil { - // Primary storage failed, try fallback - fmt.Println("Warning: Primary storage failed, falling back to FileSystemStorage for OAuth tokens") + if errors.Is(err, keyring.ErrSetDataTooBig) { + cmdutil.Warn("Data was too big to save to the keyring, falling back to filesystem storage") + } err = utils.SaveJSON(pts.fallbackStorage, oauthSecretsFile, secrets) if err == nil { // Successfully saved to fallback, switch to using it diff --git a/pkg/utils/keyring_storage.go b/pkg/utils/keyring_storage.go index 1547746b..3fb420d1 100644 --- a/pkg/utils/keyring_storage.go +++ b/pkg/utils/keyring_storage.go @@ -4,7 +4,6 @@ import ( "bytes" "compress/zlib" "errors" - "fmt" "io" "github.com/zalando/go-keyring" @@ -82,9 +81,6 @@ func compressData(value []byte) (string, error) { } compressedValue := compressed.String() - if len(compressedValue) > maxKeyringValueLength { - return "", fmt.Errorf("data is too large to save in the keyring, max length is %d bytes, got %d bytes", maxKeyringValueLength, len(compressedValue)) - } return compressedValue, nil } From 606fedda413542e48afefedfbe5bb0484ded6946 Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Sun, 16 Nov 2025 11:34:51 -0500 Subject: [PATCH 43/70] (feat) add warning for when the filesystem storage is used --- pkg/oauth/oauth.go | 1 + pkg/oauth/tokens.go | 1 + 2 files changed, 2 insertions(+) diff --git a/pkg/oauth/oauth.go b/pkg/oauth/oauth.go index 249ec2bd..c6ff7666 100644 --- a/pkg/oauth/oauth.go +++ b/pkg/oauth/oauth.go @@ -144,6 +144,7 @@ func Configure(login string) (*ConfigureTokenResponse, error) { if err != nil { return nil, fmt.Errorf("failed to store OAuth secrets: %w", err) } + cmdutil.Warn("Saved credentials to owner-restricted filesystem storage") } return &ConfigureTokenResponse{ diff --git a/pkg/oauth/tokens.go b/pkg/oauth/tokens.go index 09e8a8a0..7eaf2a3a 100644 --- a/pkg/oauth/tokens.go +++ b/pkg/oauth/tokens.go @@ -141,6 +141,7 @@ func (pts *PersistentTokenSource) saveSecrets(secrets *OAuthSecrets) error { } err = utils.SaveJSON(pts.fallbackStorage, oauthSecretsFile, secrets) if err == nil { + cmdutil.Warn("Saved credentials to owner-restricted filesystem storage") // Successfully saved to fallback, switch to using it pts.storage = pts.fallbackStorage pts.usingFallback = true From 471e1f4ef7417e33a6ff4624f3e545081b4eac73 Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Sun, 16 Nov 2025 11:35:35 -0500 Subject: [PATCH 44/70] =?UTF-8?q?(improv)=20the=20HTML=20page=20was=20ugly?= =?UTF-8?q?=20=F0=9F=A4=B7=F0=9F=8F=BE=E2=99=82=EF=B8=8F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- pkg/oauth/oauth.go | 46 +++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 43 insertions(+), 3 deletions(-) diff --git a/pkg/oauth/oauth.go b/pkg/oauth/oauth.go index c6ff7666..60cd2371 100644 --- a/pkg/oauth/oauth.go +++ b/pkg/oauth/oauth.go @@ -265,11 +265,51 @@ func performOAuthFlow(config *OAuthConfig, httpTimeout time.Duration, openBrowse // Send success response to browser w.Header().Set("Content-Type", "text/html") if _, err := w.Write([]byte(` + + + +You can close this window and return to the terminal.
- +You can safely close this window and return to the terminal.
+You can close this window and return to the terminal.
- - - - `)) - } - } - }) - - req := httptest.NewRequest("GET", "http://localhost:9876/callback?code=test-code", http.NoBody) - w := httptest.NewRecorder() - - handler.ServeHTTP(w, req) - - assert.Equal(t, http.StatusOK, w.Code) - assert.Equal(t, "text/html", w.Header().Get("Content-Type")) - assert.Contains(t, w.Body.String(), "Authorization successful!") - assert.Contains(t, w.Body.String(), "window.close()") - }) -} - func TestToScopeStrings(t *testing.T) { t.Parallel() From a7270c62dea7daf28388fa6ce335fa45d7370e93 Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Tue, 18 Nov 2025 19:43:58 -0500 Subject: [PATCH 63/70] (nit) this should be before setting the apiServer not after --- internal/config/generator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/config/generator.go b/internal/config/generator.go index 7cdb29cc..b8f84699 100644 --- a/internal/config/generator.go +++ b/internal/config/generator.go @@ -512,8 +512,8 @@ func (c *JiraCLIConfigGenerator) setApiServer() { } func (c *JiraCLIConfigGenerator) verifyServer() error { - c.setApiServer() c.value.server = strings.TrimRight(c.value.server, "/") + c.setApiServer() return c.verifyLoginDetails() } From bdabb55e68e71253c30099138a1fd53ce94616d9 Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Tue, 18 Nov 2025 19:44:43 -0500 Subject: [PATCH 64/70] Update README.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 171c6621..901bbe40 100644 --- a/README.md +++ b/README.md @@ -93,9 +93,9 @@ Follow the [installation guide](https://github.com/ankitpokhrel/jira-cli/wiki/In a `JIRA_API_TOKEN` variable. Add it to your shell configuration file, for instance, `$HOME/.bashrc`, so that the variable is always available. Alternatively, you can also use `.netrc` file or `keychain` to set the token. Learn more [here](https://github.com/ankitpokhrel/jira-cli/discussions/356). -2. Run `jira init`, select installation type as `Cloud`, and provide required details to generate a config file required - for the tool. -3. Run the `jira init`, Select the `Cloud` installation type and then select the `OAuth` authentication type. This will prompt for your Jira App Client ID and Client Secret. You can learn more about how to create a Jira App [here](https://github.com/ankitpokhrel/jira-cli/discussions/879#discussion-8604411) +2. Run `jira init`, select installation type as `Cloud`, and provide the required details to generate a config file for the tool. + - By default, you can use basic authentication with your Jira email and API token. + - Alternatively, if you wish to use OAuth authentication, after selecting `Cloud`, choose the `OAuth` authentication type. This will prompt for your Jira App Client ID and Client Secret. Learn more about creating a Jira App [here](https://github.com/ankitpokhrel/jira-cli/discussions/879#discussion-8604411). #### On-premise installation From 69a45c38e71c3f2d56c5d23773794fbea9caffef Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Tue, 18 Nov 2025 19:45:44 -0500 Subject: [PATCH 65/70] Update pkg/utils/keyring_storage.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- pkg/utils/keyring_storage.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pkg/utils/keyring_storage.go b/pkg/utils/keyring_storage.go index d41b1175..15716ebb 100644 --- a/pkg/utils/keyring_storage.go +++ b/pkg/utils/keyring_storage.go @@ -84,9 +84,7 @@ func decompressData(compressedData string) ([]byte, error) { return nil, err } defer func() { - if closeErr := reader.Close(); closeErr != nil { - _ = closeErr - } + _ = reader.Close() }() decompressed, err := io.ReadAll(reader) From 00a37927e084a98e663b48d93d0e2d81049dafdc Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Tue, 18 Nov 2025 22:04:35 -0500 Subject: [PATCH 66/70] (pr-nit) oauth name --- internal/config/generator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/config/generator.go b/internal/config/generator.go index b8f84699..ef1b3579 100644 --- a/internal/config/generator.go +++ b/internal/config/generator.go @@ -286,7 +286,7 @@ func (c *JiraCLIConfigGenerator) configureCloudAuthType() error { Message: "Authentication type:", Help: `Authentication type could be: cloud or oauth ? If you are using your login credentials, the auth type is probably 'cloud' (most common for cloud installation) -? If you are authenticating using oauth 3LO, the auth type is probably 'oauth'`, +? If you are authenticating using OAuth 3LO, the auth type is probably 'oauth'`, Options: []string{jira.AuthTypeCloud.String(), jira.AuthTypeOAuth.String()}, Default: jira.AuthTypeCloud.String(), } From 3b2414c094c11e642b1fb95d51344d294e8fe5bb Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Tue, 18 Nov 2025 22:04:49 -0500 Subject: [PATCH 67/70] (pr-nit) add AccessibleURL --- pkg/jira/cloudid.go | 4 ++-- pkg/oauth/oauth.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/jira/cloudid.go b/pkg/jira/cloudid.go index 090a3738..171d2882 100644 --- a/pkg/jira/cloudid.go +++ b/pkg/jira/cloudid.go @@ -14,7 +14,7 @@ var ( ) const ( - JiraAccessibleResourcesURL = "https://api.atlassian.com/oauth/token/accessible-resources" + AccessibleResourcesURL = "https://api.atlassian.com/oauth/token/accessible-resources" ) type CloudIDResponse struct { @@ -31,7 +31,7 @@ func (c *Client) GetCloudID() (string, error) { return envCloudID, nil } - res, err := c.request(context.Background(), http.MethodGet, JiraAccessibleResourcesURL, nil, Header{ + res, err := c.request(context.Background(), http.MethodGet, AccessibleResourcesURL, nil, Header{ "Accept": "application/json", }) if err != nil { diff --git a/pkg/oauth/oauth.go b/pkg/oauth/oauth.go index 629db17f..97f0d6bc 100644 --- a/pkg/oauth/oauth.go +++ b/pkg/oauth/oauth.go @@ -149,7 +149,7 @@ func Configure(login string) (*ConfigureTokenResponse, error) { } // Get Cloud ID for Atlassian API - cloudID, err := getCloudID(jira.JiraAccessibleResourcesURL, tokens.AccessToken) + cloudID, err := getCloudID(jira.AccessibleResourcesURL, tokens.AccessToken) if err != nil { return nil, fmt.Errorf("failed to get cloud ID: %w", err) } From fb011af16c0020e81acbb6d8629a523aaf809898 Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Tue, 18 Nov 2025 22:10:41 -0500 Subject: [PATCH 68/70] (pr-nit) godoc comments --- pkg/jira/client.go | 2 +- pkg/oauth/oauth.go | 2 +- pkg/oauth/tokens.go | 10 +++++----- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/pkg/jira/client.go b/pkg/jira/client.go index 579e5818..4bb50d49 100644 --- a/pkg/jira/client.go +++ b/pkg/jira/client.go @@ -300,7 +300,7 @@ func (c *Client) request(ctx context.Context, method, endpoint string, body []by req.Header.Add("Authorization", "Bearer "+c.token) } case string(AuthTypeOAuth): - // OAuth authentication is handled by oauth2.Transport automatically + // OAuth authentication is handled by [oauth2.Transport] automatically // Only add manual auth header if we don't have a TokenSource (fallback mode) if c.tokenSource == nil && c.token != "" { req.Header.Add("Authorization", "Bearer "+c.token) diff --git a/pkg/oauth/oauth.go b/pkg/oauth/oauth.go index 97f0d6bc..f2886665 100644 --- a/pkg/oauth/oauth.go +++ b/pkg/oauth/oauth.go @@ -108,7 +108,7 @@ func toScopeStrings(scopes []OAuthScope) []string { return scopeStrings } -// GetOAuth2Config creates an OAuth2 config for the given client credentials. +// GetOAuth2Config creates an [oauth2.Config] for the given client credentials. func GetOAuth2Config(clientID, clientSecret, redirectURI string, scopes []string) *oauth2.Config { if scopes == nil { scopes = toScopeStrings(defaultScopes) diff --git a/pkg/oauth/tokens.go b/pkg/oauth/tokens.go index 5eeab67b..e7c8e58d 100644 --- a/pkg/oauth/tokens.go +++ b/pkg/oauth/tokens.go @@ -22,7 +22,7 @@ type OAuthSecrets struct { Expiry time.Time `json:"expiry"` } -// PersistentTokenSource implements oauth2.TokenSource with automatic token persistence. +// PersistentTokenSource implements [oauth2.TokenSource] with automatic token persistence. type PersistentTokenSource struct { clientID string clientSecret string @@ -41,7 +41,7 @@ func (o *OAuthSecrets) IsValid() bool { return o.AccessToken != "" && !o.IsExpired() } -// ToOAuth2Token converts OAuthSecrets to oauth2.Token. +// ToOAuth2Token converts OAuthSecrets to [oauth2.Token]. func (o *OAuthSecrets) ToOAuth2Token() *oauth2.Token { return &oauth2.Token{ AccessToken: o.AccessToken, @@ -51,7 +51,7 @@ func (o *OAuthSecrets) ToOAuth2Token() *oauth2.Token { } } -// FromOAuth2Token updates OAuthSecrets from oauth2.Token. +// FromOAuth2Token updates OAuthSecrets from [oauth2.Token]. func (o *OAuthSecrets) FromOAuth2Token(token *oauth2.Token) { o.AccessToken = token.AccessToken o.RefreshToken = token.RefreshToken @@ -79,7 +79,7 @@ func NewPersistentTokenSource(login, clientID, clientSecret string) (*Persistent }, nil } -// Token implements oauth2.TokenSource interface. +// Token implements [oauth2.TokenSource] interface. func (pts *PersistentTokenSource) Token() (*oauth2.Token, error) { // Load current token from storage secrets, err := utils.LoadJSON[OAuthSecrets](pts.storage, oauthSecretsFile) @@ -149,7 +149,7 @@ func (pts *PersistentTokenSource) saveSecrets(secrets *OAuthSecrets) error { return err } -// LoadOAuth2TokenSource creates a TokenSource from stored OAuth secrets. +// LoadOAuth2TokenSource creates a [oauth2.TokenSource] from stored OAuth secrets. func LoadOAuth2TokenSource(login string) (oauth2.TokenSource, error) { // Load OAuth secrets to get client credentials secrets, err := LoadOAuthSecrets(login) From 8eed94278f3464002fe5d1be291681cec3736006 Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Tue, 18 Nov 2025 22:12:09 -0500 Subject: [PATCH 69/70] (pr-nit) rename to follow conventions --- pkg/utils/{fs_storage.go => fsstorage.go} | 0 pkg/utils/{fs_storage_test.go => fsstorage_test.go} | 0 pkg/utils/{keyring_storage.go => keyringstorage.go} | 0 pkg/utils/{keyring_storage_test.go => keyringstorage_test.go} | 0 4 files changed, 0 insertions(+), 0 deletions(-) rename pkg/utils/{fs_storage.go => fsstorage.go} (100%) rename pkg/utils/{fs_storage_test.go => fsstorage_test.go} (100%) rename pkg/utils/{keyring_storage.go => keyringstorage.go} (100%) rename pkg/utils/{keyring_storage_test.go => keyringstorage_test.go} (100%) diff --git a/pkg/utils/fs_storage.go b/pkg/utils/fsstorage.go similarity index 100% rename from pkg/utils/fs_storage.go rename to pkg/utils/fsstorage.go diff --git a/pkg/utils/fs_storage_test.go b/pkg/utils/fsstorage_test.go similarity index 100% rename from pkg/utils/fs_storage_test.go rename to pkg/utils/fsstorage_test.go diff --git a/pkg/utils/keyring_storage.go b/pkg/utils/keyringstorage.go similarity index 100% rename from pkg/utils/keyring_storage.go rename to pkg/utils/keyringstorage.go diff --git a/pkg/utils/keyring_storage_test.go b/pkg/utils/keyringstorage_test.go similarity index 100% rename from pkg/utils/keyring_storage_test.go rename to pkg/utils/keyringstorage_test.go From 8fe3460df5818261b30c09956a5fa249e3d5a4f5 Mon Sep 17 00:00:00 2001 From: Christian Arty <40256027+christianarty@users.noreply.github.com> Date: Tue, 18 Nov 2025 22:15:34 -0500 Subject: [PATCH 70/70] (pr-improv) use slices.SortFunc for a simpler sort when printing expectedScopes --- pkg/oauth/oauth.go | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/pkg/oauth/oauth.go b/pkg/oauth/oauth.go index f2886665..d77a0ddb 100644 --- a/pkg/oauth/oauth.go +++ b/pkg/oauth/oauth.go @@ -8,7 +8,7 @@ import ( "net/http" "os" "path/filepath" - "sort" + "slices" "time" "github.com/AlecAivazis/survey/v2" @@ -272,18 +272,24 @@ func printExpectedScopes(scopes []OAuthScope) { } // Sort by scope type (classic first, then granular) and then by name alphabetically - sort.Slice(visibleScopes, func(i, j int) bool { - if visibleScopes[i].ScopeType != visibleScopes[j].ScopeType { + slices.SortFunc(visibleScopes, func(a, b OAuthScope) int { + if a.ScopeType != b.ScopeType { // Classic comes before granular - if visibleScopes[i].ScopeType == ScopeTypeClassic { - return true + if a.ScopeType == ScopeTypeClassic { + return -1 } - if visibleScopes[j].ScopeType == ScopeTypeClassic { - return false + if b.ScopeType == ScopeTypeClassic { + return 1 } } // If same scope type, sort alphabetically by name - return visibleScopes[i].Name < visibleScopes[j].Name + if a.Name < b.Name { + return -1 + } + if a.Name > b.Name { + return 1 + } + return 0 }) fmt.Printf("Expected Scopes:\n")