refactor: use `oras-credentials-go` for credential management by Wwwsylvia · Pull Request #654 · notaryproject/notation

Wwwsylvia · 2023-04-27T11:13:11Z

Resolves: #600
Resolves: #637

codecov-commenter · 2023-04-27T11:17:47Z

Codecov Report

Attention: Patch coverage is 20.61856% with 77 lines in your changes missing coverage. Please review.

Project coverage is 59.13%. Comparing base (5fdefed) to head (16be413).
Report is 201 commits behind head on main.

Files with missing lines	Patch %	Lines
cmd/notation/login.go	0.00%	50 Missing ⚠️
internal/auth/credentials.go	0.00%	15 Missing ⚠️
cmd/notation/registry.go	55.55%	5 Missing and 3 partials ⚠️
cmd/notation/logout.go	0.00%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #654      +/-   ##
==========================================
- Coverage   61.35%   59.13%   -2.23%     
==========================================
  Files          42       40       -2     
  Lines        2280     2183      -97     
==========================================
- Hits         1399     1291     -108     
- Misses        782      795      +13     
+ Partials       99       97       -2

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

Wwwsylvia · 2023-04-27T11:57:54Z

Behaviors of this PR

notation login

If the provided credential is invalid or the connection to registry fails, throw an error
If there is a credentials store configured in the notation config file
- Save the credential into the configured credentials store
If no credentials store is configured, or the notation config file does not exist
- Use the platform-default credentials store, and set it back to the notation config file
- If the platform-default credentials store is not installed
  - If the provided credential is identical with the existing credential, print:
  - If the provided credential is different with the saved one, throw an error

notation logout

If there is a credentials store configured in the notation config file
- Remove the credential of the registry
If no credentials store is configured, or the notation config file does not exist
- No operation

Other notation operations that require authentication

Look for the credentials in following order:

notation config file

Configured credential helpers
Configured credentials store
Platform-default credentials store
Plaintext credentials

Docker config file

Configured credential helpers
Configured credentials store
Platform-default credentials store
Plaintext credentials

Related links

cmd/notation/registry.go

pkg/auth/native_store.go

cmd/notation/logout.go

JeyJeyGao

LGTM

shizhMSFT

LGTM with nit.

cmd/notation/login.go

JeyJeyGao

LGTM

ghost

LGTM

shizhMSFT

LGTM

Signed-off-by: Sylvia Lei <lixlei@microsoft.com>

cmd/notation/login.go

priteshbandi

LGTM

JeyJeyGao

LGTM

ghost

LGTM

shizhMSFT

LGTM

…project#654) Resolves: notaryproject#600 Resolves: notaryproject#637 --------- Signed-off-by: Sylvia Lei <lixlei@microsoft.com>

Wwwsylvia changed the title ~~refactor: use oras-credentials-go~~ refactor: use oras-credentials-go for credential management Apr 27, 2023

Wwwsylvia changed the title ~~refactor: use oras-credentials-go for credential management~~ refactor: use oras-credentials-go for credential management Apr 27, 2023

This comment was marked as duplicate.

Sign in to view

Wwwsylvia marked this pull request as ready for review April 28, 2023 02:26

Wwwsylvia requested review from JeyJeyGao, NiazFK, SteveLasker, gokarnm, justincormack, patrickzheng200, priteshbandi, rgnote and shizhMSFT as code owners April 28, 2023 02:26

Wwwsylvia marked this pull request as draft April 28, 2023 03:20

Wwwsylvia force-pushed the use_orascreds branch 4 times, most recently from a3b8249 to 9059f99 Compare April 28, 2023 03:50

Wwwsylvia marked this pull request as ready for review April 28, 2023 04:19

shizhMSFT reviewed Apr 28, 2023

View reviewed changes

cmd/notation/registry.go Outdated Show resolved Hide resolved

shizhMSFT reviewed Apr 28, 2023

View reviewed changes

pkg/auth/native_store.go Show resolved Hide resolved

JeyJeyGao reviewed May 4, 2023

View reviewed changes

cmd/notation/logout.go Show resolved Hide resolved

Wwwsylvia marked this pull request as draft May 8, 2023 12:05

Wwwsylvia marked this pull request as ready for review May 9, 2023 03:01

Wwwsylvia force-pushed the use_orascreds branch 3 times, most recently from a2adf1a to 2850ed0 Compare May 10, 2023 11:46

Wwwsylvia mentioned this pull request May 10, 2023

Trace support for executables oras-project/oras-credentials-go#63

Closed

JeyJeyGao previously approved these changes May 19, 2023

View reviewed changes

shizhMSFT previously approved these changes May 19, 2023

View reviewed changes

cmd/notation/login.go Outdated Show resolved Hide resolved

Wwwsylvia dismissed stale reviews from shizhMSFT, JeyJeyGao, and ghost via dac7cd4 May 19, 2023 06:37

JeyJeyGao previously approved these changes May 19, 2023

View reviewed changes

ghost previously approved these changes May 19, 2023

View reviewed changes

shizhMSFT previously approved these changes May 19, 2023

View reviewed changes

Wwwsylvia requested a review from ningziwen May 19, 2023 10:08

use oras-credentials-go

accf976

Signed-off-by: Sylvia Lei <lixlei@microsoft.com>

Wwwsylvia dismissed stale reviews from shizhMSFT, ghost , and JeyJeyGao via accf976 May 23, 2023 03:29

Wwwsylvia force-pushed the use_orascreds branch from dac7cd4 to accf976 Compare May 23, 2023 03:29

update comment

16be413

Signed-off-by: Sylvia Lei <lixlei@microsoft.com>

Wwwsylvia requested review from a user, JeyJeyGao and shizhMSFT May 23, 2023 03:41

ghost reviewed May 23, 2023

View reviewed changes

cmd/notation/login.go Show resolved Hide resolved

cmd/notation/login.go Show resolved Hide resolved

priteshbandi approved these changes May 23, 2023

View reviewed changes

JeyJeyGao approved these changes May 23, 2023

View reviewed changes

ghost approved these changes May 23, 2023

View reviewed changes

JeyJeyGao mentioned this pull request May 23, 2023

E2E testing missing login test #680

Open

shizhMSFT approved these changes May 23, 2023

View reviewed changes

shizhMSFT merged commit a695b60 into notaryproject:main May 23, 2023

Wwwsylvia deleted the use_orascreds branch May 23, 2023 08:15

Wwwsylvia mentioned this pull request May 24, 2023

docs: notation login error message improvement #681

Closed

priteshbandi mentioned this pull request May 26, 2023

Vote for release of notation v1.0.0-rc.6 #686

Closed

6 tasks

Conversation

Wwwsylvia commented Apr 27, 2023

Uh oh!

codecov-commenter commented Apr 27, 2023 • edited by codecov bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Wwwsylvia commented Apr 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Behaviors of this PR

notation login

notation logout

Other notation operations that require authentication

Related links

Uh oh!

This comment was marked as duplicate.

Uh oh!

Uh oh!

Uh oh!

JeyJeyGao left a comment

Choose a reason for hiding this comment

Uh oh!

shizhMSFT left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JeyJeyGao left a comment

Choose a reason for hiding this comment

Uh oh!

ghost left a comment

Choose a reason for hiding this comment

Uh oh!

shizhMSFT left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

priteshbandi left a comment

Choose a reason for hiding this comment

Uh oh!

JeyJeyGao left a comment

Choose a reason for hiding this comment

Uh oh!

ghost left a comment

Choose a reason for hiding this comment

Uh oh!

shizhMSFT left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Comments

codecov-commenter commented Apr 27, 2023 •

edited by codecov bot

Loading

Wwwsylvia commented Apr 27, 2023 •

edited

Loading