Skip to content

feat(oauth): add refresh token and expiry support to OAuth token flow - #1047

Open
vinokurig wants to merge 8 commits into
mainfrom
CRW-4121
Open

vinokurig wants to merge 8 commits into
mainfrom
CRW-4121

Conversation

@vinokurig

@vinokurig vinokurig commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Persist OAuth refresh tokens and expiration times in Kubernetes secrets alongside access tokens, enabling token refresh without re-authorization after server restarts.
Adds a POST /oauth/refresh endpoint, updates the OAuthToken DTO, PersonalAccessToken, and PersonalAccessTokenParams with refreshToken/expiresIn fields, and restores in-memory credentials from persisted secrets when the credential store is empty.

Screenshot/screencast of this PR

What issues does this PR fix or reference?

https://redhat.atlassian.net/browse/CRW-4121

How to test this PR?

  1. Start Che from the che-server pull request image: quay.io/eclipse/che-server:pr-1047
  2. Configure an oauth, e.g. for gitlab: https://eclipse.dev/che/docs/stable/administration-guide/configuring-oauth-2-for-gitlab/
  3. Start a workspace from a Gitlab repository url, an Oauth token must be added to the user-preferences -> Personal Access Tokens tab
  4. Restart the che-server pod to clear the oauth data from the che-server pod memory.
  5. Execute the POST API request: <che host url>/api/oauth/refresh?oauth_provider=gitlab&provider_url=https://gitlab.com
  6. Check the token secret, the token and the refresh-token data must be refreshed.

PR Checklist

As the author of this Pull Request I made sure that:

Release Notes

Reviewers

Reviewers, please comment how you tested the PR when approving it.

@openshift-ci

openshift-ci Bot commented Aug 31, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: vinokurig

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@github-actions

Copy link
Copy Markdown

Docker image build succeeded: quay.io/eclipse/che-server:pr-1047

kubectl patch command
kubectl patch -n eclipse-che "checluster/eclipse-che" --type=json -p="[{"op": "replace", "path": "/spec/components/cheServer/deployment", "value": {containers: [{image: "quay.io/eclipse/che-server:pr-1047", name: che}]}}]"

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

Docker image build succeeded: quay.io/eclipse/che-server:pr-1047

kubectl patch command
kubectl patch -n eclipse-che "checluster/eclipse-che" --type=json -p="[{"op": "replace", "path": "/spec/components/cheServer/deployment", "value": {containers: [{image: "quay.io/eclipse/che-server:pr-1047", name: che}]}}]"

@vinokurig
vinokurig force-pushed the CRW-4121 branch 3 times, most recently from fb69994 to 7a2aa30 Compare September 7, 2026 12:56
Persist OAuth refresh tokens and expiration times in Kubernetes secrets
alongside access tokens, enabling token refresh without re-authorization
after server restarts. Adds a POST /oauth/refresh endpoint, updates the
OAuthToken DTO, PersonalAccessToken, and PersonalAccessTokenParams with
refreshToken/expiresIn fields, and restores in-memory credentials from
persisted secrets when the credential store is empty.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown

Docker image build succeeded: quay.io/eclipse/che-server:pr-1047

kubectl patch command
kubectl patch -n eclipse-che "checluster/eclipse-che" --type=json -p="[{"op": "replace", "path": "/spec/components/cheServer/deployment", "value": {containers: [{image: "quay.io/eclipse/che-server:pr-1047", name: che}]}}]"

public static final String REFRESH_TOKEN_DATA_FIELD = "refresh-token";

/** Kubernetes secret data field key for the token expiration time in seconds. */
public static final String EXPIRES_IN_DATA_FIELD = "expires-in";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it better to move to annotation?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

*/
@POST
@Path("refresh")
public void refresh(@Required @QueryParam("oauth_provider") String oauthProvider)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see that provider url is not considered.
What if we have different urls for a single provider, like GitHub

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, switched to provider_url parameter

token.getRefreshToken(),
token.getExpiresIn());
personalAccessTokenManager.store(personalAccessToken);
gitCredentialManager.createOrReplace(personalAccessToken);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think removePreviousTokenSecretsIfPresent is missed.
BTW, can we resue forceRefreshPersonalAccessToken(String scmServerUrl) ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice catch, switched to forceRefreshPersonalAccessToken(String scmServerUrl)

@github-actions

Copy link
Copy Markdown

Docker image build succeeded: quay.io/eclipse/che-server:pr-1047

kubectl patch command
kubectl patch -n eclipse-che "checluster/eclipse-che" --type=json -p="[{"op": "replace", "path": "/spec/components/cheServer/deployment", "value": {containers: [{image: "quay.io/eclipse/che-server:pr-1047", name: che}]}}]"

@eclipse-che eclipse-che deleted a comment from openshift-ci Bot Sep 11, 2026
@github-actions

Copy link
Copy Markdown

Docker image build succeeded: quay.io/eclipse/che-server:pr-1047

kubectl patch command
kubectl patch -n eclipse-che "checluster/eclipse-che" --type=json -p="[{"op": "replace", "path": "/spec/components/cheServer/deployment", "value": {containers: [{image: "quay.io/eclipse/che-server:pr-1047", name: che}]}}]"

@eclipse-che eclipse-che deleted a comment from openshift-ci Bot Sep 14, 2026
…en flow

Handle providers that omit `expires_in` and issue no refresh token:

* `OAuthAuthenticator`: extract `newOAuthToken(Credential)` and set the
  expiration only when the credential provides one, since
  `OAuthToken#withExpiresIn` takes a primitive.
* `EmbeddedOAuthAPI`: fall back to 0 when the token response has no
  `expires_in`.
* `KubernetesPersonalAccessTokenManager`: only write the `refresh-token`
  secret field when a refresh token is present.

Add tests for each case, including a new `OAuthAuthenticatorTest` for the
OAuth2 authenticator.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

Docker image build succeeded: quay.io/eclipse/che-server:pr-1047

kubectl patch command
kubectl patch -n eclipse-che "checluster/eclipse-che" --type=json -p="[{"op": "replace", "path": "/spec/components/cheServer/deployment", "value": {containers: [{image: "quay.io/eclipse/che-server:pr-1047", name: che}]}}]"

@eclipse-che eclipse-che deleted a comment from openshift-ci Bot Sep 14, 2026
@svor

svor commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

/retest

personalAccessToken.getScmTokenName())
.put(
ANNOTATION_SCM_TOKEN_EXPIRES_IN,
String.valueOf(personalAccessToken.getExpiresIn()))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it updates every PAT and most all of them will have 0
It misleads since 0 means expires immediately.
Should we check if personalAccessToken.getExpiresIn()) > 0 before adding annotation?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

new TokenResponse()
.setAccessToken(token.getToken())
.setRefreshToken(token.getRefreshToken())
.setExpiresInSeconds(token.getExpiresIn());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't put 0 which is a default value.
Instead it is better to check if value > 0

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, added the check and covered it with tests

…en flow

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@eclipse-che eclipse-che deleted a comment from openshift-ci Bot Sep 15, 2026
@vinokurig

Copy link
Copy Markdown
Contributor Author

/retest

@eclipse-che eclipse-che deleted a comment from github-actions Bot Sep 15, 2026
@eclipse-che eclipse-che deleted a comment from github-actions Bot Sep 15, 2026
@vinokurig

Copy link
Copy Markdown
Contributor Author

/retest

3 similar comments
@vinokurig

Copy link
Copy Markdown
Contributor Author

/retest

@svor

svor commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

/retest

@svor

svor commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

/retest

+ token
+ '\''
+ ", refreshToken='"
+ refreshToken

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can avoid adding token data into toString()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

isNullOrEmpty(refreshTokenData)
? null
: new String(Base64.getDecoder().decode(refreshTokenData)).trim();
long expiresIn = isNullOrEmpty(expiresInAnnotation) ? 0 : parseLong(expiresInAnnotation.trim());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pls try/catch parsing long data

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

String configuredCheUserId = secretAnnotations.get(ANNOTATION_CHE_USERID);
String configuredOAuthProviderName =
secretAnnotations.get(ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_NAME);
String configuredOAuthProviderName = secretAnnotations.get(ANNOTATION_SCM_PROVIDER_NAME);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pls Have a look

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a leftover fix from the pull request
The main branch is not affected by this leftover as we try to get token by url if we fail to get the token by provider name:

Optional<PersonalAccessToken> tokenOptional =
personalAccessTokenManager.get(cheSubject, oAuthProviderName, null, namespaceName);
if (tokenOptional.isPresent()) {
return fetchGitUserDataWithPersonalAccessToken(tokenOptional.get());
} else {
Optional<PersonalAccessToken> oAuthTokenOptional =
personalAccessTokenManager.get(cheSubject, null, oAuthProviderUrl, namespaceName);

I would like to keep the fix

@github-actions

Copy link
Copy Markdown

Docker image build succeeded: quay.io/eclipse/che-server:pr-1047

kubectl patch command
kubectl patch -n eclipse-che "checluster/eclipse-che" --type=json -p="[{"op": "replace", "path": "/spec/components/cheServer/deployment", "value": {containers: [{image: "quay.io/eclipse/che-server:pr-1047", name: che}]}}]"

@github-actions

Copy link
Copy Markdown

Docker image build succeeded: quay.io/eclipse/che-server:pr-1047

kubectl patch command
kubectl patch -n eclipse-che "checluster/eclipse-che" --type=json -p="[{"op": "replace", "path": "/spec/components/cheServer/deployment", "value": {containers: [{image: "quay.io/eclipse/che-server:pr-1047", name: che}]}}]"

@vinokurig

Copy link
Copy Markdown
Contributor Author

/retest

2 similar comments
@vinokurig

Copy link
Copy Markdown
Contributor Author

/retest

@vinokurig

Copy link
Copy Markdown
Contributor Author

/retest

@openshift-ci

openshift-ci Bot commented Sep 21, 2026

Copy link
Copy Markdown

@vinokurig: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/v19-azure-no-pat-oauth-flow-ssh-url 8adc005 link true /test v19-azure-no-pat-oauth-flow-ssh-url
ci/prow/v19-gitea-no-pat-oauth-flow 8adc005 link true /test v19-gitea-no-pat-oauth-flow
ci/prow/v19-github-no-pat-oauth-flow-raw-devfile-url 8adc005 link true /test v19-github-no-pat-oauth-flow-raw-devfile-url
ci/prow/v19-azure-no-pat-oauth-flow 8adc005 link true /test v19-azure-no-pat-oauth-flow
ci/prow/v19-che-smoke-test 8adc005 link true /test v19-che-smoke-test
ci/prow/v19-bitbucket-no-pat-oauth-flow-ssh-url 8adc005 link true /test v19-bitbucket-no-pat-oauth-flow-ssh-url
ci/prow/v19-github-no-pat-oauth-flow-ssh-url 8adc005 link true /test v19-github-no-pat-oauth-flow-ssh-url
ci/prow/v19-azure-no-pat-oauth-flow-raw-devfile-url 8adc005 link true /test v19-azure-no-pat-oauth-flow-raw-devfile-url
ci/prow/v19-gitea-with-pat-setup-flow 8adc005 link true /test v19-gitea-with-pat-setup-flow
ci/prow/v19-github-no-pat-oauth-flow 8adc005 link true /test v19-github-no-pat-oauth-flow
ci/prow/v19-gitlab-with-pat-setup-flow 8adc005 link true /test v19-gitlab-with-pat-setup-flow
ci/prow/v19-gitlab-no-pat-oauth-flow-raw-devfile-url 8adc005 link true /test v19-gitlab-no-pat-oauth-flow-raw-devfile-url

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants