/synapse

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Stop generating refresh_tokens #1654

Merged
merged 5 commits into from Dec 1, 2016

Conversation

Reviewers

@NegativeMjark NegativeMjark

Assignees

@NegativeMjark NegativeMjark

Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@richvdh @NegativeMjark
@richvdh
Member

richvdh commented Nov 25, 2016 • edited

Refresh tokens are dead, so let's stop trying to generate new ones.

richvdh added some commits Nov 28, 2016

@richvdh
Stop generating refresh tokens 
Since we're not doing refresh tokens any more, we should start killing off the
dead code paths. /tokenrefresh itself is a bit of a thornier subject, since
there might be apps out there using it, but we can at least not generate
refresh tokens on new logins.

Some checks were not successful

1 errored, 3 failing, and 6 successful checks
@matrixbot
Sytest Dendron (Commit) — Build #1171 origin/rav/no_more_refresh_tokens failed in 14 min
Details
@matrixbot
Sytest Dendron (Merged PR) — Build finished.
Details
@matrixbot
Sytest Postgres (Merged PR) — Build finished.
Details
@matrixbot
Sytest SQLite (Merged PR) — Build finished.
Details
@matrixbot
Flake8 + Packaging (Commit) — Build #2154 origin/rav/no_more_refresh_tokens succeeded in 51 sec
Details
@matrixbot
Flake8 + Packaging (Merged PR) — Build finished.
Details
@matrixbot
Sytest Postgres (Commit) — Build #2009 origin/rav/no_more_refresh_tokens succeeded in 8 min 54 sec
Details
@matrixbot
Sytest SQLite (Commit) — Build #2053 origin/rav/no_more_refresh_tokens succeeded in 7 min 32 sec
Details
@matrixbot
Unit Tests (Commit) — Build #2137 origin/rav/no_more_refresh_tokens succeeded in 2 min 19 sec
Details
@matrixbot
Unit Tests (Merged PR) — Build finished.
Details
5c4edc8
@richvdh
Merge branch 'develop' into rav/no_more_refresh_tokens

Some checks were not successful

1 errored, 3 failing, and 6 successful checks
@matrixbot
Sytest Dendron (Commit) — Build #1184 origin/rav/no_more_refresh_tokens failed in 12 min
Details
@matrixbot
Sytest Dendron (Merged PR) — Build finished.
Details
@matrixbot
Sytest Postgres (Merged PR) — Build finished.
Details
@matrixbot
Sytest SQLite (Merged PR) — Build finished.
Details
@matrixbot
Flake8 + Packaging (Commit) — Build #2119 origin/rav/no_more_refresh_tokens succeeded in 57 sec
Details
@matrixbot
Flake8 + Packaging (Merged PR) — Build finished.
Details
@matrixbot
Sytest Postgres (Commit) — Build #2022 origin/rav/no_more_refresh_tokens succeeded in 8 min 48 sec
Details
@matrixbot
Sytest SQLite (Commit) — Build #2064 origin/rav/no_more_refresh_tokens succeeded in 6 min 3 sec
Details
@matrixbot
Unit Tests (Commit) — Build #2173 origin/rav/no_more_refresh_tokens succeeded in 3 min 34 sec
Details
@matrixbot
Unit Tests (Merged PR) — Build finished.
Details
dc4b23e
@richvdh
Rip out more refresh_token code 
We might as well treat all refresh_tokens as invalid. Just return a 403 from
/tokenrefresh, so that we don't have a load of dead, untestable code hanging
around.

Still TODO: removing the table from the schema.

Some checks were not successful

4 failing and 2 successful checks
@matrixbot
Flake8 + Packaging (Merged PR) — Build finished.
Details
@matrixbot
Sytest Dendron (Merged PR) — Build finished.
Details
@matrixbot
Sytest Postgres (Merged PR) — Build finished.
Details
@matrixbot
Sytest SQLite (Merged PR) — Build finished.
Details
@matrixbot
Sytest SQLite (Commit) — Build #2065 origin/rav/no_more_refresh_tokens succeeded in 6 min 16 sec
Details
@matrixbot
Unit Tests (Merged PR) — Build finished.
Details
aa09d6b
@richvdh
fix imports

Some checks were not successful

1 errored, 3 failing, and 6 successful checks
@matrixbot
Sytest Dendron (Commit) — Build #1185 origin/rav/no_more_refresh_tokens failed in 12 min
Details
@matrixbot
Sytest Dendron (Merged PR) — Build finished.
Details
@matrixbot
Sytest Postgres (Merged PR) — Build finished.
Details
@matrixbot
Sytest SQLite (Merged PR) — Build finished.
Details
@matrixbot
Flake8 + Packaging (Commit) — Build #2178 origin/rav/no_more_refresh_tokens succeeded in 33 sec
Details
@matrixbot
Flake8 + Packaging (Merged PR) — Build finished.
Details
@matrixbot
Sytest Postgres (Commit) — Build #2023 origin/rav/no_more_refresh_tokens succeeded in 9 min 12 sec
Details
@matrixbot
Sytest SQLite (Commit) — Build #2066 origin/rav/no_more_refresh_tokens succeeded in 7 min 40 sec
Details
@matrixbot
Unit Tests (Commit) — Build #2148 origin/rav/no_more_refresh_tokens succeeded in 3 min 43 sec
Details
@matrixbot
Unit Tests (Merged PR) — Build finished.
Details
12f3b90

@richvdh richvdh assigned NegativeMjark Dec 1, 2016

synapse/rest/client/v2_alpha/register.py
@@ -386,8 +386,8 @@ def _create_registration_details(self, user_id, params):
"""
@NegativeMjark

NegativeMjark Dec 1, 2016

Contributor

The doc-string still refers to refresh_tokens

@NegativeMjark
Contributor

NegativeMjark commented Dec 1, 2016

We might want to remove the refresh_tokens database table at somepoint, but fitting that into the schema upgrade process might be subtle given that there are background updates run against that table. So lets leave that for another PR so that it can be reviewed properly.
@NegativeMjark
Contributor

NegativeMjark commented Dec 1, 2016 • edited

Other than the stale docstring LGTM.
@richvdh
Member

richvdh commented Dec 1, 2016

We might want to remove the refresh_tokens database table at somepoint, but fitting that into the schema upgrade process might be subtle given that there are background updates run against that table. So lets leave that for another PR so that it can be reviewed properly.

yeah, I came to exactly the same conclusion.
@richvdh
Fix doc-string 
Remove refresh_token reference

Some checks were not successful

3 failing and 3 successful checks
@matrixbot
Sytest Dendron (Merged PR) — Build finished.
Details
@matrixbot
Sytest Postgres (Merged PR) — Build finished.
Details
@matrixbot
Sytest SQLite (Merged PR) — Build finished.
Details
@matrixbot
Flake8 + Packaging (Merged PR) — Build finished.
Details
@matrixbot
Unit Tests (Commit) — Build #2166 origin/rav/no_more_refresh_tokens succeeded in 4 min 7 sec
Details
@matrixbot
Unit Tests (Merged PR) — Build finished.
Details
6841d8f

@richvdh richvdh merged commit 4712000 into develop Dec 1, 2016
0 of 5 checks passed

0 of 5 checks passed

Flake8 + Packaging (Merged PR) Build triggered. sha1 is merged.
Details
Sytest Dendron (Merged PR) Build triggered. sha1 is merged.
Details
Sytest Postgres (Merged PR) Build triggered. sha1 is merged.
Details
Sytest SQLite (Merged PR) Build triggered. sha1 is merged.
Details
Unit Tests (Merged PR) Build triggered. sha1 is merged.
Details

@richvdh richvdh deleted the rav/no_more_refresh_tokens branch Dec 1, 2016

@euank euank referenced this pull request in ruma/ruma Feb 12, 2017

Merged

Implement tokenrefresh #166

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