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.

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

[PM-2730] Add missing hide-passwords permission to api models #3125

Merged
merged 11 commits into from
Jan 9, 2024
Merged

[PM-2730] Add missing hide-passwords permission to api models #3125

merged 11 commits into from
Jan 9, 2024

Conversation

djsmith85
Copy link
Contributor

Type of change

- [X] Bug fix
- [ ] New feature development
- [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other

Objective

When trying to create or update a collection via the public api, it was possibly to set the read-only permission, but you couldn't set the hide-passwords permission.

This adds the property to the AssociationWithPermissions-models so it can be set.

Props to @r-tome for verifying my findings.

Code changes

  • src/Api/Auth/Models/Public/AssociationWithPermissionsBaseModel.cs.: Add HidePasswords property to BaseModel
  • src/Api/Auth/Models/Public/Request/AssociationWithPermissionsRequestModel.cs: Set HidePasswords property on RequestModel
  • src/Api/Auth/Models/Public/Response/AssociationWithPermissionsResponseModel.cs: Set HidePasswords property on ResponseModel

Before you submit

  • Please check for formatting errors (dotnet format --verify-no-changes) (required)
  • If making database changes - make sure you also update Entity Framework queries and/or migrations
  • Please add unit tests where it makes sense to do so (encouraged but not required)
  • If this change requires a documentation update - notify the documentation team
  • If this change has particular deployment requirements - notify the DevOps team

@djsmith85 djsmith85 requested a review from a team as a code owner July 20, 2023 15:10
@bitwarden-bot
Copy link

bitwarden-bot commented Jul 20, 2023 •
edited
Loading

Logo
Checkmarx One – Scan Summary & Details – e7407ccb-f94f-4e30-a897-0d5a49b6eab0

No New Or Fixed Issues Found

JaredSnider-Bitwarden
JaredSnider-Bitwarden previously approved these changes Jul 20, 2023
View reviewed changes
Copy link
Contributor

@JaredSnider-Bitwarden JaredSnider-Bitwarden left a comment

Choose a reason for hiding this comment

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

LGTM! Ty!

@djsmith85 djsmith85 mentioned this pull request Sep 28, 2023
Merged
djsmith85 and others added 3 commits October 30, 2023 20:10
@djsmith85
Merge branch 'master' into tools/pm-2730/add-missing-hide-passwords-p…
0bcf4c9 
…ermission-to-api-models
@djsmith85
Merge branch 'master' of https://github.com/bitwarden/server into too…
4ee5154 
…ls/pm-2730/add-missing-hide-passwords-permission-to-api-models
@djsmith85
Merge branch 'master' of https://github.com/bitwarden/server into too…
9db6044 
…ls/pm-2730/add-missing-hide-passwords-permission-to-api-models
eliykat
eliykat requested changes Dec 15, 2023
View reviewed changes
Copy link
Member

@eliykat eliykat left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I just made these changes myself and was like "hmm maybe I should check Jira for an existing ticket before opening a PR" - lo and behold. 😁

Here are a couple of changes I made locally, I'd appreciate it if you could include them in this PR.

src/Api/Auth/Models/Public/AssociationWithPermissionsBaseModel.cs Outdated Show resolved Hide resolved
src/Api/Auth/Models/Public/Request/AssociationWithPermissionsRequestModel.cs Outdated Show resolved Hide resolved
djsmith85 and others added 2 commits December 15, 2023 15:39
@djsmith85
Merge branch 'main' of https://github.com/bitwarden/server into tools…
b46c4ef 
…/pm-2730/add-missing-hide-passwords-permission-to-api-models
@djsmith85 @eliykat
Update src/Api/Auth/Models/Public/AssociationWithPermissionsBaseModel.cs
0348e3d 
Co-authored-by: Thomas Rittson <31796059+eliykat@users.noreply.github.com>
@djsmith85 djsmith85 requested a review from a team as a code owner December 15, 2023 15:09
JaredSnider-Bitwarden
JaredSnider-Bitwarden previously approved these changes Dec 15, 2023
View reviewed changes
Copy link
Contributor

@JaredSnider-Bitwarden JaredSnider-Bitwarden left a comment

Choose a reason for hiding this comment

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

New changes LGTM!

@djsmith85 @eliykat
Update src/Api/Auth/Models/Public/Request/AssociationWithPermissionsR…
4baad0d 
…equestModel.cs

Co-authored-by: Thomas Rittson <31796059+eliykat@users.noreply.github.com>
eliykat
eliykat previously approved these changes Dec 18, 2023
View reviewed changes
JaredSnider-Bitwarden
JaredSnider-Bitwarden previously approved these changes Dec 18, 2023
View reviewed changes
Copy link
Contributor

@JaredSnider-Bitwarden JaredSnider-Bitwarden left a comment

Choose a reason for hiding this comment

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

LGTM!

@eliykat eliykat removed the request for review from addisonbeck December 19, 2023 04:00
@djsmith85
Merge branch 'main' of https://github.com/bitwarden/server into tools…
616e2fa 
…/pm-2730/add-missing-hide-passwords-permission-to-api-models
@djsmith85
Copy link
Contributor Author

@JaredSnider-Bitwarden @eliykat Had to pull in main again to fix conflicts after #3584 got merged.

addisonbeck
addisonbeck approved these changes Dec 21, 2023
View reviewed changes
Copy link
Contributor

@addisonbeck addisonbeck left a comment

Choose a reason for hiding this comment

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

Approving for @eliykat since it doesn't seem like anything changed from your merge.

@djsmith85 djsmith85 merged commit 03cbc79 into main Jan 9, 2024
41 of 42 checks passed
@djsmith85 djsmith85 deleted the tools/pm-2730/add-missing-hide-passwords-permission-to-api-models branch January 9, 2024 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@addisonbeck addisonbeck addisonbeck approved these changes

@eliykat eliykat eliykat approved these changes

@JaredSnider-Bitwarden JaredSnider-Bitwarden JaredSnider-Bitwarden approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@djsmith85 @bitwarden-bot @addisonbeck @eliykat @JaredSnider-Bitwarden