-
Notifications
You must be signed in to change notification settings - Fork 1.1k
attestations: remove double states, simplify tests #17108
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
Conversation
Signed-off-by: William Woodruff <william@trailofbits.com>
Handled latently via the Publisher identity. Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
To summarize: this makes it easier to enable attestations from other TP providers, since they no longer need to update multiple independent sites: each new attestation source only needs to add |
Signed-off-by: William Woodruff <william@trailofbits.com>
NB: This also removes our top-level dependency on |
Let's go ahead and do it here, thanks! |
Signed-off-by: William Woodruff <william@trailofbits.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM aside from 1 nit.
Signed-off-by: William Woodruff <william@trailofbits.com>
This removes some usage of stubs in favor of real models (via factories) where possible, and eliminates some potential sources of double-state/divergence in the original services.
In particular:
OIDCPublisherMixin.supports_attestations
is nowattestation_identity
, and returns aPublisher | None
that can be used directly for verificationOIDCPublisherMixin.publisher_verification_policy
is removed, since thePublisher
now encodes the verification policy_publisher_from_oidc_publisher
is removed, sinceattestation_identity
serves the same purpose