Skip to content

fix: always prefer personal access token over OAuth one #1066

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
vinokurig merged 1 commit into from
Sep 21, 2026
Merged

fix: always prefer personal access token over OAuth one #1066

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 35 additions & 2 deletions ...g/eclipse/che/api/factory/server/scm/kubernetes/KubernetesPersonalAccessTokenManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012-2025 Red Hat, Inc.
* Copyright (c) 2012-2026 Red Hat, Inc.
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
* which is available at https://www.eclipse.org/legal/epl-2.0/
Expand All @@ -12,6 +12,7 @@
package org.eclipse.che.api.factory.server.scm.kubernetes;

import static com.google.common.base.Strings.isNullOrEmpty;
import static org.eclipse.che.api.factory.server.scm.PersonalAccessTokenFetcher.OAUTH_2_PREFIX;
import static org.eclipse.che.commons.lang.StringUtils.trimEnd;

import com.google.common.collect.ImmutableMap;
Expand Down Expand Up @@ -284,9 +285,38 @@ private List<PersonalAccessToken> doGetPersonalAccessTokens(
LOG.debug("Failed to get personal access token", e);
throw new ScmConfigurationPersistenceException(e.getMessage(), e);
}
// Put personal access tokens before the OAuth ones, keeping the newest-first order within each
// group. OAuth tokens may be short-living, e.g. GitLab OAuth tokens expire in 2 hours, so a
// personal access token is always preferred when both are configured.
result.sort(Comparator.comparing(KubernetesPersonalAccessTokenManager::isOAuthToken));
return result;
}

/**
* Checks whether the token was obtained with the OAuth flow. Such tokens are stored with a
* generated {@code oauth2-<random>} name, while the manually configured personal access tokens
* are named after the SCM provider.
*
* @param token the token to check
* @return {@code true} if the token is an OAuth one
*/
private static boolean isOAuthToken(PersonalAccessToken token) {
return token.getScmTokenName() != null && token.getScmTokenName().startsWith(OAUTH_2_PREFIX);
}

/**
* The same as {@link #isOAuthToken(PersonalAccessToken)} but for the secret the token is stored
* in.
*
* @param secret the token secret to check
* @return {@code true} if the secret keeps an OAuth token
*/
private static boolean isOAuthTokenSecret(Secret secret) {
String tokenName =
secret.getMetadata().getAnnotations().get(ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_NAME);
return tokenName != null && tokenName.startsWith(OAUTH_2_PREFIX);
}

/**
* Returns the list of namespaces to search for the personal access token secrets.
*
Expand Down Expand Up @@ -439,7 +469,10 @@ private void removePreviousTokenSecretsIfPresent(String scmServerUrl)
List<Secret> secrets = doGetPersonalAccessTokenSecrets(namespaceMeta);
for (int i = 1; i < secrets.size(); i++) {
Secret secret = secrets.get(i);
if (secret.getMetadata().getAnnotations().get(ANNOTATION_SCM_URL).equals(scmServerUrl)) {
// Only the outdated OAuth token secrets are cleaned up. The manually configured personal
// access tokens must survive the refresh, as they are preferred over the OAuth ones.
if (secret.getMetadata().getAnnotations().get(ANNOTATION_SCM_URL).equals(scmServerUrl)
&& isOAuthTokenSecret(secret)) {
cheServerKubernetesClientFactory
.create()
.secrets()
Expand Down
135 changes: 133 additions & 2 deletions ...lipse/che/api/factory/server/scm/kubernetes/KubernetesPersonalAccessTokenManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
Expand Down Expand Up @@ -586,7 +587,7 @@ public void shouldReturnFirstValidTokenAndDeleteTheOlderOne() throws Exception {
.withAnnotations(
Map.of(
ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_NAME,
"github",
"oauth2-abcde",
ANNOTATION_CHE_USERID,
"user1",
ANNOTATION_SCM_URL,
Expand All @@ -600,7 +601,7 @@ public void shouldReturnFirstValidTokenAndDeleteTheOlderOne() throws Exception {
.withAnnotations(
Map.of(
ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_NAME,
"github",
"oauth2-fghij",
ANNOTATION_CHE_USERID,
"user1",
ANNOTATION_SCM_URL,
Expand Down Expand Up @@ -630,6 +631,136 @@ public void shouldReturnFirstValidTokenAndDeleteTheOlderOne() throws Exception {
verify(nonNamespaceOperation, times(1)).delete(eq(secret1));
}

@Test
public void shouldPreferPersonalAccessTokenOverOAuthToken() throws Exception {
// given
KubernetesNamespaceMeta meta = new KubernetesNamespaceMetaImpl("test");
when(namespaceFactory.list()).thenReturn(singletonList(meta));
KubernetesNamespace kubernetesnamespace = Mockito.mock(KubernetesNamespace.class);
KubernetesSecrets secrets = Mockito.mock(KubernetesSecrets.class);
when(namespaceFactory.access(eq(null), eq(meta.getName()))).thenReturn(kubernetesnamespace);
when(kubernetesnamespace.secrets()).thenReturn(secrets);
when(scmPersonalAccessTokenFetcher.getScmUsername(any(PersonalAccessTokenParams.class)))
.thenReturn(Optional.of("user"));
Map<String, String> patData =
Map.of("token", Base64.getEncoder().encodeToString("pat-token".getBytes(UTF_8)));
Map<String, String> oauthData =
Map.of("token", Base64.getEncoder().encodeToString("oauth-token".getBytes(UTF_8)));
// the personal access token secret is the older one
ObjectMeta patMeta =
new ObjectMetaBuilder()
.withCreationTimestamp("2021-07-01T12:00:00Z")
.withAnnotations(
Map.of(
ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_NAME,
"gitlab",
ANNOTATION_CHE_USERID,
"user1",
ANNOTATION_SCM_URL,
"http://host1",
ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_ID,
"pat-id"))
.build();
ObjectMeta oauthMeta =
new ObjectMetaBuilder()
.withCreationTimestamp("2021-07-02T12:00:00Z")
.withAnnotations(
Map.of(
ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_NAME,
"oauth2-abcde",
ANNOTATION_CHE_USERID,
"user1",
ANNOTATION_SCM_URL,
"http://host1",
ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_ID,
"oauth-id"))
.build();
Secret patSecret = new SecretBuilder().withMetadata(patMeta).withData(patData).build();
Secret oauthSecret = new SecretBuilder().withMetadata(oauthMeta).withData(oauthData).build();
when(secrets.get(any(LabelSelector.class))).thenReturn(Arrays.asList(patSecret, oauthSecret));

// when
Optional<PersonalAccessToken> token =
personalAccessTokenManager.get(
new SubjectImpl("user", Collections.emptyList(), "user1", "t1", false),
null,
"http://host1",
null);

// then
assertTrue(token.isPresent());
assertEquals(token.get().getScmTokenId(), "pat-id");
assertEquals(token.get().getToken(), "pat-token");
}

@Test
public void shouldKeepPersonalAccessTokenSecretOnForceRefresh() throws Exception {
// given
KubernetesNamespaceMeta meta = new KubernetesNamespaceMetaImpl("test");
when(namespaceFactory.list()).thenReturn(singletonList(meta));
KubernetesNamespace kubernetesnamespace = Mockito.mock(KubernetesNamespace.class);
KubernetesSecrets secrets = Mockito.mock(KubernetesSecrets.class);
when(namespaceFactory.access(eq(null), eq(meta.getName()))).thenReturn(kubernetesnamespace);
when(kubernetesnamespace.secrets()).thenReturn(secrets);
when(cheServerKubernetesClientFactory.create()).thenReturn(kubeClient);
when(kubeClient.secrets()).thenReturn(secretsMixedOperation);
when(secretsMixedOperation.inNamespace(eq(meta.getName()))).thenReturn(nonNamespaceOperation);
Map<String, String> patData =
Map.of("token", Base64.getEncoder().encodeToString("pat-token".getBytes(UTF_8)));
Map<String, String> oauthData =
Map.of("token", Base64.getEncoder().encodeToString("oauth-token".getBytes(UTF_8)));
ObjectMeta patMeta =
new ObjectMetaBuilder()
.withCreationTimestamp("2021-07-01T12:00:00Z")
.withAnnotations(
Map.of(
ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_NAME,
"gitlab",
ANNOTATION_CHE_USERID,
"user1",
ANNOTATION_SCM_URL,
"http://host1",
ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_ID,
"pat-id"))
.build();
ObjectMeta oauthMeta =
new ObjectMetaBuilder()
.withCreationTimestamp("2021-07-02T12:00:00Z")
.withAnnotations(
Map.of(
ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_NAME,
"oauth2-abcde",
ANNOTATION_CHE_USERID,
"user1",
ANNOTATION_SCM_URL,
"http://host1",
ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_ID,
"oauth-id"))
.build();
Secret patSecret = new SecretBuilder().withMetadata(patMeta).withData(patData).build();
Secret oauthSecret = new SecretBuilder().withMetadata(oauthMeta).withData(oauthData).build();
// the newly stored token is the first one, the rest are the candidates for the cleanup
when(secrets.get(any(LabelSelector.class))).thenReturn(Arrays.asList(patSecret, oauthSecret));
PersonalAccessToken token =
new PersonalAccessToken(
"http://host1",
"gitlab",
"user1",
"user",
"oauth2-fghij",
"new-oauth-id",
"new-oauth-token");
when(scmPersonalAccessTokenFetcher.refreshPersonalAccessToken(
any(Subject.class), eq("http://host1")))
.thenReturn(token);

// when
personalAccessTokenManager.forceRefreshPersonalAccessToken("http://host1");

// then
verify(nonNamespaceOperation, never()).delete(eq(patSecret));
}

@Test
public void shouldRemoveToken() throws Exception {
// given
Expand Down
Loading