Skip to content

feat:add feature to change password for profile as well as user - #524

Merged
bupd merged 1 commit into
goharbor:mainfrom
ChayanDass:password
Aug 26, 2025
Merged

bupd merged 1 commit into
goharbor:mainfrom
ChayanDass:password

Conversation

@ChayanDass

@ChayanDass ChayanDass commented Aug 2, 2025

Copy link
Copy Markdown
Contributor

closes #523

password change operation

image

logs for user password update and own password

image

@bupd
bupd self-requested a review August 12, 2025 11:05
Comment thread harbor-cli Outdated

@bupd bupd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Overalll LGTM, will give it a hands on review in today's CLI meeting.

@bupd bupd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Overalll LGTM, will give it a hands on review in today's CLI meeting.

@codecov

codecov Bot commented Aug 12, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 6.25000% with 180 lines in your changes missing coverage. Please review.
✅ Project coverage is 7.19%. Comparing base (60ad0bd) to head (701d624).
⚠️ Report is 37 commits behind head on main.

Files with missing lines Patch % Lines
cmd/harbor/root/password.go 11.59% 60 Missing and 1 partial ⚠️
cmd/harbor/root/user/password.go 0.00% 39 Missing ⚠️
pkg/views/password/change/view.go 0.00% 33 Missing ⚠️
pkg/views/password/reset/view.go 0.00% 28 Missing ⚠️
pkg/api/user_handler.go 0.00% 18 Missing ⚠️
cmd/harbor/root/user/cmd.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##             main    #524      +/-   ##
=========================================
- Coverage   10.99%   7.19%   -3.80%     
=========================================
  Files         173     237      +64     
  Lines        8671   14217    +5546     
=========================================
+ Hits          953    1023      +70     
- Misses       7612   13087    +5475     
- Partials      106     107       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bupd bupd changed the title feat:add feature to chnge password for profile as well as user feat:add feature to change password for profile as well as user Aug 12, 2025
@bupd

bupd commented Aug 12, 2025

Copy link
Copy Markdown
Member

@ChayanDass please do rebase your patch on main branch

@Vad1mo
Vad1mo requested a review from Copilot August 12, 2025 13:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds password change functionality for both user profiles and admin-managed users. The implementation provides two distinct workflows: users can change their own passwords by providing their old password, while admins can reset passwords for other users without requiring the old password.

  • Adds two password change views with appropriate validation
  • Implements API handlers for both user password change and admin password reset
  • Creates CLI commands for password management accessible through both profile and user management contexts

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
pkg/views/password/reset/view.go Creates password reset form for admin use (new password + confirmation)
pkg/views/password/change/view.go Creates password change form for user profile (old + new + confirmation)
pkg/api/user_handler.go Adds ResetPassword API function for admin password reset
cmd/harbor/root/user/password.go Implements user password reset command for admins
cmd/harbor/root/user/cmd.go Registers password command under user management
cmd/harbor/root/password.go Implements profile password change command
cmd/harbor/root/cmd.go Registers password command under access group

Comment thread pkg/api/user_handler.go Outdated
Comment thread cmd/harbor/root/user/password.go Outdated
Comment thread cmd/harbor/root/user/password.go Outdated
Comment thread cmd/harbor/root/password.go Outdated
@ChayanDass

ChayanDass commented Aug 12, 2025

Copy link
Copy Markdown
Contributor Author

Hey @bupd, there’s an issue , after changing the password for the current user, all API calls become unauthorized. I’m not sure if we’re sending the username and password from the config file, but if we are, we might need to update the password in the config file as well.

@ChayanDass

ChayanDass commented Aug 12, 2025

Copy link
Copy Markdown
Contributor Author
image

After changing the password for the current user, it also updates the hashed password in the config file

@ChayanDass
ChayanDass requested a review from bupd August 12, 2025 20:02

@bupd bupd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@ChayanDass Thanks for you contributions. will review this next week. Please do update the screenshots on the PR description.

@bupd
bupd self-requested a review August 14, 2025 19:43

@bupd bupd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

Comment thread cmd/harbor/root/password.go Outdated
@Vad1mo

Vad1mo commented Aug 19, 2025

Copy link
Copy Markdown
Member

lint error, other wise looks good

@ChayanDass
ChayanDass requested a review from bupd August 19, 2025 13:58

@bupd bupd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@bupd

bupd commented Aug 19, 2025

Copy link
Copy Markdown
Member

@ChayanDass I believe you need to run dagger call run-doc export I believe we have this in contributing.md when adding new commands

@ChayanDass
ChayanDass requested review from Vad1mo and bupd August 20, 2025 06:43

@bupd bupd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@ChayanDass please fix lint issue and make sure CI passes before requesting a review.

@ChayanDass

ChayanDass commented Aug 20, 2025

Copy link
Copy Markdown
Contributor Author

Hey @bupd, sorry for the earlier review request , I accidentally requested before checking the latest messages. I’ve now fixed the linting issues and updated the docs.

Maybe I missed, but I'm not able to find any instructions about the docs at contributing.md

@bupd

bupd commented Aug 20, 2025

Copy link
Copy Markdown
Member

Then we need to add that to the contributing.md feel free to create an issue.

Signed-off-by: Chayan Das <01chayandas@gmail.com>

@bupd bupd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@bupd
bupd merged commit 180af97 into goharbor:main Aug 26, 2025
7 of 9 checks passed
@ChayanDass
ChayanDass deleted the password branch August 26, 2025 14:03
@qcserestipy qcserestipy mentioned this pull request Nov 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Support for User Password Management in harbor-cli

4 participants