Skip to content

Commit

Permalink
[ADD] auth_saml: handle redirect parameter in the URI
Browse files Browse the repository at this point in the history
  • Loading branch information
vincent-hatakeyama committed Jul 4, 2023
1 parent 0dd018b commit 16b420a
Show file tree
Hide file tree
Showing 5 changed files with 100 additions and 12 deletions.
32 changes: 26 additions & 6 deletions auth_saml/controllers/main.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
# Copyright (C) 2020 GlodoUK <https://www.glodo.uk/>
# Copyright (C) 2010-2016, 2022 XCG Consulting <https://xcg-consulting.fr/>
# Copyright (C) 2010-2016, 2022-2023 XCG Consulting <https://xcg-consulting.fr/>
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).

import functools
import json
import logging

import werkzeug.utils
from werkzeug.exceptions import BadRequest
from werkzeug.urls import url_quote_plus

import odoo
Expand Down Expand Up @@ -54,7 +55,7 @@ def wrapper(self, req, **kw):


class SAMLLogin(Home):
def _list_saml_providers_domain(self):
def _list_saml_providers_domain(self): # pylint: disable=no-self-use
return []

def list_saml_providers(self, with_autoredirect: bool = False) -> models.Model:
Expand All @@ -67,7 +68,8 @@ def list_saml_providers(self, with_autoredirect: bool = False) -> models.Model:
if with_autoredirect:
domain.append(("autoredirect", "=", True))
providers = request.env["auth.saml.provider"].sudo().search_read(domain)

for provider in providers:
provider["auth_link"] = self._auth_saml_request_link(provider)
return providers

def _saml_autoredirect(self):
Expand All @@ -79,11 +81,21 @@ def _saml_autoredirect(self):
)
if autoredirect_providers and not disable_autoredirect:
return werkzeug.utils.redirect(
"/auth_saml/get_auth_request?pid=%d" % autoredirect_providers[0]["id"],
self._auth_saml_request_link(autoredirect_providers[0]),
303,
)
return None

def _auth_saml_request_link(self, provider: models.Model):
"""Return the auth request link for the provided provider"""
params = {
"pid": provider["id"],
}
redirect = request.params.get("redirect")
if redirect:
params["redirect"] = redirect
return "/auth_saml/get_auth_request?%s" % werkzeug.urls.url_encode(params)

@http.route()
def web_client(self, s_action=None, **kw):
ensure_db()
Expand Down Expand Up @@ -144,7 +156,6 @@ def _get_saml_extra_relaystate(self):
The provider will automatically set things like the dbname, provider
id, etc.
"""

redirect = request.params.get("redirect") or "web"
if not redirect.startswith(("//", "http://", "https://")):
redirect = "{}{}".format(
Expand Down Expand Up @@ -199,6 +210,8 @@ def signin(self, req, **kw):
state = json.loads(kw["RelayState"])
provider = state["p"]
dbname = state["d"]
if not http.db_filter([dbname]):
return BadRequest()

Check warning on line 214 in auth_saml/controllers/main.py

View check run for this annotation

Codecov / codecov/patch

auth_saml/controllers/main.py#L214

Added line #L214 was not covered by tests
context = state.get("c", {})
registry = registry_get(dbname)

Expand All @@ -216,8 +229,15 @@ def signin(self, req, **kw):
)
action = state.get("a")
menu = state.get("m")
redirect = (
werkzeug.urls.url_unquote_plus(state["r"])
if state.get("r")
else False
)
url = "/"
if action:
if redirect:
url = redirect
elif action:
url = "/#action=%s" % action
elif menu:
url = "/#menu_id=%s" % menu
Expand Down
6 changes: 3 additions & 3 deletions auth_saml/models/auth_saml_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ def _get_cert_key_path(self, field="sp_pem_public"):

return keys_path

def _get_config_for_provider(self, base_url: str = None):
def _get_config_for_provider(self, base_url: str = None) -> Saml2Config:
"""
Internal helper to get a configured Saml2Client
"""
Expand Down Expand Up @@ -237,7 +237,7 @@ def _get_config_for_provider(self, base_url: str = None):
sp_config.allow_unknown_attributes = True
return sp_config

def _get_client_for_provider(self, base_url: str = None):
def _get_client_for_provider(self, base_url: str = None) -> Saml2Client:
sp_config = self._get_config_for_provider(base_url)
saml_client = Saml2Client(config=sp_config)
return saml_client
Expand Down Expand Up @@ -336,7 +336,7 @@ def _store_outstanding_request(self, reqid):
{"saml_provider_id": self.id, "saml_request_id": reqid}
)

def _metadata_string(self, valid=None, base_url=None):
def _metadata_string(self, valid=None, base_url: str = None):
self.ensure_one()

sp_config = self._get_config_for_provider(base_url)
Expand Down
5 changes: 5 additions & 0 deletions auth_saml/readme/HISTORY.rst
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
15.0.1.2.0
~~~~~~~~~~

Handle redirect after authentification.

15.0.1.1.0
~~~~~~~~~~

Expand Down
67 changes: 65 additions & 2 deletions auth_saml/tests/test_pysaml.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).
import base64
import html
import os
from unittest.mock import patch

from odoo.exceptions import AccessDenied, UserError, ValidationError
from odoo.tests import HttpCase, tagged
Expand Down Expand Up @@ -85,6 +88,21 @@ def test_ensure_provider_appears_on_login(self):
self.assertIn("Login with Authentic", response.text)
self.assertIn(self.url_saml_request, response.text)

def test_ensure_provider_appears_on_login_with_redirect_param(self):
"""Test that SAML provider is listed in the login page keeping the redirect"""
response = self.url_open(
"/web/login?redirect=%2Fweb%23action%3D37%26model%3Dir.module.module%26view"
"_type%3Dkanban%26menu_id%3D5"
)
self.assertIn("Login with Authentic", response.text)
self.assertIn(
"/auth_saml/get_auth_request?pid={}&amp;redirect=%2Fweb%23action%3D37%26mod"
"el%3Dir.module.module%26view_type%3Dkanban%26menu_id%3D5".format(
self.saml_provider.id
),
response.text,
)

def test_ensure_metadata_present(self):
response = self.url_open(
"/auth_saml/metadata?p=%d&d=%s"
Expand All @@ -96,7 +114,7 @@ def test_ensure_metadata_present(self):

def test_ensure_get_auth_request_redirects(self):
response = self.url_open(
"/auth_saml/get_auth_request?pid=%d" % (self.saml_provider.id),
"/auth_saml/get_auth_request?pid=%d" % self.saml_provider.id,
allow_redirects=False,
)
self.assertTrue(response.ok)
Expand Down Expand Up @@ -160,14 +178,15 @@ def test_login_with_saml(self):
self.assertEqual(200, response.status_code)
unpacked_response = response._unpack()

(_database, login, token) = (
(database, login, token) = (
self.env["res.users"]
.sudo()
.auth_saml(
self.saml_provider.id, unpacked_response.get("SAMLResponse"), None
)
)

self.assertEqual(database, self.env.cr.dbname)
self.assertEqual(login, self.user.login)

# We should not be able to log in with the wrong token
Expand Down Expand Up @@ -273,3 +292,47 @@ def test_disallow_user_admin_can_have_password(self):
).value = "False"
# Test base.user_admin exception
self.env.ref("base.user_admin").password = "nNRST4j*->sEatNGg._!"

def test_db_filtering(self):
# change filter to only allow our db.
with patch("odoo.http.db_filter", new=lambda *args, **kwargs: []):
self.add_provider_to_user()

redirect_url = self.saml_provider._get_auth_request()
response = self.idp.fake_login(redirect_url)
unpacked_response = response._unpack()

for key in unpacked_response:
unpacked_response[key] = html.unescape(unpacked_response[key])
response = self.url_open("/auth_saml/signin", data=unpacked_response)
self.assertFalse(response.ok)
self.assertIn(response.status_code, [400, 404])

def test_redirect_after_login(self):
"""Test that providing a redirect will be kept after SAML login."""
self.add_provider_to_user()

redirect_url = self.saml_provider._get_auth_request(
{
"r": "%2Fweb%23action%3D37%26model%3Dir.module.module%26view_type%3Dkan"
"ban%26menu_id%3D5"
}
)
response = self.idp.fake_login(redirect_url)
unpacked_response = response._unpack()

for key in unpacked_response:
unpacked_response[key] = html.unescape(unpacked_response[key])
response = self.url_open(
"/auth_saml/signin",
data=unpacked_response,
allow_redirects=True,
timeout=300,
)
self.assertTrue(response.ok)
self.assertEqual(response.status_code, 200)
self.assertEqual(
response.url,
self.base_url()
+ "/web#action=37&model=ir.module.module&view_type=kanban&menu_id=5",
)
2 changes: 1 addition & 1 deletion auth_saml/views/auth_saml.xml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
t-foreach="saml_providers"
t-as="p"
class="list-group-item list-group-item-action py-2"
t-att-href="'/auth_saml/get_auth_request?pid=%s'%p['id']"
t-att-href="p['auth_link']"
>
<i t-att-class="p['css_class']" />
<t t-esc="p['body']" />
Expand Down

0 comments on commit 16b420a

Please sign in to comment.