Skip to content

Add admin API token fallback for watcher notifications #1298

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Open
nevyangelova wants to merge 2 commits into
base: master
Choose a base branch
from
+58 −6
Open

Add admin API token fallback for watcher notifications #1298

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
90a4ca8
Add admin API token fallback for watcher notifications
Apr 13, 2026
a0ade1c
Separate hard errors from no-connected-user fallback and add HTTP cli…
Apr 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 58 additions & 6 deletions server/issue.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"strconv"
"strings"
"sync"
"time"

jira "github.com/andygrunwald/go-jira"
"github.com/pkg/errors"
Expand Down Expand Up @@ -1679,15 +1680,28 @@ func (p *Plugin) checkIssueWatchers(wh *webhook, instanceID types.ID) {
commentAuthor, jwhook.mdKeySummaryLink(), jwhook.mdIssueType())
}
client, connection, err := wh.fetchConnectedUser(p, instanceID)
if err != nil || client == nil {
p.errorf("error while fetching connected users for the instanceID %v , Error : %v", instanceID, err)
if err != nil {
p.errorf("error while fetching connected users for the instanceID %v, Error: %v", instanceID, err)
return
}

watchers, err := client.GetWatchers(instanceID.String(), wh.Issue.ID, connection)
if err != nil {
p.errorf("error while getting watchers for the issue id %v , err : %v", wh.Issue.ID, err)
return
var watchers *jira.Watches
if client == nil {
if p.getConfig().AdminAPIToken == "" {
p.errorf("no connected user found for instanceID %v and no admin API token configured", instanceID)
return
}
watchers, err = p.GetWatchersWithAPIToken(wh.Issue.ID, instanceID.String())
if err != nil {
p.errorf("error while getting watchers with admin API token for issue id %v , err : %v", wh.Issue.ID, err)
return
}
} else {
watchers, err = client.GetWatchers(instanceID.String(), wh.Issue.ID, connection)
if err != nil {
p.errorf("error while getting watchers for the issue id %v , err : %v", wh.Issue.ID, err)
return
}
}

authorVal := jwhook.Comment.UpdateAuthor
Expand Down Expand Up @@ -1894,6 +1908,44 @@ func (p *Plugin) GetProjectListWithAPIToken(instanceID string) (*jira.ProjectLis
return &projectResponse.Values, nil
}

func (p *Plugin) GetWatchersWithAPIToken(issueID, instanceID string) (*jira.Watches, error) {
httpClient := &http.Client{Timeout: 30 * time.Second}
req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("%s/rest/api/2/issue/%s/watchers", instanceID, issueID), nil)
if err != nil {
return nil, errors.Wrapf(err, "failed to create http request for fetching watchers. IssueID: %s", issueID)
Comment on lines +1911 to +1915
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 13, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Use the Jira base URL here, not the instance ID.

Line 1694 passes instanceID.String(), and this helper formats that value directly into the request URL on Line 1913. In this file, instanceID is the store key used by LoadInstance, not the Jira host, so the new fallback can build an invalid URL and never reach Jira.

🔧 Suggested fix 
 func (p *Plugin) GetWatchersWithAPIToken(issueID, instanceID string) (*jira.Watches, error) {
+	instance, err := p.instanceStore.LoadInstance(types.ID(instanceID))
+	if err != nil {
+		return nil, errors.Wrapf(err, "failed to load instance. InstanceID: %s", instanceID)
+	}
+
 	httpClient := &http.Client{Timeout: 30 * time.Second}
-	req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("%s/rest/api/2/issue/%s/watchers", instanceID, issueID), nil)
+	req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("%s/rest/api/2/issue/%s/watchers", instance.GetJiraBaseURL(), issueID), nil)
 	if err != nil {
 		return nil, errors.Wrapf(err, "failed to create http request for fetching watchers. IssueID: %s", issueID)
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func (p *Plugin) GetWatchersWithAPIToken(issueID, instanceID string) (*jira.Watches, error) {
httpClient := &http.Client{Timeout: 30 * time.Second}
req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("%s/rest/api/2/issue/%s/watchers", instanceID, issueID), nil)
if err != nil {
return nil, errors.Wrapf(err, "failed to create http request for fetching watchers. IssueID: %s", issueID)
func (p *Plugin) GetWatchersWithAPIToken(issueID, instanceID string) (*jira.Watches, error) {
instance, err := p.instanceStore.LoadInstance(types.ID(instanceID))
if err != nil {
return nil, errors.Wrapf(err, "failed to load instance. InstanceID: %s", instanceID)
}
httpClient := &http.Client{Timeout: 30 * time.Second}
req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("%s/rest/api/2/issue/%s/watchers", instance.GetJiraBaseURL(), issueID), nil)
if err != nil {
return nil, errors.Wrapf(err, "failed to create http request for fetching watchers. IssueID: %s", issueID)
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@server/issue.go` around lines 1911 - 1915, The request URL in
GetWatchersWithAPIToken currently uses the store key parameter instanceID
instead of the Jira base URL; update the function to resolve the instance object
(via the existing LoadInstance or equivalent on Plugin) using the instanceID key
and use the instance's BaseURL/URL (e.g., instance.BaseURL or instance.URL) when
formatting the request to "/rest/api/2/issue/{issueID}/watchers"; ensure you
still handle LoadInstance errors and preserve the existing error wrapping and
http client usage.

Copy link
Copy Markdown

@nang2049 nang2049 Apr 15, 2026

Choose a reason for hiding this comment

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

This is a false positive. No fix needed.

In this codebase, types.ID for instances is the Jira base URL (e.g., https://jira-stage.veevadev.com), not an opaque store key. The evidence:

instances.go line 267: instanceID := types.ID(instanceURL) — it's literally constructed from the normalized Jira URL
GetIssueDataWithAPIToken uses instanceID the exact same way: fmt.Sprintf("%s/rest/api/2/issue/%s", instanceID, issueID)
GetProjectListWithAPIToken uses it the same way: fmt.Sprintf("%s/rest/api/3/project/search", instanceID)
The original bug report even shows the URL in the logs: "error while fetching connected users for the instanceID https://jira-stage.veevadev.com"
All three WithAPIToken functions follow the identical pattern.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 15, 2026

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify that instanceID for instances is the Jira base URL
rg -n "instanceID.*types.ID\|types.ID.*instanceURL\|instanceURL" --type go -C2 server/instances.go | head -60

# Also check the WithAPIToken pattern usage
rg -n "WithAPIToken" --type go -C3

Length of output: 2785

@nang2049, you're right — my comment was incorrect. The script output confirms the pattern across the codebase: GetIssueDataWithAPIToken and GetProjectListWithAPIToken both use instanceID directly as the URL base in the same way, and webhook_jira.go Line 76 shows instance.GetID().String() being passed as the instanceID argument to GetIssueDataWithAPIToken, confirming the instance ID is the normalized Jira base URL. GetWatchersWithAPIToken correctly follows the same established pattern. Apologies for the noise.

✏️ Learnings added 
Learnt from: nang2049
Repo: mattermost/mattermost-plugin-jira PR: 1298
File: server/issue.go:1911-1915
Timestamp: 2026-04-15T12:42:52.679Z
Learning: In the mattermost-plugin-jira codebase (server/), `types.ID` for a Jira instance is the normalized Jira base URL (e.g., "https://jira-stage.veevadev.com"), not an opaque store key. All three `*WithAPIToken` helpers (`GetIssueDataWithAPIToken`, `GetProjectListWithAPIToken`, `GetWatchersWithAPIToken`) correctly use `instanceID` directly as the URL base when building REST API request URLs. Do not flag this pattern as a bug.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

}

err = p.SetAdminAPITokenRequestHeader(req)
if err != nil {
return nil, err
}

resp, err := httpClient.Do(req)
if err != nil {
return nil, errors.Wrapf(err, "failed to get watchers. IssueID: %s", issueID)
}
if resp == nil || resp.Body == nil {
return nil, errors.Errorf("missing response for watchers. IssueID: %s", issueID)
}
defer resp.Body.Close()

if resp.StatusCode != http.StatusOK {
return nil, errors.Errorf("unexpected status code %d while fetching watchers. IssueID: %s", resp.StatusCode, issueID)
}

body, err := io.ReadAll(resp.Body)
if err != nil {
return nil, errors.Wrapf(err, "failed to read watchers response. IssueID: %s", issueID)
}

var watchers jira.Watches
if err = json.Unmarshal(body, &watchers); err != nil {
return nil, errors.Wrapf(err, "failed to unmarshal watchers data. IssueID: %s", issueID)
}

return &watchers, nil
}

func shouldNotifyWatcherUser(watcher jira.Watcher, author *jira.User) bool {
if author == nil {
return true
Expand Down
Loading