-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Support for scraping email addresses from OIDC providers #9245
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Add support to the OpenID Connect integration for adding the user's email address. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1056,7 +1056,8 @@ class OidcSessionData: | |
|
||
|
||
UserAttributeDict = TypedDict( | ||
"UserAttributeDict", {"localpart": Optional[str], "display_name": Optional[str]} | ||
"UserAttributeDict", | ||
{"localpart": Optional[str], "display_name": Optional[str], "emails": List[str]}, | ||
) | ||
C = TypeVar("C") | ||
|
||
|
@@ -1135,11 +1136,12 @@ def jinja_finalize(thing): | |
env = Environment(finalize=jinja_finalize) | ||
|
||
|
||
@attr.s | ||
@attr.s(slots=True, frozen=True) | ||
class JinjaOidcMappingConfig: | ||
subject_claim = attr.ib(type=str) | ||
localpart_template = attr.ib(type=Optional[Template]) | ||
display_name_template = attr.ib(type=Optional[Template]) | ||
email_template = attr.ib(type=Optional[Template]) | ||
extra_attributes = attr.ib(type=Dict[str, Template]) | ||
|
||
|
||
|
@@ -1156,23 +1158,17 @@ def __init__(self, config: JinjaOidcMappingConfig): | |
def parse_config(config: dict) -> JinjaOidcMappingConfig: | ||
subject_claim = config.get("subject_claim", "sub") | ||
|
||
localpart_template = None # type: Optional[Template] | ||
if "localpart_template" in config: | ||
def parse_template_config(option_name: str) -> Optional[Template]: | ||
if option_name not in config: | ||
return None | ||
try: | ||
localpart_template = env.from_string(config["localpart_template"]) | ||
return env.from_string(config[option_name]) | ||
except Exception as e: | ||
raise ConfigError( | ||
"invalid jinja template", path=["localpart_template"] | ||
) from e | ||
raise ConfigError("invalid jinja template", path=[option_name]) from e | ||
|
||
display_name_template = None # type: Optional[Template] | ||
if "display_name_template" in config: | ||
try: | ||
display_name_template = env.from_string(config["display_name_template"]) | ||
except Exception as e: | ||
raise ConfigError( | ||
"invalid jinja template", path=["display_name_template"] | ||
) from e | ||
localpart_template = parse_template_config("localpart_template") | ||
display_name_template = parse_template_config("display_name_template") | ||
email_template = parse_template_config("email_template") | ||
|
||
extra_attributes = {} # type Dict[str, Template] | ||
if "extra_attributes" in config: | ||
|
@@ -1192,6 +1188,7 @@ def parse_config(config: dict) -> JinjaOidcMappingConfig: | |
subject_claim=subject_claim, | ||
localpart_template=localpart_template, | ||
display_name_template=display_name_template, | ||
email_template=email_template, | ||
extra_attributes=extra_attributes, | ||
) | ||
|
||
|
@@ -1213,16 +1210,23 @@ async def map_user_attributes( | |
# a usable mxid. | ||
localpart += str(failures) if failures else "" | ||
|
||
display_name = None # type: Optional[str] | ||
if self._config.display_name_template is not None: | ||
display_name = self._config.display_name_template.render( | ||
user=userinfo | ||
).strip() | ||
def render_template_field(template: Optional[Template]) -> Optional[str]: | ||
if template is None: | ||
return None | ||
return template.render(user=userinfo).strip() | ||
|
||
display_name = render_template_field(self._config.display_name_template) | ||
if display_name == "": | ||
display_name = None | ||
|
||
if display_name == "": | ||
display_name = None | ||
emails = [] # type: List[str] | ||
email = render_template_field(self._config.email_template) | ||
if email: | ||
emails.append(email) | ||
|
||
return UserAttributeDict(localpart=localpart, display_name=display_name) | ||
return UserAttributeDict( | ||
localpart=localpart, display_name=display_name, emails=emails | ||
) | ||
Comment on lines
+1227
to
+1229
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FWIW I had wonder if we should just return No change necessary, just idly thinking. :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had the same thought. Then I realised this was an extension module, so changing the expected return type could break existing implementations, and suddenly I lost enthusiasm... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm pretty sure that was what I did as well. 😟 |
||
|
||
async def get_extra_attributes(self, userinfo: UserInfo, token: Token) -> JsonDict: | ||
extras = {} # type: Dict[str, str] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was just documented wrong it looks like?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup.