Skip to content

Fix(Credential): handle password correctly#45

Merged
Rom1-B merged 6 commits intomainfrom
fix_password
Feb 26, 2025
Merged

Fix(Credential): handle password correctly#45
Rom1-B merged 6 commits intomainfrom
fix_password

Conversation

@stonebuzz
Copy link
Contributor

@stonebuzz stonebuzz commented Feb 26, 2025

Checklist before requesting a review

Please delete options that are not relevant.

  • I have performed a self-review of my code.
  • I have added tests (when available) that prove my fix is effective or that my feature works.
  • I have updated the CHANGELOG with a short functional description of the fix or new feature.
  • This change requires a documentation update.

Description

Decrypt password before sending to twig (with is_disclosable: true for fields.passwordField)

image

Screenshots (if appropriate):

@stonebuzz stonebuzz requested a review from Rom1-B February 26, 2025 08:22
@stonebuzz stonebuzz self-assigned this Feb 26, 2025
@stonebuzz stonebuzz added the bug Something isn't working label Feb 26, 2025
Copy link
Contributor

@Rom1-B Rom1-B left a comment

Choose a reason for hiding this comment

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

I'm not a fan of this is_disclosable option. I don't think it's justified in this case to set it to true, so I'd be in favour of setting it to false.

@stonebuzz
Copy link
Contributor Author

now field is not disclosable, I've added the ability to clear the field

@stonebuzz stonebuzz requested a review from Rom1-B February 26, 2025 10:39
{
$this->initForm($ID, $options);

$this->fields['password'] = (new GLPIKey())->decrypt($this->fields['password']);
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it's still necessary now that it's no longer on display?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

indeed

@Rom1-B Rom1-B merged commit ef04e4d into main Feb 26, 2025
3 checks passed
@Rom1-B Rom1-B deleted the fix_password branch February 26, 2025 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants