Opened 11 years ago
Closed 11 years ago
#548 closed enhancement (fixed)
Add support for token refresh to the OAuth plugin
| Reported by: | nyergler | Owned by: | |
|---|---|---|---|
| Priority: | major | Milestone: | 0.4.0 |
| Component: | programming | Keywords: | oauth review |
| Cc: | joar, nyergler | Parent Tickets: |
Description
The OAuth plugin currently does not support token refresh, as described in the OAuth specification. This is needed to allow Android clients (among others) to refresh their tokens as they expire. It appears there was some planning for this, as a refresh_token field exists in the OAuthToken model, but it is currently unpopulated.
This work may overlap with work on #517.
Change History (6)
comment:1 by , 11 years ago
| Keywords: | needsreview added |
|---|
comment:2 by , 11 years ago
| Keywords: | review added; needsreview removed |
|---|
comment:3 by , 11 years ago
| Milestone: | → 0.4.0 |
|---|
comment:4 by , 11 years ago
| Cc: | added |
|---|
Yeowch, that's a huge diff and is kind of making this bug report long! I wonder if we should be careful about that...
anyway, adding nyergler as CC'ed. I don't think I'm well qualified to review this. Maybe Nathan is?
If it comes close to 0.4.0 and nobody else has time to review I can do a rough review and we can just merge it but I'm not really well qualified at all.
comment:5 by , 11 years ago
I read https://github.com/joar/mediagoblin/compare/master...oauth;refresh_tokens carefully and believe this is merge-able. I haven't tried executing it, but have don't see anything glaring. The relational model for our OAuth plugin is a little weird (foreign keys to both client and user, when I believe a client is specific to a user already), but this patch is consistent with the existing code.
I say merge it!
comment:6 by , 11 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
I've pushed a fix for this in the
oauth/refresh_tokensbranch atgit@github.com:joar/mediagoblin.gitEdited to remove really long diff.