Periodically refresh git OAuth2-tokens for GitLab and Bitbucket (expiration 2h)

[nils-mosbach]

Summary

Configuring OAuth2 (e.g. GitLab) for accessing private projects is straight forward as described by:
https://www.eclipse.org/che/docs/che-7/administration-guide/configuring-authorization/#configuring-gitlab-oauth2-with-devworkspace-engine_che

The token is stored in a Kubernetes secret and is mounted to the dev-container. That OAuth2 token is then passed to the git credential store. Which works as expected.

Problem is that tokens provided by GitLab expire after 2h. So after this time authentication fails for all Git operations (push, pull, ...). The only way I found is to delete the secrets in the users namespace that are created by che-server. Next time a user starts a workspace, new tokens/ secrets are created.

How should that be handled?

Relevant information

Che: Next
Devworkspace-Operator: 0.13.0
Kubernetes
GitLab OnPremise

[mshaposhnik]

Hi Nils, right now the tokens are validated only during workspace startup, and didn't refreshed during runtime.
Can you please check is simple WS restart helps (without manual secrets deletion). It should work.
BTW it's kinda strange configuraion, as OAuth tokens generally supposed to be long-lived.

[nils-mosbach]

Generally you're right. GitLabs Access Tokens are quite "short-living". Most applications we have integrated with, use something from a couple of days to months, but still require tokens to get refreshed as well.

GitLab allows creating tokens that don't expire, but that's deprecated.

I'll check if restarting the workspace solves the issue...

[nils-mosbach]

@mshaposhnik Simply restarting a workspace does not fix it. It looks like the authentication procedure is skipped in this case.

Starting a new workspace works. This seems to update the token.

[mshaposhnik]

Right, i've looked again and seems validation is now possible only on factory acceptance stage, when all the necessary conditions are met (e.g. we know the remote URL, user is able to interact with OAuth and workspace is running). Even on WS startup we did not have them, so all we can is to cleanup outdated token which is not helps much with UX. We will continue to think about possible solution.

[nils-mosbach]

@mshaposhnik Since GitLab has now removed the Expire access tokens setting, workaround is gone.

Git credential secret-file is also mounted read only, which prevents users from making any change to the file. Am I missing something? From a user perspective this actually makes the GitLab integration unusable after 2h since they won't be able to push and pull their changes.

Bottom lined: If a user opens a new workspace from a GitLab repository that's what he could do in order to get changes back to the origin after 2h:

• define a different origin that's not in the list of /.git-credentials/credentials (since that file is read only).
• Stop the current workspace, start a new workspace, stop the new workspace, start the old workspace, commit changes.

I'm not sure if the setting in /config/initializers/doorkeeper.rb will allow to set expiration to a much higher value (e.g. 1 Month). We'll give it a try (https://gitlab.com/gitlab-org/gitlab/-/blob/master/config/initializers/doorkeeper.rb#L38) At least gitlab.com does not have this option.

Same should be the case on Bitbucket. At least the documentation mentions "Our access tokens expire in two hours."(https://support.atlassian.com/bitbucket-cloud/docs/use-oauth-on-bitbucket-cloud/#OAuthonBitbucketCloud-Refreshtokens).

---

From what I can see, one solution could be to store the refresh token and expiration info in git-credentials-secret-* and periodically refresh the tokens for all secrets labelled by controller.devfile.io/git-credential and have a refresh token. This could be done by the devworkspace operator or a service deployed in the users namespace.

Despite the expiration issue, OAuth integration acutally performs quite well in the latest release. :)

References:

• https://gitlab.com/gitlab-org/gitlab/-/issues/21745
• https://docs.gitlab.com/ee/api/oauth2.html

[nils-mosbach]

changing authorization_code_expires_in in /config/initializers/doorkeeper.rb doesn't ssem to solve the issue.

[l0rd]

Based on your feedback @nils-mosbach I think this is becoming critical. I am setting the priority to P1.

The DevWorkspace controller cannot periodically refresh the tokens (its responsibility is to reconcile DevWorkspaces) but there are a couple of things that come to my mind.

First, if OAuth is configured and there is a secret with a previously generated token, Che should always check it at startup. If the token has expired, Che should generate a new one. That has been fixed for GitHub but not for GitLab and Bitbucket. @vinokurig can you confirm?

Second, those short lived tokens look problematic for development. When using a workspace for more than 2 hours the token should be refreshed at runtime but with current mechanism (secret mount) that requires a workspace restart. An hybrid mechanism (OAuth token for workspace startup and SSH key mounted in the workspace) looks like a better solution. @nils-mosbach what do you think?

[l0rd]

I would say that this issue is similar to #21421 but for GitLab and Bitbucket.

[vinokurig]

@l0rd Gitlab and Bitbucket already work like GitHub in the scope of #21421. The reason of this issue is to catch an event when an oauth token has expired and regenerate it.
We have to decide when we are going to catch the event: on wokrspace start or / and during workspace runtime.
Do we need to have the refresh process asynchronus? If so we would need a new way of storing the token in a container, currently we mount at the workspace provision step.

[ibuziuk]

We have to decide when we are going to catch the event: on wokrspace start or / and during workspace runtime.

I do not think we need to refresh the token during runtime, refreshing it during workspace startup should be enough.

@vinokurig I actually thought that the token is refreshed during workspace startup already if expired, no?

[vinokurig]

@ibuziuk

I actually thought that the token is refreshed during workspace startup already if expired, no?

Unfortunatelly no, we check it only when creating a new factory.

[ibuziuk]

@vinokurig ok, so I believe we need to check it on workspace startup and if there is exception - renew the token

[nils-mosbach]

We have to decide when we are going to catch the event: on wokrspace start or / and during workspace runtime.

From a user perspective I consider it mandatory that tickets are refreshed during workspace runtime. I guess that most developers tend to spend more than two hours on a project/ task/ workspace. I don't think users get the reason when and why they must restart in order to push changes. That also leads to bad habbits, like "do it quick, otherwise you have to restart". The idea of using oauth is that users don't have to deal with authentication.

An hybrid mechanism (OAuth token for workspace startup and SSH key mounted in the workspace) looks like a better solution.

Problem here would be that if I have a devfile that points to the origin https://git.company.org/project/abc.git you still won't be able to push changes since ssh keys usally point to another uri git@git.company.org:project/abc.git. Users then would have to add the ssh origin uri before pushing changes after starting a workspace.

---

I'm not a 100% sure which solution would be the best. @l0rd You're right, devworkspace Operator would definately the wrong place. I'm summarizing a couple of ideas.

Remove read only flag

I would at least remove the read-only flag for the secret mount. That allows local git commands to deal with the situation. So in most cases Git clears the invalid credentials file and prompts for a username and password. That information is lost when a workspace is restarted, but restarting should also refresh Git tokens. (like @l0rd requested)

    - mountPath: /.git-credentials/credentials
      name: devworkspace-merged-git-credentials
      readOnly: true  # remove this flag
      subPath: credentials

Store refresh token

I think we should include the refresh token in the git-credentials-secret- (not devworkspace-merged-git-credentials) so we actually have the information somewhere. It might also be useful to store information like refresh url and expiration time.

Allow for kubernetes updates on secret contents

Correct me if I'm wrong, but from what I know, secret file mounts are updated if you don't use subPath. That might allow updating the token without restarting the workspace. So it doesn't depend which process updates the secret, credentials should get updated in the running workspaces. since .git-credentials-folder is only used by credentials-file, the follwing config might be an option.

    - mountPath: /.git-credentials
      name: devworkspace-merged-git-credentials

Update access tokens based on refresh tokens

At the end we might just call the refresh url of the OAuth provider and pass the updated token to the secret. I'm not sure whats the best place to do this right now, but as a quick solution I could think of a VSCode Plugin that handles refreshing tokens.

Another idea would be if che server is responsible for creating these secrets we could create an endpoint that's triggered after a couple of minutes like "refresh-my-tokens". That could also be done by a vscode plugin, or by the same mechanism that triggers the keep-alive-signal for active workspaces (which otherwise suspend after 15'). But I'm not a 100% sure how that's implemented at the moment. Dashboard could handle this api endpoint as well, if che-server should be depracted. Or we integrate this in the workspace keep-alive logic, if that's an option.

@l0rd what do you think?

[l0rd]

I think we can split this in 2 separate issues:

• Check OAuth tokens validity when workspace restarts (I have filed this issue)
• Periodically refresh a token from a running workspace (this issue)

with the first one easier to address and that I would prioritize as that's look like a quick win.

[l0rd]

@nils-mosbach thanks for your analysis and yes, you are right and I was wrong about secrets that get updated automatically (without the need to restart a workspace).

I am still divided between periodically refreshing the PAT or using SSH key instead. An HTTPS to GIT+SSH link converter looks simpler to write and maintain than a token refresher job (not to mention the security impications). So my proposal is to work on alternate flow that uses an kubernetes.io/ssh-auth secret if the developer has pre-created the secret in his namespace (and the secret has some specific labels that match the scm). The SSH flow would have higher priority compared to the PAT flow.

[nils-mosbach]

From a users perspective PAT is a really nice option. For internal users, I find SSH a good option. If dealing with users outside of the organisation, starting a new workspace, create ssh keys, go to GitLab and paste them, and then clone the repo which you actually want to use will lead to a lot of support.

I would at least remove the read only flag of the git-settings-mount, so Git can deal with the situation.

If updating tokens in a vscode plugin is an option, I'll be happy to create the plugin. Since there's already a kube client initialized for reading exposed ports, periodically running a job that fetches a new token and updates the current secret wouldn't be a big deal. The only thing is, that it would be nice if someone could make the change that will add the refresh token and url in the secret. We could also add an annotion on the secret for this case e.g. che.eclipse.org/git-refresh which could be used by the plugin to filter for credentials that needs to be refreshed. @l0rd Would that be an option? If this works well, maybe someone could port the plugin to jetbrains and theia.

From a security point of view I don't think this is way more insecure than using GitHub Tokens that don't expire.

[l0rd]

@nils-mosbach If you want to work on a PAT refresher VS Code extension I am happy with that. I prefer my team not to maintain that extra code but you are more than welcome to work on that. Initially it can be an external VS Code extension available on open-vsx and in the future we may consider making it a built-in. We can create a git repo in the che-incubator org for that.

If I understand correctly, a prerequisite for this extension is that the PAT should be editable. Can the extension edit the PAT by patching the Kubernetes secret that contains the it? So that the fresh PAT is always available through the Kubernetes Secret API (and we don't require to mount the secret in RW mode).

[nils-mosbach]

I've tried patching the secret using kubectl in a developer container and that works for both (devworkspace-merged-git-credentials and git-credentials-secret-hifkp).

I think we need to decide if using OAuth/PAT/https should be a supported option next to SSH. I'm totally fine with a decision if Che only supports PAT for fetching devfiles from repositories and Git-Links in projects must be SSH. I would still prefer using OAuth/Pat since that makes onboarding way easier in case of a greater user base. From testings we've done internally teams won't use PAT, because it "breaks every two hours and you're stuck".

I'm happy to do most of the heavy lifting to get PAT with expiring tokens working. So while digging through the code base I think we need to change the following:

---

[Che-Server] Write refresh tokens to secrets git-credentials-*

We need to get the refreshToken from googles' oauth-client to KubernetesGitCredentialManager.

• From what I can see, the base secret for one provider is written in KubernetesGitCredentialManager.java#L150. I Would add expirationTime and refreshToken to the secret and add an annotation that allows to query for secrets that needs to be refreshed if a refresh token is set.
• So we need to extend PersonalAccessToken.java as well that it can contain expirationTime and refreshToken.
• PersonalAccessTokens are created in GitlabOAuthTokenFetcher.java#L109 (for GitLab). We need to pass expirationTime and refreshToken as well.
• OAuthToken DTO needs expirationTime and refreshToken as well. Since OAuthAuthenticator uses google-oauth that should allow retrieving these information using getRefreshToken() and getExpirationTimeMilliseconds() OAuthAuthenticator could add them to OAuthToken.

The new secret could look like this:

apiVersion: v1
data:
  credentials: YWRtaW4=
  refreshToken: YWRtaW4=
  expiration: xyar
kind: Secret
metadata:
  name: git-credentials-secret-hifkp
  annotations:
    che.eclipse.org/automount-workspace-secret: "true"
    che.eclipse.org/che-userid: CgIzOBIGZ2l0bGFi
    che.eclipse.org/git-credential: "true"
    che.eclipse.org/mount-as: file
    che.eclipse.org/mount-path: /.git-credentials
    che.eclipse.org/scm-url: https://git.company.dev
    che.eclipse.org/scm-username: username
    che.eclipse.org/refreshToken: true
    controller.devfile.io/mount-path: /.git-credentials  
type: Opaque

[Devworkspace-Operator] Mount devworkspace-merged-git-credentials-secret updatable

devworkspace-merged-git-credentials should be mounted without using subpath. See "Note: A container using a Secret as a subPath volume mount does not receive automated Secret updates." Kubernetes/Secret/Documtentation. I think secret is mounted in git-credentials.go#L134

[Devworkspace-Operator] Remove read-only flag for devworkspace-merged-git-credentials-mount

This isn't really necessary, but will allow Git to handle the situation. Since /.git-credentials/credentials is mounted read only git will always throw unauthorized if authentication fails. If that file is writable Git will at least promt for a username and password. Which allows developers to at least push their changes. Is there a reason why we not want to mount merged-credentials RW?

I think this can be done in git-credentials.go#L133

Question: When is devworkspace-merged-git-credentials updated?

From what I can see, the merged credential secrets that mounted to the container is created/updated in git-credentials.go#L74
Since I'm not that familar with Go/OperatorSDK, question is if one of the secrets gets updated, will the devworkspace operator update devworkspace-merged-git-credentials or is this only executed on workspace provisioning?

If it's only created during workspace startup, token refreshing logic that updates git-credentials-* should replicate that logic and update devworkspace-merged-git-credentials as well.

Question: How do we update the token?

My previous idea was to create a a VSCode extension that does refreshing tokens, but doesn't work with IntelliJ. If that would be the prefered option, I'm happy to provide that extension.

As an additional option, we could create a cron job for git secrets that need to be refreshed in the users namespace. Or run a small container as a sidecar to the workspace or in the users namespace as long as workspaces are running. We're happy to provide a lightweight docker image that updates token secrets based on refresh tokens. If that's an option I would have to get some help of someone familar with the devworkspace-operator codebase.

If there's another option that's better, please let me know. An editor based approach will allow displaying an error message if the refresh fails.

---

@l0rd If I should drop a PR, just let me know. I'm happy if you could point me the right direction. :)

[vinokurig]

So we need to extend PersonalAccessToken.java as well that it can contain expirationTime and refreshToken.

I don't think we should use the expirationTime value, user or the oath app owner can withdraw tokens at any time.

[nils-mosbach]

True. We could fetch new access tokens e.g. every 60'-90'. In this case tokens should get updated before they expire. At least for GitLab and Bitbucket that have a limit of 2h. That makes the implementation easier.

[l0rd]

@nils-mosbach I am currently in vacation and will be back the 30th of August. We can setup a call next week if that works for you. I am just nothing a few things:

• adding some PAT metadata (like the expiration date) to the secrets may be useful but not necessary immediately
• I am afraid that mounting devworkspace-merged-git-credentials without using subpath is problematic as the folder that contains the credentials file would get overridden cc @amisevsk
• removing the read-only flag for devworkspace-merged-git-credentials only makes sense if patching the secret doesn't work (because it uses subpath)
• devworkspace-merged-git-credentials should be updated when a secret labelled controller.devfile.io/git-credential is updated.
• an executable started by the main container entrypoint that periodically gets a fresh token or a Kubernetes CronJob that's applied as a DevWorkspace post start event are 2 approaches that may work with any editor.

[nils-mosbach]

@l0rd sure, works for me. I'll drop a PM on mattermost. Thanks.

[amisevsk]

It's worth noting that since git config and credentials are mounted via configmap/secret, the read-only flag is required -- containers cannot write to a secret by modifying the mounted files (to my knowledge).

The short expiry on GitLab PATs poses a fairly significant problem. From a cursory look, it appears that this is handled in other IDEs/editors by requiring reauthentication with GitLab every two hours. For our case, since the git credentials are currently mounted via subpath (to avoid clobbering all files in the mount path as Mario mentions), this would also mean requiring a workspace restart when the token is refreshed. Subpath volume mounts don't propagate changes to the configmap/secret, so even if the secret was refreshed, the containers using would not see the change until they were restarted.

[nils-mosbach]

True. The idea behind the read-write flag on the volume mount was actually not to write changes to the secret. From what I've tested, git removes invalid tokens from the credential-store and ask users to enter user name and password if there's no authentication available in the store. VSCode should display a username and password prompt. Since Git does not have write access to the file, it just exits with unauthorized. So if that case happens, I think from a user perspective it's nice to have a simple solution by entering username and password. That information is not stored and just used for one git operation. On container restart, it will repopulate the old/invalid credentials to that file, which is the correct behavior.

Regarding subpath mounts, from what I can see the merged-git-credentials-secret just contains one property credentials. the folder thats beeing used is empty. The place for the mount is specified by devworkspace-gitconfig configmap.

[credential]
    helper = store --file /.git-credentials/credentials
[user]
   ...

From what I can see .git-credentials is only used by devworkspaces. But if we expect other applications write to /.git-credentials/ we could move the mount to another folder e.g. /.git-credentials/merged (without subpath definition) and change devworkspace-gitconfig to helper = store --file /.git-credentials/merged/credentials. That allows refreshing tokens without restart and might not affect possible data in that folder.

[amisevsk]

After some discussion with @nils-mosbach and @l0rd, I've prepared some DevWorkspace Operator changes to test mounting the git credentials file from the configmap directly. The resulting image is available as quay.io/amisevsk/devworkspace-controller:git-credentials-file, and the diff from the current DWO main branch is amisevsk/devworkspace-operator@3c47585.

The goal of these changes is to enable one piece of the discussion above: currently we use subpath mounts, which prevent changes from being propagated to the running workspace (see issue: devfile/devworkspace-operator#915)

I've tested the linked changes and updates to the secret storing my PAT are propagated into the workspace after around 20 seconds (on minikube -- this could potentially vary depending on the cluster). Additionally, it's currently necessary to trigger an event on any DevWorkspace CR in order to cause the DevWorkspace Operator to update the merged git-credentials secret -- I do this by adding an annotation to the workspace (see issue: devfile/devworkspace-operator#914).

Edit to add: Note also that these changes result in the same path/file being mounted for Che, as the Che currently uses /.git-credentials as the mount directory, so it should not break any unforeseen expectations on the Che side.

[l0rd]

#21640 has been solved. So the problem now is about periodically update the token when a user is using a workspace for more than 2 hours. We are probably not going to work on this right now as there can be some workarounds:

• using SSH keys (instead of the token) for git credentials
• manually executing a script to refresh the token when it expires

[l0rd]

Internal RH Chat thread about this

[che-bot]

Issues go stale after 180 days of inactivity. lifecycle/stale issues rot after an additional 7 days of inactivity and eventually close.

Mark the issue as fresh with /remove-lifecycle stale in a new comment.

If this issue is safe to close now please do so.

Moderators: Add lifecycle/frozen label to avoid stale mode.

[amisevsk]

/remove-lifecycle stale

[cgruver]

#21640 has been solved. So the problem now is about periodically update the token when a user is using a workspace for more than 2 hours. We are probably not going to work on this right now as there can be some workarounds:

• using SSH keys (instead of the token) for git credentials
• manually executing a script to refresh the token when it expires

@l0rd Do you have an example of a script to refresh the OAuth token for GitLab?

[l0rd]

@cgruver no, we never looked at it.

[myarbrou-rh]

Hey all, checking in to see if there has been any movement on this issue? @l0rd you mention manually executing a script to refresh the token but how would I get access to the new token that gets sent to the OAuth callback url? Or is there some api to tell the devspaces service to go ahead and refresh it and push the new token into the secret?

[che-bot]

Issues go stale after 180 days of inactivity. lifecycle/stale issues rot after an additional 7 days of inactivity and eventually close.

Mark the issue as fresh with /remove-lifecycle stale in a new comment.

If this issue is safe to close now please do so.

Moderators: Add lifecycle/frozen label to avoid stale mode.

[ibuziuk]

lifecycle/frozen