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 56ab80015c6057280552451f6c33ef6f1da41536 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/27] 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 fd69ab00ee87d79f70e1de44ece12ede78235194 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/27] (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 c4e6ffb57a3d9863e28139ee384585945397ee4d 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/27] (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 86b5cffe..1c036c11 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 = 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 66b3890d375197fe00e41704b941540d69173847 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/27] (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 4a16853bdc09dfc62892981a02293639cc0437f7 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/27] (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 57f34c838b64e415fb32bee87aa2cb6990ace115 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/27] (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 4950c0f035f5dd30cd6d4d322d44a2d82743239e 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/27] (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 1c036c11..d9b6d7c5 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 3213b620dbfb6b3f8af0952a8dd3f4dc7f52694b 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/27] (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 0b1369482dde59457a269fe5a34ee6b169e0eb39 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/27] (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 2d949034adbc5985eaeb09c8a1b6bebcef6c0cf8 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/27] (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 fd85490dd01d4080b21f8c1da08100ed46375b1f 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/27] (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 d9b6d7c5..8abd6d76 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 = ans.Server + } if ans.Login != "" { c.value.login = 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 5dbd309f65e832b5ec004a296173149113590b0a 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/27] (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 e9117eee32521440f8e0fab1f7f0da0dd263148c 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/27] (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 b55ca773709f0fdfd2d935809062934cf8714858 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/27] (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 d395fe6b5dcb903eda54718e11219e3b955f994d 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/27] (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 8abd6d76..d5b593f2 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 = ans.Server - } if ans.Login != "" { c.value.login = 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 c3c4f6b912e19a74b662148e309a01be46e5f7ee 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/27] (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 0ebaa17a043e9a6bf695f868d1ce4a3f06d877e4 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/27] (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 2ca15a5136d660d51cf58f4ad6677afbe1120841 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/27] (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 030adc04d3b62fe977ede0bf8c975092be687a7b 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/27] 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 0708a1c524a08e09ee476502d7713ba2d921d937 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/27] 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 d5b593f2..fd013e6d 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 3cd2c327b6035d2dd4b41017ca9712fa6e387ad4 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/27] 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 d666c89d41ad4507933d4e8db0c5ebf0726896c4 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/27] 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 25cd230966932f40f55f1c2b79689c736ed16e4b 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/27] 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()