Sign in

Search issue titles and bodies, change requests and comments. Private repositories you cannot read are not included.
vole
Clone

Read only, no credentials:

https://rodent.software/vole.git

Write over SSH with a registered key:

git clone ssh://git@rodent.software:2222/vole.git

A free and open-source forge that lives in your Git repo.

Name branches after the account, and labels after the display

474a812d199a109ba894124cd6f26be12c73285d · Keenan Nemetz · 16d ago · parent ade6107e 

Name branches after the account, and labels after the display

A contributor's folder was `refs/heads/contrib/<account-id>/`, which told a
reader nothing without decoding a ULID. It is now the account name,

    refs/heads/contrib/<name>/<branch>

and the name is therefore fixed at creation: renaming an account would strand
every ref under its old folder, so `vole admin user rename` is gone and the
users row itself — kept after retirement — is what stops a name being reissued.

What people see is now a separate, mutable display name, validated by the same
rules wherever it is set (CLI, remote admin, registration, `/account`) and shown
on pages, approvals, listings and refusals. Nothing is authorized by it and
nothing on disk is keyed by it: metadata authors stay account names because the
push boundary resolves them for ownership, and approvals stay account ids.

`user_names`, `Snapshot.Reserved` and `Account.Former` are dropped: with a fixed
name there are no former names to remember, and the UNIQUE column already does
the reserving. Opening an older roster adds the `display` column and drops the
alias table in place; contributor refs written under the old id namespace are
left where they are, readable, and landable by a maintainer.

forge/identity/identity_test.go +138−63

@@ -49,7 +49,7 @@ func mustCreate(t *testing.T, s *Store, name string) *Account {
49 49 func mustCreateBy(t *testing.T, s *Store, name, actor string) *Account {
50 50 t.Helper()
51 51 line, _ := pubLine(t)
52 a, err := s.Create(name, line, actor)
52 a, err := s.Create(name, "", line, actor)
53 53 if err != nil {
54 54 t.Fatalf("create %s: %v", name, err)
55 55 }
@@ -75,7 +75,7 @@ func TestCreateAccountWithKeyAndAddKey(t *testing.T) {
75 75 t.Parallel()
76 76 s := newStore(t)
77 77 line, wantFP := pubLine(t)
78 a, err := s.Create("keenan", line, "test")
78 a, err := s.Create("keenan", "", line, "test")
79 79 if err != nil {
80 80 t.Fatal(err)
81 81 }
@@ -111,7 +111,7 @@ func TestCreateRequiresAKeyAndDoesNotReserveTheName(t *testing.T) {
111 111 t.Parallel()
112 112 s := newStore(t)
113 113 for _, bad := range []string{"", "not an SSH key"} {
114 if _, err := s.Create("alice", bad, "test"); err == nil {
114 if _, err := s.Create("alice", "", bad, "test"); err == nil {
115 115 t.Fatalf("Create accepted key %q", bad)
116 116 }
117 117 }
@@ -119,7 +119,7 @@ func TestCreateRequiresAKeyAndDoesNotReserveTheName(t *testing.T) {
119 119 t.Fatal("a failed create left an account behind")
120 120 }
121 121 line, _ := pubLine(t)
122 a, err := s.Create("alice", line, "test")
122 a, err := s.Create("alice", "", line, "test")
123 123 if err != nil {
124 124 t.Fatalf("the failed attempts reserved the name: %v", err)
125 125 }
@@ -134,10 +134,10 @@ func TestCreateRejectsAKeyAlreadyRegistered(t *testing.T) {
134 134 t.Parallel()
135 135 s := newStore(t)
136 136 line, _ := pubLine(t)
137 if _, err := s.Create("alice", line, "test"); err != nil {
137 if _, err := s.Create("alice", "", line, "test"); err != nil {
138 138 t.Fatal(err)
139 139 }
140 if _, err := s.Create("bob", line, "test"); err == nil {
140 if _, err := s.Create("bob", "", line, "test"); err == nil {
141 141 t.Fatal("a registered key was accepted for a second account")
142 142 } else if !strings.Contains(err.Error(), "already registered") {
143 143 t.Fatalf("unclear error: %v", err)
@@ -146,7 +146,7 @@ func TestCreateRejectsAKeyAlreadyRegistered(t *testing.T) {
146 146 t.Fatal("the duplicate-key failure left an account behind")
147 147 }
148 148 fresh, _ := pubLine(t)
149 if _, err := s.Create("bob", fresh, "test"); err != nil {
149 if _, err := s.Create("bob", "", fresh, "test"); err != nil {
150 150 t.Fatalf("the duplicate-key failure reserved bob's name: %v", err)
151 151 }
152 152 }
@@ -155,7 +155,7 @@ func TestMultipleKeysPerAccount(t *testing.T) {
155 155 t.Parallel()
156 156 s := newStore(t)
157 157 line, _ := pubLine(t)
158 if _, err := s.Create("keenan", line, "test"); err != nil {
158 if _, err := s.Create("keenan", "", line, "test"); err != nil {
159 159 t.Fatal(err)
160 160 }
161 161 var fps []string
@@ -188,11 +188,11 @@ func TestDuplicateKeyAcrossAccountsRejected(t *testing.T) {
188 188 t.Parallel()
189 189 s := newStore(t)
190 190 line, _ := pubLine(t)
191 if _, err := s.Create("alice", line, "test"); err != nil {
191 if _, err := s.Create("alice", "", line, "test"); err != nil {
192 192 t.Fatal(err)
193 193 }
194 194 bobLine, _ := pubLine(t)
195 if _, err := s.Create("bob", bobLine, "test"); err != nil {
195 if _, err := s.Create("bob", "", bobLine, "test"); err != nil {
196 196 t.Fatal(err)
197 197 }
198 198 if _, err := s.AddKey("bob", line, "test"); err == nil {
@@ -260,7 +260,7 @@ func TestRemoveKey(t *testing.T) {
260 260 s := newStore(t)
261 261 keepLine, keepFP := pubLine(t)
262 262 dropLine, dropFP := pubLine(t)
263 if _, err := s.Create("keenan", keepLine, "test"); err != nil {
263 if _, err := s.Create("keenan", "", keepLine, "test"); err != nil {
264 264 t.Fatal(err)
265 265 }
266 266 if _, err := s.AddKey("keenan", dropLine, "test"); err != nil {
@@ -283,7 +283,7 @@ func TestRetireStopsAuthenticationAndReservesTheName(t *testing.T) {
283 283 t.Parallel()
284 284 s := newStore(t)
285 285 line, fp := pubLine(t)
286 a, err := s.Create("wilbur", line, "test")
286 a, err := s.Create("wilbur", "", line, "test")
287 287 if err != nil {
288 288 t.Fatal(err)
289 289 }
@@ -297,7 +297,7 @@ func TestRetireStopsAuthenticationAndReservesTheName(t *testing.T) {
297 297 t.Fatal("a retired name still resolves")
298 298 }
299 299 reissue, _ := pubLine(t)
300 if _, err := s.Create("wilbur", reissue, "test"); err == nil {
300 if _, err := s.Create("wilbur", "", reissue, "test"); err == nil {
301 301 t.Fatalf("the retired name was reissued to a new account")
302 302 } else if !strings.Contains(err.Error(), "cannot be reused") {
303 303 t.Fatalf("unclear error: %v", err)
@@ -308,48 +308,82 @@ func TestRetireStopsAuthenticationAndReservesTheName(t *testing.T) {
308 308 }
309 309 }
310 310
311 // TestRenameKeepsTheIdAndReservesBothNames: the id is what ACLs and approvals
312 // hold, so a rename must not disturb it, and neither name may be reused.
313 func TestRenameKeepsTheIdAndReservesBothNames(t *testing.T) {
311 // TestDisplayNameIsALabelAndTheNameIsFixed: the display name is what people
312 // read, and it may change; the account name is what refs and metadata files were
313 // written with, so it may not.
314 func TestDisplayNameIsALabelAndTheNameIsFixed(t *testing.T) {
314 315 t.Parallel()
315 316 s := newStore(t)
316 317 line, fp := pubLine(t)
317 a, err := s.Create("wilbur", line, "test")
318 a, err := s.Create("wilbur", "Wilbur Wright", line, "test")
318 319 if err != nil {
319 320 t.Fatal(err)
320 321 }
321 renamed, err := s.Rename("wilbur", "robert", "test")
322 if a.Label() != "Wilbur Wright" {
323 t.Fatalf("Label = %q, want the display name", a.Label())
324 }
325 // The display name is not an identity: only the fixed name resolves.
326 if _, ok := s.ResolveID("Wilbur Wright"); ok {
327 t.Fatal("a display name resolved as an account name")
328 }
329 if got, ok := s.ResolveID("wilbur"); !ok || got != a.ID {
330 t.Fatalf("ResolveID(wilbur) = %q,%v; want %s", got, ok, a.ID)
331 }
332
333 changed, err := s.SetDisplay("wilbur", "Wilbur", "test")
322 334 if err != nil {
323 335 t.Fatal(err)
324 336 }
325 if renamed.ID != a.ID {
326 t.Fatalf("rename changed the id: %s -> %s", a.ID, renamed.ID)
337 if changed.ID != a.ID || changed.Name != "wilbur" {
338 t.Fatalf("SetDisplay touched the name: %+v", changed)
339 }
340 if changed.Label() != "Wilbur" {
341 t.Fatalf("Label after SetDisplay = %q", changed.Label())
327 342 }
328 343 if got, _, ok := s.Lookup(fp); !ok || got != a.ID {
329 t.Fatalf("Lookup after rename = %q,%v; want %s", got, ok, a.ID)
344 t.Fatalf("Lookup after SetDisplay = %q,%v; want %s", got, ok, a.ID)
330 345 }
331 if _, ok := s.ResolveID("wilbur"); ok {
332 t.Fatal("the old name still resolves to an active account")
346
347 // Clearing the display falls back to the name.
348 cleared, err := s.SetDisplay(a.ID, "", "test")
349 if err != nil {
350 t.Fatal(err)
333 351 }
334 if got, ok := s.ResolveID("robert"); !ok || got != a.ID {
335 t.Fatalf("new name does not resolve: %q,%v", got, ok)
352 if cleared.Display != "" || cleared.Label() != "wilbur" {
353 t.Fatalf("cleared display = %q, label %q", cleared.Display, cleared.Label())
336 354 }
337 // Both names stay reserved.
355
356 // The name cannot be taken by a second account, and the API that used to
357 // move it is gone.
338 358 reissue, _ := pubLine(t)
339 if _, err := s.Create("wilbur", reissue, "test"); err == nil {
340 t.Fatal("the former name was handed to a new account")
359 if _, err := s.Create("wilbur", "", reissue, "test"); err == nil {
360 t.Fatal("a second account took the name")
341 361 }
342 otherLine, _ := pubLine(t)
343 other, err := s.Create("dana", otherLine, "test")
344 if err != nil {
362 if _, err := s.SetDisplay("nobody", "x", "test"); err == nil {
363 t.Fatal("SetDisplay accepted an unknown account")
364 }
365 }
366
367 func TestInvalidDisplaysRefused(t *testing.T) {
368 t.Parallel()
369 s := newStore(t)
370 line, _ := pubLine(t)
371 if _, err := s.Create("keenan", "", line, "test"); err != nil {
345 372 t.Fatal(err)
346 373 }
347 if _, err := s.Rename(other.ID, "wilbur", "test"); err == nil {
348 t.Fatal("another account took the former name")
374 for _, display := range []string{"two\nlines", "tab\there", strings.Repeat("x", maxDisplayRunes+1)} {
375 if err := ValidateDisplay(display); err == nil {
376 t.Fatalf("ValidateDisplay accepted %q", display)
377 }
378 if _, err := s.SetDisplay("keenan", display, "test"); err == nil {
379 t.Fatalf("SetDisplay accepted %q", display)
380 }
349 381 }
350 // The old name still identifies the same account for display.
351 if got, err := s.Find("wilbur"); err != nil || got.ID != a.ID {
352 t.Fatalf("Find(former name) = %v, %v", got, err)
382 // Spaces, capitals and non-Latin scripts are exactly what a label is for.
383 for _, display := range []string{"Keenan O'Brien", "田中 健", " padded "} {
384 if _, err := s.SetDisplay("keenan", display, "test"); err != nil {
385 t.Fatalf("SetDisplay(%q): %v", display, err)
386 }
353 387 }
354 388 }
355 389
@@ -358,7 +392,7 @@ func TestInvalidNamesRefused(t *testing.T) {
358 392 s := newStore(t)
359 393 line, _ := pubLine(t)
360 394 for _, name := range []string{"Keenan", "has space", "a", "with/slash", "trailing."} {
361 if _, err := s.Create(name, line, "test"); err == nil {
395 if _, err := s.Create(name, "", line, "test"); err == nil {
362 396 t.Fatalf("%q accepted as a name", name)
363 397 }
364 398 }
@@ -374,7 +408,7 @@ func TestConcurrentAddsAllLand(t *testing.T) {
374 408 for i := range lines {
375 409 lines[i], _ = pubLine(t)
376 410 }
377 if _, err := s.Create("keenan", lines[0], "test"); err != nil {
411 if _, err := s.Create("keenan", "", lines[0], "test"); err != nil {
378 412 t.Fatal(err)
379 413 }
380 414 var wg sync.WaitGroup
@@ -404,17 +438,17 @@ func TestAuditLogRecordsChanges(t *testing.T) {
404 438 t.Parallel()
405 439 s := newStore(t)
406 440 line, fp := pubLine(t)
407 if _, err := s.Create("keenan", line, "test"); err != nil {
441 if _, err := s.Create("keenan", "", line, "test"); err != nil {
408 442 t.Fatal(err)
409 443 }
410 444 second, secondFP := pubLine(t)
411 445 if _, err := s.AddKey("keenan", second, "test"); err != nil {
412 446 t.Fatal(err)
413 447 }
414 if _, err := s.Rename("keenan", "robert", "test"); err != nil {
448 if _, err := s.SetDisplay("keenan", "Keenan", "test"); err != nil {
415 449 t.Fatal(err)
416 450 }
417 if err := s.RemoveKey("robert", secondFP, "test"); err != nil {
451 if err := s.RemoveKey("keenan", secondFP, "test"); err != nil {
418 452 t.Fatal(err)
419 453 }
420 454 data, err := os.ReadFile(s.AuditPath)
@@ -422,13 +456,69 @@ func TestAuditLogRecordsChanges(t *testing.T) {
422 456 t.Fatal(err)
423 457 }
424 458 log := string(data)
425 for _, want := range []string{"identity.create", "identity.add-key", "identity.rename", "identity.remove-key", fp, secondFP} {
459 for _, want := range []string{"identity.create", "identity.add-key", "identity.display", "identity.remove-key", fp, secondFP} {
426 460 if !strings.Contains(log, want) {
427 461 t.Fatalf("audit log missing %q:\n%s", want, log)
428 462 }
429 463 }
430 464 }
431 465
466 // TestRosterUpgradesFromTheAliasTable: rosters written before names were fixed
467 // kept a user_names alias table and no display column. Opening one must add the
468 // column, keep every account readable, and drop the alias table rather than
469 // leave a list of former names nothing resolves.
470 func TestRosterUpgradesFromTheAliasTable(t *testing.T) {
471 t.Parallel()
472 path := filepath.Join(t.TempDir(), "users.db")
473 db, err := openDB(path)
474 if err != nil {
475 t.Fatal(err)
476 }
477 if _, err := db.Exec(`
478 CREATE TABLE users (
479 id TEXT PRIMARY KEY,
480 name TEXT NOT NULL UNIQUE,
481 status TEXT NOT NULL DEFAULT 'active' CHECK (status IN ('active','retired')),
482 created INTEGER NOT NULL
483 );
484 CREATE TABLE user_names (
485 name TEXT PRIMARY KEY,
486 user_id TEXT NOT NULL REFERENCES users(id) ON DELETE CASCADE
487 );
488 INSERT INTO users(id, name, status, created) VALUES('01M5JR2JK29KKMAA1T9MDNTPBP', 'wilbur', 'active', 0);
489 INSERT INTO user_names(name, user_id) VALUES('wilbur', '01M5JR2JK29KKMAA1T9MDNTPBP');
490 INSERT INTO user_names(name, user_id) VALUES('robert', '01M5JR2JK29KKMAA1T9MDNTPBP');`); err != nil {
491 t.Fatal(err)
492 }
493 if err := db.Close(); err != nil {
494 t.Fatal(err)
495 }
496
497 s, err := Open(path)
498 if err != nil {
499 t.Fatal(err)
500 }
501 defer s.Close()
502 a, err := s.Find("wilbur")
503 if err != nil {
504 t.Fatal(err)
505 }
506 if a.Display != "" || a.Label() != "wilbur" {
507 t.Fatalf("upgraded account display = %q label = %q", a.Display, a.Label())
508 }
509 // The former name was only ever an alias, so it no longer resolves.
510 if _, err := s.Find("robert"); err == nil {
511 t.Fatal("an entry from the dropped alias table still resolves")
512 }
513 var n int
514 if err := s.db.QueryRow(`SELECT COUNT(*) FROM sqlite_master WHERE name='user_names'`).Scan(&n); err != nil {
515 t.Fatal(err)
516 }
517 if n != 0 {
518 t.Fatal("the user_names table survived the upgrade")
519 }
520 }
521
432 522 // TestRosterIsOneDatabaseFile proves the backup story: the roster is a single
433 523 // SQLite file, and Backup takes a consistent copy of it without a system repo or
434 524 // a directory of key files beside it.
@@ -436,13 +526,10 @@ func TestRosterIsOneDatabaseFile(t *testing.T) {
436 526 t.Parallel()
437 527 s := newStore(t)
438 528 line, fp := pubLine(t)
439 a, err := s.Create("keenan", line, "test")
529 a, err := s.Create("keenan", "Keenan O'Brien", line, "test")
440 530 if err != nil {
441 531 t.Fatal(err)
442 532 }
443 if _, err := s.Rename(a.ID, "robert", "test"); err != nil {
444 t.Fatal(err)
445 }
446 533
447 534 copied := filepath.Join(t.TempDir(), "users.db")
448 535 if err := s.Backup(copied); err != nil {
@@ -453,14 +540,14 @@ func TestRosterIsOneDatabaseFile(t *testing.T) {
453 540 t.Fatal(err)
454 541 }
455 542 defer other.Close()
456 if got, ok := other.ResolveID("robert"); !ok || got != a.ID {
457 t.Fatalf("copied roster resolves robert to %q,%v; want %s", got, ok, a.ID)
543 if got, ok := other.ResolveID("keenan"); !ok || got != a.ID {
544 t.Fatalf("copied roster resolves keenan to %q,%v; want %s", got, ok, a.ID)
458 545 }
459 546 if _, _, ok := other.Lookup(fp); !ok {
460 547 t.Fatal("copied roster lost the key")
461 548 }
462 if want := []string{"keenan"}; !equal(other.Snapshot().ByID[a.ID].Former, want) {
463 t.Fatalf("copied roster former names = %v, want %v", other.Snapshot().ByID[a.ID].Former, want)
549 if got := other.Label(a.ID); got != "Keenan O'Brien" {
550 t.Fatalf("copied roster display = %q", got)
464 551 }
465 552
466 553 for _, absent := range []string{"system.git", "trusted_keys"} {
@@ -470,18 +557,6 @@ func TestRosterIsOneDatabaseFile(t *testing.T) {
470 557 }
471 558 }
472 559
473 func equal(got, want []string) bool {
474 if len(got) != len(want) {
475 return false
476 }
477 for i := range got {
478 if got[i] != want[i] {
479 return false
480 }
481 }
482 return true
483 }
484
485 560 func TestInstanceAdminsAreExplicitAndAudited(t *testing.T) {
486 561 t.Parallel()
487 562 s := newStore(t)
@@ -506,7 +581,7 @@ func TestInstanceAdminsAreExplicitAndAudited(t *testing.T) {
506 581 t.Fatalf("admin audit line does not name the actor:\n%s", log)
507 582 }
508 583 bobKey, _ := pubLine(t)
509 if _, err := s.Create("bob", bobKey, "operator"); err != nil {
584 if _, err := s.Create("bob", "", bobKey, "operator"); err != nil {
510 585 t.Fatal(err)
511 586 }
512 587 if _, err := s.SetAdmin("bob", true, "operator"); err != nil {