Template editing should be available for non-administrative users

[emclain]

Expected Behavior

Non-administrators should be able to create and edit a Webhook Template

Current Behavior

Creating, editing, or deleting Webhook Templates requires full administrative rights to the TeamCity instance. This discourages administrators from providing self service to Webhook Templates, because it would give a user too much power over TeamCity.

Solution Ideas

Create a custom permission for creating, editing, and deleting Webhook Templates

The simplest solution would be to have a new kind of permission that would grant access to Webhook Templates and nothing else. However, this is an all-or-nothing permission that would allow them to break existing webhooks.

Associate Templates with a Project

To allow users more autonomy without allowing them to break important global webhooks, the Templates could be associated with the project hierarchy, and then administrative permissions to a project would control whether the users had permission to modify the Templates or not.

Your Environment

• tcWebHooks Version: 1.1.301.388
• TeamCity Version: TeamCity Professional 2018.2.2 (build 61245)
• TeamCity server Operating System: Windows
• Are you using a WebHook Template?: Yes

[netwolfuk]

I agree. That was always the end goal. It just turns out the UX is quite tricky to surface to the user.

Now that the statistics model is more complete, I could add the payload to the history page.

If you're interested in helping solve it, I'm happy to have a discussion here or on gitter.

[netwolfuk]

Btw. I was referring to the project level template. I was imagining it being able to be overridden at a project level. But it introduces a few issues.

Does the template override the top level one for all subprojects?
Can a user choose a 'version' of the template? Or do they just get the overridden one unless they "re-override" on their subproject.

Your first option is probably easier to implement, but I think it requires some manually defined permissions. I'm not sure if I can programmatically create extra permission types.

[netwolfuk]

I asked TeamCity support. It's not possible to extend permissions to include our own. So option 1 is out.

[netwolfuk]

Another consideration: what happens if a project level template does not support a specific build event? Does the webhook fail to fire, or is the template resolved from a parent template?

[pgrefviau]

Hey @netwolfuk , I was about to open a new issue when I stumbled on this one. I was actually going to request the same kind of change, but only in the context of testing a given template (i.e. using these endpoints)

Would this specific change be "less-tricky" to implement compared to the full-on create/edit/delete operations? If so, it would be a good starting point.

[netwolfuk]

Hi @pgrefviau. Is what you want already available via the Test tab on the webhook edit dialog?
Underneath, that dialog uses a pseudo REST endpoint to execute or preview a webhook based on a Template.

[netwolfuk]

I managed to get it working from cURL, although it's a bit of a pain.

curl -u 'username:password' -H "Accept:application/json" -H "Content-Type: application/json" http://192.168.1.72:8111/webhooks/testWebHook.html?action=execute -X POST -d '{ "url": "http://localhost:8111/webhooks/endpoint.html", "projectExternalId": "TcPlugins", "uniqueKey": "new", "testBuildState": "buildFailed", "buildId": "136640", "templateId": "elasticsearch", "payloadFormat": "jsontemplate", "authType": null, "authEnabled": false, "authPreemptive": false, "authParameters": { }, "configBuildStates": { } }'

The request payload looks like this:

{
	"url": "http://localhost:8111/webhooks/endpoint.html",
	"projectExternalId": "TcPlugins",
	"uniqueKey": "new",
	"testBuildState": "buildFailed",
	"buildId": "136640",
	"templateId": "elasticsearch",
	"payloadFormat": "jsontemplate",
	"authType": null,
	"authEnabled": false,
	"authPreemptive": false,
	"authParameters": {},
	"configBuildStates": {}
}

And the response is:

{
  "dateTime": "2019-08-16T01:19:18.860",
  "trackingId": "45949b29-2cc5-4aac-9396-5e043302908b",
  "url": "http://localhost:8111/webhooks/endpoint.html",
  "executionTime": "9 ms",
  "statusCode": 204,
  "statusReason": "No Content"
}

Error handling is atrocious :-( If it fails, you get a HTMLified java stracktrace from teamcity's default error handler, but it might be of some use to you.

edit: configBuildStates is not validated, so can be empty.

[netwolfuk]

I've been thinking about this some more.

When project template support is added, I'm wondering if we just say that templates can't be overridden at the project level.

With this model, if you want a project template, it needs to have its own template id. Then we don't need to worry about inheritance issues and my concerns above are negated.

It would still be possible to copy existing templates (assuming it belongs to a project you have read access to) or create a new template from scratch.

Templates would be available to webhooks in the associated project and any subprojects.

[pgrefviau]

Is what you want already available via the Test tab on the webhook edit dialog?

I'm not exactly sure on what tab the user that reported this was on when she encountered the error. All I was sent is this picture:

[netwolfuk]

Yes. That's the template editing screen. The reason I did it that way was because I was too lazy to figure out all the the edge cases for permissions. Eg, I'd only need to show projects the user is permitted to execute webhooks on, otherwise there's a chance they could execute a webhook to their own URL with a template or build which someone may have baked some sensitive data into.

I guess it depends on what she was wanting to test. But it does raise a good point with regards to implementing project level templates.

[netwolfuk]

This is available in 1.2.0 alpha 4. I'll leave this ticket open (but tagged as waiting for release) and will close it when 1.2.0 final is released.

Please do try it out. The alpha is stable. 1.2.0 final might be quite a while away yet.

[netwolfuk]

1.2.0 alpha 4 is here: https://github.com/tcplugins/tcWebHooks/releases/tag/v1.2.0-alpha.4

[liam-mackie]

Hey! Sorry to resurrect an old issue; we noticed that when non-administrators tried copying a template from the root project to a different destination project, they saw 405s, but when they created a template directly in the destination project, it worked fine!

[netwolfuk]

Hi @liam-mackie Thanks for commmenting. Can you let me know which version of the tcWebHooks plugin you're using?

I'll have a look and check if I can reproduce it locally.

[netwolfuk]

Hmm. I am seeing a similar thing. I get a 403 because it tries to get the existing template and load it into the browser, so that it can save it back as a new template. I am using 1.2.0-alpha.10

I made a change this week so that getting an existing template requires edit project. I suspect I need to re-think that because of this feature.

If you say you get a 405, I am guessing that it's trying to POST the template but to the wrong URL. Can you please have a look at the browser JS console's network tab and check the URL that it's trying to post to? That would help nail down where the issue could be.

And let me know the version you're using and I'll downgrade to the same so that we are comparing the same code. Thanks!

[netwolfuk]

Hi @liam-mackie Thanks for your comment. I looked at your profile and assumed you work with @matt-richardson and are therefore probably running the very recent 1.2.0 alpha.

I installed alpha9 and have found two bugs that are present when using templates with a non-admin user.

1. When copying an existing an existing _Root project template (which includes all bundled templates), the permissions prevent the original template from downloading. This is what gives the 403 I previously commented about. I think I will have to be smarter about determining the read permissions on a template and maybe even walk the project tree to see if the current user has permission to access the project to which the template belongs.
2. I have also reproduced the bug that you are seeing where it returns a 405. The URL POST'd to is incorrect.

I made a change a few months ago to support a kind of hierachy of templates. This is what allows a "per project" template to be possible and also allows "overriding" of templates in a project.

For instance. Imagine a template with templateId abc123.
Template abc123 could be defined at the _Root level (associated with the _Root project).
The _Root project may contain two sub-projects called "MyProject1" and "MyProject2". Inside the "MyProject1" a user could create a new template with the same id of abc123. A webhook is then defined in the _Root project and configured to use the template abc123.

When that webhook is executed, it will reolve the template for abc123. If the build that webhook runs against is configured inside the "MyProject1" project (or any sub-project of MyProject1), the template resolved will be the one associated with the "MyProject1" project.

Additionally, if a webhook is executed from a build in the "MyProject2" project and we assume there is no template defined in that project, then the webhook will find the template associated with the "_Root" project.

However, it looks like I didn't fix the REST API properly. I strongly believe a template should now be located at an endpoint like /app/rest/webhooks/templates/MyProject1/id:abc123. This explicitly refers to the MyProject1 version of the template. To refer to the template of the same id, but associated with the _Root project, the url would be /app/rest/webhooks/templates/_Root/id:abc123.

The code to create (POST) a new template has been updated to expect the project in the URL (eg, POST to /app/rest/webhooks/templates/MyProject1 but I neglected to fix the UI code to use this new URL for creating the template. Hence the 405 Method not allowed. It simply does not support POST unless the project name is included.

However, the REST API is also "broken" because getting the list of templates /app/rest/webhooks/templates returns the hypermedia links without the MyProject1 prefix, and effectively prevents users from being able to have the same template id in multiple projects.

I'll need to fix both of these issues, and unfortunately that will be a breaking change for anyone using the existing template REST API endpoint.

Edit: It looks like I was dreaming. It's not possible to override a template in a sub-project. I looked at the code, and the TemplateManager has a single Map<String,Template>, so only one template can be defined with a specific templateId (used as the key of the map).

It does introduce a subtle bug. Say, we defined a template in MyProject1 with the same ID as a bundled template. It would be overriden for that project, but no other project would get a chance to override it. I think I need to make a change to only allow overriding a bundled template if the new version is associated with the _Root project.

[netwolfuk]

Hi @liam-mackie I've just pushed a fix for the permissions, which should fix your issue. If you want to take look at the latest build on this branch

[netwolfuk]

I'm planning to merge this and release as 1.2.0-alpha.10 on the weekend. I'm hoping it's the last alpha before release candidate.

[liam-mackie]

I'm planning to merge this and release as 1.2.0-alpha.10 on the weekend. I'm hoping it's the last alpha before release candidate.

Sorry about the poor replies, I didn't get any emails about your replies for some reason! shakes fist at gmail

Yes, we are on 1.2.0 Alpha! Thanks so much for the swift fix! Excited for the RC! I will install alpha.10 on our prerelease TeamCity server in the coming days and test that it's been fixed! Once the RC is released, I'll update as well!

Thanks again for the fix and your time, it's much appreciated!

[netwolfuk]

No worries. I'm not getting emails either. shakes fist at mail client.

I found a bug last night, but still hoping to get a release out this weekend.

[netwolfuk]

Alpha 10 released https://github.com/tcplugins/tcWebHooks/releases

[netwolfuk]

Release candidate 1 released https://github.com/tcplugins/tcWebHooks/releases/tag/v1.2.0-rc1