diff --git a/auth_saml/controllers/main.py b/auth_saml/controllers/main.py
index fb635d3a72..54a7d0c29f 100644
--- a/auth_saml/controllers/main.py
+++ b/auth_saml/controllers/main.py
@@ -5,10 +5,11 @@
import functools
import json
import logging
+from urllib.parse import quote_plus, unquote_plus, urlencode
import werkzeug.utils
+from saml2.validate import ResponseLifetimeExceed
from werkzeug.exceptions import BadRequest
-from werkzeug.urls import url_quote_plus
from odoo import (
SUPERUSER_ID,
@@ -100,7 +101,7 @@ def _auth_saml_request_link(self, provider: models.Model):
redirect = request.params.get("redirect")
if redirect:
params["redirect"] = redirect
- return "/auth_saml/get_auth_request?%s" % werkzeug.urls.url_encode(params)
+ return "/auth_saml/get_auth_request?%s" % urlencode(params)
@http.route()
def web_client(self, s_action=None, **kw):
@@ -132,10 +133,13 @@ def web_login(self, *args, **kw):
response = super().web_login(*args, **kw)
if response.is_qweb:
error = request.params.get("saml_error")
+ # TODO c’est par là qu’il faut changer des trucs
if error == "no-signup":
error = _("Sign up is not allowed on this database.")
elif error == "access-denied":
error = _("Access Denied")
+ elif error == "response-lifetime-exceed":
+ error = _("Response Lifetime Exceeded")
elif error == "expired":
error = _(
"You do not have access to this database. Please contact"
@@ -169,7 +173,7 @@ def _get_saml_extra_relaystate(self):
)
state = {
- "r": url_quote_plus(redirect),
+ "r": quote_plus(redirect),
}
return state
@@ -231,9 +235,7 @@ def signin(self, **kw):
)
action = state.get("a")
menu = state.get("m")
- redirect = (
- werkzeug.urls.url_unquote_plus(state["r"]) if state.get("r") else False
- )
+ redirect = unquote_plus(state["r"]) if state.get("r") else False
url = "/web"
if redirect:
url = redirect
@@ -255,6 +257,9 @@ def signin(self, **kw):
redirect = werkzeug.utils.redirect(url, 303)
redirect.autocorrect_location_header = False
return redirect
+ except ResponseLifetimeExceed as e:
+ _logger.debug("Response Lifetime Exceed - %s", str(e))
+ url = "/web/login?saml_error=response-lifetime-exceed"
except Exception as e:
# signup error
diff --git a/auth_saml/models/auth_saml_attribute_mapping.py b/auth_saml/models/auth_saml_attribute_mapping.py
index 6fb6190538..ec9537b6b2 100644
--- a/auth_saml/models/auth_saml_attribute_mapping.py
+++ b/auth_saml/models/auth_saml_attribute_mapping.py
@@ -13,6 +13,7 @@ class AuthSamlAttributeMapping(models.Model):
"auth.saml.provider",
index=True,
required=True,
+ ondelete="cascade",
)
attribute_name = fields.Char(
string="IDP Response Attribute",
diff --git a/auth_saml/models/auth_saml_provider.py b/auth_saml/models/auth_saml_provider.py
index 4b323b7c26..2409420bd0 100644
--- a/auth_saml/models/auth_saml_provider.py
+++ b/auth_saml/models/auth_saml_provider.py
@@ -81,6 +81,7 @@ class AuthSamlProvider(models.Model):
"auth.saml.attribute.mapping",
"provider_id",
string="Attribute Mapping",
+ copy=True,
)
active = fields.Boolean(default=True)
sequence = fields.Integer(index=True)
@@ -136,6 +137,20 @@ class AuthSamlProvider(models.Model):
default=True,
help="Whether metadata should be signed or not",
)
+ # User creation fields
+ create_user = fields.Boolean(
+ default=False,
+ help="Create user if not found. The login and name will defaults to the SAML "
+ "user matching attribute. Use the mapping attributes to change the value "
+ "used.",
+ )
+ create_user_template_id = fields.Many2one(
+ comodel_name="res.users",
+ # Template users, like base.default_user, are disabled by default so allow them
+ domain="[('active', 'in', (True, False))]",
+ default=lambda self: self.env.ref("base.default_user"),
+ help="When creating user, this user is used as a template",
+ )
@api.model
def _sig_alg_selection(self):
@@ -256,9 +271,7 @@ def _get_auth_request(self, extra_state=None, url_root=None):
}
state.update(extra_state)
- sig_alg = ds.SIG_RSA_SHA1
- if self.sig_alg:
- sig_alg = getattr(ds, self.sig_alg)
+ sig_alg = getattr(ds, self.sig_alg)
saml_client = self._get_client_for_provider(url_root)
reqid, info = saml_client.prepare_for_authenticate(
@@ -287,27 +300,15 @@ def _validate_auth_response(self, token: str, base_url: str = None):
saml2.entity.BINDING_HTTP_POST,
self._get_outstanding_requests_dict(),
)
- matching_value = None
-
- if self.matching_attribute == "subject.nameId":
- matching_value = response.name_id.text
- else:
- attrs = response.get_identity()
-
- for k, v in attrs.items():
- if k == self.matching_attribute:
- matching_value = v
- break
-
- if not matching_value:
- raise Exception(
- f"Matching attribute {self.matching_attribute} not found "
- f"in user attrs: {attrs}"
- )
-
- if matching_value and isinstance(matching_value, list):
- matching_value = next(iter(matching_value), None)
-
+ try:
+ matching_value = self._get_attribute_value(
+ response, self.matching_attribute
+ )
+ except KeyError:
+ raise Exception(
+ f"Matching attribute {self.matching_attribute} not found "
+ f"in user attrs: {response.get_identity()}"
+ ) from None
if isinstance(matching_value, str) and self.matching_attribute_to_lower:
matching_value = matching_value.lower()
@@ -349,24 +350,59 @@ def _metadata_string(self, valid=None, base_url: str = None):
sign=self.sign_metadata,
)
+ @staticmethod
+ def _get_attribute_value(response, attribute_name: str):
+ """
+
+ :raise: KeyError if attribute is not in the response
+ :param response:
+ :param attribute_name:
+ :return: value of the attribut. if the value is an empty list, return None
+ otherwise return the first element of the list
+ """
+ if attribute_name == "subject.nameId":
+ return response.name_id.text
+ attrs = response.get_identity()
+ attribute_value = attrs[attribute_name]
+ if isinstance(attribute_value, list):
+ attribute_value = next(iter(attribute_value), None)
+ return attribute_value
+
def _hook_validate_auth_response(self, response, matching_value):
self.ensure_one()
vals = {}
- attrs = response.get_identity()
for attribute in self.attribute_mapping_ids:
- if attribute.attribute_name not in attrs:
- _logger.debug(
+ try:
+ vals[attribute.field_name] = self._get_attribute_value(
+ response, attribute.attribute_name
+ )
+ except KeyError:
+ _logger.warning(
"SAML attribute '%s' found in response %s",
attribute.attribute_name,
- attrs,
+ response.get_identity(),
)
- continue
- attribute_value = attrs[attribute.attribute_name]
- if isinstance(attribute_value, list):
- attribute_value = attribute_value[0]
+ return {"mapped_attrs": vals}
- vals[attribute.field_name] = attribute_value
+ def _user_copy_defaults(self, validation):
+ """
+ Returns defaults when copying the template user.
- return {"mapped_attrs": vals}
+ Can be overridden with extra information.
+ :param validation: validation result
+ :return: a dictionary for copying template user, empty to avoid copying
+ """
+ self.ensure_one()
+ if not self.create_user:
+ return {}
+ saml_uid = validation["user_id"]
+ return {
+ "name": saml_uid,
+ "login": saml_uid,
+ "active": True,
+ # if signature is not provided by mapped_attrs, it will be computed
+ # due to call to compute method in calling method.
+ "signature": None,
+ } | validation.get("mapped_attrs", {})
diff --git a/auth_saml/models/res_users.py b/auth_saml/models/res_users.py
index 412b5c6994..68a1733409 100644
--- a/auth_saml/models/res_users.py
+++ b/auth_saml/models/res_users.py
@@ -7,7 +7,7 @@
import passlib
-from odoo import SUPERUSER_ID, _, api, fields, models, registry, tools
+from odoo import SUPERUSER_ID, Command, _, api, fields, models, registry, tools
from odoo.exceptions import AccessDenied, ValidationError
from .ir_config_parameter import ALLOW_SAML_UID_AND_PASSWORD
@@ -45,19 +45,52 @@ def _auth_saml_signin(self, provider: int, validation: dict, saml_response) -> s
limit=1,
)
user = user_saml.user_id
- if len(user) != 1:
- raise AccessDenied()
+ user_copy_defaults = {}
+ if not user:
+ user_copy_defaults = (
+ self.env["auth.saml.provider"]
+ .browse(provider)
+ ._user_copy_defaults(validation)
+ )
+ if not user_copy_defaults:
+ raise AccessDenied()
with registry(self.env.cr.dbname).cursor() as new_cr:
new_env = api.Environment(new_cr, self.env.uid, self.env.context)
+ if user_copy_defaults:
+ new_user = (
+ new_env["auth.saml.provider"]
+ .browse(provider)
+ .create_user_template_id.with_context(no_reset_password=True)
+ .copy(
+ {
+ **user_copy_defaults,
+ "saml_ids": [
+ Command.create(
+ {
+ "saml_provider_id": provider,
+ "saml_uid": saml_uid,
+ "saml_access_token": saml_response,
+ }
+ )
+ ],
+ }
+ )
+ )
+ # Update signature as needed.
+ new_user._compute_signature()
+ return new_user.login
+
# Update the token. Need to be committed, otherwise the token is not visible
# to other envs, like the one used in login_and_redirect
user_saml.with_env(new_env).write({"saml_access_token": saml_response})
- if validation.get("mapped_attrs", {}):
- user.write(validation.get("mapped_attrs", {}))
+ # if a login is changed by a mapped attribute, it needs to be commited too
+ user = user.with_env(new_env)
+ if validation.get("mapped_attrs", {}):
+ user.write(validation.get("mapped_attrs", {}))
- return user.login
+ return user.login
@api.model
def auth_saml(self, provider: int, saml_response: str, base_url: str = None):
diff --git a/auth_saml/models/res_users_saml.py b/auth_saml/models/res_users_saml.py
index d7cbd308d3..a60f493535 100644
--- a/auth_saml/models/res_users_saml.py
+++ b/auth_saml/models/res_users_saml.py
@@ -7,7 +7,9 @@ class ResUserSaml(models.Model):
_name = "res.users.saml"
_description = "User to SAML Provider Mapping"
- user_id = fields.Many2one("res.users", index=True, required=True)
+ user_id = fields.Many2one(
+ "res.users", index=True, required=True, ondelete="cascade"
+ )
saml_provider_id = fields.Many2one(
"auth.saml.provider", string="SAML Provider", index=True
)
diff --git a/auth_saml/readme/CONFIGURE.md b/auth_saml/readme/CONFIGURE.md
index 68072d142c..5a0f1ea84b 100644
--- a/auth_saml/readme/CONFIGURE.md
+++ b/auth_saml/readme/CONFIGURE.md
@@ -2,7 +2,8 @@ To use this module, you need an IDP server, properly set up.
1. Configure the module according to your IdP’s instructions (Settings
\> Users & Companies \> SAML Providers).
-2. Pre-create your users and set the SAML information against the user.
+2. Pre-create your users and set the SAML information against the user,
+ or use the module ability to create users as they log in.
By default, the module let users have both a password and SAML ids. To
increase security, disable passwords by using the option in Settings.
diff --git a/auth_saml/readme/HISTORY.md b/auth_saml/readme/HISTORY.md
index 89020f8c3c..0c0341713c 100644
--- a/auth_saml/readme/HISTORY.md
+++ b/auth_saml/readme/HISTORY.md
@@ -1,3 +1,12 @@
-## 16.0.1.0.0
+## 17.0.1.1.0
-Initial migration for 16.0.
+- custom message when response is too old
+- avoid using werkzeug.urls method, they are deprecated
+- add missing ondelete cascade when user is deleted
+- attribute mapping is now also duplicated when the provider is duplicated
+- factorize getting SAML attribute value, allowing using subject.nameId in mapping attributes too
+- allow creating user if not found by copying a template user
+
+## 17.0.1.0.0
+
+Initial migration for 17.0.
diff --git a/auth_saml/tests/fake_idp.py b/auth_saml/tests/fake_idp.py
index f2865b403d..6e4c91ef2e 100644
--- a/auth_saml/tests/fake_idp.py
+++ b/auth_saml/tests/fake_idp.py
@@ -73,13 +73,21 @@
}
+class DummyNameId:
+ """Dummy name id with text value"""
+
+ def __init__(self, text):
+ self.text = text
+
+
class DummyResponse:
- def __init__(self, status, data, headers=None):
+ def __init__(self, status, data, headers=None, name_id: str = ""):
self.status_code = status
self.text = data
self.headers = headers or []
self.content = data
self._identity = {}
+ self.name_id = DummyNameId(name_id)
def _unpack(self, ver="SAMLResponse"):
"""
@@ -127,6 +135,7 @@ def __init__(self, metadatas=None):
config.load(settings)
config.allow_unknown_attributes = True
Server.__init__(self, config=config)
+ self.mail = "test@example.com"
def get_metadata(self):
return create_metadata_string(
@@ -163,7 +172,7 @@ def authn_request_endpoint(self, req, binding, relay_state):
"surName": "Example",
"givenName": "Test",
"title": "Ind",
- "mail": "test@example.com",
+ "mail": self.mail,
}
resp_args.update({"sign_assertion": True, "sign_response": True})
diff --git a/auth_saml/tests/test_pysaml.py b/auth_saml/tests/test_pysaml.py
index 9eedaa5405..7f510173ac 100644
--- a/auth_saml/tests/test_pysaml.py
+++ b/auth_saml/tests/test_pysaml.py
@@ -7,6 +7,7 @@
from odoo.exceptions import AccessDenied, UserError, ValidationError
from odoo.tests import HttpCase, tagged
+from odoo.tools import mute_logger
from .fake_idp import DummyResponse, FakeIDP
@@ -101,6 +102,8 @@ def test_ensure_provider_appears_on_login_with_redirect_param(self):
def test__onchange_name(self):
temp = self.saml_provider.body
+ r = self.saml_provider._onchange_name()
+ self.assertEqual(self.saml_provider.body, temp)
self.saml_provider.body = ""
r = self.saml_provider._onchange_name()
self.assertEqual(r, None)
@@ -135,14 +138,15 @@ def test__compute_sp_metadata_url__provider_has_sp_baseurl(self):
def test__hook_validate_auth_response(self):
# Create a fake response with attributes
- fake_response = DummyResponse(200, "fake_data")
+ fake_response = DummyResponse(200, "fake_data", name_id="new.user")
fake_response.set_identity(
{"email": "new_user@example.com", "first_name": "New", "last_name": "User"}
)
# Add attribute mappings to the provider
self.saml_provider.attribute_mapping_ids = [
- (0, 0, {"attribute_name": "email", "field_name": "login"}),
+ (0, 0, {"attribute_name": "email", "field_name": "email"}),
+ (0, 0, {"attribute_name": "subject.nameId", "field_name": "login"}),
(0, 0, {"attribute_name": "first_name", "field_name": "name"}),
(
0,
@@ -152,13 +156,15 @@ def test__hook_validate_auth_response(self):
]
# Call the method
- result = self.saml_provider._hook_validate_auth_response(
- fake_response, "test@example.com"
- )
+ with mute_logger("odoo.addons.auth_saml.models.auth_saml_provider"):
+ result = self.saml_provider._hook_validate_auth_response(
+ fake_response, "test@example.com"
+ )
# Check the result
self.assertIn("mapped_attrs", result)
- self.assertEqual(result["mapped_attrs"]["login"], "new_user@example.com")
+ self.assertEqual(result["mapped_attrs"]["email"], "new_user@example.com")
+ self.assertEqual(result["mapped_attrs"]["login"], "new.user")
self.assertEqual(result["mapped_attrs"]["name"], "New")
self.assertNotIn("middle_name", result["mapped_attrs"])
@@ -235,7 +241,9 @@ def add_provider_to_user(self):
def test_login_with_saml(self):
self.add_provider_to_user()
+ self._login_with_saml()
+ def _login_with_saml(self):
redirect_url = self.saml_provider._get_auth_request()
self.assertIn("http://localhost:8000/sso/redirect?SAMLRequest=", redirect_url)
@@ -252,7 +260,7 @@ def test_login_with_saml(self):
)
self.assertEqual(database, self.env.cr.dbname)
- self.assertEqual(login, self.user.login)
+ self.assertEqual(login, "test@example.com")
# We should not be able to log in with the wrong token
with self.assertRaises(AccessDenied):
@@ -261,6 +269,23 @@ def test_login_with_saml(self):
# User should now be able to log in with the token
self.authenticate(user="test@example.com", password=token)
+ def test_login_with_saml_to_lower(self):
+ self.add_provider_to_user()
+ self.saml_provider.matching_attribute_to_lower = True
+ self.idp.mail = "TEST@example.com"
+ self._login_with_saml()
+
+ def test_login_with_saml_non_existing_mapping_attribute(self):
+ self.saml_provider.matching_attribute = "nick_name"
+ self.add_provider_to_user()
+ with self.assertRaises(Exception):
+ self._login_with_saml()
+
+ def test_create_user(self):
+ self.user.unlink()
+ self.saml_provider.create_user = True
+ self._login_with_saml()
+
def test_disallow_user_password_when_changing_ir_config_parameter(self):
"""Test that disabling users from having both a password and SAML ids remove
users password."""
diff --git a/auth_saml/views/auth_saml.xml b/auth_saml/views/auth_saml.xml
index 9ee7dc0335..11f0582d8b 100644
--- a/auth_saml/views/auth_saml.xml
+++ b/auth_saml/views/auth_saml.xml
@@ -182,6 +182,16 @@
+
+
+
+