Stop putting a time caveat on access tokens #1656

Merged
merged 2 commits into from Nov 30, 2016

Projects

None yet

2 participants

@richvdh
Member
richvdh commented Nov 28, 2016

The 'time' caveat on the access tokens was something of a lie, since we weren't
enforcing it; more pertinently its presence stops us ever adding useful time
caveats.

Let's move in the right direction by not lying in our caveats.

@richvdh richvdh Stop putting a time caveat on access tokens
The 'time' caveat on the access tokens was something of a lie, since we weren't
enforcing it; more pertinently its presence stops us ever adding useful time
caveats.

Let's move in the right direction by not lying in our caveats.
1c4f05d
@@ -810,6 +810,10 @@ def validate_macaroon(self, macaroon, type_string, verify_expiry, user_id):
else:
v.satisfy_general(lambda c: c.startswith("time < "))
@NegativeMjark
NegativeMjark Nov 29, 2016 Contributor

Might as well remove the verify_expiry config option while you are at it.

@NegativeMjark
NegativeMjark Nov 29, 2016 Contributor

and put a comment explaining why we aren't ever going to check the "time < " caveats.

@NegativeMjark
NegativeMjark Nov 29, 2016 Contributor

But leave the v.satisfy_general(lambda c: c.startswith("time < ")) so that existing tokens will still work.

@NegativeMjark
NegativeMjark Nov 29, 2016 Contributor

Except you can't remove the verify_expiry option because it's used in validate_short_term_login_token_and_get_user_id

@NegativeMjark
NegativeMjark Nov 29, 2016 Contributor

But you probably want to add a comment to explain what's going on.

@richvdh richvdh Comments
Update comments in verify_macaroon
4febfe4
@NegativeMjark
Contributor

LGTM

@richvdh
Member
richvdh commented Nov 30, 2016

test fails seem unrelated.

@richvdh richvdh merged commit 321fe5c into develop Nov 30, 2016

8 of 10 checks passed

Sytest Dendron (Commit) Build #1181 origin/rav/remove_time_caveat failed in 12 min
Details
Sytest SQLite (Merged PR) Build finished.
Details
Flake8 + Packaging (Commit) Build #2116 origin/rav/remove_time_caveat succeeded in 1 min 11 sec
Details
Flake8 + Packaging (Merged PR) Build finished.
Details
Sytest Dendron (Merged PR) Build finished.
Details
Sytest Postgres (Commit) Build #2017 origin/rav/remove_time_caveat succeeded in 11 min
Details
Sytest Postgres (Merged PR) Build finished.
Details
Sytest SQLite (Commit) Build #2060 origin/rav/remove_time_caveat succeeded in 6 min 1 sec
Details
Unit Tests (Commit) Build #2144 origin/rav/remove_time_caveat succeeded in 2 min 44 sec
Details
Unit Tests (Merged PR) Build finished.
Details
@richvdh richvdh deleted the rav/remove_time_caveat branch Nov 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment