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

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

Merged

Conversation

dany74q
Copy link
Contributor

@dany74q dany74q commented Sep 4, 2022

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:
dany74q/ctap-keyring-device#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

#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

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

This commit pins ctap-keyring-device to 1.0.6, before I'm release a new
v2.0.0 that requires fido2 v1.0.0, which has numerous breaking changes
that would affect gimme-aws-creds.

Once pinned, I'll create a new tag from:
dany74q/ctap-keyring-device#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.
@dany74q
Copy link
Contributor Author

dany74q commented Sep 4, 2022

/cc @gcbeltramini - would love your review (:

Copy link
Contributor

@gcbeltramini gcbeltramini left a comment

Choose a reason for hiding this comment

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

I have been using gimme-aws-creds with ctap-keyring-device==1.0.6 without problems. I'm also OK with the next steps proposed in the PR description.

@dany74q
Copy link
Contributor Author

dany74q commented Sep 5, 2022

Thanks @gcbeltramini !

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

@dany74q
Copy link
Contributor Author

dany74q commented Sep 20, 2022

Ping about merging this PR @gcbeltramini, thanks !

@zelch
Copy link

zelch commented Oct 7, 2022

@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
Copy link
Contributor Author

dany74q commented Oct 7, 2022

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
Copy link

zelch commented Oct 13, 2022

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.

@epierce epierce merged commit e29d52b into Nike-Inc:master Oct 16, 2022
@zelch zelch mentioned this pull request Oct 26, 2022
9 tasks
@dany74q dany74q deleted the pin-ctap-keyring-device-before-fido2-upgrade branch November 8, 2022 04:34
@dany74q
Copy link
Contributor Author

dany74q commented Nov 8, 2022

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
Copy link
Contributor

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

@zelch
Copy link

zelch commented Nov 28, 2022

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

@epierce epierce mentioned this pull request Jan 5, 2023
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.

4 participants