Incompatibility with bazel credential helper?

[jayconrod]

Hello! I'm one of the authors of engflow_auth, a Bazel credential helper that works with EngFlow.

One of our users asked if they can use engflow_auth together with reclient. Sadly, it looks like this doesn't work. It looks like this repo supports a credential helper in credshelper.go, but the protocol is not the same.

1. Would you consider implementing the Bazel credential helper protocol? It would be nice to avoid incompatibility here. I'm happy to send a PR if that would be useful.
2. If not, is the current credential helper protocol stable? Can it be documented?
3. Is there any way to use the credential helper without OAuth2? Our credential helper does use OAuth2 to obtain a token in the background, but Bazel isn't aware of that, and it isn't surfaced in the credential helper protocol. We'd like to set HTTP headers to opaque strings.

[mrahs]

Hi Jay.

We intended for our creds helper to be compatible with Bazel's. Perhaps that didn't survive some changes.

1. Yes, we'll gladly review PRs. However, please post your plan here before working on a PR so we can assess any potential breaking changes to how we use it internally.
2. Good point. We'll document it asap.
3. That should be covered by (2). If not, it can probably be addressed in (1)?

[jayconrod]

Apologies for the slow response, I took a bit of cool-off time after Bazelcon last week.

It does look like these implementations have diverged. Bazel invokes the credential helper with a single argument, get, then sends a message like this on stdin:

{"uri":"https://example.com"}

It expects the credential helper to print a message like this on stdout:

{
  "headers": {
    "Authorization": ["Bearer sometoken"]
  },
  "expires": "2006-01-02T15:04:05Z07:00"
}

Differences I'm seeing in credshelper.go:

• NewExternalCredentials may be called with arguments other than get.
• Nothing is passed to the credential helper on stdin.
• The token expiration field is named "expiry" rather than "expires" and is in time.UnixDate format rather than time.RFC3339.
• The "token" field is required and must be an OAuth2 access token. The Bazel credential helper protocol doesn't require OAuth2 and relies on the `"headers" field only.

What do you think is the best approach here? What helper implementations exist today?

I see two possible plans here:

1. Add a new, separate implementation to avoid any breaking changes. It could still be in the credshelper package but with a different constructor and different type.
2. Adapt the current implementation with minor breaking changes to packages importing credshelper. The only public packages are in reclient, but there may be non-public importers.
   • Add a new constructor instead of NewExternalCredentials and the -credentials_helper_args that accepts a URI and sets arguments to get.
   • Add a new flag to select the new constructor
   • If the URI was set, pass it to the credential helper process in a JSON message on stdin.
   • Let Credentials.TokenSource return nil if the helper process did not set the "token" field.
   • If the URI was set, let Credentials.TokenSource return nil.
   • Accept either "expiry" or "expires" fields in their preferred formats.
   • Adapt callers to use the returned headers only when TokenSource returns nil.

[banikharbanda]

Hi Jay,

Really appreciate the time you took recognizing the differences and where the implementations have diverged. I think both of your plans are feasible but I'd like some more time to think and discuss with our team if one would work better than the other. I will get back to you very soon to discuss if we have a preference either way.

Thank you!

[banikharbanda]

Hi Jay,
After discussing with my team, we are considering some changes ourselves to make this easier for anyone using a Bazel style credshelper. We still need another week or two to work it into the schedule, so I’d hold off on any PRs for now. If this changes on our end I'll let you know. Thanks!

[jayconrod]

Thanks for the update, I'll keep an eye on this space!

[banikharbanda]

Hi @jayconrod,
Here's a PR which adds a wrapper binary you could use for engflow_auth. I've added a small README file along with it. Let me know if you have any issues when you try it or if the instructions aren't very clear (I understand they are super minimal at the moment) and I can help out.
#604