newrelic / nr1-github

NR1 Github allows you to create more context to your entities by having access to the GitHub repository, contributors and README.
https://github.com/newrelic/nr1-github/discussions
Apache License 2.0
19 stars 27 forks source link

A user can no longer set a GitHub URL universally for an org #79

Closed jpvajda closed 3 years ago

jpvajda commented 3 years ago

A user can no longer set a GitHub URL universally, each user must set this individually, and they are stored in that users secrets vault. This prevents one user from setting the GitHub URL to a fake server, and stealing another users PAT when they use the NerdPack

Acceptance Criteria

rudouglas commented 3 years ago

This is what the experience is now.

If a user has set an Enterprise GitHub URL for the App, they will now be directed to the Setup section and have to set it again. They will now see this Callout which will include the URL they set previously so they can easily set it again.

Screenshot 2021-06-14 at 11 50 18

Thinking about it I just thought of an edge case that i need to test so will update this again before review

jpvajda commented 3 years ago

@rudouglas awesome. it's always great to catch those edge cases. 👍

rudouglas commented 3 years ago

@jpvajda i've made those changes so this is ready for review

jpvajda commented 3 years ago

@rudouglas I'm unable to validate this locally, as I get this error, I think we just need to pair to run through it tomorrow. Shouldn't take long.

Screen Shot 2021-06-14 at 4 06 06 PM
jpvajda commented 3 years ago

I've pull down this PR, run NPM Install and nerdpack:serve and see this, but I can't seem to pull up the code locally.

Screen Shot 2021-06-14 at 4 10 37 PM
rudouglas commented 3 years ago

You may need to regenerate the GUID. Could you pull down the latest branch and run:

nr1 nerdpack:uuid -gf
npm install
npm start

Also make sure you are on the right profile for your account:

nr1 profiles:default

The other thing is that the reproduction of the new behaviour requires you to publish the Nerdpack to your account, and access it with 2 different users, maybe we should 🏕️ it later @jpvajda

jpvajda commented 3 years ago

Feedback from our review of this today: @rudouglas

1. the tooltip should be more explicit telling the user what to do.

We recently made security improvements to this Nerdpack which may require you to setup up your Github URL again.  
The following Github URL was previously set on your account {GITHUB_URL}. 
If this is a trusted source, copy and paste this URL into the field below.

2. The URL within the field for Github Enterprise should not be clickable.

3. Provide a final screen shot of the experience in the body of this ticket

4. Ping the Wizards slack channel to make others aware of this change prior to pushing to prod.

rudouglas commented 3 years ago

Altered this based on our chat yesterday. Also did some double-checks to make sure this step is not needed for Apps set to the Public GitHub API URL

Screenshot 2021-06-16 at 11 49 56
rudouglas commented 3 years ago

Made changes in the latest push and notified #wizards-programmability

jpvajda commented 3 years ago

By end of day if we don't hear any concerns on this change, we can deploy it. I'll keep an eye on the 🧙 channel.

jpvajda commented 3 years ago

I didn't see any concerns in the Wizards channel, so let's deploy this change 🥳