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.

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

fix(editor): Issue showing Auth2 callback section when all properties are overriden #8999

Conversation

RicardoE105
Copy link
Contributor

@RicardoE105 RicardoE105 commented Mar 28, 2024

Summary

We show the copy section when the credentials are OAuth2 and no properties are left after the override. Sadly, there are a couple of problems with this approach:

  1. If the OAuth2 credential has more properties than the base ones (client ID and client secret), when we override the base ones, we still show the OAuth2 callback section, and we should not.
  2. We show notices that most likely refer to an overridden property that the user cannot see.

To locally override credentials, see -> https://docs.n8n.io/embed/configuration/#credential-overwrites

Before - overridden credentials:

image

Now- overridden credential:

image

Before - non overridden credential:

image

Now - non-overridden credential:

image

Related tickets and issues

https://linear.app/n8n/issue/ADO-1627/bug-oauth-redirect-url-shows-in-slack-cred-on-cloud

Review / Merge checklist

  • PR title and summary are descriptive. Remember, the title automatically goes into the changelog. Use (no-changelog) otherwise. (conventions)
  • Tests included.

    A bug is not considered fixed, unless a test is added to prevent it from happening again.
    A feature is not complete without tests.

@RicardoE105 RicardoE105 changed the title fix issue showing Auth2 callback section when all properties are overriden fix(editor) issue showing Auth2 callback section when all properties are overriden Mar 28, 2024
@RicardoE105 RicardoE105 marked this pull request as draft March 28, 2024 17:02
@RicardoE105 RicardoE105 changed the title fix(editor) issue showing Auth2 callback section when all properties are overriden fix(editor): Issue showing Auth2 callback section when all properties are overriden Mar 28, 2024
@n8n-assistant n8n-assistant bot added n8n team Authored by the n8n team ui Enhancement in /editor-ui or /design-system labels Mar 28, 2024
@RicardoE105 RicardoE105 marked this pull request as ready for review April 2, 2024 12:31
Copy link

cypress bot commented Apr 4, 2024

3 flaky tests on run #4556 ↗︎

0 344 12 0 Flakiness 3

Details:

🌳 🖥️ browsers:node18.12.0-chrome107 🤖 RicardoE105 🗃️ e2e/*
Project: n8n Commit: e2ea40c42d
Status: Passed Duration: 03:48 💡
Started: Apr 4, 2024 9:26 AM Ended: Apr 4, 2024 9:30 AM
Flakiness  5-ndv.cy.ts • 1 flaky test

View Output Video

Test Artifacts
NDV > should not retrieve remote options when required params throw errors Test Replay Screenshots Video
Flakiness  30-editor-after-route-changes.cy.ts • 1 flaky test

View Output Video

Test Artifacts
Editor actions should work > after saving a new workflow Test Replay Screenshots Video
Flakiness  17-sharing.cy.ts • 1 flaky test

View Output Video

Test Artifacts
Sharing > should work for admin role on credentials created by others (also can share it with themselves) Test Replay Screenshots Video

Review all test suite changes for PR #8999 ↗︎

Copy link
Contributor

@MiloradFilipovic MiloradFilipovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

github-actions bot commented Apr 4, 2024

✅ All Cypress E2E specs passed

@RicardoE105 RicardoE105 removed the request for review from mutdmour April 4, 2024 09:30
@RicardoE105 RicardoE105 merged commit dff8f7a into master Apr 4, 2024
51 checks passed
@RicardoE105 RicardoE105 deleted the ado-1627-bug-oauth-redirect-url-shows-in-slack-cred-on-cloud branch April 4, 2024 09:30
MiloradFilipovic added a commit that referenced this pull request Apr 4, 2024
* master:
  fix(core): Ensure only leader handles waiting executions (#9014)
  fix(editor): Fix execution with wait node (#9051)
  fix(editor): Issue showing Auth2 callback section when all properties are overriden (#8999)
  fix(editor): Rerun failed nodes in manual executions (#9050)
  fix(editor): Canvas showing error toast when clicking outside of "import workflow by url" modal (#9001)
  fix: Workflows executed from other workflows not stopping (#9010)
  fix: Fix missing input panel in node details view (#9043)
  fix(editor): Prevent saving workflow while another save is in progress (#9048)
despairblue pushed a commit that referenced this pull request Apr 4, 2024
@github-actions github-actions bot mentioned this pull request Apr 4, 2024
despairblue pushed a commit that referenced this pull request Apr 4, 2024
@janober
Copy link
Member

janober commented Apr 5, 2024

Got released with n8n@1.36.1

@github-actions github-actions bot mentioned this pull request Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
n8n team Authored by the n8n team Released ui Enhancement in /editor-ui or /design-system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants