refactor: use `oras-credentials-go` for credential management #654

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

Resolves: #600
Resolves: #637

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

Codecov Report

Merging #654 (16be413) into main (5fdefed) will decrease coverage by 2.23%.
The diff coverage is 20.61%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            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

Impacted Files	Coverage Δ
cmd/notation/login.go	`26.08% <0.00%> (-3.19%)`	⬇️
cmd/notation/logout.go	`50.00% <0.00%> (+5.88%)`	⬆️
internal/auth/credentials.go	`0.00% <0.00%> (ø)`
cmd/notation/registry.go	`61.95% <55.55%> (-5.97%)`	⬇️
cmd/notation/common.go	`100.00% <100.00%> (ø)`

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

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

Wwwsylvia · 2023-05-18T07:01:11Z

I'm still trying to understand this process. So, under what circumstances does Notation login/logout perform write operations (if any) on the Docker config file?

notation will never write to the Docker config file.
If no credentials store is configured, or the notation config file does not exist, notation will use the platform-default credentials store, and set the "credsStore" field back to the notation config file.

Two-Hearts

LGTM

JeyJeyGao

LGTM

shizhMSFT

LGTM with nit.

cmd/notation/login.go

JeyJeyGao

LGTM

Two-Hearts

LGTM

shizhMSFT

LGTM

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

cmd/notation/login.go

priteshbandi

LGTM

JeyJeyGao

LGTM

Two-Hearts

LGTM

shizhMSFT

LGTM

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 gokarnm, JeyJeyGao, justincormack, NiazFK, patrickzheng200, priteshbandi, rgnote, shizhMSFT and SteveLasker 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

Two-Hearts previously approved these changes May 18, 2023

View reviewed changes

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 Two-Hearts via dac7cd4 May 19, 2023 06:37

JeyJeyGao previously approved these changes May 19, 2023

View reviewed changes

Two-Hearts 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, Two-Hearts, 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 shizhMSFT, JeyJeyGao and Two-Hearts May 23, 2023 03:41

Two-Hearts 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

Two-Hearts 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
4 checks passed

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

Open

priteshbandi mentioned this pull request May 26, 2023

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

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: use `oras-credentials-go` for credential management #654

refactor: use `oras-credentials-go` for credential management #654

Wwwsylvia commented Apr 27, 2023

codecov-commenter commented Apr 27, 2023 •

edited

Wwwsylvia commented Apr 27, 2023 •

edited

This comment was marked as duplicate.

Wwwsylvia commented May 18, 2023

Two-Hearts left a comment

JeyJeyGao left a comment

shizhMSFT left a comment

JeyJeyGao left a comment

Two-Hearts left a comment

shizhMSFT left a comment

priteshbandi left a comment

JeyJeyGao left a comment

Two-Hearts left a comment

shizhMSFT left a comment

refactor: use oras-credentials-go for credential management #654

refactor: use oras-credentials-go for credential management #654

Conversation

Wwwsylvia commented Apr 27, 2023

codecov-commenter commented Apr 27, 2023 • edited

Codecov Report

Wwwsylvia commented Apr 27, 2023 • edited

Behaviors of this PR

notation login

notation logout

Other notation operations that require authentication

Related links

This comment was marked as duplicate.

Wwwsylvia commented May 18, 2023

Two-Hearts left a comment

Choose a reason for hiding this comment

JeyJeyGao left a comment

Choose a reason for hiding this comment

shizhMSFT left a comment

Choose a reason for hiding this comment

JeyJeyGao left a comment

Choose a reason for hiding this comment

Two-Hearts left a comment

Choose a reason for hiding this comment

shizhMSFT left a comment

Choose a reason for hiding this comment

priteshbandi left a comment

Choose a reason for hiding this comment

JeyJeyGao left a comment

Choose a reason for hiding this comment

Two-Hearts left a comment

Choose a reason for hiding this comment

shizhMSFT left a comment

Choose a reason for hiding this comment

refactor: use `oras-credentials-go` for credential management #654

refactor: use `oras-credentials-go` for credential management #654

codecov-commenter commented Apr 27, 2023 •

edited

Wwwsylvia commented Apr 27, 2023 •

edited