add static openid provider #41
Conversation
doodzik
force-pushed
the
add_static_openid_provider_2
branch
3 times, most recently
from
4808273 to
af1bd30
Compare
doodzik
force-pushed
the
add_static_openid_provider_2
branch
3 times, most recently
from
6bfd422 to
26171c9
Compare
| @@ -2,6 +2,7 @@ | |||
| require 'webmock/test_unit' | |||
| require 'rubygems/mock_gem_ui' | |||
| require 'json/jwt' | |||
| require 'byebug' | |||
There was a problem hiding this comment.
making debugging in tests a bit easier so we don't have to type require 'byebug';
| end | ||
|
|
||
| def public_keys | ||
| @public_keys ||= oidc_discovery.jwks |
There was a problem hiding this comment.
We are loading the public key from the provider specified in the config file, which is probably good enough for now, but we might want to revisit it in the future.
There was a problem hiding this comment.
Your oidc issuer may be different for dynamic vs static flows. In the dynamic flow, you would rely on oauth.sigstore.dev. For CI, it would be GitHub.
Does it make sense to add a new config key in settings.yml, static_token_oidc_issuer ? Or perhaps look for an oidc-issuer command parameter/environment variable?
There was a problem hiding this comment.
I think it makes sense to come up with another way of specifying the oidc-issuer, because the current solution will not scale as you pointed out. I'm not sure how it should look through. My plan was to make the static flow work with Shipit and adjust the command based on my learnings.
| end | ||
| end | ||
|
|
||
| def subject |
There was a problem hiding this comment.
I'm copying cosign's flow of retrieving the subject of a claim as seen in this flow. We might want to chat with the people of sigstore about they utilize the subject field.
| @@ -41,7 +46,8 @@ def execute | |||
| gemfile = Gem::Sigstore::Gemfile.new(get_one_gem_name) | |||
| rekor_entry = Gem::Sigstore::GemSigner.new( | |||
| gemfile: gemfile, | |||
| config: Gem::Sigstore::Config.read | |||
| config: Gem::Sigstore::Config.read, | |||
| token: options[:identity_token], | |||
There was a problem hiding this comment.
Nitpick: perhaps token: should be identity_token: for consistency with :identity_token and --identity-token.
lib/rubygems/sigstore/gem_signer.rb
Outdated
| @@ -9,9 +9,10 @@ class Gem::Sigstore::GemSigner | |||
|
|
|||
| Data = Struct.new(:digest, :signature, :raw) | |||
|
|
|||
| def initialize(gemfile:, config:) | |||
| def initialize(gemfile:, config:, token: nil) | |||
There was a problem hiding this comment.
I suggest this also be identity_token.
| if parsed_token["email"] | ||
| # ensure that the OIDC provider has verified the email address | ||
| # note: this may have happened some time in the past | ||
| if parsed_token["email_verified"] != true |
There was a problem hiding this comment.
Would unless parsed_token["email_verified"] be better here?
| if parsed_token["email_verified"] != true | ||
| abort 'Email address in OIDC token has not been verified by provider' | ||
| end | ||
| return parsed_token["email"] |
There was a problem hiding this comment.
Any reason for using return here?
There was a problem hiding this comment.
It is an early return, but I could move the email retrieval into a separate method to clear things up a bit.
| # ensure that the OIDC provider has verified the email address | ||
| # note: this may have happened some time in the past | ||
| if parsed_token["email_verified"] != true | ||
| abort 'Email address in OIDC token has not been verified by provider' |
There was a problem hiding this comment.
Nit: has not been verified could be replaced with the shorter was not verified.
I'd also change provider to identity provider, because provider is a bit ambiguous by itself.
doodzik
force-pushed
the
add_static_openid_provider_2
branch
from
26171c9 to
574e658
Compare
@jchestershopify Have a look at this PR and verification flow. |
| @unparsed_token = token | ||
| end | ||
|
|
||
| # https://www.youtube.com/watch?v=ZsgA77j5LyY |
Problem
The current implementation of the id provider requires interaction with a web browser, which doesn't work in an automated environment.
Solution
The folks over at sigstore/cosign solved this problem by allowing cosign to accept an id token. This PR ports that approach over.
At a later point (#42), we can also enable the usage of env variables, but that isn't required to get this tool working in ShipIt, which is the biggest unknown in my opinion. See sigstore/cosign#644 for reference.
ref #24