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

Fix false invalidation when decoding token User Ids #1278

Merged
merged 3 commits into from
Mar 16, 2019

Conversation

Chris-Johnston
Copy link
Collaborator

This fixes an issue when trying to validate older bot tokens. The first segment is a base64-encoded string that contains a ulong. The format that discord provides will not contain any padding, while Convert.FromBase64String will throw a FormatException when trying to decode from a string that requires it (string length is not a multiple of 4).

This PR:

  • Adds a util method for padding base64 strings + tests
  • Pads the first segment of the token before trying to decode it
  • Adds a test for tokens that require base64 padding

Original conversation can be found here.

@Chris-Johnston Chris-Johnston changed the title Fix false invalidation when deoding token User Ids Fix false invalidation when decoding token User Ids Mar 4, 2019
Copy link
Member

@foxbot foxbot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm beginning to question whether or not our token invalidation logic is growing too complicated; however, this is probably a reasonable fix to have on the current codebase, so we'll accept now, reconsider later.

@Chris-Johnston
Copy link
Collaborator Author

Given that it's run into a few problems I agree that it might be best to reconsider, but it seems like it's been somewhat effective at helping people identify when they are passing in invalid tokens.

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

Successfully merging this pull request may close these issues.

2 participants