Skip to content

Conversation

@pbrisbin
Copy link
Member

@pbrisbin pbrisbin commented Jan 5, 2021

When the state token is sent to an OAuth2 provider, it undergoes
%-encoding as a URL parameter. Presumably, the OAuth2 provider decodes
it as part of handling things (because it would take work to prevent
their own web frameworks from doing so), and then re-%-encodes it coming
back to us again as a callback parameter.

For us, and all existing providers, + is not a %-encoded character, so
it's sent as-is and sent back as-is. So far so good.

ClassLink, though, chooses to decode + to space. I'm not aware of the
actual spec or if this is a reasonable thing to do, but they do. This
results in them sending %20 back to us, which doesn't match and we fail.

We can't predict or prescribe what providers do in this area, so our
options are:

  • Look for a match in our Session as-is OR with spaces replaced by +

    This is harder than it sounds: a token could contain +'s or spaces,
    and we'd be getting back only spaces. To succeed, we'd actually have
    to check every permutation of space/+ substitution.

  • Filter + from our tokens

    The only downside is we may generate slightly fewer than 30
    characters, and so produce slightly less secure tokens.

    I chose this option.

  • Generate tokens without + to begin with

    This would be ideal, but I'm just not familiar enough with
    Crypto.Random. I would happily accept a PR to use this option.

@pbrisbin pbrisbin changed the title WIP: ClassLink plugin Avoid + in state token, to fix ClassLink Jan 13, 2021
When the state token is sent to an OAuth2 provider, it undergoes %-encoding as a URL parameter. Presumably, the OAuth2 provider decodes it as part of handling things (because it would take work to prevent their own web frameworks from doing so), and then re-%-encodes it coming back to us again as a callback parameter. For us, and all existing providers, + is not a %-encoded character, so it's sent as-is and sent back as-is. So far so good. ClassLink, though, chooses to decode + to space. I'm not aware of the actual spec or if this is a reasonable thing to do, but they do. This results in them sending %20 back to us, which doesn't match and we fail. We can't predict or prescribe what providers do in this area, so our options are: - Look for a match in our Session as-is OR with spaces replaced by + This is harder than it sounds: a token could contain +'s or spaces, and we'd be getting back only spaces. To succeed, we'd actually have to check every permutation of space/+ substitution. - Filter + from our tokens The only downside is we may generate slightly fewer than 30 characters, and so produce slightly less secure tokens. I chose this option. - Generate tokens without + to begin with This would be ideal, but I'm just not familiar enough with Crypto.Random. I would happily accept a PR to use this option.
@pbrisbin pbrisbin merged commit a09528a into master Jan 14, 2021
@pbrisbin pbrisbin deleted the pb/classlink branch January 14, 2021 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant