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

[15.0][ADD] auth_saml: handle redirect parameter in the URI #483

Merged
merged 1 commit into from
Sep 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@


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 @@
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 @@
)
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 @@
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 @@
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 @@
)
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
10 changes: 10 additions & 0 deletions auth_saml/readme/HISTORY.rst
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
15.0.1.4.0
~~~~~~~~~~

Handle redirect after authentification.

15.0.1.3.0
~~~~~~~~~~

Improve login page.

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
Loading