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.

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

New flag --oidc-providers-disable to disable OIDC providers #1832

Merged
merged 3 commits into from May 2, 2022

Conversation

puerco
Copy link
Member

@puerco puerco commented May 2, 2022

Summary

This PR adds a new command line flag --oidc-providers-disable to cosign sign and cosign sign-blob to disable the internal OIDC providers. This does not break compatibility with the current cli and skips the providers logic to jump straight to the OIDC flow.

/cc @di @eddiezane

Signed-off-by: Adolfo García Veytia (Puerco) puerco@chainguard.dev

Ticket Link

Fixes #1819

Release Note

New command line flag `--oidc-providers-disable` to `cosign sign` and `cosign sign-blob` to disable the internal OIDC providers. This does not break compatibility with the current cli and skips the providers logic to jump straight to the OIDC flow.

@@ -66,4 +67,7 @@ func (o *OIDCOptions) AddFlags(cmd *cobra.Command) {

cmd.Flags().StringVar(&o.RedirectURL, "oidc-redirect-url", "",
"[EXPERIMENTAL] OIDC redirect URL (Optional). The default oidc-redirect-url is 'http://localhost:0/auth/callback'.")

cmd.Flags().BoolVar(&o.DisableAmbientProviders, "oidc-providers-disable", false,
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe the cli documentation is missing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks hector! I just regenerated them.

@codecov-commenter
Copy link

codecov-commenter commented May 2, 2022

Codecov Report

Merging #1832 (4f5ed66) into main (88b68f4) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #1832      +/-   ##
==========================================
- Coverage   33.01%   32.99%   -0.02%     
==========================================
  Files         147      147              
  Lines        9350     9355       +5     
==========================================
  Hits         3087     3087              
- Misses       5909     5914       +5     
  Partials      354      354              
Impacted Files Coverage Δ
cmd/cosign/cli/fulcio/fulcio.go 21.42% <0.00%> (ø)
cmd/cosign/cli/options/oidc.go 0.00% <0.00%> (ø)
cmd/cosign/cli/sign.go 0.00% <0.00%> (ø)
cmd/cosign/cli/signblob.go 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 88b68f4...4f5ed66. Read the comment docs.

@@ -66,4 +67,7 @@ func (o *OIDCOptions) AddFlags(cmd *cobra.Command) {

cmd.Flags().StringVar(&o.RedirectURL, "oidc-redirect-url", "",
"[EXPERIMENTAL] OIDC redirect URL (Optional). The default oidc-redirect-url is 'http://localhost:0/auth/callback'.")

cmd.Flags().BoolVar(&o.DisableAmbientProviders, "oidc-providers-disable", false,
Copy link
Member

Choose a reason for hiding this comment

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

This is yakshaving, but IMO the --oidc-providers-disable flag could be a little more descriptive.

In sigstore-python we refer to these providers as "ambient" providers to distinguish them from explicit OAuth flows and other sources of identities. I think a flag name like --disable-ambient-oidc or similar (--oidc-disable-ambient-providers, etc.) would better capture the semantics here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good to me, thanks for the suggestion @woodruffw , changed.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks a ton!

This commit adds an option to tell the fulcio client to avoid trying
to get an auth token from the internal OIDC providers.

Signed-off-by: Adolfo García Veytia (Puerco) <puerco@chainguard.dev>
…oviders

This commit adds a command line flag `--oidc-providers-disable` to
`cosign sign` and `cosign sign-blob` to disable the internal OIDC
providers. This does not break compatibility with the current cli and
skips the providers logic to jump straight to the OIDC flow.

Signed-off-by: Adolfo García Veytia (Puerco) <puerco@chainguard.dev>
@puerco puerco force-pushed the disable-oidc-providers branch 4 times, most recently from 2346074 to a440205 Compare May 2, 2022 23:02
Signed-off-by: Adolfo García Veytia (Puerco) <puerco@chainguard.dev>
@dlorenc dlorenc merged commit 2e00e8a into sigstore:main May 2, 2022
@github-actions github-actions bot added this to the v1.9.0 milestone May 2, 2022
mlieberman85 pushed a commit to mlieberman85/cosign that referenced this pull request May 6, 2022
…#1832)

* Add KeyOpt to disable internal providers

This commit adds an option to tell the fulcio client to avoid trying
to get an auth token from the internal OIDC providers.

Signed-off-by: Adolfo García Veytia (Puerco) <puerco@chainguard.dev>

* Add --oidc-disable-ambient-providers flag to disable internal OIDC providers

This commit adds a command line flag `--oidc-providers-disable` to
`cosign sign` and `cosign sign-blob` to disable the internal OIDC
providers. This does not break compatibility with the current cli and
skips the providers logic to jump straight to the OIDC flow.

Signed-off-by: Adolfo García Veytia (Puerco) <puerco@chainguard.dev>

* Update internal docs for new OIDC flag

Signed-off-by: Adolfo García Veytia (Puerco) <puerco@chainguard.dev>
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 a flag to disable ambient credential usage for cosign sign
5 participants