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

grant_types should follow response_types in a client registration req… #493

Merged
merged 7 commits into from
Feb 17, 2018
Merged

grant_types should follow response_types in a client registration req… #493

merged 7 commits into from
Feb 17, 2018

Conversation

rohe
Copy link
Collaborator

@rohe rohe commented Feb 16, 2018

grant_types should follow response_types in a client registration request

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

@rohe rohe requested a review from tpazderka February 16, 2018 09:11
CHANGELOG.md Outdated
@@ -8,6 +8,7 @@ The format is based on the [KeepAChangeLog] project.
## 0.13.0 [Unreleased]

### Added
- [#493] grant_types specification should follow the response_types sprecification in a client registration request.
Copy link
Collaborator

Choose a reason for hiding this comment

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

specification, not sprecification

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right

@codecov-io
Copy link

codecov-io commented Feb 16, 2018

Codecov Report

Merging #493 into master will increase coverage by 0.04%.
The diff coverage is 92.85%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #493      +/-   ##
==========================================
+ Coverage      59%   59.04%   +0.04%     
==========================================
  Files          62       62              
  Lines       11200    11214      +14     
  Branches     1957     1959       +2     
==========================================
+ Hits         6608     6621      +13     
  Misses       4036     4036              
- Partials      556      557       +1
Impacted Files Coverage Δ
src/oic/oic/__init__.py 63.34% <92.85%> (+0.4%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2feb099...5fc8ec5. Read the comment docs.

Copy link
Collaborator

@tpazderka tpazderka left a comment

Choose a reason for hiding this comment

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

Also we should wrap lines to 120 characters and not split them unnecessarily.

}


def response_types_to_grant_types(response_types):
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be covered by tests.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Especially the part that response_types may be in random order (so no one removes the 'sort' from the code by accident).

Copy link
Collaborator

Choose a reason for hiding this comment

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

One last thing then this is good to go.

The test with an unknown response_type should also be present.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added

@@ -1319,6 +1345,12 @@ def create_registration_request(self, **kwargs):
except KeyError:
pass

try:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it is better to change try .. except to a condition if response_types in req.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm OK with either or so we'll go for your proposal.

@tpazderka tpazderka merged commit 23efe6d into CZ-NIC:master Feb 17, 2018
@schlenk schlenk mentioned this pull request May 2, 2018
2 tasks
@rohe rohe deleted the grant_types branch June 8, 2018 07:57
andrewkrug pushed a commit to mozilla-iam/pyoidc that referenced this pull request Jun 6, 2019
CZ-NIC#493)

* grant_types should follow response_types in a client registration request.
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.

4 participants