Support specifying bearerToken for git http token authentication

[blurpy]

As an alternative to username and password with http basic authentication.

Makes it possible to fix fluxcd/source-controller#251 by configuring the secret referenced by a GitRepository like this:

apiVersion: v1
kind: Secret
metadata:
  name: token-auth
data:
  bearerToken: abc...
type: Opaque

My requirement is being able to use Flux with repository HTTP access tokens in Bitbucket Server. That way we can limit access to a single repository without making dedicated users for every repository.

With dedicated users, we can use basic auth and personal access tokens in combination with the username, but I've failed to get repository access tokens to work with basic auth. Setting a random username gives access denied, and setting a real username gives access denied and gets the user blocked.

According to the documentation they recommend using bearer auth for repository tokens, and with the changes in this PR I was able to get the Flux source controller to successfully sync with a repository token.

I've tested this against Bitbucket Server v8.4.1, and the main branch of https://github.com/fluxcd/source-controller with this in go.mod:

replace github.com/fluxcd/pkg/git => ../pkg/git
replace github.com/fluxcd/pkg/git/gogit => ../pkg/git/gogit

make all completes without errors in both this repository and source-controller.

[blurpy]

Force pushed with sign-off in commit message, which DCO was unhappy about.

[aryan9600]

thank you @blurpy for this PR!

[aryan9600]

@blurpy i'm sorry i think i spoke too soon regarding the casing. I just noticed that we use both camel case and snake case for the secret currently (caFile and known_hosts). @hiddeco @pjbgf this can cause confusion for users who manually create this secret, but i guess that ship has sailed since changing the format would break existing installations when flux is upgraded. what do you think should we follow from here on?

[pjbgf]

I just noticed that we use both camel case and snake case for the secret currently (caFile and known_hosts)

@aryan9600 I would leave as is until fluxcd/flux2#3366 (or a better alternative) is agreed on as both caFile and known_hosts may cease to exist. However, we need a documented pattern for other similar things.

[blurpy]

Any preference for now, to decide between bearerToken or bearer_token?

[hiddeco]

Think we would prefer bearerToken as known_hosts originates from the well-known file name on systems, for which there is none in the context of (bearer) tokens.

[blurpy]

Should I delete my last commit, to revert back to bearerToken?

[hiddeco]

That would be the most effective way of removing it, so fine with me :-)

[blurpy]

Commit has been removed :)

[hiddeco]

This looks good to me, but someone else might want to take a look as well.

Thank you @blurpy 🙇

[aryan9600]

lgtm, thanks! 🎖️

[blurpy]

Excellent :)

Are there any more steps I need to do in order for this to be included in the next release of source-controller?

[darkowlzz]

A few observations, some of which may require some discussion.

This feature is being added only to the go-git implementation. Should we just ignore the libgit2 implementation? It wouldn't be hard to just add a header in the corresponding transport code of libgit2. Or if we decide to not add such improvements/new features to libgit2, how about adding a check for such new things in the Client.validateUrl() method which already performs some extra validation that are not just URL but related to auth options, refer

pkg/git/gogit/client.go

Line 259 in 946d9ac

return errors.New("basic auth cannot be sent over HTTP")

. I think it'd be better to explicitly fail one implementation as we diverge further, else a user of the package may have to learn about it the hard way.

The Client.validateUrl() method referred above has a check to not allow basic auth over HTTP. The same should be true for token auth. We can add a similar validation for token auth and corresponding tests for it.

After reading about token auth support in some of the different git providers, it seems like not all of them support token based authentication easily. Some have very specific requirements like creating an app account for which such token can be generated. Not so automation friendly. I think it'd be better to have at least some kind of test that makes use of token auth. GitKit seems to only have support for basic auth at present, but maybe we can extend it to add support for token auth as well and use it in our git e2e tests.

[pjbgf]

On the libgit2 conundrum, I would vote for the libgit2 module to be removed on a separate PR once the dust of its deprecation has settled a bit further (it's been 3 weeks since its removal from the controllers).

I don't mind the the adhoc change to libgit2 for now, but it may send the wrong message that we still maintaining it. But if we do, it would be good to add some logging to it to highlight it is also being deprecated.

[aryan9600]

I am indifferent to adding this to the libgit2 implementation, but if I had to choose, I'd say no mainly because of the reasons @pjbgf mentions above.
I definitely agree with extending gitkit to support token authentication and adding tests here that make use of that. We can do that in a different PR or do it in this PR by putting it on hold until the required changes are made in gitkit.

I would vote for the libgit2 module to be removed on a separate PR

I completely agree, this would remove the direct use of CGO and make the project Makefile much simpler.

[blurpy]

Regarding the recent discussion, does that mean it could take a while to get this feature in a release? We would love to use this soon 😃

[darkowlzz]

@blurpy I think for this PR, it should be enough to add the last thing I mentioned about not allowing token auth over HTTP without an option to explicitly configure it to do so:

The Client.validateUrl() method referred above has a check to not allow basic auth over HTTP. The same should be true for token auth. We can add a similar validation for token auth and corresponding tests for it.

It would be nice to add e2e test for it separately as discussed above. Since this doesn't break or change any existing feature, based on your testing against bitbucket, I think we should be able to release it soon, before we have those tests. Others may have a different opinion about it.
Would you like to work on extending gitkit with token auth and adding e2e test for it? If not, we can open an issue for it and someone else can work on it.

[darkowlzz]

Also, we have a self-hosted bitbucket server that we run our e2e tests against. Maybe in addition to gitkit, we can add token auth e2e test against bitbucket too.
Adding @souleb to this discussion, who may have some more information about our bitbucket server, if we can use it to set up a test similar to the scenario described here https://confluence.atlassian.com/bitbucketserver084/http-access-tokens-1167707640.html#HTTPaccesstokens-UsingHTTPaccesstokens .

[blurpy]

@blurpy I think for this PR, it should be enough to add the last thing I mentioned about not allowing token auth over HTTP without an option to explicitly configure it to do so.

Ok, I will try to get that done tomorrow when I'm back at work.

It would be nice to add e2e test for it separately as discussed above. Since this doesn't break or change any existing feature, based on your testing against bitbucket, I think we should be able to release it soon, before we have those tests.

Sounds great!

Would you like to work on extending gitkit with token auth and adding e2e test for it? If not, we can open an issue for it and someone else can work on it.

That sounds like fun, but due to my time constraints I think it's better if you make a separate issue and open it for others to work on.

[souleb]

Also, we have a self-hosted bitbucket server that we run our e2e tests against. Maybe in addition to gitkit, we can add token auth e2e test against bitbucket too. Adding @souleb to this discussion, who may have some more information about our bitbucket server, if we can use it to set up a test similar to the scenario described here https://confluence.atlassian.com/bitbucketserver084/http-access-tokens-1167707640.html#HTTPaccesstokens-UsingHTTPaccesstokens .

We need to upgrade our test instance. I will do that this week.

[blurpy]

@darkowlzz I think it's ready now.

I added some new (slightly duplicating) tests in client_test.go as well, since I couldn't find a good way to verify happy path with https in clone_test.go where the existing tests were.

I have tested that both basic auth and bearer token still works in a new build of source-controller.
And when I tried to configure a gitrepository with http and a bearer token, I got authentication required, which I guess means the validation did it's job?

[darkowlzz]

And when I tried to configure a gitrepository with http and a bearer token, I got authentication required, which I guess means the validation did it's job?

@blurpy that sounds right to me.
UPDATE: See the next comment. It doesn't sound right to me after thinking more about it.

Thanks for testing it again. Based on your testing, would you like to create a source-controller PR to update the docs, adding a section about bearer token in the GitRepository Secret reference docs?
Once this PR gets merged, we can update the go module in source-controller. It'd be good to have the docs ready to be merged alongside. Please let us know if you have any time constraints. Someone else can take care of the docs.

[blurpy]

@blurpy regarding the authentication required error that you saw, looking at the source-controller code https://github.com/fluxcd/source-controller/blob/8785ebc9ae881f5aad17328dad5fccfe1e5b0cf6/controllers/gitrepository_controller.go#L793-L794 , I'm not sure how it failed for you. If I'm not misreading it, when it's a http URL, source-controller configures the insecure client option. Are you sure that's what's happening? authentication required seems to be something that the server would respond with. A client validation failure would result in the not allowed error that you added in this PR, but based on the source-controller code, that'd not happen. authentication required may mean that the bearer token authentication didn't work for some reason.

Good catch. This is the exact message I got with a bearer token over http:

failed to checkout and determine revision: unable to list remote for 'http://git....': authentication required

So I rolled back to stable source-controller.

Then I tried to clone using basic auth and http, and got this message:

failed to checkout and determine revision: unable to list remote for 'http://git...': authentication required

Next test was removing secret ref from gitrepository, so it connects without credentials (tested both http and https), and I got this message:

failed to checkout and determine revision: unable to list remote for 'https://git...': authentication required

All those 3 tests gave the same error message, perhaps suggesting that none of them sent any credentials?

I also tried to clone using the regular git client with a bearer token over http to see if that even works:

$ git clone -c http.extraHeader='Authorization: Bearer ...' http://git...
Cloning into 'repository'...
warning: redirecting to https://git...
remote: Enumerating objects: 56, done.
remote: Counting objects: 100% (56/56), done.
remote: Compressing objects: 100% (42/42), done.
remote: Total 56 (delta 8), reused 0 (delta 0), pack-reused 0
Receiving objects: 100% (56/56), 6.00 KiB | 6.00 MiB/s, done.
Resolving deltas: 100% (8/8), done.

Yes, that worked fine. But notice this line:

warning: redirecting to https://git...

So, my theory now is that the http client in go is following redirects, but loosing the authentication headers in the process. Could that be the case? I think I have seen that in other http clients.

[blurpy]

I did find some old comment regarding this here: https://stackoverflow.com/a/32753865

the Go client does not propagate the authorization header through the redirects.

Not sure if it's still valid, but might look like it.

[aryan9600]

@blurpy yep, its valid, i was able to reproduce this

[aryan9600]

after doing some research, it looks like the best practice is to NOT forward the authorization header when following a redirect. (ref: composer/composer#6716)

[blurpy]

after doing some research, it looks like the best practice is to NOT forward the authorization header when following a redirect. (ref: composer/composer#6716)

I think that makes sense too, even though it's a bit confusing.

[blurpy]

Thanks for testing it again. Based on your testing, would you like to create a source-controller PR to update the docs, adding a section about bearer token in the GitRepository Secret reference docs? Once this PR gets merged, we can update the go module in source-controller. It'd be good to have the docs ready to be merged alongside. Please let us know if you have any time constraints. Someone else can take care of the docs.

Looks like adding a section to the docs should be quick, so I can give it a go.

Update: see fluxcd/source-controller#1000

[blurpy]

Rebased from main.

[souleb]

Also, we have a self-hosted bitbucket server that we run our e2e tests against. Maybe in addition to gitkit, we can add token auth e2e test against bitbucket too. Adding @souleb to this discussion, who may have some more information about our bitbucket server, if we can use it to set up a test similar to the scenario described here https://confluence.atlassian.com/bitbucketserver084/http-access-tokens-1167707640.html#HTTPaccesstokens-UsingHTTPaccesstokens .

We need to upgrade our test instance. I will do that this week.

All good, we can now test this with our bitbucket infrastructure.

[michas2]

Can we include github app authentication, too?
With this we need to store a private key, which can be used to generate a http token dynamically. (And it's not a lot additional work.)

Something like this is great if you are working in an environment, where you need to use a github server instance, where access is only available using https and personal access tokens are valide only for a certain time.

[darkowlzz]

Can we include github app authentication, too?

@michas2 Thanks for sharing the github app authentication docs. This PR adds support for accepting and using a bearer token for git authentication. We have other ongoing work for testing this feature against different git providers separately.
For github, this change should enable a user to use github app based authentication if they write their own tooling to generate a jwt token and update the git Secret which will be used by source-controller. It'll be good to add this basic support in first and then separately work on adding platform/provider specific automation. I think we need to have more discussion and proposals about the implementation of jwt token generation which is out of scope for this change.

[darkowlzz]

LGTM!
Thanks a lot of adding this feature and all the improvements.

[darkowlzz]

Here's a source-controller RC image with this feature to try ghcr.io/fluxcd/source-controller:rc-f50e968f.
Update: See below for new image.

[blurpy]

Here's a source-controller RC image with this feature to try ghcr.io/fluxcd/source-controller:rc-f50e968f.

Thanks @darkowlzz! I tested it, but I can't get it to sync with bearer tokens.

I then tried to build the image myself with these changes:

	github.com/fluxcd/pkg/git v0.8.0
	github.com/fluxcd/pkg/git/gogit v0.5.0

And then it worked again. Could rc-f50e968f have been built with the old dependencies?

[darkowlzz]

@blurpy yeah, sorry I selected the wrong branch for it. This one should work ghcr.io/fluxcd/source-controller:rc-9ae77bbb@sha256:261645477a0359b3126ece60ef9486108d92fed558c139aa016c082ebe1c6fc4.
Thanks for checking again.

Also, regarding github app authentication, I gave it a try and found out that they don't support bearer token, it's in the same doc posted above. They use a fixed username for app auth for http access. Based on that, I just created a small go program to generate the token and write to a Secret, and run it as a Kubernetes CronJob - https://github.com/darkowlzz/github-app-secret .
@michas2 if you can give it a try, it'd be very helpful to understand what's needed. I've tested it with public github only but Github Enterprise should also work with a different API URL.
We can discuss more about it in a separate issue or on flux CNCF slack channel.

[blurpy]

@blurpy yeah, sorry I selected the wrong branch for it. This one should work ghcr.io/fluxcd/source-controller:rc-9ae77bbb@sha256:261645477a0359b3126ece60ef9486108d92fed558c139aa016c082ebe1c6fc4.
Thanks for checking again.

Ah, thank you, that one worked :)