Skip to content

Commit 10eb8df

Browse files
SantiagoDePoloniaSantiagoDePolonia
committed
fix(auth): preserve issued keys and skipped-route auth state
1 parent b486213 commit 10eb8df

7 files changed

Lines changed: 168 additions & 8 deletions

File tree

‎internal/admin/dashboard/static/js/modules/auth-keys.js‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,9 @@
3535
}
3636
this.authKeysAvailable = true;
3737
if (!this.handleFetchResponse(res, 'auth keys')) {
38-
this.authKeys = [];
38+
if (res.status !== 401) {
39+
this.authKeyError = await this._authKeyResponseMessage(res, 'Unable to load API keys.');
40+
}
3941
return;
4042
}
4143
const payload = await res.json();
@@ -50,20 +52,28 @@
5052
},
5153

5254
openAuthKeyForm() {
55+
if (this.authKeyFormSubmitting || this.authKeyFormOpen) {
56+
return;
57+
}
5358
this.authKeyFormOpen = true;
5459
this.authKeyError = '';
5560
this.authKeyNotice = '';
56-
this.authKeyIssuedValue = '';
57-
this.resetAuthKeyCopyFeedback();
58-
this.authKeyForm = this.defaultAuthKeyForm();
61+
if (!this.authKeyIssuedValue) {
62+
this.resetAuthKeyCopyFeedback();
63+
this.authKeyForm = this.defaultAuthKeyForm();
64+
}
5965
},
6066

6167
closeAuthKeyForm() {
68+
if (!this.authKeyFormOpen) {
69+
return;
70+
}
6271
this.authKeyFormOpen = false;
6372
this.authKeyError = '';
64-
this.authKeyIssuedValue = '';
6573
this.resetAuthKeyCopyFeedback();
66-
this.authKeyForm = this.defaultAuthKeyForm();
74+
if (!this.authKeyFormSubmitting && !this.authKeyIssuedValue) {
75+
this.authKeyForm = this.defaultAuthKeyForm();
76+
}
6777
},
6878

6979
clearAuthKeyCopyResetTimer() {
@@ -208,6 +218,8 @@
208218
}
209219
const issued = await res.json();
210220
this.authKeyIssuedValue = issued.value || '';
221+
this.authKeyFormOpen = true;
222+
this.resetAuthKeyCopyFeedback();
211223
this.authKeyForm = this.defaultAuthKeyForm();
212224
await this.fetchAuthKeys();
213225
} catch (e) {

‎internal/admin/dashboard/static/js/modules/auth-keys.test.js‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,3 +192,110 @@ test('copyAuthKeyValue falls back to document.execCommand when clipboard API is
192192
assert.equal(module.authKeyCopied, true);
193193
assert.equal(module.authKeyCopyError, false);
194194
});
195+
196+
test('fetchAuthKeys preserves existing rows and surfaces non-auth HTTP errors', async () => {
197+
const module = createAuthKeysModule({
198+
fetch: async () => ({
199+
status: 500,
200+
ok: false,
201+
statusText: 'Internal Server Error',
202+
async json() {
203+
return {
204+
error: {
205+
message: 'storage unavailable'
206+
}
207+
};
208+
}
209+
}),
210+
console: {
211+
error() {}
212+
}
213+
});
214+
215+
module.authKeys = [{ id: 'existing-key' }];
216+
module.headers = () => ({});
217+
module.handleFetchResponse = () => false;
218+
219+
await module.fetchAuthKeys();
220+
221+
assert.deepEqual(module.authKeys, [{ id: 'existing-key' }]);
222+
assert.equal(module.authKeyError, 'storage unavailable');
223+
});
224+
225+
test('fetchAuthKeys preserves existing rows on authentication failures handled by handleFetchResponse', async () => {
226+
const module = createAuthKeysModule({
227+
fetch: async () => ({
228+
status: 401,
229+
ok: false,
230+
statusText: 'Unauthorized'
231+
})
232+
});
233+
234+
module.authKeys = [{ id: 'existing-key' }];
235+
module.headers = () => ({});
236+
module.handleFetchResponse = (res) => {
237+
if (res.status === 401) {
238+
module.authError = true;
239+
module.needsAuth = true;
240+
}
241+
return false;
242+
};
243+
244+
await module.fetchAuthKeys();
245+
246+
assert.deepEqual(module.authKeys, [{ id: 'existing-key' }]);
247+
assert.equal(module.authKeyError, '');
248+
assert.equal(module.authError, true);
249+
assert.equal(module.needsAuth, true);
250+
});
251+
252+
test('openAuthKeyForm and closeAuthKeyForm preserve an issued key instead of clearing it', () => {
253+
const module = createAuthKeysModule();
254+
module.authKeyIssuedValue = 'sk_gom_once';
255+
256+
module.openAuthKeyForm();
257+
assert.equal(module.authKeyFormOpen, true);
258+
assert.equal(module.authKeyIssuedValue, 'sk_gom_once');
259+
260+
module.closeAuthKeyForm();
261+
assert.equal(module.authKeyFormOpen, false);
262+
assert.equal(module.authKeyIssuedValue, 'sk_gom_once');
263+
});
264+
265+
test('submitAuthKeyForm reopens the editor if issuance finishes after a manual close', async () => {
266+
let resolveResponse;
267+
const responsePromise = new Promise((resolve) => {
268+
resolveResponse = resolve;
269+
});
270+
const module = createAuthKeysModule({
271+
fetch: async () => responsePromise
272+
});
273+
274+
module.headers = () => ({ 'Content-Type': 'application/json' });
275+
module.fetchAuthKeys = async () => {};
276+
module.authKeyFormOpen = true;
277+
module.authKeyForm = {
278+
name: 'ci-deploy',
279+
description: '',
280+
expires_at: ''
281+
};
282+
283+
const submitPromise = module.submitAuthKeyForm();
284+
module.closeAuthKeyForm();
285+
286+
assert.equal(module.authKeyFormOpen, false);
287+
assert.equal(module.authKeyFormSubmitting, true);
288+
289+
resolveResponse({
290+
status: 201,
291+
async json() {
292+
return { value: 'sk_gom_async' };
293+
}
294+
});
295+
296+
await submitPromise;
297+
298+
assert.equal(module.authKeyFormOpen, true);
299+
assert.equal(module.authKeyIssuedValue, 'sk_gom_async');
300+
assert.equal(module.authKeyFormSubmitting, false);
301+
});

‎internal/admin/dashboard/static/js/modules/dashboard-layout.test.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,9 @@ test('auth key expirations render as a UTC date with the full UTC timestamp in t
9494

9595
assert.match(indexTemplate, /x-text="key\.expires_at \? formatDateUTC\(key\.expires_at\) : '\\u2014'"/);
9696
assert.match(indexTemplate, /:title="key\.expires_at \? formatTimestampUTC\(key\.expires_at\) : ''"/);
97+
assert.match(indexTemplate, /:disabled="authKeyFormSubmitting"/);
98+
assert.match(indexTemplate, /@click="if \(!authKeyFormSubmitting\) openAuthKeyForm\(\)"/);
99+
assert.match(indexTemplate, /x-show="authKeys\.length === 0 && !authKeysLoading && !authError && !authKeyError && authKeysAvailable"/);
97100
});
98101

99102
test('usage and audit pages reuse a shared pagination template', () => {

‎internal/admin/dashboard/templates/index.html‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,8 @@ <h2>API Keys</h2>
237237
<div class="page-header-controls">
238238
<button type="button" class="pagination-btn pagination-btn-primary"
239239
x-show="authKeysAvailable && !authError"
240-
@click="openAuthKeyForm()">Create API Key</button>
240+
:disabled="authKeyFormSubmitting"
241+
@click="if (!authKeyFormSubmitting) openAuthKeyForm()">Create API Key</button>
241242
</div>
242243
</div>
243244

@@ -360,7 +361,7 @@ <h3>Create API Key</h3>
360361
</div>
361362

362363
<p class="empty-state"
363-
x-show="authKeys.length === 0 && !authKeysLoading && !authError && authKeysAvailable">
364+
x-show="authKeys.length === 0 && !authKeysLoading && !authError && !authKeyError && authKeysAvailable">
364365
No API keys yet. Issue a key to get started.
365366
</p>
366367
</div>

‎internal/auditlog/middleware_auth_method_test.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,16 @@ func TestEnrichEntryWithAuthMethodTrimsAndValidatesAllowedValues(t *testing.T) {
2525
if entry.AuthMethod != AuthMethodMasterKey {
2626
t.Fatalf("entry auth method = %q, want %q", entry.AuthMethod, AuthMethodMasterKey)
2727
}
28+
29+
EnrichEntryWithAuthMethod(c, "no_key")
30+
if entry.AuthMethod != AuthMethodNoKey {
31+
t.Fatalf("entry auth method = %q, want %q", entry.AuthMethod, AuthMethodNoKey)
32+
}
33+
34+
EnrichEntryWithAuthMethod(c, "unknown")
35+
if entry.AuthMethod != "unknown" {
36+
t.Fatalf("entry auth method = %q, want %q", entry.AuthMethod, "unknown")
37+
}
2838
}
2939

3040
func TestEnrichEntryWithAuthMethodIgnoresBlankAndUnsupportedValues(t *testing.T) {

‎internal/server/auth.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,11 @@ func AuthMiddlewareWithAuthenticator(masterKey string, authenticator BearerToken
4444
if strings.HasSuffix(skipPath, "/*") {
4545
prefix := strings.TrimSuffix(skipPath, "*")
4646
if strings.HasPrefix(requestPath, prefix) {
47+
auditlog.EnrichEntryWithAuthMethod(c, auditlog.AuthMethodNoKey)
4748
return next(c)
4849
}
4950
} else if requestPath == skipPath {
51+
auditlog.EnrichEntryWithAuthMethod(c, auditlog.AuthMethodNoKey)
5052
return next(c)
5153
}
5254
}

‎internal/server/auth_test.go‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,31 @@ func TestAuthMiddleware_WildcardSkipPaths(t *testing.T) {
365365
}
366366
}
367367

368+
func TestAuthMiddleware_SkipPathEnrichesNoKeyAuditMethod(t *testing.T) {
369+
e := echo.New()
370+
handler := AuthMiddlewareWithAuthenticator("secret-key", nil, []string{"/health"})(func(c *echo.Context) error {
371+
entryVal := c.Get(string(auditlog.LogEntryKey))
372+
entry, ok := entryVal.(*auditlog.LogEntry)
373+
if !ok || entry == nil {
374+
t.Fatal("audit log entry missing from context")
375+
}
376+
if entry.AuthMethod != auditlog.AuthMethodNoKey {
377+
t.Fatalf("audit entry auth method = %q, want %q", entry.AuthMethod, auditlog.AuthMethodNoKey)
378+
}
379+
return c.String(http.StatusOK, "ok")
380+
})
381+
382+
req := httptest.NewRequest(http.MethodGet, "/health", nil)
383+
rec := httptest.NewRecorder()
384+
c := e.NewContext(req, rec)
385+
c.Set(string(auditlog.LogEntryKey), &auditlog.LogEntry{Data: &auditlog.LogData{}})
386+
387+
err := handler(c)
388+
require.NoError(t, err)
389+
assert.Equal(t, http.StatusOK, rec.Code)
390+
assert.Equal(t, "ok", rec.Body.String())
391+
}
392+
368393
func TestAuthMiddleware_ConstantTimeComparison(t *testing.T) {
369394
t.Run("constant-time comparison prevents timing attacks", func(t *testing.T) {
370395
// Test that the constant-time comparison works correctly for various inputs

0 commit comments

Comments
 (0)