SM-1402 - review and update php sdk

tangowithfoxtrot commented 2 months ago

🎟️ Tracking

https://bitwarden.atlassian.net/browse/SM-1402

📔 Objective

Update PHP bindings in accordance with our other wrappers. This renames any "put" methods to "update", refactors access_token_login to auth().login_access_token, re-orders function args for create and update, and adds secret syncing.

This update required quite a few changes to the schemas. However, since we cannot auto-generate them with quicktype (see the error referenced in glideapps/quicktype/pull/2407), schemas were generated with the swaggest/json-cli:

json-cli gen-php ../../support/schemas/schema_types/SchemaTypes.json --ns '\Bitwarden\Sdk\Schemas' --ns-path ./src/schemas/

The generated schemas still required hand modification to get human-readable class names for things like ProjectCommand, SecretCommand, etc.

To validate the changes, I've run the example.php file after updating the schemas.

⏰ Reminders before review

Contributor guidelines followed
All formatters and local linters executed and passed
Written new unit and / or integration tests where applicable
Protected functional changes with optionality (feature flags)
Used internationalization (i18n) for all UI strings
CI builds passed
Communicated to DevOps any deployment requirements
Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

👍 (:+1:) or similar for great changes
📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
❓ (:question:) for questions
🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
🎨 (:art:) for suggestions / improvements
❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
⛏ (:pick:) for minor or nitpick changes

github-actions[bot] commented 2 months ago

Checkmarx One – Scan Summary & Details – 9f274337-322f-45a6-aa53-2bf9cd742c0a

New Issues

Issue	Source File / Package	Checkmarx Insight
Privacy_Violation	/languages/java/src/main/java/com/bitwarden/sdk/BitwardenClient.java: 41	Attack Vector
Privacy_Violation	/languages/java/src/main/java/com/bitwarden/sdk/SecretsClient.java: 40	Attack Vector
Unpinned Actions Full Length Commit SHA	/build-cli.yml: 341	Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
Unpinned Actions Full Length Commit SHA	/publish-rust-crates.yml: 56	Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
Unpinned Actions Full Length Commit SHA	/build-swift.yml: 84	Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...

Fixed Issues

Severity	Issue	Source File / Package
	Privacy_Violation	/languages/java/example/Example.java: 46
	Privacy_Violation	/languages/java/src/main/java/com/bitwarden/sdk/BitwardenClient.java: 43
	Privacy_Violation	/languages/java/example/Example.java: 43
	Privacy_Violation	/languages/java/src/main/java/com/bitwarden/sdk/BitwardenClient.java: 43
	Privacy_Violation	/languages/java/src/main/java/com/bitwarden/sdk/BitwardenClient.java: 43
	Privacy_Violation	/languages/java/src/main/java/com/bitwarden/sdk/BitwardenClient.java: 43
	Privacy_Violation	/languages/java/src/main/java/com/bitwarden/sdk/BitwardenClient.java: 43
	Privacy_Violation	/languages/java/src/main/java/com/bitwarden/sdk/SecretsClient.java: 138
	Privacy_Violation	/languages/java/src/main/java/com/bitwarden/sdk/SecretsClient.java: 41
	Privacy_Violation	/languages/java/src/main/java/com/bitwarden/sdk/SecretsClient.java: 41
	Privacy_Violation	/languages/java/example/Example.java: 53
	Privacy_Violation	/languages/java/src/main/java/com/bitwarden/sdk/SecretsClient.java: 20
	Unpinned Actions Full Length Commit SHA	/release-cli.yml: 58
	Unpinned Actions Full Length Commit SHA	/build-cli.yml: 338
	Unpinned Actions Full Length Commit SHA	/release-cli.yml: 192
	Unpinned Actions Full Length Commit SHA	/build-cli-docker.yml: 54
	Unpinned Actions Full Length Commit SHA	/build-cli-docker.yml: 61
	Unpinned Actions Full Length Commit SHA	/build-cli.yml: 173
	Unpinned Actions Full Length Commit SHA	/release-cli.yml: 198
	Unpinned Actions Full Length Commit SHA	/release-cli.yml: 66
	Unpinned Actions Full Length Commit SHA	/publish-rust-crates.yml: 43
	Unpinned Actions Full Length Commit SHA	/build-swift.yml: 91
	Unpinned Actions Full Length Commit SHA	/build-cli-docker.yml: 131
	Unpinned Actions Full Length Commit SHA	/build-cli.yml: 86
	Unpinned Actions Full Length Commit SHA	/release-cli.yml: 124

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 58.16%. Comparing base (c0859c4) to head (f255347). Report is 18 commits behind head on main.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #1032 +/- ## ========================================== - Coverage 58.18% 58.16% -0.03% ========================================== Files 197 197 Lines 13487 13523 +36 ========================================== + Hits 7847 7865 +18 - Misses 5640 5658 +18 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

mzieniukbw commented 2 months ago

The #878 have conflicting changes. Maybe you join them together or pick changes individually from other one, so we won't have to redu the testing ?

tangowithfoxtrot commented 2 months ago

@mzieniukbw , Thanks for the heads-up. I've merged the changes from your branch in #878.

bitwarden / sdk