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

Feat(Inventory): Basic auth for Agent #17834

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open

Conversation

MyvTsv
Copy link

@MyvTsv MyvTsv commented Sep 11, 2024

Checklist before requesting a review

Please delete options that are not relevant.

  • I have read the CONTRIBUTING document.
  • I have performed a self-review of my code.
  • I have added tests that prove my fix is effective or that my feature works.

Description

Implement Basic Auth form for inventory endpoint

Screenshots (if appropriate):

Capture d’écran du 2024-09-11 15-00-48

src/Glpi/Inventory/Conf.php Outdated Show resolved Hide resolved
src/Glpi/Inventory/Conf.php Show resolved Hide resolved
src/Glpi/Inventory/Conf.php Show resolved Hide resolved
front/inventory.conf.php Outdated Show resolved Hide resolved
front/inventory.conf.php Outdated Show resolved Hide resolved
src/Glpi/Agent/Communication/AbstractRequest.php Outdated Show resolved Hide resolved
src/Glpi/Agent/Communication/AbstractRequest.php Outdated Show resolved Hide resolved
src/Glpi/Inventory/Conf.php Outdated Show resolved Hide resolved
src/Glpi/Inventory/Conf.php Outdated Show resolved Hide resolved
src/Glpi/Inventory/Conf.php Outdated Show resolved Hide resolved
front/inventory.conf.php Outdated Show resolved Hide resolved
src/Glpi/Inventory/Conf.php Outdated Show resolved Hide resolved
src/Glpi/Inventory/Conf.php Outdated Show resolved Hide resolved
front/inventory.conf.php Outdated Show resolved Hide resolved
src/Glpi/Inventory/Conf.php Outdated Show resolved Hide resolved
src/Glpi/Inventory/Conf.php Outdated Show resolved Hide resolved
@stonebuzz stonebuzz self-requested a review September 13, 2024 14:19
front/inventory.conf.php Outdated Show resolved Hide resolved
src/Glpi/Agent/Communication/Headers/Common.php Outdated Show resolved Hide resolved
src/Glpi/Inventory/Conf.php Outdated Show resolved Hide resolved
@stonebuzz stonebuzz added this to the 11.0.0 milestone Sep 13, 2024
@stonebuzz stonebuzz self-requested a review September 16, 2024 13:03
src/Glpi/Inventory/Conf.php Outdated Show resolved Hide resolved
@stonebuzz stonebuzz self-requested a review September 17, 2024 12:42
src/Glpi/Inventory/Conf.php Outdated Show resolved Hide resolved
tests/web/Glpi/Inventory/Request.php Outdated Show resolved Hide resolved
tests/web/Glpi/Inventory/Request.php Show resolved Hide resolved
tests/web/Glpi/Inventory/Request.php Outdated Show resolved Hide resolved
@stonebuzz stonebuzz self-requested a review September 18, 2024 13:22
src/Glpi/Inventory/Conf.php Outdated Show resolved Hide resolved
@MyvTsv MyvTsv marked this pull request as ready for review September 19, 2024 13:46
tests/web/Glpi/Inventory/Request.php Show resolved Hide resolved
tests/web/Glpi/Inventory/Request.php Show resolved Hide resolved
tests/web/Glpi/Inventory/Request.php Show resolved Hide resolved
tests/web/Glpi/Inventory/Request.php Show resolved Hide resolved
tests/web/Glpi/Inventory/Request.php Show resolved Hide resolved
tests/web/Glpi/Inventory/Request.php Show resolved Hide resolved
@stonebuzz
Copy link
Contributor

$DB->commit() should not be used
I was able to check that the configuration had been saved correctly

@cconard96
Copy link
Contributor

I think there should be a warning displayed when trying to use basic authentication. It really should never be used, except as a short-term solution for anyone that was handling this at the web server level previously until they are able to switch to using OAuth.

@trasher
Copy link
Contributor

trasher commented Oct 2, 2024

I think there should be a warning displayed when trying to use basic authentication. It really should never be used, except as a short-term solution for anyone that was handling this at the web server level previously until they are able to switch to using OAuth.

If we warn about using an unsecure auth mechanism, we have to warn about using default no auth at all one. I wonder if a better place would be in documentation.

@stonebuzz stonebuzz requested a review from trasher October 3, 2024 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants