Skip to content

Commit 433c67f

Browse files
committed
Fix potential issue where users' access token gets expired and they lose ability to push their files. Also fix issue where some organization users might run into issues if they don't have the right permissions set.
1 parent 6407ea4 commit 433c67f

14 files changed

Lines changed: 350 additions & 39 deletions

apps/adr-manager/src/App.vue

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
<template>
22
<router-view />
3+
<DialogReconnect />
34
</template>
45

5-
<script setup lang="ts"></script>
6+
<script setup lang="ts">
7+
import DialogReconnect from "@/components/DialogReconnect.vue";
8+
</script>

apps/adr-manager/src/components/DialogCommit.vue

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<template>
2-
<BaseDialog v-model="show" title="Commit &amp; Push" icon="publish" :width="700">
2+
<BaseDialog v-model="show" title="Commit & Push" icon="publish" :width="700">
33
<div class="step-head">
44
<span class="mdi mdi-file" aria-hidden="true"></span>
55
<span class="step-title">Select files</span>
@@ -73,7 +73,7 @@
7373
:disabled="commitMessage === '' || !anyFileSelected"
7474
@click="onCommit"
7575
>
76-
Commit &amp; Push
76+
Commit & Push
7777
</button>
7878
<button type="button" class="btn btn-text-error" @click="show = false">Cancel</button>
7979
</template>
@@ -88,7 +88,7 @@ import AutoGrowTextarea from "./AutoGrowTextarea.vue";
8888
import BaseDialog from "./BaseDialog.vue";
8989
import LoadingOverlay from "./LoadingOverlay.vue";
9090
import { pushSelectedFiles } from "@/composables/useCommitPush";
91-
import { getActiveProvider } from "@/plugins/git";
91+
import { describeGitError, getActiveProvider } from "@/plugins/git";
9292
import { store } from "@/plugins/store";
9393
import { useAlert } from "@/composables/useAlert";
9494
import { useToast } from "@/composables/useToast";
@@ -218,12 +218,7 @@ async function push(): Promise<void> {
218218
alert("Successfully pushed", "Success", "success");
219219
} catch (error) {
220220
console.error(error);
221-
alert(
222-
"Error during pushing. Your changes were not pushed. Please try again later. \nError code: " +
223-
String(error),
224-
"Error",
225-
"error"
226-
);
221+
alert(`Your changes were not pushed. ${describeGitError(error)}`, "Error", "error");
227222
} finally {
228223
loading.value = false;
229224
}
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
<template>
2+
<BaseDialog
3+
:model-value="reconnectVisible"
4+
title="Session expired"
5+
icon="key-alert"
6+
:width="440"
7+
@update:model-value="cancel"
8+
>
9+
<p>Your GitHub session expired. Reconnect to pick up where you left off.</p>
10+
<template #actions>
11+
<button type="button" class="btn btn-text-success" :disabled="connecting" @click="reconnect">
12+
{{ connecting ? "Reconnecting…" : "Reconnect to GitHub" }}
13+
</button>
14+
<button type="button" class="btn btn-text-error" :disabled="connecting" @click="cancel">Cancel</button>
15+
</template>
16+
</BaseDialog>
17+
</template>
18+
19+
<script setup lang="ts">
20+
import { ref, watch } from "vue";
21+
import BaseDialog from "./BaseDialog.vue";
22+
import { reconnectVisible, settleReauth } from "@/plugins/git/providers/github/reauth";
23+
import { prewarmGitHubAuth, signInWithGitHubPopup } from "@/plugins/git/providers/github/auth";
24+
import { useToast } from "@/composables/useToast";
25+
26+
const { showErrorToast } = useToast();
27+
const connecting = ref(false);
28+
29+
// Warm the Firebase modules while the dialog is up so the popup opens within the click gesture.
30+
watch(reconnectVisible, (visible) => {
31+
if (visible) {
32+
void prewarmGitHubAuth();
33+
}
34+
});
35+
36+
async function reconnect(): Promise<void> {
37+
connecting.value = true;
38+
try {
39+
await signInWithGitHubPopup();
40+
settleReauth(true);
41+
} catch (error) {
42+
console.error(error);
43+
showErrorToast("Could not reconnect to GitHub. Please try again.");
44+
settleReauth(false);
45+
} finally {
46+
connecting.value = false;
47+
}
48+
}
49+
50+
function cancel(): void {
51+
settleReauth(false);
52+
}
53+
</script>

apps/adr-manager/src/plugins/git/errors.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,12 @@ export function describeGitError(error: unknown): string {
1111
}
1212
const status = error.response.status;
1313
if (status === 401) {
14-
return "Your session has expired. Please sign out and sign in again.";
14+
return "Your session has expired. Reconnect when prompted, or sign out and sign in again.";
1515
}
1616
if (status === 403) {
17+
if (error.response.headers["x-github-sso"]) {
18+
return "This organization requires SSO authorization for your GitHub token. Authorize it in your GitHub account settings, then retry.";
19+
}
1720
return error.response.headers["x-ratelimit-remaining"] === "0"
1821
? "API rate limit reached. Please try again later."
1922
: "You don't have permission to access this resource.";

apps/adr-manager/src/plugins/git/providers/github/api.ts

Lines changed: 47 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
import axios from "axios";
2+
import type { AxiosInstance, InternalAxiosRequestConfig } from "axios";
3+
import { lsGet } from "@/plugins/storage";
24
import { request } from "../../request";
5+
import { requestReauth } from "./reauth";
36
import type { Branch, CommitAuthor, RepoPage, RepoSummary, UserInfo } from "@/types/git";
47
import type {
58
GitHubBranch,
@@ -16,6 +19,39 @@ import type {
1619
const BASE_URL_USER = "https://api.github.com/user";
1720
const BASE_URL_REPO = "https://api.github.com/repos";
1821

22+
let client: AxiosInstance | null = null;
23+
24+
type RetriableConfig = InternalAxiosRequestConfig & { _retried?: boolean };
25+
26+
/** Injects the stored token per request and, after a 401, prompts for a fresh one and retries once. */
27+
export function attachGitHubInterceptors(http: AxiosInstance): void {
28+
http.interceptors.request.use((config) => {
29+
const token = lsGet("authId");
30+
if (token) {
31+
config.headers.Authorization = `Bearer ${token}`;
32+
}
33+
return config;
34+
});
35+
http.interceptors.response.use(undefined, async (error: unknown) => {
36+
if (axios.isAxiosError(error) && error.response?.status === 401 && error.config) {
37+
const config = error.config as RetriableConfig;
38+
if (!config._retried && (await requestReauth())) {
39+
config._retried = true;
40+
return http.request(config);
41+
}
42+
}
43+
throw error;
44+
});
45+
}
46+
47+
function githubHttp(): AxiosInstance {
48+
if (!client) {
49+
client = axios.create();
50+
attachGitHubInterceptors(client);
51+
}
52+
return client;
53+
}
54+
1955
function toRepoSummary(repo: GitHubRepoSummary): RepoSummary {
2056
return {
2157
fullName: repo.full_name,
@@ -26,7 +62,7 @@ function toRepoSummary(repo: GitHubRepoSummary): RepoSummary {
2662
}
2763

2864
export async function listRepositories(page: number, perPage: number): Promise<RepoPage> {
29-
const response = await axios.get<GitHubRepoSummary[]>(
65+
const response = await githubHttp().get<GitHubRepoSummary[]>(
3066
`${BASE_URL_USER}/repos?sort=updated&direction=desc&page=${page}&per_page=${perPage}`
3167
);
3268
return {
@@ -69,13 +105,13 @@ export async function searchRepositories(query: string, maxResults: number): Pro
69105
}
70106

71107
export async function listBranches(repoFullName: string): Promise<Branch[]> {
72-
const response = await axios.get<GitHubBranch[]>(`${BASE_URL_REPO}/${repoFullName}/branches?per_page=999`);
108+
const response = await githubHttp().get<GitHubBranch[]>(`${BASE_URL_REPO}/${repoFullName}/branches?per_page=999`);
73109
return response.data;
74110
}
75111

76112
export async function listFiles(repoFullName: string, branch: string): Promise<string[]> {
77113
try {
78-
const response = await axios.get<GitHubFileTree>(
114+
const response = await githubHttp().get<GitHubFileTree>(
79115
`${BASE_URL_REPO}/${repoFullName}/git/trees/${branch}?recursive=1`
80116
);
81117
return response.data.tree.filter((entry) => entry.type === "blob").map((entry) => entry.path);
@@ -90,7 +126,7 @@ export async function listFiles(repoFullName: string, branch: string): Promise<s
90126

91127
export async function readFile(repoFullName: string, branch: string, filePath: string): Promise<string | undefined> {
92128
const content = await request(
93-
axios.get<GitHubContent>(`${BASE_URL_REPO}/${repoFullName}/contents/${filePath}?ref=${branch}`)
129+
githubHttp().get<GitHubContent>(`${BASE_URL_REPO}/${repoFullName}/contents/${filePath}?ref=${branch}`)
94130
);
95131
return content ? decodeUnicode(content.content) : undefined;
96132
}
@@ -106,23 +142,23 @@ function decodeUnicode(str: string): string {
106142
}
107143

108144
export async function getUser(): Promise<UserInfo | undefined> {
109-
const user = await request(axios.get<GitHubUser>(BASE_URL_USER));
145+
const user = await request(githubHttp().get<GitHubUser>(BASE_URL_USER));
110146
if (!user) {
111147
return undefined;
112148
}
113-
const emails = await request(axios.get<GitHubEmail[]>(`${BASE_URL_USER}/public_emails`));
149+
const emails = await request(githubHttp().get<GitHubEmail[]>(`${BASE_URL_USER}/public_emails`));
114150
const publicEmail = (emails?.find((entry) => entry.primary) ?? emails?.[0])?.email;
115151
const email = publicEmail ?? user.email ?? `${user.id}+${user.login}@users.noreply.github.com`;
116152
return { username: user.login, displayName: user.name ?? user.login, email };
117153
}
118154

119155
export async function getLastCommit(repoFullName: string, branch: string): Promise<GitHubBranch | undefined> {
120-
return request(axios.get<GitHubBranch>(`${BASE_URL_REPO}/${repoFullName}/branches/${branch}`));
156+
return request(githubHttp().get<GitHubBranch>(`${BASE_URL_REPO}/${repoFullName}/branches/${branch}`));
121157
}
122158

123159
export async function createBlob(repoFullName: string, content: string): Promise<GitHubShaResponse | undefined> {
124160
return request(
125-
axios.post<GitHubShaResponse>(`${BASE_URL_REPO}/${repoFullName}/git/blobs`, {
161+
githubHttp().post<GitHubShaResponse>(`${BASE_URL_REPO}/${repoFullName}/git/blobs`, {
126162
content,
127163
encoding: "utf-8"
128164
})
@@ -135,7 +171,7 @@ export async function createTree(
135171
tree: GitHubTreeInput[]
136172
): Promise<GitHubShaResponse | undefined> {
137173
return request(
138-
axios.post<GitHubShaResponse>(`${BASE_URL_REPO}/${repoFullName}/git/trees`, {
174+
githubHttp().post<GitHubShaResponse>(`${BASE_URL_REPO}/${repoFullName}/git/trees`, {
139175
base_tree: baseTreeSha,
140176
tree
141177
})
@@ -150,7 +186,7 @@ export async function createCommit(
150186
treeSha: string
151187
): Promise<GitHubShaResponse | undefined> {
152188
return request(
153-
axios.post<GitHubShaResponse>(`${BASE_URL_REPO}/${repoFullName}/git/commits`, {
189+
githubHttp().post<GitHubShaResponse>(`${BASE_URL_REPO}/${repoFullName}/git/commits`, {
154190
message,
155191
author,
156192
parents: [parentSha],
@@ -165,7 +201,7 @@ export async function updateBranchRef(
165201
commitSha: string
166202
): Promise<GitHubRef | undefined> {
167203
return request(
168-
axios.post<GitHubRef>(`${BASE_URL_REPO}/${repoFullName}/git/refs/heads/${branch}`, {
204+
githubHttp().post<GitHubRef>(`${BASE_URL_REPO}/${repoFullName}/git/refs/heads/${branch}`, {
169205
ref: "refs/heads/" + branch,
170206
sha: commitSha
171207
})
Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,16 @@
1-
import axios from "axios";
2-
import { lsGet, lsRemove, lsSet } from "@/plugins/storage";
1+
import { lsRemove, lsSet } from "@/plugins/storage";
32

43
const OAUTH_SCOPES = "repo read:user gist workflow read:org";
54

5+
const loadFirebase = () => Promise.all([import("firebase/auth"), import("@/plugins/firebase/client")]);
6+
67
/**
78
* Runs the Firebase GitHub popup flow and persists the resulting token.
89
* Firebase is imported lazily so the GitLab path (and unit tests) never load it.
910
*/
1011
export async function signInWithGitHubPopup(): Promise<void> {
1112
const [{ signInWithPopup, GithubAuthProvider, getAdditionalUserInfo }, { auth, GithubProvider }] =
12-
await Promise.all([import("firebase/auth"), import("@/plugins/firebase/client")]);
13+
await loadFirebase();
1314
GithubProvider.addScope(OAUTH_SCOPES);
1415
const result = await signInWithPopup(auth, GithubProvider);
1516
const credential = GithubAuthProvider.credentialFromResult(result);
@@ -18,20 +19,16 @@ export async function signInWithGitHubPopup(): Promise<void> {
1819
}
1920
lsSet("authId", credential.accessToken);
2021
lsSet("user", getAdditionalUserInfo(result)?.username ?? "");
21-
applyAuthHeader();
2222
}
2323

24-
export function applyAuthHeader(): void {
25-
const token = lsGet("authId");
26-
if (token) {
27-
axios.defaults.headers.common["Authorization"] = `Bearer ${token}`;
28-
}
24+
/** Loads Firebase ahead of a popup so the window opens within the user's click gesture. */
25+
export function prewarmGitHubAuth(): Promise<unknown> {
26+
return loadFirebase();
2927
}
3028

3129
export async function clearSession(): Promise<void> {
3230
lsRemove("authId");
3331
lsRemove("user");
34-
delete axios.defaults.headers.common["Authorization"];
35-
const [{ signOut }, { auth }] = await Promise.all([import("firebase/auth"), import("@/plugins/firebase/client")]);
32+
const [{ signOut }, { auth }] = await loadFirebase();
3633
await signOut(auth);
3734
}

apps/adr-manager/src/plugins/git/providers/github/index.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import {
1212
searchRepositories,
1313
updateBranchRef
1414
} from "./api";
15-
import { applyAuthHeader, clearSession, signInWithGitHubPopup } from "./auth";
15+
import { clearSession, signInWithGitHubPopup } from "./auth";
1616
import { encodeFilePath } from "../../paths";
1717
import type { GitProvider } from "../../provider";
1818
import type { GitHubTreeInput } from "./types";
@@ -42,7 +42,7 @@ export const githubProvider: GitProvider = {
4242
},
4343

4444
restoreSession() {
45-
applyAuthHeader();
45+
// Tokens are injected per request by the auth interceptor; nothing to apply globally.
4646
},
4747

4848
getUser,
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import { ref } from "vue";
2+
3+
export const reconnectVisible = ref(false);
4+
5+
let pending: Promise<boolean> | null = null;
6+
let resolvePending: ((ok: boolean) => void) | null = null;
7+
8+
/** Single-flight: concurrent 401s share one reconnect prompt. */
9+
export function requestReauth(): Promise<boolean> {
10+
if (!pending) {
11+
pending = new Promise<boolean>((resolve) => {
12+
resolvePending = resolve;
13+
});
14+
reconnectVisible.value = true;
15+
}
16+
return pending;
17+
}
18+
19+
export function settleReauth(ok: boolean): void {
20+
reconnectVisible.value = false;
21+
resolvePending?.(ok);
22+
pending = null;
23+
resolvePending = null;
24+
}

apps/adr-manager/tests/git-errors.test.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@ test("a network failure without response suggests checking the connection", () =
1212
});
1313

1414
test("a 401 reads as an expired session", () => {
15-
expect(describeGitError(axiosError(401))).toBe("Your session has expired. Please sign out and sign in again.");
15+
expect(describeGitError(axiosError(401))).toBe(
16+
"Your session has expired. Reconnect when prompted, or sign out and sign in again."
17+
);
1618
});
1719

1820
test("a 403 with exhausted rate limit reads as rate limiting", () => {
@@ -25,6 +27,12 @@ test("a plain 403 reads as missing permission", () => {
2527
expect(describeGitError(axiosError(403))).toBe("You don't have permission to access this resource.");
2628
});
2729

30+
test("a 403 from organization SSO explains the authorization step", () => {
31+
expect(describeGitError(axiosError(403, { "x-github-sso": "required; organizations=12345" }))).toBe(
32+
"This organization requires SSO authorization for your GitHub token. Authorize it in your GitHub account settings, then retry."
33+
);
34+
});
35+
2836
test("a 404 mentions a deleted or renamed repository or branch", () => {
2937
expect(describeGitError(axiosError(404))).toBe(
3038
"Not found. The repository or branch may have been deleted or renamed."

apps/adr-manager/tests/github-list-repositories.test.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,14 @@ import { listRepositories } from "@/plugins/git/providers/github/api";
33

44
vi.mock("axios", async (importOriginal) => {
55
const actual = await importOriginal<typeof import("axios")>();
6-
return { ...actual, default: { ...actual.default, get: vi.fn() } };
6+
const get = vi.fn();
7+
const instance = {
8+
get,
9+
post: vi.fn(),
10+
request: vi.fn(),
11+
interceptors: { request: { use: vi.fn() }, response: { use: vi.fn() } }
12+
};
13+
return { ...actual, default: { ...actual.default, get, create: () => instance } };
714
});
815

916
const get = vi.mocked(axios.get);

0 commit comments

Comments
 (0)