Nike-Inc / gimme-aws-creds

A CLI that utilizes Okta IdP via SAML to acquire temporary AWS credentials
Apache License 2.0
925 stars 263 forks source link

Fido2 v1.0.0 pre-upgrade - pin ctap-keyring-device #360

Closed dany74q closed 2 years ago

dany74q commented 2 years ago

This commit pins ctap-keyring-device to 1.0.6, before I'm release a new tag that upgrade to fido2 v1.0.0, which contains breaking changes.

Description

ctap-keyring-device v2.0.0 would require fido2 v1.0.0, which has numerous breaking changes that would affect gimme-aws-creds. As this project is the biggest user of said library, I'd like to take this precaution to prevent breaking it for new installations.

Once pinned, I'll create a new tag from: https://github.com/dany74q/ctap-keyring-device/pull/9

Then I'll create a separate PR here which bumps the pinned versions of fido2 and ctap-keyring-device, along w/ fixing all breaking changes.

Related Issue

https://github.com/Nike-Inc/gimme-aws-creds/issues/355

Motivation and Context

The new fido2 v1.0.0 release has several breaking changes that needs to be addressed; Also, currently ctap-keyring-device uses the forsaken winrt module - in the new tag I'll migrate to the community based winsdk, which support Python >= 3.10.

How Has This Been Tested?

$> pip install --editable .

Screenshots (if appropriate):

Types of changes

Checklist:

dany74q commented 2 years ago

/cc @gcbeltramini - would love your review (:

dany74q commented 2 years ago

Thanks @gcbeltramini !

Can anyone w/ write access merge the PR ? I'll prepare the v2 of ctap-keyring-device right after

dany74q commented 2 years ago

Ping about merging this PR @gcbeltramini, thanks !

zelch commented 2 years ago

@dany74q and @gcbeltramini , what are your thoughts on replacing this PR outright with #366?

The biggest downside is that I'm currently having to target an unreleased version of ctap-keyring-device, but on the other hand this brings us all the way up to Fido2 v1.0.0. (And the current Okta release, and, well, a few other things.)

dany74q commented 2 years ago

Hey @zelch ! Your PR looks great - I think it's a good direction to take regardless of the current changes.

I'm the author of ctap-keyring-device - I think it would be beneficial for this small PR to get merged first, for me to then release a proper version - and later to go ahead and have your PR approved, using an official version of the lib.

Waiting for someone w/ write access to merge this (:

zelch commented 2 years ago

Hey @dany74q,

Any thoughts on paths forward if we can't get any response on #367 ?

Absent activity from Nike, any release that you roll under the current name would break all users newly installing gimme-aws-creds, but without that, even if we were to fork the project we couldn't pull in ctap-keyring-device in that fork by a released version number.

Giving v2 of ctap-keyring-device a new name seems like a crazy answer as well.

I suppose we could fork gimme-aws-creds (would need a name for it to put in pypi, and, well, I wouldn't be willing to go down that road without at least one additional person, not with my current company, willing to be a maintainer. Replicating the maintenance problem somewhere else isn't a fix.), open an issue pointing people at the fork, ask for the release of ctap-keyring-device to happen, and... Let gimme-aws-creds break.

But that also doesn't seem like a great option.

And I'm somewhat short of ideas beyond that.

Thanks, Zeph / Liz.

dany74q commented 1 year ago

Hey @epierce / @gcbeltramini - Any chance we can create a new release of gimme that includes this change and pins ctap-keyring-device to 1.0.6 ?

Once that's done, I'll continue w/ the plan and will upgrade it to 2.0

gcbeltramini commented 1 year ago

Sorry, but I'm not a maintainer of this repo, so I can't help you with that.

zelch commented 1 year ago

Ping @epierce on the request for a release, so we can get the next steps done? :)