git-pr
created pr with
111.1
cmds
checkout latest patchset:
ssh pr.pico.sh print 111 | git am -3checkout any patchset in a patch request:
ssh pr.pico.sh print 111.[rev] | git am -3add changes to patch request:
git format-patch main --stdout | ssh pr.pico.sh pr add 111
Patchset
111.1
refactor: require user to create an account
Eric Bower
2026-02-23T16:40:18ZWe have noticed that users are accidentally creating accounts because we automatically create an account for any operation. So instead we are going to require a one-line remote cli command to register the account first we can help users figure out they are using the wrong pubkey.
Semantic diff summary
2 added,
12 modified,
0 signature changed,
1 removed
across 3 analyzed files
+34
-15
cli.go
#
| ... | ... | @@ -13,6 +13,10 @@ import ( | |
| 13 | 13 | "github.com/urfave/cli/v2" | |
| 14 | 14 | ) | |
| 15 | 15 | ||
| 16 | + | func errNotExist(host, pubkey string) error { | |
| 17 | + | return fmt.Errorf("User does not exist, run `ssh <username>@%s register` to create an account\nPubkey: %s", host, pubkey) | |
| 18 | + | } | |
| 19 | + | ||
| 16 | 20 | func NewTabWriter(out io.Writer) *tabwriter.Writer { | |
| 17 | 21 | return tabwriter.NewWriter(out, 0, 0, 1, ' ', tabwriter.TabIndent) | |
| 18 | 22 | } |
| ... | ... | @@ -237,7 +241,7 @@ To get started, submit a new patch request: | |
| 237 | 241 | pubkey := be.Pubkey(sesh.PublicKey()) | |
| 238 | 242 | user, err := pr.GetUserByPubkey(pubkey) | |
| 239 | 243 | if err != nil { | |
| 240 | - | return err | |
| 244 | + | return errNotExist(be.Cfg.Host, pubkey) | |
| 241 | 245 | } | |
| 242 | 246 | isPubkey := cCtx.Bool("pubkey") | |
| 243 | 247 | prID := cCtx.Int64("pr") |
| ... | ... | @@ -290,6 +294,21 @@ To get started, submit a new patch request: | |
| 290 | 294 | return nil | |
| 291 | 295 | }, | |
| 292 | 296 | }, | |
| 297 | + | { | |
| 298 | + | Name: "register", | |
| 299 | + | Usage: "Create an account", | |
| 300 | + | Args: true, | |
| 301 | + | Flags: []cli.Flag{}, | |
| 302 | + | Action: func(cCtx *cli.Context) error { | |
| 303 | + | pubkey := be.Pubkey(sesh.PublicKey()) | |
| 304 | + | user, err := pr.RegisterUser(pubkey, userName) | |
| 305 | + | if err != nil { | |
| 306 | + | return err | |
| 307 | + | } | |
| 308 | + | wish.Printf(sesh, "User created successfully!\nUser: %s\nPubkey: %s\n", user.Name, pubkey) | |
| 309 | + | return nil | |
| 310 | + | }, | |
| 311 | + | }, | |
| 293 | 312 | { | |
| 294 | 313 | Name: "ps", | |
| 295 | 314 | Usage: "Mange patchsets", |
| ... | ... | @@ -347,9 +366,9 @@ To get started, submit a new patch request: | |
| 347 | 366 | Args: true, | |
| 348 | 367 | ArgsUsage: "[repoName]", | |
| 349 | 368 | Action: func(cCtx *cli.Context) error { | |
| 350 | - | user, err := pr.UpsertUser(pubkey, userName) | |
| 369 | + | user, err := pr.GetUserByPubkey(pubkey) | |
| 351 | 370 | if err != nil { | |
| 352 | - | return err | |
| 371 | + | return errNotExist(be.Cfg.Host, pubkey) | |
| 353 | 372 | } | |
| 354 | 373 | ||
| 355 | 374 | args := cCtx.Args() |
| ... | ... | @@ -528,9 +547,9 @@ To get started, submit a new patch request: | |
| 528 | 547 | Args: true, | |
| 529 | 548 | ArgsUsage: "[repoName]", | |
| 530 | 549 | Action: func(cCtx *cli.Context) error { | |
| 531 | - | user, err := pr.UpsertUser(pubkey, userName) | |
| 550 | + | user, err := pr.GetUserByPubkey(pubkey) | |
| 532 | 551 | if err != nil { | |
| 533 | - | return err | |
| 552 | + | return errNotExist(be.Cfg.Host, pubkey) | |
| 534 | 553 | } | |
| 535 | 554 | ||
| 536 | 555 | args := cCtx.Args() |
| ... | ... | @@ -631,9 +650,9 @@ To get started, submit a new patch request: | |
| 631 | 650 | return err | |
| 632 | 651 | } | |
| 633 | 652 | ||
| 634 | - | user, err := pr.UpsertUser(pubkey, userName) | |
| 653 | + | user, err := pr.GetUserByPubkey(pubkey) | |
| 635 | 654 | if err != nil { | |
| 636 | - | return err | |
| 655 | + | return errNotExist(be.Cfg.Host, pubkey) | |
| 637 | 656 | } | |
| 638 | 657 | ||
| 639 | 658 | repo, err := pr.GetRepoByID(prq.RepoID) |
| ... | ... | @@ -717,9 +736,9 @@ To get started, submit a new patch request: | |
| 717 | 736 | return fmt.Errorf("PR has already been closed") | |
| 718 | 737 | } | |
| 719 | 738 | ||
| 720 | - | user, err := pr.UpsertUser(pubkey, userName) | |
| 739 | + | user, err := pr.GetUserByPubkey(pubkey) | |
| 721 | 740 | if err != nil { | |
| 722 | - | return err | |
| 741 | + | return errNotExist(be.Cfg.Host, pubkey) | |
| 723 | 742 | } | |
| 724 | 743 | ||
| 725 | 744 | err = pr.UpdatePatchRequestStatus(prID, user.ID, StatusClosed, cCtx.String("comment")) |
| ... | ... | @@ -782,9 +801,9 @@ To get started, submit a new patch request: | |
| 782 | 801 | return fmt.Errorf("PR is already open") | |
| 783 | 802 | } | |
| 784 | 803 | ||
| 785 | - | user, err := pr.UpsertUser(pubkey, userName) | |
| 804 | + | user, err := pr.GetUserByPubkey(pubkey) | |
| 786 | 805 | if err != nil { | |
| 787 | - | return err | |
| 806 | + | return errNotExist(be.Cfg.Host, pubkey) | |
| 788 | 807 | } | |
| 789 | 808 | ||
| 790 | 809 | err = pr.UpdatePatchRequestStatus(prID, user.ID, StatusOpen, cCtx.String("comment")) |
| ... | ... | @@ -814,9 +833,9 @@ To get started, submit a new patch request: | |
| 814 | 833 | return err | |
| 815 | 834 | } | |
| 816 | 835 | ||
| 817 | - | user, err := pr.UpsertUser(pubkey, userName) | |
| 836 | + | user, err := pr.GetUserByPubkey(pubkey) | |
| 818 | 837 | if err != nil { | |
| 819 | - | return err | |
| 838 | + | return errNotExist(be.Cfg.Host, pubkey) | |
| 820 | 839 | } | |
| 821 | 840 | ||
| 822 | 841 | repo, err := pr.GetRepoByID(prq.RepoID) |
| ... | ... | @@ -885,9 +904,9 @@ To get started, submit a new patch request: | |
| 885 | 904 | return err | |
| 886 | 905 | } | |
| 887 | 906 | ||
| 888 | - | user, err := pr.UpsertUser(pubkey, userName) | |
| 907 | + | user, err := pr.GetUserByPubkey(pubkey) | |
| 889 | 908 | if err != nil { | |
| 890 | - | return err | |
| 909 | + | return errNotExist(be.Cfg.Host, pubkey) | |
| 891 | 910 | } | |
| 892 | 911 | ||
| 893 | 912 | isReview := cCtx.Bool("review") |
+6
-0
e2e_test.go
#
| ... | ... | @@ -31,6 +31,9 @@ func testSingleTenantE2E(t *testing.T) { | |
| 31 | 31 | // Hack to wait for startup | |
| 32 | 32 | time.Sleep(time.Millisecond * 100) | |
| 33 | 33 | ||
| 34 | + | suite.userKey.MustCmd(suite.patch, "register") | |
| 35 | + | suite.adminKey.MustCmd(suite.patch, "register") | |
| 36 | + | ||
| 34 | 37 | t.Log("User cannot create repo") | |
| 35 | 38 | _, err := suite.userKey.Cmd(suite.patch, "pr create test") | |
| 36 | 39 | if err == nil { |
| ... | ... | @@ -63,6 +66,9 @@ func testMultiTenantE2E(t *testing.T) { | |
| 63 | 66 | ||
| 64 | 67 | time.Sleep(time.Millisecond * 100) | |
| 65 | 68 | ||
| 69 | + | suite.userKey.MustCmd(suite.patch, "register") | |
| 70 | + | suite.adminKey.MustCmd(suite.patch, "register") | |
| 71 | + | ||
| 66 | 72 | t.Log("Admin should be able to create a repo") | |
| 67 | 73 | suite.adminKey.MustCmd(nil, "repo create test") | |
| 68 | 74 |
+6
-6
pr.go
#
| ... | ... | @@ -31,7 +31,7 @@ type GitPatchRequest interface { | |
| 31 | 31 | GetRepoByID(repoID int64) (*Repo, error) | |
| 32 | 32 | GetRepoByName(user *User, repoName string) (*Repo, error) | |
| 33 | 33 | CreateRepo(user *User, repoName string) (*Repo, error) | |
| 34 | - | UpsertUser(pubkey, name string) (*User, error) | |
| 34 | + | RegisterUser(pubkey, name string) (*User, error) | |
| 35 | 35 | IsBanned(pubkey, ipAddress string) error | |
| 36 | 36 | SubmitPatchRequest(repoID int64, userID int64, patchset io.Reader) (*PatchRequest, error) | |
| 37 | 37 | SubmitPatchset(prID, userID int64, op PatchsetOp, patchset io.Reader) ([]*Patch, error) |
| ... | ... | @@ -194,16 +194,16 @@ func (pr PrCmd) createUser(pubkey, name string) (*User, error) { | |
| 194 | 194 | return user, err | |
| 195 | 195 | } | |
| 196 | 196 | ||
| 197 | - | func (pr PrCmd) UpsertUser(pubkey, name string) (*User, error) { | |
| 197 | + | func (pr PrCmd) RegisterUser(pubkey, name string) (*User, error) { | |
| 198 | 198 | sanName := strings.ToLower(name) | |
| 199 | 199 | if pubkey == "" { | |
| 200 | 200 | return nil, fmt.Errorf("must provide pubkey during upsert") | |
| 201 | 201 | } | |
| 202 | - | user, err := pr.GetUserByPubkey(pubkey) | |
| 203 | - | if err != nil { | |
| 204 | - | user, err = pr.createUser(pubkey, sanName) | |
| 202 | + | _, err := pr.GetUserByPubkey(pubkey) | |
| 203 | + | if err == nil { | |
| 204 | + | return nil, fmt.Errorf("pubkey is already registered by another user") | |
| 205 | 205 | } | |
| 206 | - | return user, err | |
| 206 | + | return pr.createUser(pubkey, sanName) | |
| 207 | 207 | } | |
| 208 | 208 | ||
| 209 | 209 | func (pr PrCmd) GetPatchsetsByPrID(prID int64) ([]*Patchset, error) { |