Skip to content

Bob handles failure cases with a bit more tact#874

Merged
KPrasch merged 13 commits intonucypher:epic-schabowskifrom
jMyles:bob
Apr 27, 2019
Merged

Bob handles failure cases with a bit more tact#874
KPrasch merged 13 commits intonucypher:epic-schabowskifrom
jMyles:bob

Conversation

@jMyles
Copy link
Contributor

@jMyles jMyles commented Mar 24, 2019

Bob previously had trouble retrieving twice. Ursula raised an error instead of 404'ing if a KFrag was NotFound. Both of these are fixed, as well as smoothing out the retrieval flow and adding some test logic.

Fixes #833
Fixes #851

Opened #892

Note: Presently, even though Bob can retrieve multiple times, he does not pass cache=True to retrieve(...) in any of the control APIs, so he will still fail in this case. If we determine that this is the sane default, I'll add it in this PR. If more discussion is in order, it can be added later or as an option for dapp developers.

@jMyles jMyles changed the title [WIP] Bob handles failure cases with a bit more tact Bob handles failure cases with a bit more tact Mar 28, 2019
@jMyles jMyles force-pushed the bob branch 2 times, most recently from 2e1a924 to f738977 Compare April 1, 2019 22:53
_unknown_ursulas, _known_ursulas, m = self.follow_treasure_map(map_id=map_id, block=True)

# TODO: Consider blocking until map is done being followed.
already_retrieved = len(message_kit.capsule._attached_cfrags) >= m
Copy link
Member

Choose a reason for hiding this comment

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

Why follow the TMap here - Is it possible to push this check earlier in this method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's interesting. It of course implies that we do have (or at least have the possibility of having) cached cfrags. Wanna make an Issue?

Copy link
Member

@KPrasch KPrasch Apr 15, 2019

Choose a reason for hiding this comment

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

@jMyles - Following up : #934

@jMyles jMyles changed the base branch from master to epic-schabowski April 9, 2019 13:49
@KPrasch KPrasch added the Bob 👨‍💼 Effects the "Bob" development area label Apr 10, 2019
@KPrasch
Copy link
Member

KPrasch commented Apr 16, 2019

Are you still thinking that cache=False is a good default for control?

@nucypher nucypher deleted a comment from KPrasch Apr 16, 2019
@jMyles
Copy link
Contributor Author

jMyles commented Apr 16, 2019

@KPrasch: As for cache=False being a default - I honest-to-goodness feel like it's a toss-up.

On one hand, it's more confusing, without a doubt. I think that it's very reasonable for users to expect the behavior they'll get with cache=True (and in fact it's reasonable for them to never even consider that there might be another possibility).

On the other hand, it's possibly more secure for some (albeit very rare) real-world scenarios. Obviously, in these scenarios, we hope that people RTFM and set cache=False themselves. I think that we'll feel somewhat bad (although I don't think that we'll be at fault in any logically or morally defensible way) if someone ever tries to revoke access for a known compromised device, not realizing that they had stored cached CFrags on it, while continuing to encrypt for a "double-chimney" sym key.

I truly can go either way on it.

Copy link
Contributor

@tuxxy tuxxy left a comment

Choose a reason for hiding this comment

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

🐧

policy_arrangement = datastore.get_policy_arrangement(arrangement_id=id_as_hex.encode(),
session=session)
work_order = WorkOrder.from_rest_payload(arrangement_id, request.data)
log.info("Work Order from {}, signed {}".format(work_order.bob, work_order.receipt_signature))
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice logging addition.

for work_order in work_orders:
try:
cfrags = self.get_reencrypted_cfrags(work_order)
except (requests.exceptions.ConnectTimeout, NotFound):
Copy link
Contributor

Choose a reason for hiding this comment

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

No logging here?

@KPrasch
Copy link
Member

KPrasch commented Apr 27, 2019

Merging into epic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bob 👨‍💼 Effects the "Bob" development area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants