From ad85b9a62c91c3af3c8fe1ede88f4173284b0a58 Mon Sep 17 00:00:00 2001 From: Ama Asare Date: Tue, 17 May 2016 14:01:18 -0500 Subject: [PATCH 1/5] Ama/Kumar: Configure authorization for SAML --- docs/dev/saml.rst | 34 +++++++++++++++++++++++ redash/authentication/google_oauth.py | 2 ++ redash/authentication/saml_auth.py | 7 ++++- redash/models.py | 13 +++++++++ tests/test_models.py | 39 ++++++++++++++++++++++++++- 5 files changed, 93 insertions(+), 2 deletions(-) create mode 100644 docs/dev/saml.rst diff --git a/docs/dev/saml.rst b/docs/dev/saml.rst new file mode 100644 index 000000000..5fff029b4 --- /dev/null +++ b/docs/dev/saml.rst @@ -0,0 +1,34 @@ +SAML Authentication and Authorization +##################################### + +Authentication +============== + +Add to your .env file REDASH_SAML_METADATA_URL config value which +needs to point to the SAML provider metadata url, eg https://app.onelogin.com/saml/metadata/ + +And an optional REDASH_SAML_CALLBACK_SERVER_NAME which contains the + server name of the redash server for the callbacks from the SAML provider (eg demo.redash.io) + +On the SAML provider side, example configuration for OneLogin is: +SAML Consumer URL: http://demo.redash.io/saml/login +SAML Audience: http://demo.redash.io/saml/callback +SAML Recipient: http://demo.redash.io/saml/callback + +Example configuration for Okta is: +Single Sign On URL: http://demo.redash.io/saml/callback +Recipient URL: http://demo.redash.io/saml/callback +Destination URL: http://demo.redash.io/saml/callback + +with parameters 'FirstName' and 'LastName', both configured to be included in the SAML assertion. + + +Authorization +============= +To manage group assignments in Redash using your SAML provider, configure SAML response to include +attribute with key 'RedashGroups', and value as names of groups in Redash. + +Example configuration for Okta is: +In the Group Attribute Statements - +Name: RedashGroups +Filter: Starts with: this-is-a-group-in-redash \ No newline at end of file diff --git a/redash/authentication/google_oauth.py b/redash/authentication/google_oauth.py index 1e21fd1df..0dba2ea05 100644 --- a/redash/authentication/google_oauth.py +++ b/redash/authentication/google_oauth.py @@ -69,6 +69,8 @@ def create_and_login_user(org, name, email): login_user(user_object, remember=True) + return user_object + @blueprint.route('//oauth/google', endpoint="authorize_org") def org_login(org_slug): diff --git a/redash/authentication/saml_auth.py b/redash/authentication/saml_auth.py index 918c4b3f6..d3faeb5fb 100644 --- a/redash/authentication/saml_auth.py +++ b/redash/authentication/saml_auth.py @@ -85,7 +85,12 @@ def idp_initiated(): # This is what as known as "Just In Time (JIT) provisioning". # What that means is that, if a user in a SAML assertion # isn't in the user store, we create that user first, then log them in - create_and_login_user(current_org, name, email) + user = create_and_login_user(current_org, name, email) + + if 'RedashGroups' in authn_response.ava: + group_names = authn_response.ava.get('RedashGroups') + user.update_group_assignments(group_names, current_org) + url = url_for('redash.index') return redirect(url) diff --git a/redash/models.py b/redash/models.py index 3d30788ce..f57d5f344 100644 --- a/redash/models.py +++ b/redash/models.py @@ -244,6 +244,13 @@ class Group(BaseModel, BelongsToOrgMixin): def members(cls, group_id): return User.select().where(peewee.SQL("%s = ANY(groups)", group_id)) + @classmethod + def find_by_name(cls, org, group_names): + if len(group_names) == 0: + return [] + result = cls.select().where(cls.org == org, cls.name << group_names) + return list(result) if result.count() > 0 else [] + def __unicode__(self): return unicode(self.id) @@ -330,6 +337,12 @@ class User(ModelTimestampsMixin, BaseModel, BelongsToOrgMixin, UserMixin, Permis def verify_password(self, password): return self.password_hash and pwd_context.verify(password, self.password_hash) + def update_group_assignments(self, group_names, org): + groups = Group.find_by_name(org, group_names) + groups.append(org.default_group) + self.groups = map(lambda g: g.id, groups) + self.save() + class ConfigurationField(peewee.TextField): def db_value(self, value): diff --git a/tests/test_models.py b/tests/test_models.py index e4190c1d9..91cdfa5b5 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -276,7 +276,6 @@ class QueryArchiveTest(BaseTestCase): self.assertEqual(None, query.schedule) - class DataSourceTest(BaseTestCase): def test_get_schema(self): return_value = [{'name': 'table', 'columns': []}] @@ -415,6 +414,44 @@ class TestQueryAll(BaseTestCase): self.assertIn(q2, models.Query.all_queries([group1, group2])) +class TestUser(BaseTestCase): + def test_default_group_always_added(self): + user = self.factory.user + org1 = self.factory.create_org() + + user.update_group_assignments(["g_unknown"], org1) + self.assertItemsEqual([org1.default_group.id], user.groups) + + def test_update_group_assignments(self): + user = self.factory.user + org1 = self.factory.create_org() + new_group = models.Group.create(id='999', name="g1", org=org1) + + user.update_group_assignments(["g1"], org1) + self.assertItemsEqual([org1.default_group.id, new_group.id], user.groups) + + +class TestGroup(BaseTestCase): + def test_returns_groups_with_specified_names(self): + org1 = self.factory.create_org() + org2 = self.factory.create_org() + + matching_group1 = models.Group.create(id='999', name="g1", org=org1) + matching_group2 = models.Group.create(id='888', name="g2", org=org1) + non_matching_group = models.Group.create(id='777', name="g1", org=org2) + + groups = models.Group.find_by_name(org1, ["g1", "g2"]) + self.assertIn(matching_group1, groups) + self.assertIn(matching_group2, groups) + self.assertNotIn(non_matching_group, groups) + + def test_returns_no_groups(self): + org1 = self.factory.create_org() + + models.Group.create(id='999', name="g1", org=org1) + self.assertEqual([], models.Group.find_by_name(org1, ["non-existing"])) + + class TestQueryResultStoreResult(BaseTestCase): def setUp(self): super(TestQueryResultStoreResult, self).setUp() From c1c2db4a7380305d1a7d51726db864ddc87d891a Mon Sep 17 00:00:00 2001 From: Kumar Vora Date: Fri, 20 May 2016 14:28:08 -0500 Subject: [PATCH 2/5] use user.org instead of passing org as a separate argument --- redash/authentication/saml_auth.py | 2 +- redash/models.py | 6 +++--- tests/test_models.py | 14 ++++++-------- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/redash/authentication/saml_auth.py b/redash/authentication/saml_auth.py index d3faeb5fb..80fa5d2f9 100644 --- a/redash/authentication/saml_auth.py +++ b/redash/authentication/saml_auth.py @@ -89,7 +89,7 @@ def idp_initiated(): if 'RedashGroups' in authn_response.ava: group_names = authn_response.ava.get('RedashGroups') - user.update_group_assignments(group_names, current_org) + user.update_group_assignments(group_names) url = url_for('redash.index') diff --git a/redash/models.py b/redash/models.py index f57d5f344..2303a03e3 100644 --- a/redash/models.py +++ b/redash/models.py @@ -337,9 +337,9 @@ class User(ModelTimestampsMixin, BaseModel, BelongsToOrgMixin, UserMixin, Permis def verify_password(self, password): return self.password_hash and pwd_context.verify(password, self.password_hash) - def update_group_assignments(self, group_names, org): - groups = Group.find_by_name(org, group_names) - groups.append(org.default_group) + def update_group_assignments(self, group_names): + groups = Group.find_by_name(self.org, group_names) + groups.append(self.org.default_group) self.groups = map(lambda g: g.id, groups) self.save() diff --git a/tests/test_models.py b/tests/test_models.py index 91cdfa5b5..bde70958b 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -416,19 +416,17 @@ class TestQueryAll(BaseTestCase): class TestUser(BaseTestCase): def test_default_group_always_added(self): - user = self.factory.user - org1 = self.factory.create_org() + user = self.factory.create_user() - user.update_group_assignments(["g_unknown"], org1) - self.assertItemsEqual([org1.default_group.id], user.groups) + user.update_group_assignments(["g_unknown"]) + self.assertItemsEqual([user.org.default_group.id], user.groups) def test_update_group_assignments(self): user = self.factory.user - org1 = self.factory.create_org() - new_group = models.Group.create(id='999', name="g1", org=org1) + new_group = models.Group.create(id='999', name="g1", org=user.org) - user.update_group_assignments(["g1"], org1) - self.assertItemsEqual([org1.default_group.id, new_group.id], user.groups) + user.update_group_assignments(["g1"], user.org) + self.assertItemsEqual([user.org.default_group.id, new_group.id], user.groups) class TestGroup(BaseTestCase): From 8900d02c955514d1c0c1da365527df9f73c5120b Mon Sep 17 00:00:00 2001 From: Kumar Vora Date: Fri, 20 May 2016 14:35:49 -0500 Subject: [PATCH 3/5] fixing test --- tests/test_models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_models.py b/tests/test_models.py index bde70958b..12c5d5ae2 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -425,7 +425,7 @@ class TestUser(BaseTestCase): user = self.factory.user new_group = models.Group.create(id='999', name="g1", org=user.org) - user.update_group_assignments(["g1"], user.org) + user.update_group_assignments(["g1"]) self.assertItemsEqual([user.org.default_group.id, new_group.id], user.groups) From b5be5a8fa4ae35c59e5fbc402be610f1b97b9966 Mon Sep 17 00:00:00 2001 From: Kumar Vora Date: Tue, 24 May 2016 16:38:41 -0500 Subject: [PATCH 4/5] no need to check count of results --- redash/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redash/models.py b/redash/models.py index 2303a03e3..368cbfda0 100644 --- a/redash/models.py +++ b/redash/models.py @@ -249,7 +249,7 @@ class Group(BaseModel, BelongsToOrgMixin): if len(group_names) == 0: return [] result = cls.select().where(cls.org == org, cls.name << group_names) - return list(result) if result.count() > 0 else [] + return list(result) def __unicode__(self): return unicode(self.id) From 69177752bcb72268f1635c78b78546caeacb4d22 Mon Sep 17 00:00:00 2001 From: Kumar Vora Date: Thu, 26 May 2016 14:46:25 -0500 Subject: [PATCH 5/5] addresses PR feedback! --- redash/models.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/redash/models.py b/redash/models.py index 368cbfda0..8eb346521 100644 --- a/redash/models.py +++ b/redash/models.py @@ -246,8 +246,6 @@ class Group(BaseModel, BelongsToOrgMixin): @classmethod def find_by_name(cls, org, group_names): - if len(group_names) == 0: - return [] result = cls.select().where(cls.org == org, cls.name << group_names) return list(result)