Skip to content

Commit ce5eadb

Browse files
[IMP] auth_saml: user provisioning on login
- 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
1 parent 0beb977 commit ce5eadb

File tree

10 files changed

+185
-57
lines changed

10 files changed

+185
-57
lines changed

auth_saml/controllers/main.py

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,12 @@
66
import json
77
import logging
88

9+
from urllib.parse import quote_plus
910
import werkzeug.utils
1011
from werkzeug.exceptions import BadRequest
11-
from werkzeug.urls import url_quote_plus
12+
from urllib.parse import unquote_plus, urlencode
13+
14+
from saml2.validate import ResponseLifetimeExceed
1215

1316
from odoo import (
1417
SUPERUSER_ID,
@@ -100,7 +103,7 @@ def _auth_saml_request_link(self, provider: models.Model):
100103
redirect = request.params.get("redirect")
101104
if redirect:
102105
params["redirect"] = redirect
103-
return "/auth_saml/get_auth_request?%s" % werkzeug.urls.url_encode(params)
106+
return "/auth_saml/get_auth_request?%s" % urlencode(params)
104107

105108
@http.route()
106109
def web_client(self, s_action=None, **kw):
@@ -132,10 +135,13 @@ def web_login(self, *args, **kw):
132135
response = super().web_login(*args, **kw)
133136
if response.is_qweb:
134137
error = request.params.get("saml_error")
138+
# TODO c’est par là qu’il faut changer des trucs
135139
if error == "no-signup":
136140
error = _("Sign up is not allowed on this database.")
137141
elif error == "access-denied":
138142
error = _("Access Denied")
143+
elif error == "response-lifetime-exceed":
144+
error = _("Response Lifetime Exceeded")
139145
elif error == "expired":
140146
error = _(
141147
"You do not have access to this database. Please contact"
@@ -169,7 +175,7 @@ def _get_saml_extra_relaystate(self):
169175
)
170176

171177
state = {
172-
"r": url_quote_plus(redirect),
178+
"r": quote_plus(redirect),
173179
}
174180
return state
175181

@@ -232,7 +238,7 @@ def signin(self, **kw):
232238
action = state.get("a")
233239
menu = state.get("m")
234240
redirect = (
235-
werkzeug.urls.url_unquote_plus(state["r"]) if state.get("r") else False
241+
unquote_plus(state["r"]) if state.get("r") else False
236242
)
237243
url = "/web"
238244
if redirect:
@@ -255,6 +261,9 @@ def signin(self, **kw):
255261
redirect = werkzeug.utils.redirect(url, 303)
256262
redirect.autocorrect_location_header = False
257263
return redirect
264+
except ResponseLifetimeExceed as e:
265+
_logger.debug("Response Lifetime Exceed - %s", str(e))
266+
url = "/web/login?saml_error=response-lifetime-exceed"
258267

259268
except Exception as e:
260269
# signup error

auth_saml/models/auth_saml_attribute_mapping.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ class AuthSamlAttributeMapping(models.Model):
1313
"auth.saml.provider",
1414
index=True,
1515
required=True,
16+
ondelete="cascade",
1617
)
1718
attribute_name = fields.Char(
1819
string="IDP Response Attribute",

auth_saml/models/auth_saml_provider.py

Lines changed: 70 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ class AuthSamlProvider(models.Model):
8181
"auth.saml.attribute.mapping",
8282
"provider_id",
8383
string="Attribute Mapping",
84+
copy=True,
8485
)
8586
active = fields.Boolean(default=True)
8687
sequence = fields.Integer(index=True)
@@ -136,6 +137,20 @@ class AuthSamlProvider(models.Model):
136137
default=True,
137138
help="Whether metadata should be signed or not",
138139
)
140+
# User creation fields
141+
create_user = fields.Boolean(
142+
default=False,
143+
help="Create user if not found. The login and name will defaults to the SAML "
144+
"user matching attribute. Use the mapping attributes to change the value "
145+
"used.",
146+
)
147+
create_user_template_id = fields.Many2one(
148+
comodel_name="res.users",
149+
# Template users, like base.default_user, are disabled by default so allow them
150+
domain="[('active', 'in', (True, False))]",
151+
default=lambda self: self.env.ref("base.default_user"),
152+
help="When creating user, this user is used as a template",
153+
)
139154

140155
@api.model
141156
def _sig_alg_selection(self):
@@ -256,9 +271,7 @@ def _get_auth_request(self, extra_state=None, url_root=None):
256271
}
257272
state.update(extra_state)
258273

259-
sig_alg = ds.SIG_RSA_SHA1
260-
if self.sig_alg:
261-
sig_alg = getattr(ds, self.sig_alg)
274+
sig_alg = getattr(ds, self.sig_alg)
262275

263276
saml_client = self._get_client_for_provider(url_root)
264277
reqid, info = saml_client.prepare_for_authenticate(
@@ -287,27 +300,15 @@ def _validate_auth_response(self, token: str, base_url: str = None):
287300
saml2.entity.BINDING_HTTP_POST,
288301
self._get_outstanding_requests_dict(),
289302
)
290-
matching_value = None
291-
292-
if self.matching_attribute == "subject.nameId":
293-
matching_value = response.name_id.text
294-
else:
295-
attrs = response.get_identity()
296-
297-
for k, v in attrs.items():
298-
if k == self.matching_attribute:
299-
matching_value = v
300-
break
301-
302-
if not matching_value:
303-
raise Exception(
304-
f"Matching attribute {self.matching_attribute} not found "
305-
f"in user attrs: {attrs}"
306-
)
307-
308-
if matching_value and isinstance(matching_value, list):
309-
matching_value = next(iter(matching_value), None)
310-
303+
try:
304+
matching_value = self._get_attribute_value(
305+
response, self.matching_attribute
306+
)
307+
except KeyError:
308+
raise Exception(
309+
f"Matching attribute {self.matching_attribute} not found "
310+
f"in user attrs: {response.get_identity()}"
311+
) from None
311312
if isinstance(matching_value, str) and self.matching_attribute_to_lower:
312313
matching_value = matching_value.lower()
313314

@@ -349,24 +350,59 @@ def _metadata_string(self, valid=None, base_url: str = None):
349350
sign=self.sign_metadata,
350351
)
351352

353+
@staticmethod
354+
def _get_attribute_value(response, attribute_name: str):
355+
"""
356+
357+
:raise: KeyError if attribute is not in the response
358+
:param response:
359+
:param attribute_name:
360+
:return: value of the attribut. if the value is an empty list, return None
361+
otherwise return the first element of the list
362+
"""
363+
if attribute_name == "subject.nameId":
364+
return response.name_id.text
365+
attrs = response.get_identity()
366+
attribute_value = attrs[attribute_name]
367+
if isinstance(attribute_value, list):
368+
attribute_value = next(iter(attribute_value), None)
369+
return attribute_value
370+
352371
def _hook_validate_auth_response(self, response, matching_value):
353372
self.ensure_one()
354373
vals = {}
355-
attrs = response.get_identity()
356374

357375
for attribute in self.attribute_mapping_ids:
358-
if attribute.attribute_name not in attrs:
359-
_logger.debug(
376+
try:
377+
vals[attribute.field_name] = self._get_attribute_value(
378+
response, attribute.attribute_name
379+
)
380+
except KeyError:
381+
_logger.warning(
360382
"SAML attribute '%s' found in response %s",
361383
attribute.attribute_name,
362-
attrs,
384+
response.get_identity(),
363385
)
364-
continue
365386

366-
attribute_value = attrs[attribute.attribute_name]
367-
if isinstance(attribute_value, list):
368-
attribute_value = attribute_value[0]
387+
return {"mapped_attrs": vals}
369388

370-
vals[attribute.field_name] = attribute_value
389+
def _user_copy_defaults(self, validation):
390+
"""
391+
Returns defaults when copying the template user.
371392
372-
return {"mapped_attrs": vals}
393+
Can be overridden with extra information.
394+
:param validation: validation result
395+
:return: a dictionary for copying template user, empty to avoid copying
396+
"""
397+
self.ensure_one()
398+
if not self.create_user:
399+
return {}
400+
saml_uid = validation["user_id"]
401+
return {
402+
"name": saml_uid,
403+
"login": saml_uid,
404+
"active": True,
405+
# if signature is not provided by mapped_attrs, it will be computed
406+
# due to call to compute method in calling method.
407+
"signature": None,
408+
} | validation.get("mapped_attrs", {})

auth_saml/models/res_users.py

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
import passlib
99

10-
from odoo import SUPERUSER_ID, _, api, fields, models, registry, tools
10+
from odoo import SUPERUSER_ID, Command, _, api, fields, models, registry, tools
1111
from odoo.exceptions import AccessDenied, ValidationError
1212

1313
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
4545
limit=1,
4646
)
4747
user = user_saml.user_id
48-
if len(user) != 1:
49-
raise AccessDenied()
48+
user_copy_defaults = {}
49+
if not user:
50+
user_copy_defaults = (
51+
self.env["auth.saml.provider"]
52+
.browse(provider)
53+
._user_copy_defaults(validation)
54+
)
55+
if not user_copy_defaults:
56+
raise AccessDenied()
5057

5158
with registry(self.env.cr.dbname).cursor() as new_cr:
5259
new_env = api.Environment(new_cr, self.env.uid, self.env.context)
60+
if user_copy_defaults:
61+
new_user = (
62+
new_env["auth.saml.provider"]
63+
.browse(provider)
64+
.create_user_template_id.with_context(no_reset_password=True)
65+
.copy(
66+
{
67+
**user_copy_defaults,
68+
"saml_ids": [
69+
Command.create(
70+
{
71+
"saml_provider_id": provider,
72+
"saml_uid": saml_uid,
73+
"saml_access_token": saml_response,
74+
}
75+
)
76+
],
77+
}
78+
)
79+
)
80+
# Update signature as needed.
81+
new_user._compute_signature()
82+
return new_user.login
83+
5384
# Update the token. Need to be committed, otherwise the token is not visible
5485
# to other envs, like the one used in login_and_redirect
5586
user_saml.with_env(new_env).write({"saml_access_token": saml_response})
5687

57-
if validation.get("mapped_attrs", {}):
58-
user.write(validation.get("mapped_attrs", {}))
88+
# if a login is changed by a mapped attribute, it needs to be commited too
89+
user = user.with_env(new_env)
90+
if validation.get("mapped_attrs", {}):
91+
user.write(validation.get("mapped_attrs", {}))
5992

60-
return user.login
93+
return user.login
6194

6295
@api.model
6396
def auth_saml(self, provider: int, saml_response: str, base_url: str = None):

auth_saml/models/res_users_saml.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@ class ResUserSaml(models.Model):
77
_name = "res.users.saml"
88
_description = "User to SAML Provider Mapping"
99

10-
user_id = fields.Many2one("res.users", index=True, required=True)
10+
user_id = fields.Many2one(
11+
"res.users", index=True, required=True, ondelete="cascade"
12+
)
1113
saml_provider_id = fields.Many2one(
1214
"auth.saml.provider", string="SAML Provider", index=True
1315
)

auth_saml/readme/CONFIGURE.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@ To use this module, you need an IDP server, properly set up.
22

33
1. Configure the module according to your IdP’s instructions (Settings
44
\> Users & Companies \> SAML Providers).
5-
2. Pre-create your users and set the SAML information against the user.
5+
2. Pre-create your users and set the SAML information against the user,
6+
or use the module ability to create users as they log in.
67

78
By default, the module let users have both a password and SAML ids. To
89
increase security, disable passwords by using the option in Settings.

auth_saml/readme/HISTORY.md

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,12 @@
1-
## 16.0.1.0.0
1+
## 17.0.1.1.0
22

3-
Initial migration for 16.0.
3+
- custom message when response is too old
4+
- avoid using werkzeug.urls method, they are deprecated
5+
- add missing ondelete cascade when user is deleted
6+
- attribute mapping is now also duplicated when the provider is duplicated
7+
- factorize getting SAML attribute value, allowing using subject.nameId in mapping attributes too
8+
- allow creating user if not found by copying a template user
9+
10+
## 17.0.1.0.0
11+
12+
Initial migration for 17.0.

auth_saml/tests/fake_idp.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,13 +73,20 @@
7373
}
7474

7575

76+
class DummyNameId:
77+
"""Dummy name id with text value"""
78+
def __init__(self, text):
79+
self.text = text
80+
81+
7682
class DummyResponse:
77-
def __init__(self, status, data, headers=None):
83+
def __init__(self, status, data, headers=None, name_id: str = ""):
7884
self.status_code = status
7985
self.text = data
8086
self.headers = headers or []
8187
self.content = data
8288
self._identity = {}
89+
self.name_id = DummyNameId(name_id)
8390

8491
def _unpack(self, ver="SAMLResponse"):
8592
"""
@@ -127,6 +134,7 @@ def __init__(self, metadatas=None):
127134
config.load(settings)
128135
config.allow_unknown_attributes = True
129136
Server.__init__(self, config=config)
137+
self.mail = "[email protected]"
130138

131139
def get_metadata(self):
132140
return create_metadata_string(
@@ -163,7 +171,7 @@ def authn_request_endpoint(self, req, binding, relay_state):
163171
"surName": "Example",
164172
"givenName": "Test",
165173
"title": "Ind",
166-
"mail": "[email protected]",
174+
"mail": self.mail,
167175
}
168176

169177
resp_args.update({"sign_assertion": True, "sign_response": True})

0 commit comments

Comments
 (0)