Skip to content

Commit 1454089

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 3bc493a commit 1454089

File tree

11 files changed

+221
-55
lines changed

11 files changed

+221
-55
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: 72 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,21 @@ 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. If a deactivated user has a matching saml uid, activate it rather than"
146+
"create a new one.",
147+
)
148+
create_user_template_id = fields.Many2one(
149+
comodel_name="res.users",
150+
# Template users, like base.default_user, are disabled by default so allow them
151+
domain="[('active', 'in', (True, False))]",
152+
default=lambda self: self.env.ref("base.default_user"),
153+
help="When creating user, this user is used as a template",
154+
)
139155

140156
@api.model
141157
def _sig_alg_selection(self):
@@ -256,9 +272,7 @@ def _get_auth_request(self, extra_state=None, url_root=None):
256272
}
257273
state.update(extra_state)
258274

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

263277
saml_client = self._get_client_for_provider(url_root)
264278
reqid, info = saml_client.prepare_for_authenticate(
@@ -272,6 +286,7 @@ def _get_auth_request(self, extra_state=None, url_root=None):
272286
for key, value in info["headers"]:
273287
if key == "Location":
274288
redirect_url = value
289+
break
275290

276291
self._store_outstanding_request(reqid)
277292

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

@@ -349,24 +352,59 @@ def _metadata_string(self, valid=None, base_url: str = None):
349352
sign=self.sign_metadata,
350353
)
351354

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

357377
for attribute in self.attribute_mapping_ids:
358-
if attribute.attribute_name not in attrs:
359-
_logger.debug(
378+
try:
379+
vals[attribute.field_name] = self._get_attribute_value(
380+
response, attribute.attribute_name
381+
)
382+
except KeyError:
383+
_logger.warning(
360384
"SAML attribute '%s' not found in response %s",
361385
attribute.attribute_name,
362-
attrs,
386+
response.get_identity(),
363387
)
364-
continue
365388

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

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

auth_saml/models/res_users.py

Lines changed: 39 additions & 3 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,47 @@ 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 (
50+
not user.active
51+
and self.env["auth.saml.provider"].browse(provider).create_user
52+
):
53+
user.active = True
54+
if not user:
55+
user_copy_defaults = (
56+
self.env["auth.saml.provider"]
57+
.browse(provider)
58+
._user_copy_defaults(validation)
59+
)
60+
if not user_copy_defaults:
61+
raise AccessDenied()
5062

5163
with registry(self.env.cr.dbname).cursor() as new_cr:
5264
new_env = api.Environment(new_cr, self.env.uid, self.env.context)
65+
if user_copy_defaults:
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+
# Update signature as needed.
86+
new_user._compute_signature()
87+
return new_user.login
88+
5389
# Update the token. Need to be committed, otherwise the token is not visible
5490
# to other envs, like the one used in login_and_redirect
5591
user_saml.with_env(new_env).write({"saml_access_token": saml_response})

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: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
- Avoid redirecting when there is a SAML error.
66

77

8-
## 17.0.1.1.0
8+
## 17.0.1.0.1
99

1010
When using attribute mapping, only write value that changes.
1111
No writing the value systematically avoids getting security mail on login/email
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
- custom message when response is too old
2+
- avoid using werkzeug.urls method, they are deprecated
3+
- add missing ondelete cascade when user is deleted
4+
- attribute mapping is now also duplicated when the provider is duplicated
5+
- factorize getting SAML attribute value, allowing using subject.nameId in mapping attributes too
6+
- allow creating user if not found by copying a template user, or activating a deactivated user.

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)