Skip to content

add SensitiveParameter to sensitive arguments #119

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.

Sign up for GitHub

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

Jump to bottom
Merged
RobThree merged 5 commits into from
Apr 17, 2024
Merged

add SensitiveParameter to sensitive arguments #119

RobThree merged 5 commits into from
Apr 17, 2024

Conversation

@NicolasCARPi
Copy link
Collaborator

@NicolasCARPi NicolasCARPi commented Apr 15, 2024

This change adds the PHP attribute SensitiveParameter to the secret holding variables.
See: https://www.php.net/manual/en/class.sensitiveparameter

This feature is only available in PHP 8.2, so the minimum php version required has been updated.

Github Actions now use PHP 8.2 and 8.3 for the tests. The checkout action has been updated to v4, too.

Fix issue #118

@NicolasCARPi
add SensitiveParameter to sensitive arguments
76f420e 
This change adds the PHP attribute SensitiveParameter to the secret
holding variables.
See: https://www.php.net/manual/en/class.sensitiveparameter

This feature is only available in PHP 8.2, so the minimum php version
required has been updated.

Github Actions now use PHP 8.2 and 8.3 for the tests.
The checkout action has been updated to v4, too.

Fix issue #118
@NicolasCARPi
don't include the sensitive param change in breaking changes section
061a2a3 
in changelog
@NicolasCARPi
use global import
bba4c20
@NicolasCARPi NicolasCARPi mentioned this pull request Apr 15, 2024
Closed
RobThree
RobThree reviewed Apr 15, 2024
View reviewed changes
@willpower232
Copy link
Collaborator

willpower232 commented Apr 15, 2024

I finally merged #103 so hopefully the v3 composer installs will be slightly smaller, not sure if explicitly worth noting in the changelog

@RobThree
Copy link
Owner

RobThree commented Apr 15, 2024

not sure if explicitly worth noting in the changelog

Meh...

I think the .phpproj and .sln can also be deleted; I don't use VS2022 with PHP tools for PHP development anymore.

@NicolasCARPi
Copy link
Collaborator Author

NicolasCARPi commented Apr 15, 2024 •
edited
Loading

not sure if explicitly worth noting in the changelog

About the changelog, I would suggest going over the commits added after the latest version, and mentioning the changes useful to users in this file.

Also, it looks like the PR are merged without squashing the commits, which pollutes the git log with useless commits such as "Fix typo". I would strongly suggest merging PR squashing the commits into 1.

@NicolasCARPi NicolasCARPi mentioned this pull request Apr 15, 2024
Merged
@RobThree
Copy link
Owner

RobThree commented Apr 15, 2024

Hey, guys, not sure if I expressed this before (if I haven't, I should've, I've thought this many a times) but I really appreciate your work on this library. Especially the both of you, @NicolasCARPi and @willpower232 .

Thanks guys! 🙏 👊

@willpower232
Copy link
Collaborator

willpower232 commented Apr 16, 2024

About the changelog, I would suggest going over the commits added after the latest version, and mentioning the changes useful to users in this file.

big agree

Also, it looks like the PR are merged without squashing the commits, which pollutes the git log with useless commits such as "Fix typo". I would strongly suggest merging PR squashing the commits into 1.

depends on the PR and whether the commits were relatively atomic or not but yeah, mostly agree

willpower232 pushed a commit that referenced this pull request Apr 16, 2024
@NicolasCARPi
delete files specific to code editors (#120)
323053b 
* delete files specific to code editors

* remove TwoFactorAuth.phpproj
* remove TwoFactorAuth.sln

They are not used anymore. See #119 (comment)

* remove deleted files from excluded files in composer.json
@NicolasCARPi
Merge branch 'master' into nico-sensitive-attr
e08b0a5 
* master:
  add CI4-auth link in README. fix #107 (#123)
  remove insecure rng providers and remove polyfill for hash_equals (#122)
  delete files specific to code editors (#120)
  Exclude useless files from dist archive #103
willpower232
willpower232 reviewed Apr 16, 2024
View reviewed changes
Copy link
Collaborator

@willpower232 willpower232 left a comment

Choose a reason for hiding this comment

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

might as well make this the v3 pr haha

@NicolasCARPi
Copy link
Collaborator Author

NicolasCARPi commented Apr 17, 2024

Is there any blocker for the merge?

@RobThree RobThree merged commit b909cb3 into RobThree:master Apr 17, 2024
@RobThree
Copy link
Owner

RobThree commented Apr 17, 2024

I merged it, AFAIK it was all ok.

@NicolasCARPi NicolasCARPi deleted the nico-sensitive-attr branch April 17, 2024 18:32
@willpower232
Copy link
Collaborator

willpower232 commented Apr 18, 2024

I meant to poke you all yesterday, all good with me

pH-7 referenced this pull request in pH7Software/pH7-Social-Dating-CMS Sep 3, 2024
@pH-7
Quick class cleanup + type hints
e359a5b 
* Use PHP 8 type declarations (on params and class props)
* Remove unnecessary PHPDoc tags
* Add return types
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RobThree RobThree RobThree left review comments

@willpower232 willpower232 willpower232 left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@NicolasCARPi @willpower232 @RobThree