This does require a new version of python-openidc-client (v0.6.0) for the printfd argument.

https://pagure.io/fedpkg/pull-request/225 contains the fedpkg config

@puiterwijk Could you give some background of this PR?

@cqi this is meant to allow HTTPS pushing to src.fedoraproject.org.
Basically, this implements the git credential helper API, by requesting an OpenID Connect token and submitting that as the password to authenticate to the remote server.
The remote server (https://src.fedoraproject.org/) will accept this token and use it to authenticate the push.
This way, users don't need to manually retrieve a token everytime they try to push, since this code will take care of it for them.

@cqi to test this, you can grab this and the corresponding fedpkg config change, run "fedpkg-stage clone -a rpms/$somerpm", go into the repo, change it, commit it, and git push.
Even though you cloned with -a (https clone), it should be possible to push after it asks you to authorize it.

Can you upload the 0.6.0 version to PyPI?

I tried to use it with stage server. On first run, I get redirected to a web page asking for username and password. On second run this information seems to be cached in ~/.openidc/ (why not under ~/.cache/openid/?).

However the push itself does not work for me, but I have no idea if that's related to this change (although the same commit can be pushed over ssh without issues).

Counting objects: 3, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (3/3), done.
Writing objects: 100% (3/3), 354 bytes | 354.00 KiB/s, done.
Total 3 (delta 1), reused 0 (delta 0)
send-pack: protocol error: bad band #35
fatal: The remote end hung up unexpectedly
fatal: The remote end hung up unexpectedly
error: failed to push some refs to 'https://src.stg.fedoraproject.org/rpms/compose-utils.git'

What's the advantage of using this approach over existing ssh?

Can you upload the 0.6.0 version to PyPI?

Will do so.

I tried to use it with stage server. On first run, I get redirected to a web page asking for username and password. On second run this information seems to be cached in ~/.openidc/ (why not under ~/.cache/openid/?).

No particular reason, just what I've done so far.

However the push itself does not work for me, but I have no idea if that's related to this change (although the same commit can be pushed over ssh without issues).

This is probably related to the server-side handling of the push, and I'd like to debug this, but not related to the rpkg/fedpkg changes, since by the time it gets here, it's already authenticated you.

Counting objects: 3, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (3/3), done.
Writing objects: 100% (3/3), 354 bytes | 354.00 KiB/s, done.
Total 3 (delta 1), reused 0 (delta 0)
send-pack: protocol error: bad band #35
fatal: The remote end hung up unexpectedly
fatal: The remote end hung up unexpectedly
error: failed to push some refs to 'https://src.stg.fedoraproject.org/rpms/compose-utils.git'

What's the advantage of using this approach over existing ssh?

That this does not require ssh access to more people (non-packagers drive-by contributions), and requires less people with ssh access (in the long run, I want to move everyone over to https, but first opt-in).

The push error is fixed now. That was a misconfiguration.

Need this import to make this work in EL6

try:
    import unittest2 as unittest
except ImportError:
    import unittest

Fixed, thanks.

From where to feed the input?

Why does this method do nothing? Is it just a placeholder?

This is the IO protocol as defined by the git-credential helper API.

Because git-credential will call it after the credentials were accepted, but when we return, we have already stored the token on disk. But we can't error out, as that would lead to ugly errors.

https://git-scm.com/docs/git-credential#IOFMT

I see. Thank you.

Do note that this option is actually suppressed in the help output, so it's not expected that users will call this by hand unless they know what they're doing. That's why I coded it assuming the git-credential IO format, without too much handling of invalid input.

"garbage in == garbage out"

I failed to push rpkg to src.stg.fp.o

fedpkg-stage --config ~/code/fedpkg/conf/etc/rpkg/fedpkg-stage.conf -d -v push
Creating repo object from /home/cqi/packages/fedora/stg/rpkg
Running: git push
OpenID Connect param oidc_id_provider not configured
No token received. OpenIDC configured?
fatal: credential helper '/home/cqi/virtualenvs/rpkg-py3-env/bin/fedpkg-stage gitcred' told us to quit
Could not execute push: Command '['git', 'push']' returned non-zero exit status 128.
Traceback (most recent call last):
...

Same error happens when run in Python 2.

It does work for me with fedpkg push as well as git push. Your error looks like it's not using the correct config.

Right, so what happened is that when you did the "git clone -a", it stored fedpkg-stage gitcred as the git config credential.helper value probably, since it uses the --config value from clone time.
It will not read the --config flag from your current "push" command, but it will use the config that it installed when you cloned.
So you should look at the .git/config file, and update the credential.helper value to include the --config ....
Alternatively, remove this clone, and do the fedpkg-stgae --config ~/code/.... clone -a ...

It works for me now. Several questions:

• Is there some packaging policy that is being changed?
• When this change is expected to be released and available for packagers?
• When will prod src.fedoraproject.org be ready for this new feature?
• Will there someone else to annouce the news of https push to src.fedoraproject.org and when?
• What will be the difference for Fedora between pushing via https (from an anonymous clone) and ssh? Which one should be the default that is recommended for packagers?
• What would be cases of https pushing?

It works for me now. Several questions:

Is there some packaging policy that is being changed?

No. The main repos for packages are still only for packages, this is primarily meant for drive-by contributors so they can push to forks on src.fp.o.

When this change is expected to be released and available for packagers?

It is expected to be available as soon as they upgrade rpkg/fedpkg.

When will prod src.fedoraproject.org be ready for this new feature?

It is as of last week.

Will there someone else to annouce the news of https push to src.fedoraproject.org and when?

We will initially do a low-key introduction on an opt-in basis. The plan is to in time (weeks) change the default to https, and then even later (months) disable ssh pushing.

What will be the difference for Fedora between pushing via https (from an anonymous clone) and ssh? Which one should be the default that is recommended for packagers?

For now, the plan is to not change anything for packages yet. As said, in due time, the plan is to move all new clones over, later on migrate all clones, and at some point disable ssh.

What would be cases of https pushing?

At first: opt-in, primarily intended for drive-by contributors.
In due time, for everyone.

The advantage for the Fedora Infrastructure is that https pushing makes for a single authentication strategy for everything (OpenID Connect/OAuth2), and requires less people to have ssh access into our systems.

Should this go to fedpkg instead? As far as I know, no other package tools built on top of rpkg do not allow HTTPS pushing. It seems this is only for Fedora.

Meanwhile, I'm thinking if it would be good to create a separate binary e.g. fedpkg-gitcred. Then, no gitcred command will appear in the commands list, and it will be transparent to users.

                                  Should this go to fedpkg instead? As far as I know, no other package tools built on top of rpkg do not allow HTTPS pushing. It seems this is only for Fedora.

I figured it might be useful if other people want it down the road, in addition to the actual clone code all being in rpkg.

Meanwhile, I'm thinking if it would be good to create a separate binary e.g. fedpkg-gitcred. Then, no gitcred command will appear in the commands list, and it will be transparent to users.

Right now we're doing a better way of hiding it I think: it should already be hidden from the help functionality (help=argparse.SUPPRESS).

Except that suppress doesn't work for some reason?

Ah, that would be https://bugs.python.org/issue22848

I remove the help=argparse.SUPPRESS for now, which removes it from the --help, but leaves it in the usage.
I personally think that having it "hidden" among the already long list of fedpkg subcommands in the usage command is still a lot less user-visible than having a fedpkg-gitcred.

Should this go to fedpkg instead?

Note that there is another pagure instance whose user are also interacting with via a sub-client of rpkg who could maybe benefit from this work as well.

Looks good to me.

@puiterwijk Could you please build openidc-client 0.6.0 packages for el6, epel7 and all active Fedora releases? fedpkg has to work in all of those releases.

Sorry, just additional comment. git credential helper is injected into git config and git-push command line directly, which might affect any other tools built on top of rpkg except fedpkg, e.g. rhpkg, that still only allows push via ssh and no openidc is required.

I think we need a flag in config file so that code can know if https pushing authentication should be enabled or not. Probably we can add a new option https_pushing = True in [fedpkg]. Read oidc_* and inject git credential helper only when there is such an optional and set to True.

I'm not sure what the option would achieve: in current status if the site does not support pushing via http, you get an error if you try. If the config is updated as in this PR, you would get a different error. Either way it does not work. That doesn't seem to me to be worth the extra complexity in code.

If the repo is cloned over ssh, the gitcred helper is not used as far as I can tell.

If the clone is non-https, this line will make us just bail out.
This is basically the signal to git "this credential helper does not have anything for this push, continue as normal".

Note the inline comment as to why over non-https is not a problem.

Also, because I do the clone-time injection of gitcred before the other clone_config, if someone was using rpkg.conf to inject another credential.helper, that would still take priority over this, so even if a setup uses another credential.helper, this won't impact them.

Thus the only case where this will change anything for users is if a setup is currently using https based pushing and expecting users to manually enter a username/password every single time, because gitcred returns a "quit" if we can't get a token, meaning that if the credential helper can't get its hand on a token, it will tell git to not prompt the user for a password.

With this latest patch, even the case where the user is expected to manually enter a username/password on https push will work.
Because if OpenID Connect is unconfigured, it will not return the quit=1 output, meaning that git will continue and ask the user for their password.
But if OpenID Connect is configured and we just don't get a valid token, we do still quit=1, because the chance of the user successfully entering an OIDC token by hand is reasonably low.

Also, if the global OpenID Connect client is not configured, we won't even try to import openidc_client anymore.

This means that in setups where OpenID Connect is not used, users won't even know we're there: on ssh push, we silently tell git to ignore us, on https push, we tell git we don't have anything to say for ourselves.

@puiterwijk Could you please build openidc-client 0.6.0 packages for el6, epel7 and all active Fedora releases? fedpkg has to work in all of those releases.

Builds made.
Please add them to the fedpkg update so they go out at the same time: python-openidc-client-0.6.0-1.20180605gitcd8d91c.{fc27,fc28,fc29,el6,el7}

My concern is, for example rhpkg, there is no git credential-helper in rhpkg.conf or rhpkg-stage.conf, with this patch, push over https will fail with this output

rhpkg-stage push
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.
Could not execute push: Command '['git', '-c', 'credential.helper=/home/cqi/virtualenvs/rhpkg-py2-env/bin/rhpkg-stage gitcred', '-c', 'credential.useHttpPath=true', 'push']' returned non-zero exit status 128

That would confuse user. rpkgClient.push can be overridden in rhpkg, but that means every downstream tools like rhpkg, which does not want that auto-injected git credential-helper options, has to override rpkgClient.push.

@puiterwijk Thank you. There is another issue just found. doc/rpkg_man_page.py is broken.

Traceback (most recent call last):
  File "doc/rpkg_man_page.py", line 162, in <module>
    generate(client.parser, client.subparsers)
  File "doc/rpkg_man_page.py", line 131, in generate
    if help_texts[command]:
KeyError: 'gitcred'

Probably gitcred could be skipped explicitly since it does not aim to be used by user.

Why set gitcred config in command line again?

The broken doc/rpkg_man_page.py should be another problem. That is, fedpkg, rhpkg, rfpkg and probably others have to fix the same issue in their rpkg_man_page.py.

Can we reconsider the implementation to avoid coupling gitcred into rpkg so deeply? As rpkg is a general framework for building package tools, not only for Fedora infra. So, it would not be a good idea to inject gitcred into git config by default.

Regarding my previous comment to move gitcred into a separate command, the point is not only for hiding gitcred in commands list, but also for decoupling it from rpkg in order to keep rpkg be general to enable features by configuration for different environment, e.g. rhpkg, rfpkg.

In my mind, it could be to create a new command fed-gitcred with its own OIDC configuration. For staging, it could be fed-gitcred-stage just like fedpkg-stage. In fedpkg configuration, add git config

clone_config =
    ...
    credential.helper fed-gitcred
    credential.useHttpPath true

rpkg is already able to set the git config after clone, so everything will work properly.

This new command could be provided by rpkg or its own package. Either is fine I think.

What do you think?

I think that this points out that the *_man_page.py tools need serious refactoring.
Since right now, they're just forks of eachother, with the main thing changed their identity (rpkg vs fedpkg), and some config handling.
Rpkg should just have a base class for this, and the subtools should just override the identity and specific things, but not fork the entire file.

The problems with making another command are that:

1. It's going to be even more obvious for users, because they'll suddenly see new system commands
2. It's going to require either significant refactoring to __main__.py or a lot of duplication of config handling code
3. It's going to be a lot harder to find_self and handle the different configuration options: right now, I can just set credential.helper to sys.argv[0] <possibly --config ...> to make sure that if someone uses a custom config, we pick that up. If we reuse the existing clone_config, I'll need to make that a lot more complicated with injecting the config file we started with.

Because the configuration one is done for the case where someone cloned it with a new rpkg, this version is for the case you encountered during your testing: you cloned it with a rpkg without, and then you run rpkg push, to still get the credential helper.

Also, just looked, and you already have other subcommands that don't show up in the man page.
That's the whole idea behind add_help=False seemingly.
With my latest patch, the man script works without changes.

Looks good to me. Merge to move it forward. :tada: