-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Do not retry failed authorization #86
Conversation
} | ||
|
||
@Override | ||
public String getAccessToken(User user) throws IOException, NotAuthorizedException { | ||
if (!user.isAuthorized()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wondering, How would this prevent temporary failures when refreshing tokens being completely removed out of the system? In this case the researchers have to recruit again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, but temporary failures are handled by the rest source authoriser. This update is just to ensure that unauthorised users do not pulled again and again. If the rest source authoriser fails to update a token because it gets a 400, 401 or 403 error from the REST source API (e.g. Fitbit), it will permanently mark the user as unauthorised, until the user is manually authorised again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Ignore users for future requests if their authentication has failed.