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

How to upload new project with two factor auth enabled? #14070

Closed
grantjenks opened this issue Jul 6, 2023 · 15 comments · Fixed by #14082
Closed

How to upload new project with two factor auth enabled? #14070

grantjenks opened this issue Jul 6, 2023 · 15 comments · Fixed by #14082
Assignees
Labels

Comments

@grantjenks
Copy link

Describe the bug

I want to upload a new project and my account has two factor auth enabled. When I try to upload locally with twine, I get an error saying that I cannot use my password because I have two factor auth enabled. Then I try uploading from GitHub using the new OIDC functionality but I get another error saying that non-user accounts cannot create new projects.

Expected behavior

There is a way to upload to PyPI with a two factor auth account.

To Reproduce

GitHub action log: https://github.com/grantjenks/python-typesense-server-wrapper/actions/runs/5480180242/jobs/9983014023

My Platform

Python 3.11 from GitHub Codespaces and GitHub Actions on ubuntu-latest.

Additional context

@grantjenks grantjenks added bug 🐛 requires triaging maintainers need to do initial inspection of issue labels Jul 6, 2023
@di
Copy link
Member

di commented Jul 6, 2023

Hi @grantjenks, sorry you're having trouble and thanks for the report. The two ways to create projects when 2FA is enabled are:

This might be a bug on our end: I see that you correctly added a "pending publisher" for https://github.com/grantjenks/python-typesense-server-wrapper.

I also see that during the workflow run, https://pypi.org/p/python-typesense-server-wrapper/ actually did get created and the pending publisher was correctly converted it to an actual publisher (you can see it here: https://pypi.org/manage/project/python-typesense-server-wrapper/settings/publishing/). However, no releases were published.

In this case, the token exchange prior to the upload successfully created the project, but by the time the request to upload the releases was made, the following request thought the project didn't exist for some reason, and so it failed:

if project is None:
# Another sanity check: we should be preventing non-user identities
# from creating projects in the first place with scoped tokens,
# but double-check anyways.
if not request.user:
raise _exc_with_message(
HTTPBadRequest,
(
"Non-user identities cannot create new projects. "
"You must first create a project as a user, and then "
"configure the project to use trusted publishers."
),
)

I'm not sure what's happening here: it's possible the two requests happened too quickly and the creation of the project wasn't committed to our database before the upload request happened. At any rate, the error message is wrong and potentially confusing, so we should look into improving that as well.

The good news is you should be able to re-run that same workflow and it should work successfully this time.

(cc @webknjaz @woodruffw for any thoughts they might have)

@di di removed the requires triaging maintainers need to do initial inspection of issue label Jul 6, 2023
@woodruffw
Copy link
Member

Yeah, this is odd -- to have gotten that far request.identity must be non-None which in turn means it must be a valid publisher identity, so the corresponding project should have been created and made available at that point.

Agreed about the error message being confusing though; I'll give some thought to what a better one would be.

@webknjaz
Copy link
Member

webknjaz commented Jul 6, 2023

That assessment seems fair. Too bad we can't reliably know if it's safe to call twine, otherwise sticking a retry would've helped on the action side...

@di
Copy link
Member

di commented Jul 7, 2023

Too bad we can't reliably know if it's safe to call twine

Can you clarify what you mean by this?

@grantjenks
Copy link
Author

Thanks for the quick responses, all! I tried re-running the workflow but failed the same way. Log at https://github.com/grantjenks/python-typesense-server-wrapper/actions/runs/5480180242/jobs/9985978215

Anything else I can do to get the GHA working?

@di
Copy link
Member

di commented Jul 7, 2023

Aha, @grantjenks, should the project name be typesense_server_wrapper or python-typesense-server-wrapper? Looks like you've configured the latter on the PyPI publisher, but the former for the project itself.

@webknjaz
Copy link
Member

webknjaz commented Jul 7, 2023

Too bad we can't reliably know if it's safe to call twine

Can you clarify what you mean by this?

When we run twine upload from the action, there's no way to act on its error message. This is because there's no API and we can only see if the entire operation succeeded or failed. With a failure, it could've succeeded uploading a few artifacts but crashed on the rest, for example.
If twine were provide these details, it'd make it possible for the action to attempt another upload safely.
Also, we can't even display the information and recommendations in better places since twine only outputs to the console and that's about it..
I wish twine had better integration capabilities, it would open up some possibilities of being more helpful to the end-users with better explanations and hints in GHA...

@webknjaz
Copy link
Member

webknjaz commented Jul 7, 2023

Looks like you've configured the latter on the PyPI publisher, but the former for the project itself.

This must be it. If these don't match, PyPI can't find a publisher trust connection for that... I wonder if PyPI should silently normalize the project name both on creation and upload.. cc @woodruffw

Also, I'd argue that the error message could be improved — just by reading the log, I couldn't understand why it's talking about some service identity. And that's me with likely better understanding of the context than an average user. Can we provide simpler explanation there?
Of course, we could attempt giving a hint on the action side but adding a new hint for every corner case feels wasteful..

@di
Copy link
Member

di commented Jul 7, 2023

I wonder if PyPI should silently normalize the project name both on creation and upload.. cc @woodruffw

@webknjaz We already normalize the project name, but even with normalization, the project names here aren't the same.

Also, I'd argue that the error message could be improved

Yes, looks like this is an edge case we hadn't considered. I think at this point we have enough information to provide an error message like:

You attempted to upload to {some project name}, but the publisher you used is configured for {some other project name}.

@woodruffw
Copy link
Member

Yep, agreed with @di -- I can make a PR to improve this error message in a moment.

@webknjaz
Copy link
Member

webknjaz commented Jul 7, 2023

Ah, I missed that they differ more..

@woodruffw how about improving the form on PyPI with extra explanation that the name shouldn't come from the repo slug but from something like pyproject.toml?

@woodruffw
Copy link
Member

@woodruffw how about improving the form on PyPI with extra explanation that the name shouldn't come from the repo slug but from something like pyproject.toml?

Yeah, I'll think about the language there -- we might not want to reference specific sources of metadata since PyPI is agnostic to whether it's coming from pyproject.toml or some other source.

Either way, I think we should add this as a troubleshooting item to the trusted publisher docs as well. I'll include that in the error message work.

@woodruffw
Copy link
Member

#14082 improves the error here and adds a corresponding troubleshooting section.

@woodruffw woodruffw self-assigned this Jul 7, 2023
@grantjenks
Copy link
Author

Thanks all. I fixed the name on PyPI and the job on GitHub worked. Sorry for the mistake.

@woodruffw
Copy link
Member

Thanks all. I fixed the name on PyPI and the job on GitHub worked. Sorry for the mistake.

Nothing to be sorry for -- this is a reasonable source of confusion, and you helped us find and fix a misleading error message!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants