-
-
Notifications
You must be signed in to change notification settings - Fork 23.6k
feat(jira): Add Bearer Token Authentication and SSL Certificate Support #5634
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat(jira): Add Bearer Token Authentication and SSL Certificate Support #5634
Conversation
## Summary Add enterprise-grade authentication options for the JIRA Tool, enabling connections to self-hosted JIRA instances that require Bearer tokens and/or custom SSL certificates. ## Changes ### Added - Bearer Token authentication support for enterprise/self-hosted JIRA - SSL certificate configuration (sslCertPath, sslKeyPath, verifySslCerts) - Comprehensive test suite (unit, integration, regression tests) - JIRA Tool documentation (README.md) - CHANGELOG.md for release notes ### Changed - JiraApi credential updated to v2.0 with auth type selector - BaseJiraTool refactored with buildHeaders() and buildFetchOptions() ### Backward Compatibility - Basic Auth (email + apiToken) continues to work unchanged - Existing users require no configuration changes Closes #XXXX
Summary of ChangesHello @vellanki-santhosh, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Jira Tool's capabilities by introducing enterprise-grade authentication and security features. It allows users to connect to self-hosted Jira instances using Bearer Tokens and provides robust support for custom SSL certificates, including client certificates and CA certificates, with an option to disable verification for development. These additions ensure broader compatibility and secure communication with various Jira deployments, all while maintaining full backward compatibility with existing Basic Authentication setups. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces significant enhancements to the JIRA Tool by adding support for Bearer Token authentication and custom SSL certificates, which is crucial for enterprise and self-hosted JIRA instances. The changes are well-structured, including updates to credentials, core logic, and the addition of comprehensive documentation and a test suite. My feedback focuses on improving the clarity of error messages, correcting a potential misunderstanding in the test suite's error handling assertions, and refining a UI description for better user understanding. Overall, this is a solid contribution that greatly improves the JIRA tool's capabilities.
| }) | ||
|
|
||
| const getIssueTool = tools.find((t) => t.name === 'jira_get_issue') | ||
| await expect(getIssueTool!.invoke({ issueKey: 'PROJ-123' })).rejects.toThrow('Failed to load SSL certificate') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests using rejects.toThrow() seem to be based on an incorrect assumption. The _call methods in the tool classes (e.g., GetIssueTool) are designed to catch exceptions and return a formatted error string, causing the invoke() promise to resolve with an error message rather than reject. These tests will likely fail and should be updated to check for a resolved promise containing the error message, for example: await expect(getIssueTool.invoke(...)).resolves.toContain('Error message');. This issue applies to all similar rejects.toThrow() assertions in this test suite.
| name: 'sslCertPath', | ||
| type: 'string', | ||
| placeholder: '/path/to/cert.pem', | ||
| description: 'Path to SSL certificate file for self-signed certificates', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The description for sslCertPath is a bit restrictive. It can be used for custom CA certificates or client certificates for mTLS, not just self-signed certificates. A more general description would be more accurate and helpful for users, aligning better with the scenarios described in the new README.md.
| description: 'Path to SSL certificate file for self-signed certificates', | |
| description: 'Path to the SSL certificate file (e.g., custom CA, client certificate for mTLS).', |
| } catch (error) { | ||
| if (error instanceof Error) { | ||
| throw new Error(`Failed to connect to Jira: ${error.message}`) | ||
| } | ||
| throw error | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current error handling wraps all errors, including Jira API errors (like 401 Unauthorized), with the prefix "Failed to connect to Jira:". This can be misleading, as an API error implies the connection was successful but the request was invalid for other reasons (e.g., authentication). It would be better to only wrap network-level errors and re-throw API errors as they are for better error diagnosis.
} catch (error) {
if (error instanceof Error) {
// Avoid re-wrapping Jira API errors, as they indicate a successful connection but a failed request.
if (error.message.startsWith('Jira API Error')) {
throw error
}
// Wrap network or other connection-level errors.
throw new Error(`Failed to connect to Jira: ${error.message}`)
}
throw error
}- Fix test assertions: Change rejects.toThrow() to resolves.toContain() since tool _call methods catch exceptions and return error strings - Update sslCertPath description to be more general and accurate: 'Path to the SSL certificate file (e.g., custom CA, client certificate for mTLS)' - Refine error handling in core.ts: Don't re-wrap API errors, only wrap network/connection errors for better diagnostics
✅ Addressed Gemini Code Review FeedbackThanks for the review! I've pushed fixes addressing all the flagged issues: Changes Made
Commit: The PR is now ready for maintainer approval. 🚀 |
|
Thanks for the review @gemini-code-assist! |
|
Thanks for the update, @vellanki-santhosh! I appreciate you addressing the feedback and making these improvements. The changes you've outlined, especially refining the test assertions to Great work on these fixes! The pull request looks good to proceed for workflow approval and merge. 🚀 |
Summary
Add enterprise-grade authentication options for the JIRA Tool, enabling connections to self-hosted JIRA instances that require Bearer tokens and/or custom SSL certificates.
Motivation
Many enterprise environments use self-hosted JIRA (Data Center/Server) instances that:
This PR enables these use cases while maintaining full backward compatibility.
Changes
Added
sslCertPath,sslKeyPath,verifySslCerts)Changed
buildHeaders()andbuildFetchOptions()methodsFiles Modified
packages/components/credentials/JiraApi.credential.tspackages/components/nodes/tools/Jira/Jira.tspackages/components/nodes/tools/Jira/core.tspackages/components/nodes/tools/Jira/Jira.test.ts(new)packages/components/nodes/tools/Jira/README.md(new)CHANGELOG.md(new)Backward Compatibility
✅ No breaking changes - Basic Auth (
email + apiToken) continues to work exactly as before.✅ Existing users require no configuration changes.
Testing
Screenshots
N/A - Backend changes only
Checklist