Skip to content
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

Delete bad API token files #54324

Open
wants to merge 1 commit into
base: 2018.3
from

Conversation

@waynew
Copy link
Contributor

commented Aug 27, 2019

What does this PR do?

Deletes eauth token files if they're empty (or otherwise fail to be read)

What issues does this PR fix or reference?

#54256
#37945

Previous Behavior

If the token file was empty or we otherwise failed to deserialize the token it would cause an exception. This was a regression.

New Behavior

If the token file is empty or we otherwise fail to deserialize the token we delete the invalid token file.

Tests written?

Yes - not just ones to cover this regression, but also added a couple more to cover this whole function.

Commits signed with GPG?

Yes


I have one question about this, though - right now I'm capturing Exception when trying to load the token file. This seems like it could be overly broad - OTOH, it's also probably accurate. For an IOError or OSError it will end out taking a different path (i.e. that load will just return an empty dict that gets returned, which bypasses the file removal process).

Any thoughts?

@waynew waynew requested a review from saltstack/team-core as a code owner Aug 27, 2019
@pull-assigner pull-assigner bot requested a review from garethgreenaway Aug 27, 2019
@waynew waynew force-pushed the waynew:54256-re-fix-empty-api-token branch from d607fed to 4867d37 Aug 27, 2019
Under the following conditions and API token should be considered
invalid:

- The file is empty.
- We cannot deserialize the token from the file.
- The token exists but has no expiration date.
- The token exists but has expired.

All of these conditions necessitate deleting the token file. Otherwise
we should simply return an empty token.
@waynew waynew force-pushed the waynew:54256-re-fix-empty-api-token branch from 4867d37 to bb21983 Aug 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.