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

sanitize_token() doesn't allow lowercase characters #347

Closed
jmbarbone opened this issue Jul 22, 2021 · 3 comments
Closed

sanitize_token() doesn't allow lowercase characters #347

jmbarbone opened this issue Jul 22, 2021 · 3 comments
Assignees

Comments

@jmbarbone
Copy link
Contributor

Adding a small change in sanitize_token() with toupper() could save some work for people with API tokens stored with lowercase character data.

library(REDCapR)
sanitize_token("12345678901234567890123456ABCDEF")
#> [1] "12345678901234567890123456ABCDEF"
sanitize_token("12345678901234567890123456abcdef")
#> Error in sanitize_token("12345678901234567890123456abcdef"): The token is not a valid 32-character hexademical value.
packageVersion("REDCapR")
#> [1] '1.0.0'

Created on 2021-07-22 by the reprex package (v2.0.0)

@wibeasley wibeasley self-assigned this Jul 22, 2021
@wibeasley
Copy link
Member

Thanks @jmbarbone. I restricted to uppercase intentionally, because REDCap's token generating code wasn't producing any lowercase letters. I assume it generated lowercase letters for you? I guess their code changed.

If so, do you mind removing the call to toupper(), and instead loosen the regex so it accommodates lowercase letters from a to z? The pattern would then be defined like this (notice the extra "a-f")

  pattern <- "^([0-9A-Fa-f]{32})(?:\\n)?$"

@jmbarbone
Copy link
Contributor Author

Yes, @wibeasley, all the API tokens I've seen use lowercase characters.

The reason I used toupper() instead of adjusting the regex was to make sure that the outputs would be the same regardless of the original case of token in case there were other checks that demanded upper case tokens. But from local testing it seems that adjusting just the pattern is safe.

wibeasley added a commit that referenced this issue Jul 22, 2021
resolves #347 - lowercase characters in token
@wibeasley
Copy link
Member

I'm glad you figured it out and told me instead of turning off the validation.

wibeasley added a commit that referenced this issue Jul 22, 2021
To reflect @jmbarbone's modifications in #347 & #348

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

No branches or pull requests

2 participants