Skip to content
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

atlassian_{confluence,jira}: clarify API authentication #3693

Merged
merged 3 commits into from
Sep 13, 2022

Conversation

efd6
Copy link
Contributor

@efd6 efd6 commented Jul 13, 2022

What does this PR do?

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.

Author's Checklist

  • [ ]

How to test this PR locally

Related issues

Screenshots

Screen Shot 2022-07-13 at 10 34 58

Screen Shot 2022-07-13 at 10 36 06

@efd6 efd6 added bug Something isn't working, use only for issues documentation Improvements or additions to documentation Team:Security-External Integrations Integration:atlassian_jira Atlassian Jira Integration:atlassian_confluence Atlassian Confluence labels Jul 13, 2022
@efd6 efd6 requested a review from a team as a code owner July 13, 2022 01:09
@efd6 efd6 self-assigned this Jul 13, 2022
@elasticmachine
Copy link

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

@elasticmachine
Copy link

elasticmachine commented Jul 13, 2022

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-09-13T01:08:53.956+0000

  • Duration: 15 min 12 sec

Test stats 🧪

Test Results
Failed 0
Passed 18
Skipped 0
Total 18

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

@elasticmachine
Copy link

elasticmachine commented Jul 13, 2022

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (2/2) 💚
Files 100.0% (6/6) 💚 2.537
Classes 100.0% (6/6) 💚 2.537
Methods 100.0% (45/45) 💚 9.77
Lines 98.822% (839/849) 👍 7.921
Conditionals 100.0% (0/0) 💚

Copy link
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

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

I was reviewing the README to see if there were any additional changes that could help. I noticed something that makes me question the original intent of the "Personal Access Token". It says

JIRA Cloud only supports Basic Auth using username and a Personal Access Token.

That makes me think the "Personal Access Token" was intended for use with Atlassian Cloud. If that's the case then perhaps we can just fully remove the "Personal Access Token" and only support Basic Auth for both self-hosted Jira and Atlassian Cloud.

@legoguy1000 Do you recall the intent behind the "Personal Access Token"? Was that for the Atlassian Cloud case?

@legoguy1000
Copy link
Contributor

legoguy1000 commented Jul 13, 2022

So if I recall everything from my testing, the self hosted confluence and JIRA support basic auth with user/password OR Bearer auth header with a PAT. Cloud versions only supported basic auth with email/token. When I updated the integrations to support cloud, I let the basic auth option for either and Auth header only for self hosted.

@efd6
Copy link
Contributor Author

efd6 commented Jul 25, 2022

Is there a decision on this?

@andrewkroh
Copy link
Member

the self hosted confluence and JIRA support basic auth with user/password OR Bearer auth header with a PAT

Yeah, it looks to be true that a PAT can be used with self-hosted JIRA via a Authentication: Bearer <pat> header. https://developer.atlassian.com/server/jira/platform/rest-apis/

So I think we should clarify the readme by

In UI description of the "Personal Access Token" I think it should mention that this is only for self-hosted JIRA (or not for Atlassian Cloud). This will help steer users toward the correct fields if the don't read the README.

@legoguy1000
Copy link
Contributor

legoguy1000 commented Jul 25, 2022 via email

@andrewkroh
Copy link
Member

IMO if we removed the abstraction from the UI / manifest.yml then determining what options to set based on use case gets simpler. Like if the UI had three config field, Basic Auth Username, Basic Auth Password, and Bearer Authentication Header then when I'm reading the Atlassian documentation I have a straight forward way to map things into our UI.

The descriptions of the fields can still be there to guide users as to what to enter based on whether that are connecting to Jira Server (self-hosted) or Atlassian Cloud.

## Logs

### Audit
z### Audit
Copy link
Contributor

Choose a reason for hiding this comment

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

typo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😆

The consequence of using hot-row modifiers.

@efd6
Copy link
Contributor Author

efd6 commented Sep 13, 2022

/test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working, use only for issues documentation Improvements or additions to documentation Integration:atlassian_confluence Atlassian Confluence Integration:atlassian_jira Atlassian Jira
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[atlassian_jira] Update username / password descriptions for Atlassian Cloud
4 participants