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

Render LDAP and remote auth login links correctly when multi org mode is enabled. #3530

Merged
merged 2 commits into from
Mar 27, 2019

Conversation

jezdez
Copy link
Member

@jezdez jezdez commented Mar 5, 2019

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix

Description

When the multi org feature and remote or LDAP auth is enabled at the same time, the login links on the login landing page aren't correctly rendered since they are not passed the current org slug.

As an aside the LDAP auth handler isn't org scoped for some reason, and it's not clear if this was an oversight or deliberate decision.

Related Tickets & Documents

3044c77 added the URL reversal to login.html and seems to be the culprit of the missing org parameter.

@ghost ghost assigned jezdez Mar 5, 2019
@ghost ghost added the in progress label Mar 5, 2019
@jezdez jezdez requested a review from arikfr March 5, 2019 12:27
@@ -91,18 +90,26 @@ def setUp(self):
super(TestLogin, self).setUp()
self.factory.org.set_setting('auth_password_login_enabled', True)

@classmethod
Copy link
Member Author

Choose a reason for hiding this comment

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

I think those methods are leftovers from a previous iteration of the multi org feature.

@jezdez jezdez requested a review from emtwo March 6, 2019 08:44
@jezdez jezdez added this to the Next milestone Mar 7, 2019
@jezdez jezdez force-pushed the remote_auth-login branch from 0b808ab to 4f4b1c3 Compare March 11, 2019 08:24
@jezdez
Copy link
Member Author

jezdez commented Mar 11, 2019

@arikfr Ready for another look.

@jezdez
Copy link
Member Author

jezdez commented Mar 27, 2019

@arikfr Ready when you're ready.

@arikfr arikfr merged commit 6c26aa7 into master Mar 27, 2019
@arikfr arikfr deleted the remote_auth-login branch March 27, 2019 15:26
The-Alchemist pushed a commit to The-Alchemist/redash that referenced this pull request Jul 17, 2019
… is enabled. (getredash#3530)

* Make LDAP auth handler org scoped.

* Render LDAP and remote auth login links correctly when multi org mode is enabled.
harveyrendell pushed a commit to pushpay/redash that referenced this pull request Nov 14, 2019
… is enabled. (getredash#3530)

* Make LDAP auth handler org scoped.

* Render LDAP and remote auth login links correctly when multi org mode is enabled.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants