#338 Add ID4me backend#339
Conversation
pawel-kow
left a comment
pawel-kow
left a comment
There was a problem hiding this comment.
Hi,
please take a look into the comments.
social_core/backends/id4me.py
Outdated
| self.validate_state() | ||
| identity = self.strategy.session_get(self.name + '_identity') | ||
| openid_configuration = self.get_identity_record(identity) | ||
| if 'clp' not in openid_configuration: |
There was a problem hiding this comment.
RP shall never need to read out 'clp', as the user-info delegation to Agent shall rely on distributed claims
|
|
||
| @handle_http_errors | ||
| def user_data(self, access_token, *args, **kwargs): | ||
| user_token = requests.get(self.userinfo_url(), headers={ |
There was a problem hiding this comment.
This one requires rewrite to utilize distributed claims.
Essential is that initial access token can be only considered valid for authorities' userinfo, but the one to be used with Agent can be different and will be provided only by distributed claims object.
See also: https://gitlab.com/ID4me/general/issues/25
| raise AuthUnreachableProvider(self) | ||
| if not response: | ||
| raise AuthUnreachableProvider(self) | ||
| records = response[0] |
| if not records: | ||
| raise AuthUnreachableProvider(self) | ||
| record = records[-1].strings[0].decode() | ||
| return {item.split("=")[0]: item.split("=")[1] for item in record.split(";")} |
There was a problem hiding this comment.
there shall be a check that record has v=OID1 label set
| association = self.get_association(iau) | ||
| if not association: | ||
| issuer_configuration = self.oidc_config_authority() | ||
| response = requests.post(issuer_configuration['registration_endpoint'], json={ |
There was a problem hiding this comment.
there shall be some configuration added for policy/logo/tos URLs
| def get_key_and_secret(self): | ||
| iau = self.strategy.session_get(self.name + '_authority') | ||
| association = self.get_association(iau) | ||
| if not association: |
There was a problem hiding this comment.
there shall be a check, that registration did not expire (client_secret_expires_at)
| def get_identity_record(self, identity): | ||
| try: | ||
| response = dns.resolver.query('_openid.' + identity, 'TXT', lifetime=5).response.answer | ||
| except NXDOMAIN or Timeout: |
There was a problem hiding this comment.
There shall be a lookup on parent domain in case of failure.
See https://gitlab.com/ID4me/documentation/blob/master/id4ME Technical Specification.adoc 2.1.2
In case _openid subdomain cannot be resolved, the record for the parent domain SHALL be resolved, by removing the leftmost label from the original ID4me Identifier.
social_core/backends/id4me.py
Outdated
| header = jwt.get_unverified_header(id_token) | ||
| if header['kid'] == key['kid']: | ||
| if 'alg' not in key: | ||
| key['alg'] = 'RS256' if key['kty'] == 'RSA' else 'ES256' |
There was a problem hiding this comment.
typically there shall be a correlation between id_token_signed_response_alg set in the client registration and the signature key. If omitted they default to 'RSA256', so usage of 'ES256' is not correct in this case.
social_core/backends/id4me.py
Outdated
| header = jwt.get_unverified_header(id_token) | ||
| if header['kid'] == key['kid']: | ||
| if 'alg' not in key: | ||
| key['alg'] = 'RS256' if key['kty'] == 'RSA' else 'ES256' |
There was a problem hiding this comment.
typically there shall be a correlation between userinfo_signed_response_alg set in the client registration and the signature key. If omitted they default to 'RSA256', so usage of 'ES256' is not correct in this case.
social_core/backends/id4me.py
Outdated
| raise AuthTokenError(self, 'Incorrect id_token: nbf') | ||
|
|
||
| # Verify the token was issued in the last 10 minutes | ||
| iat_leeway = self.setting('ID_TOKEN_MAX_AGE', self.ID_TOKEN_MAX_AGE) |
There was a problem hiding this comment.
Why this arbitrary check for 10 minutes? There is exp claim which shall be respected.
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
3 participants
No description provided.