Giter Site home page Giter Site logo

Comments (33)

patroeper avatar patroeper commented on June 3, 2024 1

Could you tell me what plug-in created the other parameter in your screenshot? Maybe I could see what that plug-in does to create that value.

I believe the settings in this screenshot were created by the teamcity-oauth plugin.

(based on that being the only plugin we have related to OAuth)

from tcwebhooks.

netwolfuk avatar netwolfuk commented on June 3, 2024 1

@patroeper I just wanted to say a huge thank you for your persistence on this. Your keen eye has found the issue with my parameter naming, and now that I have changed it, it really just works

The UX is simply...

  1. Create a Webhook Parameter, and mark it as "Password"
  2. Create a Webhook to reference the parameter.

Here is a VCS sync'd project:

    <extension id="PROJECT_EXT_4" type="tcWebHookParameter">
      <parameters>
        <param name="boolean.forceResolveTeamCityVariable" value="false" />
        <param name="boolean.includedInLegacyPayloads" value="false" />
        <param name="boolean.secure" value="true" />
        <param name="name" value="myURL" />
        <param name="secure:value" value="credentialsJSON:8126c5f8-9603-4ca6-b266-99973766514e" />
        <param name="templateEngine" value="STANDARD" />
      </parameters>
    </extension>

And a non-VCS sync'd project:

    <extension id="PROJECT_EXT_5" type="tcWebHookParameter">
      <parameters>
        <param name="boolean.forceResolveTeamCityVariable" value="false" />
        <param name="boolean.includedInLegacyPayloads" value="false" />
        <param name="boolean.secure" value="true" />
        <param name="name" value="webhooks.url" />
        <param name="secure:value" value="zxxf6382cfdca5ab69e4d0515a5a8312e0e7eb6c197a2a20bfe3c064b33d5fec7c044967a3109337cf6a0a884d030214b20b56e1c5a7a84ce64babb3a9c6ef39dba" />
        <param name="templateEngine" value="STANDARD" />
      </parameters>
    </extension>

I don't have to worry about creating tokens or referencing with the special $secure() template extensions.
Here is the webhook config for a working tokenised hidden field.

image

It is still showing the URL in the history. I'll have a think about the best way to signal to the UI that the URL was built from a secure parameter.
image

from tcwebhooks.

netwolfuk avatar netwolfuk commented on June 3, 2024 1

I asked the team and the reply implies that it's their logger that does the password masking.

I have option 1 working. Basically I keep a flag inside the property resolver that toggles to true if any value that was secure was accessed. It does mean that I can't determine whether it was for the URL or the payload. I just know it was requested during the execution of the webhook. At which point I can choose to show or hide the entire URL or payload. However, I can't mask specific values. It's all or nothing.

I'm thinking of implementing option 4 which is a combination of 1 and 3.

Eg. Have a toggle to show/hide secure values and check if any were accessed. If hiding and secure, then don't log to the log.

I have also implemented URL 'simplification'. So if your URL was api.slack.com/webhooks/some_secret then what is shown in the UI is slack.com if the payload or URL contained a secure value.

I haven't solved the issue of it being shown in the UI because when the webhook ran it was set to 'show', but now that setting is toggled to hide in that webhook configuration. I need to think about that some more. Ideally, if the webhook has 'hide' checked now we shouldn't show URLs from previous runs.

The history shown in the UI is held in memory. If someone has access to the server and manages to take a dump of memory then the full URL could be obtained from previous executions. But if they have access to the server, they could just read the logs. I can't go back in time and rewrite those either, so I'll stop worrying about that. 🤣

from tcwebhooks.

netwolfuk avatar netwolfuk commented on June 3, 2024 1

Hi @patroeper. After what can only be referred to as a terrible few months (insert reference to global pandemic here), I have a new build to test. It has the checkbox to show hide the values both in the UI and the log.

Please download a build from the issue_53-hide_password branch.

I have also created a page on the wiki about how to use secured values.

Update: I have just noticed that toggling the checkbox does not toggle historical entries in the UI. I will take a look at that and try to get a fix asap. It should be referring to the state of that setting in the live config, but it appears to have a copy of the config in the execution stats object (that's what is written to the history store).

from tcwebhooks.

netwolfuk avatar netwolfuk commented on June 3, 2024 1

@patroeper Do you want to do any testing on this, or should I just merge it? I ask because you're extremely thorough, and might pick up any issues I could have missed.

from tcwebhooks.

patroeper avatar patroeper commented on June 3, 2024 1

@netwolfuk I'll take a look this week

from tcwebhooks.

Keilaron avatar Keilaron commented on June 3, 2024

I believe the recommended practice is to use the "Generate token for a secure value" feature. If the plugin could be modified to accept such a token as a URL, I think that would be the easiest way to implement secure storage of the sensitive webhook URLs.

from tcwebhooks.

netwolfuk avatar netwolfuk commented on June 3, 2024

Hi @Keilaron
That option does not seem to be enabled on for my project. Probably because I am not using versioned settings.
I'll have a play and see if I can get something working. There is an issue #146 which uses the "password" type build parameters, and they are not available un-encrypted at the build finished event. I suspect this might be the same, but will see what I can find.

from tcwebhooks.

Keilaron avatar Keilaron commented on June 3, 2024

Hmm, #146 mentions something I wasn't aware of -- I kind of assumed that Parameters were not usable since it isn't mentioned in the UI that we can use them.
Just to be clear though, I had been talking about this: https://www.jetbrains.com/help/teamcity/storing-project-settings-in-version-control.html#StoringProjectSettingsinVersionControl-ManagingTokens
I thought it was always visible (but you have to specifically be looking at a Project in Edit view, I believe?), but maybe you are correct in saying that it is only available when using VCS storage.

from tcwebhooks.

netwolfuk avatar netwolfuk commented on June 3, 2024

D'oh. I'm a muppet! Yes I found the option in the actions menu. That looks super helpful. I will see how I can get at that from within plugin code.

from tcwebhooks.

netwolfuk avatar netwolfuk commented on June 3, 2024

FYI. Making great progress on using settings from both #146 and the secure token. I have #146 working by resolving via SFeatureDescriptor items, and have extended the webhooks rest API to be able to create and edit them. So I just have the UI to write for that.

For the secure tokens I have the code written and it works in unit tests, but I need to figure out how to get Spring to load a different version of the WebHookSecureResolver depending on the TeamCity version, because the underlying API in TeamCity has only been available since 2017.1

from tcwebhooks.

netwolfuk avatar netwolfuk commented on June 3, 2024

Raised an issue with Jetbrains about how to use Spring's factory beans.

https://youtrack.jetbrains.com/issue/TW-66712

from tcwebhooks.

netwolfuk avatar netwolfuk commented on June 3, 2024

I have worked around the issue with spring by injecting the factory and having the factory return the relevant object.

That piece of the code only contains objects that are instantiated for the duration of a webhook execution, so I don't need to rely on spring management of the instances.

I need to do some more testing, but the secrets are resolving using the Preview and Test tab of the webhook dialog.

from tcwebhooks.

patroeper avatar patroeper commented on June 3, 2024

@netwolfuk I read through #146 which brough me to this issue. I'm looking to do something similar wrt "securing" the webhook URL field. If I'm following these threads correctly, the latest alpha (currently 1.2.0-alpha.7) contains some changes that might be useful.

I've installed this alpha release (both tcWebHooks & REST API + server restart) and am able to use the new webhooks-specific per-project configuration. I added a WebHook parameter to the _Root project, setting the type to Password:

image

From there I was able to configure a webhook w/ the URL set to the value ${webhooks.url}, and I've validated that the URL is resolved as expected. I did, however, notice that the url value is stored in clear text in (teamcity)/_Root/project-config.xml (prepended with secure:):

image

I may have incorrectly assumed that the secure token infrastructure was being used, in which case the project-config.xml data would have the following characteristics:

  • the param name would be prefixed with secure:
  • the param value would look something like credentialsJSON:29fe6b10-8715-4054-9116-f956e03119bc

For example, here's a secure value from an unrelated extension:

image

I was hoping you could shed light on the intended behavior of creating a Password parameter in the latest alpha, as to how the underlying value should be stored.


For context:

  • TC version is: TeamCity Professional 2021.2.3 (build 99711)
  • Using standard json templating (not velocity)
  • TC settings are stored in VCS (git)

from tcwebhooks.

netwolfuk avatar netwolfuk commented on June 3, 2024

Hi @patroeper Thanks for your detailed comment.

Firstly, apologies for the slow reply. My email client on my phone had stopped receiving new messages. A pile of them just arrived.

WRT the secure token: yes, my intention is that the secure mechanism was invoked for parameters prefixed with secure:. Pavel mentioned this but I can't find the reference at the moment. Maybe it was in person. However, it appears that in your screenshot this is not working as I thought it would.

There are two "secure" mechanisms in TeamCity.

  1. Marking a parameter with the secure: prefix. This appears to simply hide the value in the UI, and also prevent it from being resolvable in the final stages of the build or logged to the build log. Arguably, this really only pseudo security, and one of the commenters on one of these tickets also mentioned this.
  2. Scrambled token support in Teamcity. I suspect that is what has been used in your above screenshot for the clientSecret value. The good news is that tcWebHooks also supports this in recent 1.2 alphas.

From memory, it works like this...

  • Edit your project and create a scrambled token (actions menu at the top right).
  • Create a Webhook Project Parameter of type "Standard Template" called something like scrambledUrl
  • set the value of the parameter to $secure(my_big_long_token)
  • refer to $scrabledUrl in your webhook.

In theory, this should create the secure value like your above screenshot

I have my dev teamcity in a broken state at the moment (UI refactor) and my QA server is in pieces as I replace the fans, but will spin up a new one on 1.2alpha7 on my dev box and test this over the weekend for you.

from tcwebhooks.

patroeper avatar patroeper commented on June 3, 2024

@netwolfuk

I don't fully understand the history of scrambled/secure token values; our TC install was ~2018ish. I found a thread that provided some context, specifically this comment: https://youtrack.jetbrains.com/issue/TW-45181#focus=Comments-27-2051540.0-0

We are using VCS for TC settings, and I observe that our secure values are written as credentialsJSON:uuid in project-config.xml files (included in VCS). With administrative access, I can traverse the TC server file storage to a credentials.json file where there is a 1-1 mapping of a uuid to a value that starts with zxx; I believe this is a "scrambled" value (not included in VCS).

For clarification on your notes above:

Edit your project and create a scrambled token (actions menu at the top right)

Screenshots of flow:

image

image

image

set the value of the parameter to $secure(my_big_long_token)

I think what you are saying is that my_big_long_token should be the zxx... value, the one that is kept out of source control. What I'm trying to do is reference the credentialsJSON:uuid value, and in attempting that I got this webhook error:

999 :: Unexpected exception. Please log a bug on GitHub tcplugins/tcWebHooks. Exception was: 
Illegal character in scheme name at index 0: $secure(credentialsJSON:64020928-38a7-4ae2-98e8-c859bd87b235)

Does the latest alpha understand this layer of indirection, or is it intended to work off of the underlying scrambled token?

from tcwebhooks.

netwolfuk avatar netwolfuk commented on June 3, 2024

Hi @patroeper. Yes the latest alphas are supposed to do the resolution of these scrambled values.

What's interesting is that my TeamCity has a different format for the token.

Screenshot_20220416-035639

I'll need to dig further into why they're different and where that error is occurring.

You could try changing the property from standard to Velocity and $secure to #secure but I suspect the issue is in the format of the token name.

from tcwebhooks.

netwolfuk avatar netwolfuk commented on June 3, 2024

Oh. I just reread your comment. Hmm, I've never seen this credentials file before. I don't think my code will resolve that. Can you point me at some documentation on how I can setup TeamCity to use this?

from tcwebhooks.

patroeper avatar patroeper commented on June 3, 2024

I don't think my code will resolve that.

Good to know; thanks!


Docs: https://www.jetbrains.com/help/teamcity/storing-project-settings-in-version-control.html#Storing+Secure+Settings

Here's the setting on our instance (found under Administration -> Root Project -> Versioned Settings):

image

from tcwebhooks.

patroeper avatar patroeper commented on June 3, 2024

Also, one of the links I shared above has a broader discussion around the development/use of "credentials storage": https://youtrack.jetbrains.com/issue/TW-45181

from tcwebhooks.

netwolfuk avatar netwolfuk commented on June 3, 2024

Looking closely at your screenshot, I think I need to prepend the name, not the value with secure: and could remove the boolean.secure property. I'll fix that first, and maybe this will improve this handling.

I have found that if I dig out the credentials.json file, and use the zxx value in the webhook parameter, then the value is resolved correctly, but that's a pain from UX perspective, and probably not even possible without admin privileges in TeamCity.

eg. <param name="value" value="http://${secure(zxx265e9b3e4de89b9c0ffc785b1de56499)}/webhooks/endpoint.html" />

I'll do some more experimentation, and then raise a ticket with Jetbrains for tips on how to read the credentials.json file. There must be a method they use for it, rather than me rolling my own JSON reader.

from tcwebhooks.

netwolfuk avatar netwolfuk commented on June 3, 2024

Also, I can see the URL resolved in the logs, and the UI. This is probably not ideal.

image

Perhaps if URL value is resolved from a "secure" parameter, I need to just show the hostname. I do this in other places in the UI where the user does not have edit permission on a project.

from tcwebhooks.

netwolfuk avatar netwolfuk commented on June 3, 2024

Looking closely at your screenshot, I think I need to prepend the name, not the value with secure: and could remove the boolean.secure property. I'll fix that first, and maybe this will improve this handling.

Wow. This is exactly what you said in your first comment. I was obviously not having a good day that day. Sorry for all the confusion.

from tcwebhooks.

patroeper avatar patroeper commented on June 3, 2024

I have found that if I dig out the credentials.json file, and use the zxx value in the webhook parameter, then the value is resolved correctly, but that's a pain from UX perspective, and probably not even possible without admin privileges in TeamCity.

That's correct. But even more importantly, the purpose of the credentials.json file is to keep the zxx out of source control -- specifying the zxx value as a webhooks parameter puts it back into VCS (defeating the purpose of the Store passwords and API tokens outside of VCS setting).

... how to read the credentials.json file. There must be a method they use for it, rather than me rolling my own JSON reader.

That YouTrack thread mentions a CredentialsStorage interface. I don't know if that's meaningful, having never working with the TC programming model. It sounds like the credentials.json is the default implementation of said interface, but other implementations may exist (e.g., adapters for HashiCorp Vault or AWS Secrets Manager). Ideally TC would give you access to active CredentialsStorage component so tcWebHooks isn't coupled to the credentials.json implementation.

Also, I can see the URL resolved in the logs, and the UI. This is probably not ideal.

💯

I was obviously not having a good day that day. Sorry for all the confusion.

No worries. Your response times are faster than some of our paid vendors, and higher quality 🤣

from tcwebhooks.

netwolfuk avatar netwolfuk commented on June 3, 2024

Thanks for the thoughtful and researched replies. I now have a better understanding and a good set of ideas to try.

I'm really rather hoping that setting the value correctly to secure will magically do all this for me. That's what Pavel implied.

I have some time off for Easter so hopefully I'll make progress on this.

from tcwebhooks.

netwolfuk avatar netwolfuk commented on June 3, 2024

Could you tell me what plug-in created the other parameter in your screenshot? Maybe I could see what that plug-in does to create that value.

from tcwebhooks.

patroeper avatar patroeper commented on June 3, 2024

Or those settings came from this area in TeamCity:

image

... and I'm not sure if this screen is related to the OAuth plugin I mentioned above.

from tcwebhooks.

netwolfuk avatar netwolfuk commented on June 3, 2024

I have pushed to a branch which will build on teamcity.jetbrains.com.

You're welcome to try a build on that branch. Hopefully I'll get a new alpha out this week which will contain this and couple of other changes. I'd like to try to get the URL shortening working for secured webhook parameters too before I release alpha 8.

from tcwebhooks.

netwolfuk avatar netwolfuk commented on June 3, 2024

I'm interested in feedback on the way to hide the values.

  1. Automatically hide the URL and payload in both the teamcity logs and the UI when either contain a "secure" value.
    Pros:

    • This is more secure, and won't accidentally leak secure values.

    Cons:

    • This would be really hard to debug when you can't see the actual URL being used in the logs or UI.
    • Will be quite tricky to code. I need some way to determine that a string in a templated value refers to a parameter that is marked as secure.
  2. Automatically hide the URL in the UI, but still log details to the teamcity-server.log file.
    Pros:

    • Slightly secure in that URLs are not leaked in the UI.

    Cons:

    • This would be really hard to debug when you can't see the logs.
      You might not have permission to see the teamcity-server.log as a standard TC user.
    • Will be quite tricky to code. I need some way to determine that a string in a templated value refers to a parameter that is marked as secure.
    • Does not enforce security by default. Values are still in the TeamCity log file for admins to view.
  3. Add an option to the webhook config dialog, which allows hiding URL and/or payload from logs and/or UI.
    Pros:

    • Can be toggled to see what is being used, and then turned off again when debugging completed.
    • Slightly easier to code as I have access to the config at the time the webhook is logged to screen and/or log.

    Cons:

    • Could be overlooked. Does not enforce security by default.

from tcwebhooks.

patroeper avatar patroeper commented on June 3, 2024

I don't have a strong or informed opinion on this, but thinking out loud (in text)...


Ignoring how much effort would be required, if I were writing a plugin, I'd want the plugin's behavior to closely match TeamCity's behavior for comparable tasks. WRT secure values, it looks like TC (sometimes) masks the values in logs with asterisks.

  • I'm curious how this is implemented -- it looks to be a string replace on a list of known secure values.
    • If you create a configuration parameter of type Password with the value build, it's interesting to see what gets masked in the build log.
  • Does the TC programming model expose a component responsible for this output scrubbing?
  • Although masking values introduces some pain in debugging/troubleshooting, isn't this already a challenge for someone using secure values in other aspects of TeamCity (i.e., this isn't a new problem for a plugin author)

WRT option 3, the setting could be set on by default or have inverted meaning, but possibly at the cost of getting inbound questions from existing users losing info in the UI/logs after upgrading the plugin(s)

  • mask/hide url & payload
  • show/include raw url & payload (may expose secure values)

from tcwebhooks.

netwolfuk avatar netwolfuk commented on June 3, 2024

A new build is available on the above link. This fixes that bug where toggling the Secure Values checkbox didn't show/hide the URL in the history. I now look up the value of the config just before rendering the history items in case that checkbox has changed state since the webhook was executed.

from tcwebhooks.

patroeper avatar patroeper commented on June 3, 2024

@netwolfuk I covered the happy-path for the 1.2.0-alpha.8-build.439 artifacts:

  • The default state of checked/ticked for the Show simplified URL in UI and prevent payload from logging secure values seems like the right decision
  • Secure parameters (the webhook URL, in my scenario) are scrubbed in the recent webhooks UI
  • Toggling the checkbox allows for historical webhook activity to be read in the clear (per your comment above).

Nice job! 🎉 :shipit:

from tcwebhooks.

netwolfuk avatar netwolfuk commented on June 3, 2024

Thanks so much for the taking the time to review it @patroeper

I have found one very minor unrelated bug.
When testing a webhook whilst editing it (in the last pane of the dialog), the stats object is not populated with the ProjectId.
Therefore, when the webhook did not use a secure value, so the mouseover in history for that webhook is supposed to say which project the webhook config belongs to, but it's blank.

The code path doing a test webhook execution must be slightly different when populating the execution stats.

I will endeavour to fix that bug, and then merge this branch to master and release alpha8 this weekend.
I really had hoped alpha8 would support editing filters, headers and parameters in the UI, but that work to taking forever.

I guess we'll have an alpha9 before the Release Candidate is out. I really want to ship 1.2, as it's been a couple of years now, but looking back at the last two years, I should cut myself some slack.

from tcwebhooks.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.