Skip to content
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.

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

fix:(GitHub Auth): Deprecate getAuthUsername #585

Merged
merged 13 commits into from
Feb 19, 2025
4 changes: 0 additions & 4 deletions src/targets/awsLambdaLayer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import * as path from 'path';
import { Octokit } from '@octokit/rest';
import simpleGit from 'simple-git';
import {
getAuthUsername,
getGitHubApiToken,
getGitHubClient,
GitHubRemote,
} from '../utils/githubApi';
Expand Down Expand Up @@ -139,8 +137,6 @@ export class AwsLambdaLayerTarget extends BaseTarget {
this.logger.trace('AWS regions: ', awsRegions);

const remote = this.awsLambdaConfig.registryRemote;
const username = await getAuthUsername(this.github);
remote.setAuth(username, getGitHubApiToken());

await withTempDir(
async directory => {
Expand Down
6 changes: 0 additions & 6 deletions src/targets/ghPages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ import { GitHubGlobalConfig, TargetConfig } from '../schemas/project_config';
import { ConfigurationError, reportError } from '../utils/errors';
import { withTempDir } from '../utils/files';
import {
getAuthUsername,
getGitHubApiToken,
getGitHubClient,
GitHubRemote,
} from '../utils/githubApi';
Expand Down Expand Up @@ -219,13 +217,9 @@ export class GhPagesTarget extends BaseTarget {
packageFiles[0]
);

const username = await getAuthUsername(this.github);

const remote = new GitHubRemote(
githubOwner,
githubRepo,
username,
getGitHubApiToken()
);

await withTempDir(
Expand Down
4 changes: 0 additions & 4 deletions src/targets/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ import { GitHubGlobalConfig, TargetConfig } from '../schemas/project_config';
import { ConfigurationError, reportError } from '../utils/errors';
import { withTempDir } from '../utils/files';
import {
getAuthUsername,
getGitHubApiToken,
getGitHubClient,
GitHubRemote,
} from '../utils/githubApi';
Expand Down Expand Up @@ -427,8 +425,6 @@ export class RegistryTarget extends BaseTarget {

private async cloneRegistry(directory: string): Promise<SimpleGit> {
const remote = this.remote;
const username = await getAuthUsername(this.github);
remote.setAuth(username, getGitHubApiToken());

const git = simpleGit(directory);
this.logger.info(
Expand Down
3 changes: 0 additions & 3 deletions src/targets/upm.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { Octokit } from '@octokit/rest';
import simpleGit from 'simple-git';
import {
getAuthUsername,
getGitHubApiToken,
getGitHubClient,
GitHubRemote,
Expand Down Expand Up @@ -107,11 +106,9 @@ export class UpmTarget extends BaseTarget {
packageFile
);

const username = await getAuthUsername(this.github);
const remote = new GitHubRemote(
this.config.releaseRepoOwner,
this.config.releaseRepoName,
username,
getGitHubApiToken()
);
const remoteAddr = remote.getRemoteString();
Expand Down
38 changes: 5 additions & 33 deletions src/utils/githubApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ export class GitHubRemote {
public readonly owner: string;
/** GitHub repository name */
public readonly repo: string;
/** GitHub username */
protected username?: string;
/** GitHub personal authentication token */
protected apiToken?: string;
/** GitHub hostname */
Expand All @@ -26,27 +24,14 @@ export class GitHubRemote {
public constructor(
owner: string,
repo: string,
username?: string,
apiToken?: string
) {
this.owner = owner;
this.repo = repo;
if (username && apiToken) {
this.setAuth(username, apiToken);
}
Comment on lines -34 to -36
Copy link
Member

Choose a reason for hiding this comment

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

should be able to remove these parameters entirely -- or maybe take the api key and do the header modification here?

Copy link
Member Author

Choose a reason for hiding this comment

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

parameters removed, but I'm not sure how to bake the token and header in there

this.apiToken = apiToken;
this.url = `/${this.owner}/${this.repo}/`;
}

/**
* Sets authentication arguments: username and personal API token
*
* @param username GitHub username
* @param apiToken GitHub API token
*/
public setAuth(username: string, apiToken: string): void {
this.username = username;
this.apiToken = apiToken;
}

/**
* Returns an HTTP-based git remote
Expand All @@ -60,12 +45,14 @@ export class GitHubRemote {
/**
* Returns an HTTP-based git remote with embedded HTTP basic auth
*
* Using dummy username as it does not matter for cloning
*
* It MAY contain sensitive information (e.g. API tokens)
*/
public getRemoteStringWithAuth(): string {
const authData =
this.username && this.apiToken
? `${this.username}:${this.apiToken}@`
this.apiToken
? `dummyusername:${this.apiToken}@`
: '';
return this.PROTOCOL_PREFIX + authData + this.GITHUB_HOSTNAME + this.url;
}
Expand Down Expand Up @@ -123,21 +110,6 @@ export function getGitHubClient(token = ''): Octokit {
return _GitHubClientCache[githubApiToken];
}

/**
* Gets the currently authenticated GitHub user from the client
*
* @param github GitHub client
* @returns GitHub username
*/
export async function getAuthUsername(github: Octokit): Promise<string> {
const userData = await github.users.getAuthenticated({});
const username = (userData.data || {}).login;
if (!username) {
throw new Error('Cannot reliably detect GitHub username, aborting');
}
return username;
}

/**
* Loads a file from the context's repository
*
Expand Down
Loading