Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Unable to create Default Webhooks #24624

Closed
kousu opened this issue May 10, 2023 · 5 comments · Fixed by #24626
Closed

Unable to create Default Webhooks #24624

kousu opened this issue May 10, 2023 · 5 comments · Fixed by #24626
Labels
type/bug

Comments

@kousu
Copy link
Contributor

kousu commented May 10, 2023 •
edited
Loading

Description

As of 9a0652f, the current main, it is impossible to create Default Webhooks using /admin/default-hooks/gitea/new. They are instead added as system webhooks, producing these database rows; notice that is_system_webhook = 1 in both:

sqlite> select * from webhook limit 3;
id|repo_id|owner_id|is_system_webhook|url|http_method|content_type|secret|events|is_active|type|meta|last_status|header_authorization_encrypted|created_unix|updated_unix
3|0|0|1|https://system-main|POST|1||{"push_only":true,"send_everything":false,"choose_events":false,"branch_filter":"*","events":{"create":false,"delete":false,"fork":false,"issues":false,"issue_assign":false,"issue_label":false,"issue_milestone":false,"issue_comment":false,"push":false,"pull_request":false,"pull_request_assign":false,"pull_request_label":false,"pull_request_milestone":false,"pull_request_comment":false,"pull_request_review":false,"pull_request_sync":false,"wiki":false,"repository":false,"release":false,"package":false}}|1|gitea||0||1683681122|1683681122
4|0|0|1|https://default-main|POST|1||{"push_only":true,"send_everything":false,"choose_events":false,"branch_filter":"*","events":{"create":false,"delete":false,"fork":false,"issues":false,"issue_assign":false,"issue_label":false,"issue_milestone":false,"issue_comment":false,"push":false,"pull_request":false,"pull_request_assign":false,"pull_request_label":false,"pull_request_milestone":false,"pull_request_comment":false,"pull_request_review":false,"pull_request_sync":false,"wiki":false,"repository":false,"release":false,"package":false}}|1|gitea||0||1683681147|1683681147

Gitea Version

9a0652f

Can you reproduce the bug on the Gitea demo site?

No -- but only because I don't have admin rights there

Log Gist

https://gist.github.com/kousu/8cf00c3a7a6a032679a76d1bc2fda47b

Screenshots

"Add a new default webhook":

Screenshot 2023-05-09 at 21-12-16 Gitea Git with a cup of tea

Adds it as a system webhook:

Screenshot 2023-05-09 at 21-12-30 Gitea Git with a cup of tea

Git Version

2.34.1

Operating System

Ubuntu 22.04

How are you running Gitea?

Built from git, run with ./gitea

Database

SQLite
@kousu kousu added the type/bug label May 10, 2023
@kousu
Copy link
Contributor Author

kousu commented May 10, 2023

This seems similar to #23139, in that it regards instability in the /admin/hooks API.

@lunny
Copy link
Member

lunny commented May 10, 2023

@kousu would you like to send a PR to fix it?

@kousu
Copy link
Contributor Author

kousu commented May 10, 2023

I will if I figure out what's wrong!

@kousu
Copy link
Contributor Author

kousu commented May 10, 2023

This doesn't appear on 8ceb78c = v1.19.3; with that, it works as expected:

Screenshot 2023-05-09 at 21-08-08 Gitea Git with a cup of tea

sqlite> select * from webhook limit 3;
id|repo_id|owner_id|is_system_webhook|url|http_method|content_type|secret|events|is_active|type|meta|last_status|header_authorization_encrypted|created_unix|updated_unix
1|0|0|0|https://whatsup|POST|1|sdfsdfsd|{"push_only":true,"send_everything":false,"choose_events":false,"branch_filter":"*","events":{"create":false,"delete":false,"fork":false,"issues":false,"issue_assign":false,"issue_label":false,"issue_milestone":false,"issue_comment":false,"push":false,"pull_request":false,"pull_request_assign":false,"pull_request_label":false,"pull_request_milestone":false,"pull_request_comment":false,"pull_request_review":false,"pull_request_sync":false,"wiki":false,"repository":false,"release":false,"package":false}}|1|gitea||0||1683680855|1683680855
2|0|0|1|https://system|POST|1|asfsdfsdf|{"push_only":true,"send_everything":false,"choose_events":false,"branch_filter":"*","events":{"create":false,"delete":false,"fork":false,"issues":false,"issue_assign":false,"issue_label":false,"issue_milestone":false,"issue_comment":false,"push":false,"pull_request":false,"pull_request_assign":false,"pull_request_label":false,"pull_request_milestone":false,"pull_request_comment":false,"pull_request_review":false,"pull_request_sync":false,"wiki":false,"repository":false,"release":false,"package":false}}|1|gitea||0||1683680885|1683680885

@kousu kousu mentioned this issue May 10, 2023
Merged
@kousu
Copy link
Contributor Author

kousu commented May 10, 2023 •
edited
Loading

Ah, http://localhost:3000/admin/default-hooks/gitea/new generates

<form class="ui form dirty" action="/admin/system-hooks/gitea/new" method="post">

So it's the rendering side of the GET request that's confused.

PR in #24626 🎉

kousu added a commit to neuropoly/gitea that referenced this issue May 10, 2023
@kousu
Reenable creating default webhooks.
3f044b0 
Fixes go-gitea#24624

This seems to have been broken in go-gitea#21563

Previously, the code read

```
                // Are we looking at default webhooks?
                if ctx.Params(":configType") == "default-hooks" {
                        return &orgRepoCtx{
                                IsAdmin:     true,
                                Link:        path.Join(setting.AppSubURL, "/admin/hooks"),
                                LinkNew:     path.Join(setting.AppSubURL, "/admin/default-hooks"),
                                NewTemplate: tplAdminHookNew,
                        }, nil
                }

                // Must be system webhooks instead
                return &orgRepoCtx{
                        IsAdmin:         true,
                        IsSystemWebhook: true,
                        Link:            path.Join(setting.AppSubURL, "/admin/hooks"),
                        LinkNew:         path.Join(setting.AppSubURL, "/admin/system-hooks"),
                        NewTemplate:     tplAdminHookNew,
                }, nil
```

but was simplified to

```
                return &ownerRepoCtx{
                        IsAdmin:         true,
                        IsSystemWebhook: ctx.Params(":configType") == "system-hooks",
                        Link:            path.Join(setting.AppSubURL, "/admin/hooks"),
                        LinkNew:         path.Join(setting.AppSubURL, "/admin/system-hooks"),
                        NewTemplate:     tplAdminHookNew,
                }, nil
```

combining the IsSystemWebhook check into a one-liner, but forgtting the same for LinkNew.
lunny pushed a commit that referenced this issue May 11, 2023
@kousu
Reenable creating default webhooks. (#24626)
3d9ed62 
Fixes #24624

This seems to have been broken in
#21563

Previously, this code read

```
                // Are we looking at default webhooks?
                if ctx.Params(":configType") == "default-hooks" {
                        return &orgRepoCtx{
                                IsAdmin:     true,
                                Link:        path.Join(setting.AppSubURL, "/admin/hooks"),
                                LinkNew:     path.Join(setting.AppSubURL, "/admin/default-hooks"),
                                NewTemplate: tplAdminHookNew,
                        }, nil
                }

                // Must be system webhooks instead
                return &orgRepoCtx{
                        IsAdmin:         true,
                        IsSystemWebhook: true,
                        Link:            path.Join(setting.AppSubURL, "/admin/hooks"),
                        LinkNew:         path.Join(setting.AppSubURL, "/admin/system-hooks"),
                        NewTemplate:     tplAdminHookNew,
                }, nil
```

but was simplified to

```
                return &ownerRepoCtx{
                        IsAdmin:         true,
                        IsSystemWebhook: ctx.Params(":configType") == "system-hooks",
                        Link:            path.Join(setting.AppSubURL, "/admin/hooks"),
                        LinkNew:         path.Join(setting.AppSubURL, "/admin/system-hooks"),
                        NewTemplate:     tplAdminHookNew,
                }, nil
```

In other words, combining the `IsSystemWebhook` check into a one-liner
and forgetting that `LinkNew` also depended on it. This meant the
rendered `<form>` always POSTed to `/admin/system-hooks`, even when you
had GETed `/admin/default-hooks/gitea/new`.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
type/bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Reenable creating default webhooks. neuropoly/gitea
2 participants
@lunny @kousu