Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Audience is endpoint dependent. #430

Merged
merged 4 commits into from
Oct 6, 2017
Merged

Audience is endpoint dependent. #430

merged 4 commits into from
Oct 6, 2017

Conversation

rohe
Copy link
Collaborator

@rohe rohe commented Oct 6, 2017

Thanks for contributing to this library! Please include the following check
list in your pull request submission (you can delete this message). If your
changes don't need a change log or documentation update, please ignore this.

  • Any changes relevant to users are recorded in the CHANGELOG.md.
  • The documentation has been updated, if necessary.

@coveralls
Copy link

coveralls commented Oct 6, 2017

Coverage Status

Coverage decreased (-0.001%) to 60.14% when pulling 26f4ecd on rohe:audience into bb8e71a on OpenIDC:master.

@coveralls
Copy link

coveralls commented Oct 6, 2017

Coverage Status

Coverage decreased (-0.001%) to 60.14% when pulling 26f4ecd on rohe:audience into bb8e71a on OpenIDC:master.

@rohe
Copy link
Collaborator Author

rohe commented Oct 6, 2017

I really don't care if the coverage decreases with 0.001 % .

algorithm = None
if kwargs['authn_endpoint'] in ['token', 'refresh']:
try:
algorithm = self.cli.registration_info[
'token_endpoint_auth_signing_alg']
except (KeyError, AttributeError):
pass
audience = self.cli.provider_info['token_endpoint']
else:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The drop in coverage is caused by this branch, which is never covered.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can easily be remedied

@coveralls
Copy link

coveralls commented Oct 6, 2017

Coverage Status

Coverage increased (+0.01%) to 60.153% when pulling 612ed45 on rohe:audience into bb8e71a on OpenIDC:master.

@rohe
Copy link
Collaborator Author

rohe commented Oct 6, 2017

I don't think this will break anything. It's really a corner case, slightly outside the standard.
private_key_jwt and cleint_secret_jwt was designed for client authentication at the token endpoint.
But now when the methods are defined they are creatively :-) used at other endpoints.
In which case using the token endpoint URL as audience doesn't make sense.

@rohe rohe merged commit 516bf6b into CZ-NIC:master Oct 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants