Skip to content

Commit e8b862f

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 7fe6b8a commit e8b862f

File tree

10 files changed

+223
-56
lines changed

10 files changed

+223
-56
lines changed

auth_saml/controllers/main.py

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,11 @@
55
import functools
66
import json
77
import logging
8+
from urllib.parse import quote_plus, unquote_plus, urlencode
89

910
import werkzeug.utils
11+
from saml2.validate import ResponseLifetimeExceed
1012
from werkzeug.exceptions import BadRequest
11-
from werkzeug.urls import url_quote_plus
1213

1314
from odoo import (
1415
SUPERUSER_ID,
@@ -100,7 +101,7 @@ def _auth_saml_request_link(self, provider: models.Model):
100101
redirect = request.params.get("redirect")
101102
if redirect:
102103
params["redirect"] = redirect
103-
return "/auth_saml/get_auth_request?%s" % werkzeug.urls.url_encode(params)
104+
return "/auth_saml/get_auth_request?%s" % urlencode(params)
104105

105106
@http.route()
106107
def web_client(self, s_action=None, **kw):
@@ -136,6 +137,8 @@ def web_login(self, *args, **kw):
136137
error = _("Sign up is not allowed on this database.")
137138
elif error == "access-denied":
138139
error = _("Access Denied")
140+
elif error == "response-lifetime-exceed":
141+
error = _("Response Lifetime Exceeded")
139142
elif error == "expired":
140143
error = _(
141144
"You do not have access to this database. Please contact"
@@ -169,7 +172,7 @@ def _get_saml_extra_relaystate(self):
169172
)
170173

171174
state = {
172-
"r": url_quote_plus(redirect),
175+
"r": quote_plus(redirect),
173176
}
174177
return state
175178

@@ -231,9 +234,7 @@ def signin(self, **kw):
231234
)
232235
action = state.get("a")
233236
menu = state.get("m")
234-
redirect = (
235-
werkzeug.urls.url_unquote_plus(state["r"]) if state.get("r") else False
236-
)
237+
redirect = unquote_plus(state["r"]) if state.get("r") else False
237238
url = "/web"
238239
if redirect:
239240
url = redirect
@@ -255,6 +256,9 @@ def signin(self, **kw):
255256
redirect = werkzeug.utils.redirect(url, 303)
256257
redirect.autocorrect_location_header = False
257258
return redirect
259+
except ResponseLifetimeExceed as e:
260+
_logger.debug("Response Lifetime Exceed - %s", str(e))
261+
url = "/web/login?saml_error=response-lifetime-exceed"
258262

259263
except Exception as e:
260264
# 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: 71 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(
@@ -272,6 +285,7 @@ def _get_auth_request(self, extra_state=None, url_root=None):
272285
for key, value in info["headers"]:
273286
if key == "Location":
274287
redirect_url = value
288+
break
275289

276290
self._store_outstanding_request(reqid)
277291

@@ -287,27 +301,15 @@ def _validate_auth_response(self, token: str, base_url: str = None):
287301
saml2.entity.BINDING_HTTP_POST,
288302
self._get_outstanding_requests_dict(),
289303
)
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-
304+
try:
305+
matching_value = self._get_attribute_value(
306+
response, self.matching_attribute
307+
)
308+
except KeyError:
309+
raise KeyError(
310+
f"Matching attribute {self.matching_attribute} not found "
311+
f"in user attrs: {response.get_identity()}"
312+
) from None
311313
if isinstance(matching_value, str) and self.matching_attribute_to_lower:
312314
matching_value = matching_value.lower()
313315

@@ -349,24 +351,59 @@ def _metadata_string(self, valid=None, base_url: str = None):
349351
sign=self.sign_metadata,
350352
)
351353

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

357376
for attribute in self.attribute_mapping_ids:
358-
if attribute.attribute_name not in attrs:
359-
_logger.debug(
377+
try:
378+
vals[attribute.field_name] = self._get_attribute_value(
379+
response, attribute.attribute_name
380+
)
381+
except KeyError:
382+
_logger.warning(
360383
"SAML attribute '%s' found in response %s",
361384
attribute.attribute_name,
362-
attrs,
385+
response.get_identity(),
363386
)
364-
continue
365387

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

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

auth_saml/models/res_users.py

Lines changed: 51 additions & 4 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,11 +45,58 @@ 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+
login = (
62+
user_copy_defaults.pop("login")
63+
if "login" in user_copy_defaults
64+
else None
65+
)
66+
new_user = (
67+
new_env["auth.saml.provider"]
68+
.browse(provider)
69+
.create_user_template_id.with_context(no_reset_password=True)
70+
.copy(
71+
{
72+
**user_copy_defaults,
73+
"saml_ids": [
74+
Command.create(
75+
{
76+
"saml_provider_id": provider,
77+
"saml_uid": saml_uid,
78+
"saml_access_token": saml_response,
79+
}
80+
)
81+
],
82+
}
83+
)
84+
)
85+
# login needs to be updated with SQL to avoid sending useless message
86+
# about login having been changed.
87+
if login is not None:
88+
self.env.cr.execute(
89+
"UPDATE res_users SET login = %s WHERE id = %s",
90+
(
91+
login,
92+
new_user.id,
93+
),
94+
)
95+
new_user.invalidate_recordset(["login"])
96+
# Update signature as needed.
97+
new_user._compute_signature()
98+
return new_user.login
99+
53100
# Update the token. Need to be committed, otherwise the token is not visible
54101
# to other envs, like the one used in login_and_redirect
55102
user_saml.with_env(new_env).write({"saml_access_token": saml_response})
@@ -68,7 +115,7 @@ def _auth_saml_signin(self, provider: int, validation: dict, saml_response) -> s
68115
):
69116
vals[key] = value
70117
if vals:
71-
user.write(vals)
118+
user.with_env(new_env).write(vals)
72119

73120
return user.login
74121

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: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,13 @@ When using attribute mapping, only write value that changes.
44
No writing the value systematically avoids getting security mail on login/email
55
when there is no real change.
66

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

916
Initial migration for 17.0.

auth_saml/tests/fake_idp.py

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

7575

76+
class DummyNameId:
77+
"""Dummy name id with text value"""
78+
79+
def __init__(self, text):
80+
self.text = text
81+
82+
7683
class DummyResponse:
77-
def __init__(self, status, data, headers=None):
84+
def __init__(self, status, data, headers=None, name_id: str = ""):
7885
self.status_code = status
7986
self.text = data
8087
self.headers = headers or []
8188
self.content = data
8289
self._identity = {}
90+
self.name_id = DummyNameId(name_id)
8391

8492
def _unpack(self, ver="SAMLResponse"):
8593
"""
@@ -127,6 +135,7 @@ def __init__(self, metadatas=None):
127135
config.load(settings)
128136
config.allow_unknown_attributes = True
129137
Server.__init__(self, config=config)
138+
self.mail = "test@example.com"
130139

131140
def get_metadata(self):
132141
return create_metadata_string(
@@ -163,7 +172,7 @@ def authn_request_endpoint(self, req, binding, relay_state):
163172
"surName": "Example",
164173
"givenName": "Test",
165174
"title": "Ind",
166-
"mail": "test@example.com",
175+
"mail": self.mail,
167176
}
168177

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

0 commit comments

Comments
 (0)