Save archived credential when updating last_decrypted_date #275
Conversation
confidant/routes/credentials.py
Outdated
| @@ -160,7 +161,7 @@ def get_credential(id): | |||
| metadata_only = request.args.get( | |||
| 'metadata_only', | |||
| default=False, | |||
| type=bool, | |||
| type=inputs.boolean, | |||
There was a problem hiding this comment.
See flask-restful/flask-restful#397.
Oddly, I don't remember running into this issue when using reqparse so I assume it's just because I'm using request.args.get. Either way, the bug is definitely fixed now
There was a problem hiding this comment.
I don't see how this is an issue. We're not seeing this for the get_service endpoint.
There was a problem hiding this comment.
Spoke offline - moving to a helper function
confidant/routes/credentials.py
Outdated
| @@ -6,6 +6,7 @@ | |||
|
|
|||
| from datetime import datetime | |||
| from flask import blueprints, jsonify, request | |||
| from flask_restful import inputs | |||
There was a problem hiding this comment.
Please don't add flask_restful, it's deprecated, and we're using blueprints for everything.
confidant/routes/credentials.py
Outdated
| credential.save() | ||
| # Also try to save the archived credential if ID | ||
| # corresponds to a 'credential' | ||
| if credential.data_type == 'credential': |
There was a problem hiding this comment.
Let's actually just limit this endpoint to credential.data_type == 'credential' up in line 184
There was a problem hiding this comment.
that causes potential backwards incompatibilities with existing users.
It's not a huge deal in this case though
confidant/routes/credentials.py
Outdated
| logger.warning('Archived credential {}-{} not found'.format( | ||
| id, credential.revision) | ||
| ) | ||
| return jsonify({}), 404 |
There was a problem hiding this comment.
I don't think we should return 404 here. I think we should just continue.
This fixes 2 things
metadata_onlyshould be parsed withflask_restful.inputs.boolean, notboolasbool()evaluates everything to True. The test for this has changed and now can catch this.When we update the last_decrypted_date of a credential, we also update that field on the archive credential model.
We also limit IDs for
GET /v1/credentials/<ID>to be of data typecredential(as opposed tocredentialandarchive-credential)