bug fix: SAML_LOGIN_ENABLED setting logic

[ism-k]

What type of PR is this?

• Refactor
• Feature
• Bug Fix
• New Query Runner (Data Source)
• New Alert Destination
• Other

Description

I would like to point out that an existing SAML_LOGIN_ENABLED may have inappropriately strict conditions for it to be true.

I first identified this problem as a bug in the following PR, which I attach as a reference:
getredash/contrib-helm-chart#122

In short, due to a change merged from v10 in PR #5175, SAML cannot be enabled at startup unless an environment variable REDASH_SAML_SSO_URL is set, but this variable is actually unnecessary in the case of dynamic SAML login.

So I'm adding a new conditional branch to avoid SAML_SSO_URL != "" when the SAML login type is not static.

How is this tested?

• Unit tests (pytest, jest)
• E2E Tests (Cypress)
• Manually
• N/A

Related Tickets & Documents

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

[kenchan0130]

@susodapop

This change is very important for redash admin users with SAML settings.
The change itself also appears to be appropriate.

[susodapop]

I'm not currently contributing to the Redash project.

[acefei]

Highly recommended this merge to the trunk branch, so that I don't need to set the redundant env by kubectl set env deployment/my-release-redash REDASH_SAML_SSO_URL=xxxxx to enable SAML login.
Given @susodapop didn't contribute to the Redash project, could @ism-k invite others to approve this?

[acefei]

ping @arikfr

[arikfr]

👍

[ism-k]

@acefei
Thanks for facilitating!

[arikfr]

This is not related to the Pull Request directly, but I assume that the author or followers might have SAML enabled for their deployment and should be aware of the following Security Advisory: #5961. This affects all Redash versions and should be patched immediately.