mirror of
https://github.com/RayLabsHQ/gitea-mirror.git
synced 2026-10-30 22:21:58 +01:00
fix(public-orgs): finish the destination-only paths for tokenless sources (#411)
Follow-up to #409. Public-only accounts could add and mirror an organization, then hit the source-token gates that were left behind everywhere after the first mirror. Each item was reproduced against a running instance with only a destination configured and a tokenless GitHub source, and re-checked after the fix. **The paths that still demanded a source token** - `POST /api/job/sync-repo` and `POST /api/job/schedule-sync-repo` answered 400 "Config missing for the user or token", so the Sync button never worked for these accounts. Neither route uses that token; they dispatch to the destination and resolve source clients per repository. Both now gate on the destination token, like the mirror and retry routes already do. - `recovery.ts` threw "GitHub token not found in configuration" before the per-source resolution right below it, so an interrupted mirror never recovered. The gate is gone and a tokenless GitHub source now gets the same anonymous client the job routes build. - `mirrorGitRepoIssuesToGitea` and the pull request, label and milestone functions required the config token even though the client arrives as a parameter, so with the anonymous client they threw and the per-component try/catch logged an error on every sync. They now require only the destination, and public metadata mirrors. **The bulk import** `POST /api/sync` iterated every source, including tokenless ones whose personal listing 401s, counted them as failed, and answered 502 when there was nothing else. It now skips them the way `importRepositoriesFromSources` does, reports them as `skippedPublicSources`, and says so when every source is public only. **The organization pin** `force: true` without a `sourceId` or `provider` re-pinned the organization to the primary source. Reproduced with two sources: an organization pinned to the GitLab source and one deliberately left on every source were both moved onto the primary. Only a request that names a source re-pins now; naming a provider (public mode) still re-pins as #409 intended. Covered by a new route test. **The badge** `invalidateConfigCache()` cleared the cache but nothing re-read it, so the Public only badge appeared only after a reload. Mounted `useConfigStatus` hooks now refresh on invalidation, through one shared in-flight request so a page with several of them still makes a single call. Two things from the review I deliberately did not change: the anonymous 60 requests an hour budget with its long `onRateLimit` waits, and `rediscoverOrganizationRepositories` running for every user rather than only tokenless sources. Both are judgement calls about the feature rather than defects. Tests: 906 pass. Build passes. Type errors in the touched files are unchanged from main.
This commit is contained in:
1 parent
72a3223c76
commit
6b8d0b3d78
9 files changed
+145
-36
No files matched your search
@@ -8,6 +8,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
## [Unreleased]
|
||||
|
||||
### Added
|
||||
- Public organizations without a source connection (#409)
|
||||
- The Add Organization dialog offers a Public only mode, the default when no source is connected: pick GitHub, GitLab or Gitea/Forgejo, optionally an instance URL, and the organization is imported anonymously
|
||||
- A tokenless source row is found or created for that provider and host and the organization is pinned to it, so attribution, locks, the scheduler and cleanup keep working per source
|
||||
- Anonymous GitHub clients keep the throttling and rate-limit backoff of authenticated ones, and the scheduler, per-repo mirror, retry, sync and recovery paths gate on the destination token instead of a source token
|
||||
- Sources with no username and no token are saved and shown as Public only, and the organizations and repositories pages work with no configured source
|
||||
- Per-organization source selection for multi-source accounts
|
||||
- Organizations remember which source they import and mirror from; migration 0020 backfills the pin from each organization's repositories when they agree on one
|
||||
- Source picker in the add-organization dialog and on organization cards when more than one source is connected
|
||||
|
||||
@@ -34,6 +34,28 @@ let configCache: { data: ConfigApiResponse | null; timestamp: number; userId: st
|
||||
|
||||
const CACHE_DURATION = 30000; // 30 seconds cache
|
||||
|
||||
// Mounted hooks, so invalidating the cache refreshes what is on screen
|
||||
// instead of only clearing the store behind it.
|
||||
const cacheSubscribers = new Set<() => void>();
|
||||
|
||||
// One in-flight GET shared by every mounted hook: an invalidation wakes all
|
||||
// of them at once, and without this each would fire its own request.
|
||||
let inFlightRequest: { userId: string; promise: Promise<ConfigApiResponse> } | null = null;
|
||||
|
||||
function fetchConfigOnce(userId: string): Promise<ConfigApiResponse> {
|
||||
if (inFlightRequest && inFlightRequest.userId === userId) {
|
||||
return inFlightRequest.promise;
|
||||
}
|
||||
const promise = apiRequest<ConfigApiResponse>(`/config?userId=${userId}`, { method: 'GET' });
|
||||
const tracked = promise.finally(() => {
|
||||
if (inFlightRequest?.promise === tracked) {
|
||||
inFlightRequest = null;
|
||||
}
|
||||
});
|
||||
inFlightRequest = { userId, promise: tracked };
|
||||
return tracked;
|
||||
}
|
||||
|
||||
/**
|
||||
* Hook to check if GitHub and Gitea are properly configured
|
||||
* Returns configuration status and prevents unnecessary API calls when not configured
|
||||
@@ -127,10 +149,7 @@ export function useConfigStatus(): ConfigStatus {
|
||||
setConfigStatus(prev => ({ ...prev, isLoading: true, error: null }));
|
||||
}
|
||||
|
||||
const configResponse = await apiRequest<ConfigApiResponse>(
|
||||
`/config?userId=${user.id}`,
|
||||
{ method: 'GET' }
|
||||
);
|
||||
const configResponse = await fetchConfigOnce(user.id);
|
||||
|
||||
// Update cache
|
||||
configCache = {
|
||||
@@ -196,12 +215,32 @@ export function useConfigStatus(): ConfigStatus {
|
||||
checkConfiguration();
|
||||
}, [checkConfiguration]);
|
||||
|
||||
// Re-read after invalidateConfigCache(): saving the configuration or
|
||||
// adding a public organization (which creates its tokenless source row)
|
||||
// has to reach the components already on screen, or source-dependent UI
|
||||
// like the organization card's "Public only" badge stays stale until a
|
||||
// reload.
|
||||
useEffect(() => {
|
||||
const refresh = () => {
|
||||
void checkConfiguration();
|
||||
};
|
||||
cacheSubscribers.add(refresh);
|
||||
return () => {
|
||||
cacheSubscribers.delete(refresh);
|
||||
};
|
||||
}, [checkConfiguration]);
|
||||
|
||||
return configStatus;
|
||||
}
|
||||
|
||||
// Export function to invalidate cache when config is updated
|
||||
// Export function to invalidate cache when config is updated. Mounted hooks
|
||||
// re-fetch (through the shared in-flight request, so this costs one call).
|
||||
export function invalidateConfigCache() {
|
||||
configCache = { data: null, timestamp: 0, userId: null };
|
||||
inFlightRequest = null;
|
||||
for (const refresh of [...cacheSubscribers]) {
|
||||
refresh();
|
||||
}
|
||||
}
|
||||
|
||||
// Export function to get cached config data for other hooks
|
||||
|
||||
+11
-16
@@ -2531,13 +2531,14 @@ export const mirrorGitRepoIssuesToGitea = async ({
|
||||
giteaRepoName?: string;
|
||||
}) => {
|
||||
//things covered here are- issue, title, body, labels, comments and assignees
|
||||
// The source client arrives as `octokit` (anonymous for a public-only
|
||||
// source), so only the destination has to be configured here.
|
||||
if (
|
||||
!config.githubConfig?.token ||
|
||||
!config.giteaConfig?.token ||
|
||||
!config.giteaConfig?.url ||
|
||||
!config.giteaConfig?.defaultOwner
|
||||
) {
|
||||
throw new Error("Missing GitHub or Gitea configuration.");
|
||||
throw new Error("Missing destination configuration.");
|
||||
}
|
||||
|
||||
// Decrypt config tokens for API usage
|
||||
@@ -3601,13 +3602,13 @@ export async function mirrorGitRepoPullRequestsToGitea({
|
||||
giteaOwner: string;
|
||||
giteaRepoName?: string;
|
||||
}) {
|
||||
// The source client arrives as `octokit`; see mirrorGitRepoIssuesToGitea.
|
||||
if (
|
||||
!config.githubConfig?.token ||
|
||||
!config.giteaConfig?.token ||
|
||||
!config.giteaConfig?.url ||
|
||||
!config.giteaConfig?.defaultOwner
|
||||
) {
|
||||
throw new Error("Missing GitHub or Gitea configuration.");
|
||||
throw new Error("Missing destination configuration.");
|
||||
}
|
||||
|
||||
// Decrypt config tokens for API usage
|
||||
@@ -4053,12 +4054,9 @@ export async function mirrorGitRepoLabelsToGitea({
|
||||
giteaOwner: string;
|
||||
giteaRepoName?: string;
|
||||
}) {
|
||||
if (
|
||||
!config.githubConfig?.token ||
|
||||
!config.giteaConfig?.token ||
|
||||
!config.giteaConfig?.url
|
||||
) {
|
||||
throw new Error("Missing GitHub or Gitea configuration.");
|
||||
// The source client arrives as `octokit`; see mirrorGitRepoIssuesToGitea.
|
||||
if (!config.giteaConfig?.token || !config.giteaConfig?.url) {
|
||||
throw new Error("Missing destination configuration.");
|
||||
}
|
||||
|
||||
// Decrypt config tokens for API usage
|
||||
@@ -4187,12 +4185,9 @@ export async function mirrorGitRepoMilestonesToGitea({
|
||||
giteaOwner: string;
|
||||
giteaRepoName?: string;
|
||||
}) {
|
||||
if (
|
||||
!config.githubConfig?.token ||
|
||||
!config.giteaConfig?.token ||
|
||||
!config.giteaConfig?.url
|
||||
) {
|
||||
throw new Error("Missing GitHub or Gitea configuration.");
|
||||
// The source client arrives as `octokit`; see mirrorGitRepoIssuesToGitea.
|
||||
if (!config.giteaConfig?.token || !config.giteaConfig?.url) {
|
||||
throw new Error("Missing destination configuration.");
|
||||
}
|
||||
|
||||
// Decrypt config tokens for API usage
|
||||
|
||||
+8
-7
@@ -8,7 +8,7 @@ import { resetStuckMirrorStatuses } from './stuck-status-recovery';
|
||||
import { db, repositories, organizations, mirrorJobs, configs } from './db';
|
||||
import { eq, and, lt, inArray, sql } from 'drizzle-orm';
|
||||
import { mirrorRepositoryToDestination, syncRepositoryOnDestination } from './mirror-dispatch';
|
||||
import { createGitHubClient } from './github';
|
||||
import { createGitHubClient, createPublicGitHubClient } from './github';
|
||||
import type { Octokit } from '@octokit/rest';
|
||||
import {
|
||||
decryptSourceToken,
|
||||
@@ -272,11 +272,8 @@ async function recoverMirrorJob(job: any, remainingItemIds: string[]) {
|
||||
|
||||
console.log(`Found ${repos.length} repositories to process for recovery`);
|
||||
|
||||
// Validate GitHub configuration before creating client
|
||||
if (!config.githubConfig?.token) {
|
||||
throw new Error('GitHub token not found in configuration');
|
||||
}
|
||||
|
||||
// No source-token check here: the client is resolved from each
|
||||
// repository's own source below, and a public-only source has none.
|
||||
// Only GitHub sources use an API client while mirroring (metadata).
|
||||
// Other hosts get code-only mirrors through Gitea, so no client is built.
|
||||
// Repositories picked by bare id can span sources, so the sources are
|
||||
@@ -304,6 +301,10 @@ async function recoverMirrorJob(job: any, remainingItemIds: string[]) {
|
||||
if (repoSource?.provider === 'github') {
|
||||
try {
|
||||
const decryptedToken = decryptSourceToken(repoSource.token);
|
||||
// A tokenless GitHub source gets the anonymous public client
|
||||
// (60 req/hr), the same resolution the job routes use; an empty
|
||||
// token must never reach createGitHubClient, which would send
|
||||
// `auth: ""` and 401 on every call.
|
||||
octokit = decryptedToken
|
||||
? createGitHubClient(
|
||||
decryptedToken,
|
||||
@@ -311,7 +312,7 @@ async function recoverMirrorJob(job: any, remainingItemIds: string[]) {
|
||||
repoSource.username || undefined,
|
||||
resolveGitHubApiBaseUrl(repoSource.url)
|
||||
)
|
||||
: null;
|
||||
: createPublicGitHubClient(resolveGitHubApiBaseUrl(repoSource.url));
|
||||
} catch (error) {
|
||||
throw new Error(`Failed to create GitHub client: ${error instanceof Error ? error.message : String(error)}`);
|
||||
}
|
||||
|
||||
@@ -32,11 +32,12 @@ export const POST: APIRoute = async ({ request, locals }) => {
|
||||
|
||||
const config = configResult[0];
|
||||
|
||||
if (!config || !config.githubConfig.token) {
|
||||
// Scheduling a sync only needs the destination; see sync-repo.ts.
|
||||
if (!config || !config.giteaConfig?.token) {
|
||||
return new Response(
|
||||
JSON.stringify({
|
||||
success: false,
|
||||
error: "Config missing for the user or GitHub token not found.",
|
||||
error: "Config missing for the user or destination token not found.",
|
||||
repositories: [],
|
||||
} satisfies ScheduleSyncRepoResponse),
|
||||
{ status: 400, headers: { "Content-Type": "application/json" } }
|
||||
|
||||
@@ -49,9 +49,12 @@ export const POST: APIRoute = async ({ request, locals }) => {
|
||||
|
||||
const config = configResult[0];
|
||||
|
||||
if (!config || !config.githubConfig.token) {
|
||||
// Syncing only touches the destination: the source client is resolved
|
||||
// per repository further down the dispatch, and a public-only source has
|
||||
// no token at all. Requiring one here locked those accounts out of Sync.
|
||||
if (!config || !config.giteaConfig?.token) {
|
||||
return new Response(
|
||||
JSON.stringify({ error: "Config missing for the user or token." }),
|
||||
JSON.stringify({ error: "Config missing for the user or destination token." }),
|
||||
{ status: 400, headers: { "Content-Type": "application/json" } }
|
||||
);
|
||||
}
|
||||
|
||||
@@ -40,7 +40,7 @@ export const POST: APIRoute = async ({ request, locals }) => {
|
||||
// is the primary source). A source without a token still lists public
|
||||
// repositories through its provider. Configs written outside the sources
|
||||
// API get their first source row seeded here.
|
||||
const { listSources, ensureSourcesFromConfig, selectSameRunPinsToClear } = await import("@/lib/sources");
|
||||
const { listSources, ensureSourcesFromConfig, selectSameRunPinsToClear, decryptSourceToken } = await import("@/lib/sources");
|
||||
const { createSourceProviderFromSource } = await import("@/lib/source-providers");
|
||||
await ensureSourcesFromConfig(userId);
|
||||
const sources = (await listSources(userId)).filter((source) => source.enabled);
|
||||
@@ -70,6 +70,7 @@ export const POST: APIRoute = async ({ request, locals }) => {
|
||||
let totalSkippedDisabled = 0;
|
||||
const failedOrgNames: string[] = [];
|
||||
const failedSourceNames: string[] = [];
|
||||
const skippedPublicSourceNames: string[] = [];
|
||||
|
||||
// Organization pins this run set itself (inserted, or re-pinned on
|
||||
// recovery), by normalized name. A later source listing the same name
|
||||
@@ -79,6 +80,17 @@ export const POST: APIRoute = async ({ request, locals }) => {
|
||||
|
||||
for (const source of sources) {
|
||||
try {
|
||||
// A public-only (tokenless) source has no account to list from: the
|
||||
// personal and starred listings 401 on every call, which would count
|
||||
// the source as failed and, when it is the only one, answer 502. Its
|
||||
// organizations are added through the add-organization route and
|
||||
// re-listed by the scheduler, which skips these sources the same way.
|
||||
if (decryptSourceToken(source.token) === "") {
|
||||
console.log(`Skipping personal discovery for public-only source ${source.name} (${source.id})`);
|
||||
skippedPublicSourceNames.push(source.name);
|
||||
continue;
|
||||
}
|
||||
|
||||
const sourceProvider = createSourceProviderFromSource(source, { userId });
|
||||
const sourceLabel = SOURCE_PROVIDER_LABELS[sourceProvider.kind];
|
||||
|
||||
@@ -316,7 +328,8 @@ export const POST: APIRoute = async ({ request, locals }) => {
|
||||
}
|
||||
}
|
||||
|
||||
if (sources.length > 0 && failedSourceNames.length === sources.length) {
|
||||
const attemptedSourceCount = sources.length - skippedPublicSourceNames.length;
|
||||
if (attemptedSourceCount > 0 && failedSourceNames.length === attemptedSourceCount) {
|
||||
return createSecureErrorResponse(
|
||||
new Error("every enabled source failed to sync"),
|
||||
"source data sync",
|
||||
@@ -327,12 +340,16 @@ export const POST: APIRoute = async ({ request, locals }) => {
|
||||
return jsonResponse({
|
||||
data: {
|
||||
success: true,
|
||||
message: "Repositories and organizations synced successfully",
|
||||
message:
|
||||
attemptedSourceCount === 0 && skippedPublicSourceNames.length > 0
|
||||
? "Nothing to import: every connected source is public only, so it has no account to list repositories from. Public organizations are imported from the Organizations page."
|
||||
: "Repositories and organizations synced successfully",
|
||||
newRepositories: totalInsertedRepos,
|
||||
newOrganizations: totalInsertedOrgs,
|
||||
skippedDisabledRepositories: totalSkippedDisabled,
|
||||
failedOrgs: failedOrgNames,
|
||||
failedSources: failedSourceNames,
|
||||
skippedPublicSources: skippedPublicSourceNames,
|
||||
recoveredOrgs: totalRecoveredOrgs,
|
||||
},
|
||||
});
|
||||
|
||||
@@ -454,6 +454,51 @@ describe.skipIf(!isChild)("POST /api/sync/organization public add-org", () => {
|
||||
expect(insertedRepoRows).toHaveLength(0);
|
||||
});
|
||||
|
||||
test("force:true without a named source leaves the existing pin alone", async () => {
|
||||
const primary = makeSourceRow({
|
||||
provider: "github",
|
||||
url: "https://github.com",
|
||||
username: "octocat",
|
||||
token: "enc:gh-token",
|
||||
});
|
||||
const other = makeSourceRow({
|
||||
provider: "gitlab",
|
||||
url: "https://gitlab.com",
|
||||
username: "release-bot",
|
||||
token: "enc:gl-token",
|
||||
});
|
||||
sourceRows = [primary, other];
|
||||
orgRows = [
|
||||
{
|
||||
id: "org-1",
|
||||
userId: "user-1",
|
||||
name: "acme",
|
||||
normalizedName: "acme",
|
||||
membershipRole: "admin",
|
||||
sourceId: other.id,
|
||||
},
|
||||
{
|
||||
id: "org-2",
|
||||
userId: "user-1",
|
||||
name: "widgets",
|
||||
normalizedName: "widgets",
|
||||
membershipRole: "admin",
|
||||
sourceId: null,
|
||||
},
|
||||
];
|
||||
|
||||
const pinned = await postOrg({ org: "acme", role: "member", force: true });
|
||||
expect(pinned.status).toBe(200);
|
||||
expect(orgRows[0].sourceId).toBe(other.id);
|
||||
|
||||
const unpinned = await postOrg({ org: "widgets", role: "member", force: true });
|
||||
expect(unpinned.status).toBe(200);
|
||||
expect(orgRows[1].sourceId).toBeNull();
|
||||
|
||||
// Nothing was created to hold a new pin either.
|
||||
expect(sourceRows).toHaveLength(2);
|
||||
});
|
||||
|
||||
test("a rate-limited org metadata fetch (403) still imports with a fallback record", async () => {
|
||||
orgMetadataError = Object.assign(
|
||||
new Error("rate limit exceeded"),
|
||||
|
||||
@@ -161,7 +161,10 @@ export const POST: APIRoute = async ({ request, locals }) => {
|
||||
.set({
|
||||
membershipRole: role,
|
||||
normalizedName: normalizedOrg,
|
||||
...(source ? { sourceId: source.id } : {}),
|
||||
// Only a request that named a source (an explicit pin, or public
|
||||
// mode's provider) re-pins; a bare force re-add must leave the
|
||||
// organization's own pin, including "every source", alone.
|
||||
...(source && (sourceId || body.provider) ? { sourceId: source.id } : {}),
|
||||
updatedAt: new Date(),
|
||||
})
|
||||
.where(eq(organizations.id, existingOrg.id))
|
||||
|
||||
Reference in new issue
Block a user