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

refactor(core): Extract all Auth-related User columns into a separate entity #9557

Merged
merged 3 commits into from
May 31, 2024

Conversation

netroy
Copy link
Member

@netroy netroy commented May 30, 2024

Db entities with columns marked with select: false or queries with a select param return a partial object that isn't type-safe, and creates the need for all sorts of checks and workarounds. This creates issues like in #9524 where the code does not guarantees that there will be an mfaSecret in places that need one.
This PR extracts those columns out into a separate DB entity that always contains those columns, and makes all code around this a bit more reliable with fewer runtime checks.
This also removes the need to filter out those columns for user objects that represent other users besides the one talking to the API.

Review / Merge checklist

  • PR title and summary are descriptive
  • Tests included

@n8n-assistant n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team ui Enhancement in /editor-ui or /design-system labels May 30, 2024
Before we were sending `mfaToken` and `mfaRecoveryCode` even though the user can only send one. It was working because we had an or condition in the backend. The changes on this PR change that condition, making it imposible to login with recovery codes. This change makes the FE to send only the field the user used making recovery codes work again.
Copy link

cypress bot commented May 30, 2024

Passing run #5200 ↗︎

0 362 0 0 Flakiness 0

Details:

🌳 master 🖥️ browsers:node18.12.0-chrome107 🤖 PR User 🗃️ e2e/*
Project: n8n Commit: 47d774100b
Status: Passed Duration: 04:29 💡
Started: May 31, 2024 3:10 AM Ended: May 31, 2024 3:15 AM

Review all test suite changes for PR #9557 ↗︎

RicardoE105
RicardoE105 previously approved these changes May 30, 2024
Copy link
Contributor

@RicardoE105 RicardoE105 left a comment

Choose a reason for hiding this comment

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

The code looks good. I tested it locally, and it worked fine! There is just a small fix in the FE, as the log-in with the recovery code broke.

Copy link
Contributor

⚠️ Some Cypress E2E specs are failing, please fix them before merging

Copy link
Contributor

✅ All Cypress E2E specs passed

@netroy netroy merged commit 5887ed6 into master May 31, 2024
55 checks passed
@netroy netroy deleted the AuthUser-refactor-part-1 branch May 31, 2024 07:40
MiloradFilipovic added a commit that referenced this pull request May 31, 2024
* master:
  fix(editor): Fix ts errors across the board (no-changelog) (#9561)
  refactor(core): Extract all Auth-related User columns into a separate entity (#9557)
  refactor(core): Update supertest, and fix some typing errors (no-changelog) (#9527)
  fix(editor): Render checkboxes in markdown (#9549)
  test(core): Rename and combine all credential api tests (no-changelog) (#9550)
  🚀 Release 1.44.0 (#9553)
  feat(editor): Node Creator AI nodes improvements  (#9484)
  fix(editor): Show workflow data in header when execution page is hard reloaded (#9529)
  refactor(core): Stop reporting to Sentry `NodeApiError` with 5xx status codes (no-changelog) (#9552)
  feat: HighLevel oauth2 api credentials (#9542)
  fix(editor): Fix empty node name handling (#9548)
  ci: Fix vulnerable dev dependencies (no-changelog) (#9545)
  fix: Don't throw errors for NaN in number operators in the filter component  (#9506)
  fix(core): Try setting postgres search_path on the database (#9530)
  fix(editor): Redirect to workflows list after deleting a workflow (#9546)

# Conflicts:
#	packages/editor-ui/src/views/NodeView.vue
#	pnpm-lock.yaml
@janober
Copy link
Member

janober commented Jun 5, 2024

Got released with n8n@1.45.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Enhancement outside /nodes-base and /editor-ui 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.

3 participants