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

Jira: add SSL client certificate support for authentication #9753

Conversation

weristdominik
Copy link
Contributor

@weristdominik weristdominik commented Feb 15, 2025

SUMMARY

Enabled support for Client Certificate authentification for community.general.jira module, for all usecases (creating issue, comment etc.)

Edit def request() function and replaced fetch_url() with usage of urllib.request. Allowing both Username+Password (or token) AND Username+Password (or token) + Client Certificate/ Client Key.

Can be used by simply adding 2x more values into each Ansible task:
client_cert: "/path/to/client.crt"
client_key: "/path/to/client.key"

Edit DOCUMENTATION & EXAMPLES section inside jira.py File.

ISSUE TYPE
  • Docs Pull Request
  • Feature Pull Request
COMPONENT NAME

jira

ADDITIONAL INFORMATION

@ansibullbot
Copy link
Collaborator

@ansibullbot ansibullbot added docs module module new_contributor Help guide this first time contributor plugins plugin (any type) labels Feb 15, 2025
@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-10 Automatically create a backport for the stable-10 branch labels Feb 15, 2025
@ansibullbot

This comment was marked as outdated.

@ansibullbot ansibullbot added ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Feb 15, 2025
@ansibullbot

This comment was marked as outdated.

@ansibullbot ansibullbot removed the ci_verified Push fixes to PR branch to re-run CI label Feb 16, 2025
@ansibullbot

This comment was marked as outdated.

@ansibullbot ansibullbot added the ci_verified Push fixes to PR branch to re-run CI label Feb 16, 2025
@ansibullbot ansibullbot removed the ci_verified Push fixes to PR branch to re-run CI label Feb 16, 2025
@ansibullbot

This comment was marked as outdated.

@ansibullbot ansibullbot added the ci_verified Push fixes to PR branch to re-run CI label Feb 16, 2025
Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! Please remember to add a changelog fragment. Thanks.

@ansibullbot ansibullbot removed the ci_verified Push fixes to PR branch to re-run CI label Feb 16, 2025
weristdominik and others added 2 commits February 16, 2025 15:15
@ansibullbot ansibullbot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Feb 16, 2025
@ansibullbot
Copy link
Collaborator

@weristdominik this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@ansibullbot ansibullbot added merge_commit This PR contains at least one merge commit. Please resolve! needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Feb 16, 2025
@ansibullbot ansibullbot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Feb 16, 2025
@ansibullbot ansibullbot added needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Feb 16, 2025
@weristdominik weristdominik requested a review from russoz February 16, 2025 18:31
@ansibullbot ansibullbot removed needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Feb 16, 2025
Comment on lines +62 to +73
client_cert:
type: path
description:
- Client certificate if required.
- In addition to O(username) and O(password) or O(token). Not mutually exclusive.
version_added: 10.4.0
client_key:
type: path
description:
- Client certificate key if required.
- In addition to O(username) and O(password) or O(token). Not mutually exclusive.
version_added: 10.4.0
Copy link
Collaborator

@russoz russoz Feb 16, 2025

Choose a reason for hiding this comment

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

Something that will not be solved now, let alone in this PR, is that the docs for these parameters should be in a doc_fragment, ansible-core has one for urls but that one includes a parameter .... url. I don't think ansible-core will be changing that anytime soon (in theory it should break that in two separate fragments, one for the url itself and another for all these other operational parameters for HTTP transactions, but that would imply changes in a lot modules - not gonna happen).

So, it could/should be a future PR for c.g. to create doc fragments like that and use it within the collection.

Copy link
Collaborator

@russoz russoz left a comment

Choose a reason for hiding this comment

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

LGTM

@felixfontein felixfontein merged commit fa7876b into ansible-collections:main Feb 16, 2025
138 checks passed
Copy link

patchback bot commented Feb 16, 2025

Backport to stable-10: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-10/fa7876bb406cbba9a4cd937a0e8c6d8ada3e5e24/pr-9753

Backported as #9763

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Feb 16, 2025
patchback bot pushed a commit that referenced this pull request Feb 16, 2025
* jira: add ssl client certificate support for authentification

* fix code bugs from first CI run

* fix fstring not compatible with older python and chhange urlopen module call

* removed duplicated post,put,get method

* fix urllib module detection Python2/ Python3

* edit HTTP Request back to fetch_url

* add changelog fragment

* fix python line spacing

* Update plugins/modules/jira.py

Co-authored-by: Felix Fontein <[email protected]>

* Update plugins/modules/jira.py

Co-authored-by: Felix Fontein <[email protected]>

* edit documentation certificate auth not mutually exclusive

* Update changelogs/fragments/9753-jira-add-client-certificate-auth.yml

Co-authored-by: Felix Fontein <[email protected]>

* edit documentation for client certificate auth and token

* add no_log for client_cert and client_key

* removed no_log for client_cert and client_key

---------

Co-authored-by: domin <[email protected]>
Co-authored-by: Felix Fontein <[email protected]>
(cherry picked from commit fa7876b)
@felixfontein
Copy link
Collaborator

@weristdominik thanks for your contribution!
@russoz thanks for reviewing!

felixfontein pushed a commit that referenced this pull request Feb 16, 2025
…ate support for authentication (#9763)

Jira: add SSL client certificate support for authentication (#9753)

* jira: add ssl client certificate support for authentification

* fix code bugs from first CI run

* fix fstring not compatible with older python and chhange urlopen module call

* removed duplicated post,put,get method

* fix urllib module detection Python2/ Python3

* edit HTTP Request back to fetch_url

* add changelog fragment

* fix python line spacing

* Update plugins/modules/jira.py

Co-authored-by: Felix Fontein <[email protected]>

* Update plugins/modules/jira.py

Co-authored-by: Felix Fontein <[email protected]>

* edit documentation certificate auth not mutually exclusive

* Update changelogs/fragments/9753-jira-add-client-certificate-auth.yml

Co-authored-by: Felix Fontein <[email protected]>

* edit documentation for client certificate auth and token

* add no_log for client_cert and client_key

* removed no_log for client_cert and client_key

---------

Co-authored-by: domin <[email protected]>
Co-authored-by: Felix Fontein <[email protected]>
(cherry picked from commit fa7876b)

Co-authored-by: Dominik <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-10 Automatically create a backport for the stable-10 branch docs merge_commit This PR contains at least one merge commit. Please resolve! module module needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html new_contributor Help guide this first time contributor plugins plugin (any type)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants