✨(backend) add option to configure list of required OIDC claims #525
Conversation
sampaccoud
added
python
We want to be able to refuse connection for users who have missing claims from a list of required keys.
sampaccoud
force-pushed
the
add-setting-to-make-oidc-claims-required
branch
from
67d1163 to
dca001e
Compare
| # Validate required claims | ||
| missing_claims = [ | ||
| claim | ||
| for claim in settings.USER_OIDC_REQUIRED_CLAIMS | ||
| if claim not in userinfo | ||
| ] | ||
| if missing_claims: | ||
| raise SuspiciousOperation( | ||
| _("Missing required claims in user info: %(claims)s") | ||
| % {"claims": ", ".join(missing_claims)} | ||
| ) | ||
|
|
There was a problem hiding this comment.
according to the spec, these claims should be referred as essential claims. Please take a look here https://openid.net/specs/openid-connect-core-1_0.html
Essential Claim
Claim specified by the Client as being necessary to ensure a smooth authorization experience for the specific task requested by the End-User.
There was a problem hiding this comment.
You might prefer to override the base method, and call it in get_or_create_user, as proposed in the initial implementation. get_userinfo should have a single responsibility, getting the data.
def verify_claims(self, claims):
"""Verify the provided claims to decide if authentication should be allowed."""
# Verify claims required by default configuration
scopes = self.get_settings("OIDC_RP_SCOPES", "openid email")
if "email" in scopes.split():
return "email" in claims
LOGGER.warning(
"Custom OIDC_RP_SCOPES defined. "
"You need to override `verify_claims` for custom claims verification."
)
return True
There was a problem hiding this comment.
Interesting! 🙏
Note that checking the presence of the "sub" claim was already wrong then 🤔
Let's fix that before it is released!
There was a problem hiding this comment.
@lebaudantoine Here you go: #531
There was a problem hiding this comment.
Kind of, checking the presence of "sub" was naive and simple.
As soon as we introduce more complexity (a list check, a new setting, …) I would stick with the Mozilla django oidc philosophy
Purpose
We want to be able to refuse connection for users who have missing claims from a list of required keys.
Proposal
Add option to configure list of required OIDC claims: USER_OIDC_REQUIRED_CLAIMS.
Defaults to an empty list.