Skip to content

Do not encode access token for git-credentials secrets #745

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

Closed
vinokurig wants to merge 1 commit into from
Closed

Do not encode access token for git-credentials secrets #745

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
6 changes: 2 additions & 4 deletions ...ava/org/eclipse/che/api/factory/server/scm/kubernetes/KubernetesGitCredentialManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012-2023 Red Hat, Inc.
* Copyright (c) 2012-2024 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 @@ -13,7 +13,6 @@

import static com.google.common.base.Strings.isNullOrEmpty;
import static java.lang.String.format;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.eclipse.che.api.factory.server.scm.PersonalAccessTokenFetcher.OAUTH_2_PREFIX;
import static org.eclipse.che.workspace.infrastructure.kubernetes.provision.secret.KubernetesSecretAnnotationNames.ANNOTATION_AUTOMOUNT;
import static org.eclipse.che.workspace.infrastructure.kubernetes.provision.secret.KubernetesSecretAnnotationNames.ANNOTATION_DEV_WORKSPACE_MOUNT_PATH;
Expand All @@ -31,7 +30,6 @@
import io.fabric8.kubernetes.client.KubernetesClient;
import java.net.MalformedURLException;
import java.net.URL;
import java.net.URLEncoder;
import java.util.Base64;
import java.util.HashMap;
import java.util.Map;
Expand Down Expand Up @@ -154,7 +152,7 @@ public void createOrReplace(PersonalAccessToken personalAccessToken)
"%s://%s:%s@%s%s",
scmUrl.getProtocol(),
getUsernameSegment(personalAccessToken),
URLEncoder.encode(personalAccessToken.getToken(), UTF_8),
personalAccessToken.getToken(),
Copy link
Contributor

@tolusha tolusha Nov 20, 2024

Choose a reason for hiding this comment

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

We have to know why previously it was done in that way.

Copy link
Contributor Author

@vinokurig vinokurig Nov 20, 2024

Choose a reason for hiding this comment

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

This was added a long time ago by @mshaposhnik, I don't think we can find the reason.

Copy link
Contributor

@mshaposhnik mshaposhnik Nov 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Hey guys,
it was done so cuz sometimes tokens may contain symbols which are disallowed or bad to use in URL's (like @, :, # etc)
see https://serverfault.com/a/1001324

Copy link
Contributor Author

@vinokurig vinokurig Nov 20, 2024

Choose a reason for hiding this comment

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

Thanks @mshaposhnik for the explanation, looks like we would need to find a better solution

scmUrl.getHost(),
scmUrl.getPort() != 80 && scmUrl.getPort() != -1
? ":" + scmUrl.getPort()
Expand Down
Loading