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(core): Allow Azure's SAML metadata XML containing WS-Federation nodes to pass validation #11724

Conversation

despairblue
Copy link
Contributor

@despairblue despairblue commented Nov 13, 2024

Summary

Azure is adding WS Federation data to it's metadata XML.
n8n didn't have the right xsd's to validate that xml correctly even though the rest of the metadata file contains everything else we need to SAML.

This PR adds the schemas and tests for the validation logic, making sure azure's format is now supported.

This only allows passing the xml validation against the XSDs. It does not use the WS-Federation nodes for anything.

Before:
image


[Node] SAML Validate Metadata: Invalid metadata
[Node] element RoleDescriptor: Schemas validity error : Element '{urn:oasis:names:tc:SAML:2.0:metadata}RoleDescriptor', attribute '{http://www.w3.org/2001/XMLSchema-instance}type': The QName value '{http://docs.oasis-open.org/wsfed/federation/200706}SecurityTokenServiceType' of the xsi:type attribute does not resolve to a type definition. - metadata.xml:1: element RoleDescriptor: Schemas validity error : Element '{urn:oasis:names:tc:SAML:2.0:metadata}RoleDescriptor', attribute '{http://www.w3.org/2001/XMLSchema-instance}type': The QName value '{http://docs.oasis-open.org/wsfed/federation/200706}SecurityTokenServiceType' of the xsi:type attribute does not resolve to a type definition.
[Node] element RoleDescriptor: Schemas validity error : Element '{urn:oasis:names:tc:SAML:2.0:metadata}RoleDescriptor': The type definition is abstract. - metadata.xml:1: element RoleDescriptor: Schemas validity error : Element '{urn:oasis:names:tc:SAML:2.0:metadata}RoleDescriptor': The type definition is abstract.
[Node] element RoleDescriptor: Schemas validity error : Element '{urn:oasis:names:tc:SAML:2.0:metadata}RoleDescriptor', attribute '{http://www.w3.org/2001/XMLSchema-instance}type': The QName value '{http://docs.oasis-open.org/wsfed/federation/200706}ApplicationServiceType' of the xsi:type attribute does not resolve to a type definition. - metadata.xml:1: element RoleDescriptor: Schemas validity error : Element '{urn:oasis:names:tc:SAML:2.0:metadata}RoleDescriptor', attribute '{http://www.w3.org/2001/XMLSchema-instance}type': The QName value '{http://docs.oasis-open.org/wsfed/federation/200706}ApplicationServiceType' of the xsi:type attribute does not resolve to a type definition.
[Node] element RoleDescriptor: Schemas validity error : Element '{urn:oasis:names:tc:SAML:2.0:metadata}RoleDescriptor': The type definition is abstract. - metadata.xml:1: element RoleDescriptor: Schemas validity error : Element '{urn:oasis:names:tc:SAML:2.0:metadata}RoleDescriptor': The type definition is abstract.
[Node] 400 Error fetching SAML Metadata from  https://login.microsoftonline.com/c42130b5-2c34-4815-a7a0-8a52fddd788f/federationmetadata/2007-06/federationmetadata.xml?appid=e556d554-06c9-4de6-ba3f-e887dd9b2b15: ResponseError: Data received from  https://login.microsoftonline.com/c42130b5-2c34-4815-a7a0-8a52fddd788f/federationmetadata/2007-06/federationmetadata.xml?appid=e556d554-06c9-4de6-ba3f-e887dd9b2b15 is not valid SAML metadata.

After:
image

Related Linear tickets, Github issues, and Community forum posts

Docs PR: n8n-io/n8n-docs#2617
Linear Ticket: https://linear.app/n8n/issue/HELP-643/urgent-not-able-to-login-into-n8n-after-upgrading-version#comment-1886eaa9

Review / Merge checklist

  • PR title and summary are descriptive. (conventions)
  • Docs updated or follow-up ticket created.
  • Tests included.
  • PR Labeled with release/backport (if the PR is an urgent fix that needs to be backported)

…odes to pass validation

This only allows passing the xml validation against the XSDs. It does not use the WS-Federation nodes for anything.

But this allows using the metadata URL to keep the metadata up to date automatically opposed to copy and pasting the xml from Azure, while removing the WS-Federation nodes.
@n8n-assistant n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team labels Nov 13, 2024
despairblue added a commit to n8n-io/n8n-docs that referenced this pull request Nov 13, 2024
This is not necessary anymore once
n8n-io/n8n#11724 is merged.
Copy link

codecov bot commented Nov 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

@despairblue despairblue marked this pull request as ready for review November 13, 2024 16:59
Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <[email protected]>
Copy link
Member

@netroy netroy left a comment

Choose a reason for hiding this comment

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

👏🏽

Copy link

cypress bot commented Nov 14, 2024

n8n    Run #7884

Run Properties:  status check passed Passed #7884  •  git commit 9c873f4ebc: 🌳 🖥️ browsers:node18.12.0-chrome107 🤖 despairblue 🗃️ e2e/*
Project n8n
Branch Review help-643-urgent-not-able-to-login-into-n8n-after-upgrading-version
Run status status check passed Passed #7884
Run duration 04m 20s
Commit git commit 9c873f4ebc: 🌳 🖥️ browsers:node18.12.0-chrome107 🤖 despairblue 🗃️ e2e/*
Committer Danny Martini
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 470
View all changes introduced in this branch ↗︎

Copy link
Contributor

✅ All Cypress E2E specs passed

@despairblue despairblue merged commit 3b62bd5 into master Nov 14, 2024
35 checks passed
@despairblue despairblue deleted the help-643-urgent-not-able-to-login-into-n8n-after-upgrading-version branch November 14, 2024 13:33
@github-actions github-actions bot mentioned this pull request Nov 20, 2024
@janober
Copy link
Member

janober commented Nov 20, 2024

Got released with [email protected]

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants