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.

Sign up for GitHub

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

Jump to bottom

Security Objects handling and parameter validation #112

Merged
merged 3 commits into from
Aug 25, 2016
Merged

Security Objects handling and parameter validation #112

merged 3 commits into from
Aug 25, 2016

Conversation

macisamuele
Copy link
Collaborator

Handling of security definitions for an Operation object and enforcing request parameter validation in case of apiKey security.

@coveralls
Copy link

coveralls commented Aug 23, 2016 •
edited
Loading

Coverage Status

Coverage increased (+0.07%) to 97.278% when pulling ebacd50 on macisamuele:security-objects-handling into 3f5b09a on Yelp:master.

laucia
laucia reviewed Aug 23, 2016
View reviewed changes
bravado_core/operation.py
def _build_params_from_security_objects(op):
"""
Generate the required parameters from the security object definition.
NOTE: the current implementation handles only basic and apiKey types.
Copy link

Choose a reason for hiding this comment

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

we should probably start mentioning this in the documentation.
Could you add an entry for security support in there?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sorry @laucia but is not clear to me what you meant ... could you please give me more informations? Thanks a lot

@sjaensch
Copy link
Contributor

This is looking really good! Could you please add a test or two making sure the new security object parameters behave properly for validation? I.e. we'd want a validation error if they're missing for example, at least for the api_key type.

@macisamuele
Simplified pre-commit install
7b59f94
@macisamuele
Security Objects handling
6b888ef 
NOTE: Support for:
  - Basic and ApiKey Only (no oauth2)
  - Security Objects in Operation Object

Only ApiKey Support and Security Objects
@coveralls
Copy link

coveralls commented Aug 24, 2016 •
edited
Loading

Coverage Status

Coverage increased (+0.07%) to 97.278% when pulling 76684b9 on macisamuele:security-objects-handling into 3f5b09a on Yelp:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) to 97.278% when pulling 31e4ad2 on macisamuele:security-objects-handling into 3f5b09a on Yelp:master.

@macisamuele
Copy link
Collaborator Author

I have added 2 tests test_correct_request_with_apiKey_security and test_wrong_request_with_apiKey_security for checking the proper validation behaviour.

@sjaensch
Copy link
Contributor

👍 Taking this, it's a nice foundation to build upon.

@sjaensch sjaensch merged commit 3d469f5 into Yelp:master Aug 25, 2016
@macisamuele
Copy link
Collaborator Author

@sjaensch Thanks a lot. In the next days I'll open a new PR for the documentation as suggested by @laucia.

@macisamuele macisamuele deleted the security-objects-handling branch August 26, 2016 15:27
@macisamuele macisamuele mentioned this pull request Oct 6, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@macisamuele @coveralls @sjaensch @laucia